Fix for bug 24. 93/393/5
authorChi-Vien Ly <chivly@cisco.com>
Fri, 24 May 2013 23:24:55 +0000 (16:24 -0700)
committerChi-Vien Ly <chivly@cisco.com>
Sat, 25 May 2013 05:01:30 +0000 (22:01 -0700)
Description: Ping between host hosts takes more than 1second
Solution: Make modification in ArpHandler to handle the followings
- When a ARP Request is received from an host (requestor) to an unknown target host, an requestor entry is added. This will be used by ArpHandler to relay the ARP Reply back to this requestor once the target host replies.
- These target hosts  are saved and timed for up to 2sec. If the target host does not reply within up to 2sec, the host will be removed. This housekeeping process is implemented in a periodic timer task.

Change-Id: I85c6c1ef7c55845a76920ad1023b45ec6ac41974
Signed-off-by: Chi-Vien Ly <chivly@cisco.com>
opendaylight/arphandler/pom.xml
opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/Activator.java
opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java

index 62f93b235d4f3f3b0d45aa1fcc5c324dcbe31712..8df1f4cffa183fe88d49483f265256be60d8c0a9 100644 (file)
@@ -27,6 +27,7 @@
               org.opendaylight.controller.sal.utils,
               org.opendaylight.controller.sal.packet,
               org.opendaylight.controller.switchmanager,
+              org.opendaylight.controller.topologymanager,
               org.opendaylight.controller.hosttracker,
               org.opendaylight.controller.hosttracker.hostAware,
               org.apache.felix.dm,
       <groupId>org.opendaylight.controller</groupId>
       <artifactId>switchmanager</artifactId>
       <version>0.4.0-SNAPSHOT</version>
+    </dependency>
+     <dependency>
+      <groupId>org.opendaylight.controller</groupId>
+      <artifactId>topologymanager</artifactId>
+      <version>0.4.0-SNAPSHOT</version>
     </dependency>
     <dependency>
       <groupId>org.opendaylight.controller</groupId>
index e886813b4500d196165ed2cb53bf392b30882684..705ffbfa632d94237ee8c55d4f8eec753969d935 100644 (file)
@@ -22,6 +22,7 @@ import org.opendaylight.controller.sal.core.ComponentActivatorAbstractBase;
 import org.opendaylight.controller.sal.packet.IListenDataPacket;
 import org.opendaylight.controller.sal.packet.IDataPacketService;
 import org.opendaylight.controller.switchmanager.ISwitchManager;
+import org.opendaylight.controller.topologymanager.ITopologyManager;
 
 public class Activator extends ComponentActivatorAbstractBase {
     protected static final Logger logger = LoggerFactory
@@ -86,6 +87,10 @@ public class Activator extends ComponentActivatorAbstractBase {
                     "unsetSwitchManager").setRequired(true));
 
             c.add(createContainerServiceDependency(containerName).setService(
+                    ITopologyManager.class).setCallbacks("setTopologyManager",
+                    "unsetTopologyMananger").setRequired(true));
+
+           c.add(createContainerServiceDependency(containerName).setService(
                     IDataPacketService.class).setCallbacks(
                     "setDataPacketService", "unsetDataPacketService")
                     .setRequired(true));
index 26a86c53a4694ff45ca4fbdf2ff7dbc1a2721dcb..fd37b84d7af1d9c8026ca097f4efb9e57ce8182a 100644 (file)
@@ -18,6 +18,9 @@ import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -43,15 +46,20 @@ import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.controller.sal.utils.NetUtils;
 import org.opendaylight.controller.switchmanager.ISwitchManager;
 import org.opendaylight.controller.switchmanager.Subnet;
