From 261f59df395a0b8e2ebb1259837526f60f64baa4 Mon Sep 17 00:00:00 2001 From: Chi-Vien Ly Date: Fri, 24 May 2013 16:24:55 -0700 Subject: [PATCH] Fix for bug 24. 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 --- opendaylight/arphandler/pom.xml | 6 + .../arphandler/internal/Activator.java | 5 + .../arphandler/internal/ArpHandler.java | 109 ++++++++++++++++-- 3 files changed, 113 insertions(+), 7 deletions(-) diff --git a/opendaylight/arphandler/pom.xml b/opendaylight/arphandler/pom.xml index 62f93b235d..8df1f4cffa 100644 --- a/opendaylight/arphandler/pom.xml +++ b/opendaylight/arphandler/pom.xml @@ -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, @@ -47,6 +48,11 @@ org.opendaylight.controller switchmanager 0.4.0-SNAPSHOT + + + org.opendaylight.controller + topologymanager + 0.4.0-SNAPSHOT org.opendaylight.controller diff --git a/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/Activator.java b/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/Activator.java index e886813b45..705ffbfa63 100644 --- a/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/Activator.java +++ b/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/Activator.java @@ -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)); diff --git a/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java b/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java index 26a86c53a4..fd37b84d7a 100644 --- a/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java +++ b/opendaylight/arphandler/src/main/java/org/opendaylight/controller/arphandler/internal/ArpHandler.java @@ -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 hostListener = Collections .synchronizedSet(new HashSet()); + private ConcurrentHashMap> arpRequestors; + private ConcurrentHashMap 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 requestorSet = arpRequestors + .get(targetIP); + if ((requestorSet == null) || requestorSet.isEmpty()) { + requestorSet = new HashSet(); + 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>(); + countDownTimers = new ConcurrentHashMap(); } /** @@ -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 targetIPs = countDownTimers.keySet(); + Set expiredTargets = new HashSet(); + 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 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()); + } + } } -- 2.36.6