Bug-590: Packet loss on first time ping test 54/6354/1
authorFlavio Fernandes <ffernand@redhat.com>
Thu, 24 Apr 2014 11:06:37 +0000 (07:06 -0400)
committerFlavio Fernandes <ffernand@redhat.com>
Thu, 24 Apr 2014 11:06:37 +0000 (07:06 -0400)
In this commit, we are addressing a race that could cause the exception (ie first ip packet) to
be lost. What is observed with this bug is that the non-arp packet -- in this case the icmp packet --
is being 'exceptioned' to the SimpleForwarding code path because the node has not yet been programmed
with flows that would allow it to forward the packet to its destination w/out involving the controller.

The race in this case is caused by the exceptioned packet being received before the rulesDB member
in simpleForwarding module is updated. That member -- rulesDB -- is updated via the
hostTracker::notifyHTClient() callback shortly after. In this particular race, the hostTracker is already
updated with the proper information, and rulesDB is 'stale'. Thus, the proposed fix is to make the forwarding
less strict (ie. ignore rulesDB). Note that both routing and hostTracker are all simpleForwarding needs to
know how to forward the packet.

Change-Id: Icfb909ca89abff6031212b4fcd0f5d1bd1a27d6b
Signed-off-by: Flavio Fernandes <ffernand@redhat.com>
opendaylight/samples/simpleforwarding/src/main/java/org/opendaylight/controller/samples/simpleforwarding/internal/SimpleForwardingImpl.java

index d3cefd4..94e6724 100644 (file)
@@ -1010,12 +1010,10 @@ public class SimpleForwardingImpl implements IfNewHostNotify,
                     routing.getRoute(incomingNodeConnector.getNode(), destHost.getnodeconnectorNode()) != null)) {
 
             log.trace("Host {} is at {}", dIP, destHost.getnodeConnector());
                     routing.getRoute(incomingNodeConnector.getNode(), destHost.getnodeconnectorNode()) != null)) {
 
             log.trace("Host {} is at {}", dIP, destHost.getnodeConnector());
-            HostNodePair key = new HostNodePair(destHost, destHost.getnodeconnectorNode());
 
             // If SimpleForwarding is aware of this host, it will try to install
             // a path. Forward packet until it's done.
 
             // If SimpleForwarding is aware of this host, it will try to install
             // a path. Forward packet until it's done.
-            if (dataPacketService != null && this.rulesDB.containsKey(key)) {
-
+            if (dataPacketService != null) {
 
                 /*
                  * if we know where the host is and there's a path from where this
 
                 /*
                  * if we know where the host is and there's a path from where this