+import org.opendaylight.controller.topologymanager.ITopologyManager;
 
 public class ArpHandler implements IHostFinder, IListenDataPacket {
     private static final Logger logger = LoggerFactory
             .getLogger(ArpHandler.class);
     private IfIptoHost hostTracker = null;
     private ISwitchManager switchManager = null;
+    private ITopologyManager topologyManager;
     private IDataPacketService dataPacketService = null;
     private Set<IfHostListener> hostListener = Collections
             .synchronizedSet(new HashSet<IfHostListener>());
+    private ConcurrentHashMap<InetAddress, Set<HostNodeConnector>> arpRequestors;
+    private ConcurrentHashMap<InetAddress, Short> countDownTimers;
+    private Timer periodicTimer;
 
     void setHostListener(IfHostListener s) {
         if (this.hostListener != null) {
@@ -90,6 +98,16 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
             this.hostTracker = null;
         }
     }
+    
+    public void setTopologyManager(ITopologyManager tm) {
+        this.topologyManager = tm;
+    }
+
+    public void unsetTopologyManager(ITopologyManager tm) {
+        if (this.topologyManager == tm) {
+            this.topologyManager = null;
+        }
+    }
 
     protected void sendARPReply(NodeConnector p, byte[] sMAC, InetAddress sIP,
             byte[] tMAC, InetAddress tIP) {
@@ -182,13 +200,13 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
             return;
         }
 
+        HostNodeConnector requestor = null;
         if (isUnicastMAC(sourceMAC)) {
             // TODO For not this is only OPENFLOW but we need to fix this
             if (p.getType().equals(
                     NodeConnector.NodeConnectorIDType.OPENFLOW)) {
-                HostNodeConnector host = null;
                 try {
-                    host = new HostNodeConnector(sourceMAC, sourceIP, p, subnet
+                    requestor = new HostNodeConnector(sourceMAC, sourceIP, p, subnet
                             .getVlan());
                 } catch (ConstructionException e) {
                     return;
@@ -197,27 +215,34 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
                  * Learn host from the received ARP REQ/REPLY, inform
                  * Host Tracker
                  */
-                logger.debug("Inform Host tracker of new host {}", host);
+                logger.debug("Inform Host tracker of new host {}", requestor.getNetworkAddress());
                 synchronized (this.hostListener) {
                     for (IfHostListener listener : this.hostListener) {
-                        listener.hostListener(host);
+                        listener.hostListener(requestor);
                     }
                 }
             }
         }
         /*
-         * No further action is needed if this is a gratuitous ARP
+         * Gratuitous ARP. If there are hosts (in arpRequestors) waiting for the
+         * ARP reply for this sourceIP, it's time to generate the reply and it
+         * to these hosts
          */
         if (sourceIP.equals(targetIP)) {
+            generateAndSendReply(sourceIP, sourceMAC);
             return;
         }
 
         /*
-         * No further action is needed if this is a ARP Reply
+         * ARP Reply. If there are hosts (in arpRequesttors) waiting for the ARP
+         * reply for this sourceIP, it's time to generate the reply and it to
+         * these hosts
          */
         if (pkt.getOpCode() != ARP.REQUEST) {
+            generateAndSendReply(sourceIP, sourceMAC);
             return;
         }
+    
         /*
          * ARP Request Handling:
          * If targetIP is the IP of the subnet, reply with ARP REPLY
@@ -241,6 +266,19 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
          */
         HostNodeConnector host = hostTracker.hostQuery(targetIP);
         if (host == null) {
+            // add the requestor to the list so that we can replay the reply
+            // when the host responds
+            if (requestor != null) {
+                Set<HostNodeConnector> requestorSet = arpRequestors
+                        .get(targetIP);
+                if ((requestorSet == null) || requestorSet.isEmpty()) {
+                    requestorSet = new HashSet<HostNodeConnector>();
+                    countDownTimers.put(targetIP, (short) 2); // set max timeout
+                                                              // to 2sec
+                }
+                requestorSet.add(requestor);
+                arpRequestors.put(targetIP, requestorSet);
+            }
             sendBcastARPRequest(targetIP, subnet);
             return;
         }
@@ -283,6 +321,9 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
             nodeConnectors = subnet.getNodeConnectors();
         }
         for (NodeConnector p : nodeConnectors) {
+               if (topologyManager.isInternal(p)) {
+                       continue;
+               }
             ARP arp = new ARP();
             byte[] senderIP = subnet.getNetworkAddress().getAddress();
             byte[] targetIPB = targetIP.getAddress();
@@ -309,7 +350,7 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
                     .setEtherType(EtherTypes.ARP.shortValue()).setPayload(arp);
 
             // TODO For now send port-by-port, see how to optimize to
-            // send to a bunch of port on the same node in a shoot
+            // send to multiple ports at once
             RawPacket destPkt = this.dataPacketService.encodeDataPacket(ethernet);
             destPkt.setOutgoingNodeConnector(p);
 
@@ -442,6 +483,8 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
      *
      */
     void init() {
+        arpRequestors = new ConcurrentHashMap<InetAddress, Set<HostNodeConnector>>();
+        countDownTimers = new ConcurrentHashMap<InetAddress, Short>();
     }
 
     /**
@@ -460,6 +503,7 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
      *
      */
     void start() {
+        startPeriodicTimer();
     }
 
     /**
@@ -469,6 +513,7 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
      *
      */
     void stop() {
+        cancelPeriodicTimer();
     }
 
     void setSwitchManager(ISwitchManager s) {
@@ -507,4 +552,54 @@ public class ArpHandler implements IHostFinder, IListenDataPacket {
         }
         return PacketResult.IGNORED;
     }
+    
+    private void startPeriodicTimer() {
+        this.periodicTimer = new Timer("ArpHandler Periodic Timer");
+        this.periodicTimer.scheduleAtFixedRate(new TimerTask() {
+            @Override
+            public void run() {
+                Set<InetAddress> targetIPs = countDownTimers.keySet();
+                Set<InetAddress> expiredTargets = new HashSet<InetAddress>();
+                for (InetAddress t : targetIPs) {
+                    short tick = countDownTimers.get(t);
+                    tick--;
+                    if (tick <= 0) {
+                        expiredTargets.add(t);
+                    } else {
+                        countDownTimers.replace(t, tick);
+                    }
+                }
+                for (InetAddress t : expiredTargets) {
+                    countDownTimers.remove(t);
+                    // remove the requestor(s) who have been waited for the ARP
+                    // reply from this target for more than 1sec
+                    arpRequestors.remove(t);
+                    logger.debug("{} didn't respond to ARP request", t);
+                }
+            }
+        }, 0, 1000);
+    }
+
+    private void cancelPeriodicTimer() {
+        if (this.periodicTimer != null) {
+            this.periodicTimer.cancel();
+        }
+    }
+
+    private void generateAndSendReply(InetAddress sourceIP, byte[] sourceMAC) {
+        Set<HostNodeConnector> hosts = arpRequestors.remove(sourceIP);
+        if ((hosts == null) || hosts.isEmpty()) {
+            return;
+        }
+        countDownTimers.remove(sourceIP);
+        for (HostNodeConnector host : hosts) {
+            logger.debug(
+                    "Sending ARP Reply with src {}/{}, target {}/{}",
+                    new Object[] { sourceMAC, sourceIP,
+                            host.getDataLayerAddressBytes(),
+                            host.getNetworkAddress() });
+            sendARPReply(host.getnodeConnector(), sourceMAC, sourceIP,
+                    host.getDataLayerAddressBytes(), host.getNetworkAddress());
+        }
+    }
 }