Bug 4144 - Periodic ARP resolver should use unicast MAC destination

address whenever possible; instead of broadcast

Modified ArpResolver and GatewayMacResolverService to use the unicast DA
when known.  The code treats the DA as known for a new ARP request if
one of the last N=2 ARP request was answered.

Change-Id: I2bd7791b74d41e2838edf59d6edc510b63f3c40b
Signed-off-by: Andre Fredette <afredette@redhat.com>

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpResolverMetadata.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpResolverMetadata.java
index 0ac0bc0884650f8d253a9b110e7b7069828575d6..aed4a647fe5732fc976215e32dc32cde994eab64 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpResolverMetadata.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpResolverMetadata.java
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13.services.arp;
 
+import org.opendaylight.controller.liblldp.NetUtils;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.Ipv4Address;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev100924.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.RemoveFlowInput;
@@ -26,6 +27,9 @@ public final class ArpResolverMetadata {
     private final boolean periodicRefresh;
     private RemoveFlowInput flowToRemove;
     private MacAddress gatewayMacAddress;
+    private boolean gatewayMacAddressResolved;
+    private int numberOfOutstandingArpRequests;
+    private static final int MAX_OUTSTANDING_ARP_REQUESTS = 2;
 
     public ArpResolverMetadata(final Long externalNetworkBridgeDpid,
             final Ipv4Address gatewayIpAddress, final Ipv4Address arpRequestSourceIp,
@@ -35,6 +39,9 @@ public final class ArpResolverMetadata {
         this.arpRequestSourceIp = arpRequestSourceIp;
         this.arpRequestSourceMacAddress = arpRequestMacAddress;
         this.periodicRefresh = periodicRefresh;
+        this.gatewayMacAddress = null;
+        this.gatewayMacAddressResolved = false;
+        this.numberOfOutstandingArpRequests = 0;
     }
 
     public RemoveFlowInput getFlowToRemove() {
@@ -53,6 +60,12 @@ public final class ArpResolverMetadata {
         return gatewayMacAddress;
     }
     public void setGatewayMacAddress(MacAddress gatewayMacAddress) {
+        if (gatewayMacAddress != null) {
+            gatewayMacAddressResolved = true;
+            numberOfOutstandingArpRequests = 0;
+        } else {
+            gatewayMacAddressResolved = false;
+        }
         this.gatewayMacAddress = gatewayMacAddress;
     }
 
@@ -62,10 +75,39 @@ public final class ArpResolverMetadata {
     public Ipv4Address getArpRequestSourceIp() {
         return arpRequestSourceIp;
     }
-    public MacAddress getArpRequestMacAddress() {
+    public MacAddress getArpRequestSourceMacAddress() {
         return arpRequestSourceMacAddress;
     }
 
+    /**
+     * This method is used to determine whether to use the broadcast MAC or the unicast MAC as the destination address
+     * for an ARP request packet based on whether one of the last MAX_OUTSTANDING_ARP_REQUESTS requests has been
+     * answered.
+     *
+     * A counter (numberOfOutstandingArpRequests) is maintained to track outstanding ARP requests.  This counter is
+     * incremented in this method and reset when setGatewayMacAddress() is called with an updated MAC address after an
+     * ARP reply is received. It is therefore expected that this method be called exactly once for each ARP request
+     * event, and not be called for other reasons, or it may result in more broadcast ARP request packets being sent
+     * than needed.
+     *
+     * @return Destination MAC address to be used in ARP request packet:  Either the unicast MAC or the broadcast MAC
+     * as described above.
+     */
+    public MacAddress getArpRequestDestMacAddress() {
+
+        numberOfOutstandingArpRequests++;
+
+        if (numberOfOutstandingArpRequests > MAX_OUTSTANDING_ARP_REQUESTS) {
+            gatewayMacAddressResolved = false;
+        }
+
+        if (gatewayMacAddressResolved) {
+            return gatewayMacAddress;
+        } else {
+            return ArpUtils.bytesToMac(NetUtils.getBroadcastMACAddr());
+        }
+    }
+
     @Override
     public int hashCode() {
         final int prime = 31;

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpSender.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpSender.java
index 1030915e53a35df66f7f6e9267f9d335c4cf3189..8abc1251f3cfba1e11c5175964b2e8bccfdb34a0 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpSender.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/ArpSender.java
@@ -17,6 +17,7 @@ import org.opendaylight.controller.liblldp.Ethernet;
 import org.opendaylight.controller.liblldp.NetUtils;
 import org.opendaylight.controller.liblldp.PacketException;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev100924.Ipv4Address;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev100924.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRef;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
@@ -69,7 +70,7 @@ public class ArpSender {
         NodeConnectorKey nodeConnectorKey = new NodeConnectorKey(createNodeConnectorId(OFPP_ALL,
                 nodeIid.firstKeyOf(Node.class, NodeKey.class).getId()));
         InstanceIdentifier<NodeConnector> egressNc = nodeIid.child(NodeConnector.class, nodeConnectorKey);
-        return sendArp(senderAddress, tpa, egressNc);
+        return sendArp(senderAddress, tpa, null, egressNc);
     }
 
     private NodeConnectorId createNodeConnectorId(String connectorId, NodeId nodeId) {
@@ -82,15 +83,15 @@ public class ArpSender {
      * @param senderAddress the addresses used in sender part of ARP packet
      * @param tpa the target protocol address, in this case IPv4 address for which MAC should be
      *        discovered
-     * @param egressNc the path to node connector from where the ARP packet will be sent
-     * @return future result about success of packet-out
+     * @param arpRequestDestMacAddress the destination MAC address to be used in the ARP packet or null if not known.
+     * @param egressNc the path to node connector from where the ARP packet will be sent  @return future result about success of packet-out
      */
     public ListenableFuture<RpcResult<Void>> sendArp(ArpMessageAddress senderAddress, Ipv4Address tpa,
-            InstanceIdentifier<NodeConnector> egressNc) {
+                                                     MacAddress arpRequestDestMacAddress, InstanceIdentifier<NodeConnector> egressNc) {
         checkNotNull(senderAddress);
         checkNotNull(tpa);
         checkNotNull(egressNc);
-        final Ethernet arpFrame = createArpFrame(senderAddress, tpa);
+        final Ethernet arpFrame = createArpFrame(senderAddress, tpa, arpRequestDestMacAddress);
         byte[] arpFrameAsBytes;
         try {
             arpFrameAsBytes = arpFrame.serialize();
@@ -113,10 +114,15 @@ public class ArpSender {
         return JdkFutureAdapters.listenInPoolThread(futureTransmitPacketResult);
     }
 
-    private Ethernet createArpFrame(ArpMessageAddress senderAddress, Ipv4Address tpa) {
+    private Ethernet createArpFrame(ArpMessageAddress senderAddress, Ipv4Address tpa, MacAddress arpRequestDestMacAddress) {
         byte[] senderMac = ArpUtils.macToBytes(senderAddress.getHardwareAddress());
         byte[] senderIp = ArpUtils.ipToBytes(senderAddress.getProtocolAddress());
-        byte[] targetMac = NetUtils.getBroadcastMACAddr();
+        byte[] targetMac;
+        if (arpRequestDestMacAddress != null) {
+            targetMac = ArpUtils.macToBytes(arpRequestDestMacAddress);
+        } else {
+            targetMac = NetUtils.getBroadcastMACAddr();
+        }
         byte[] targetIp = ArpUtils.ipToBytes(tpa);
         Ethernet arpFrame = new Ethernet().setSourceMACAddress(senderMac)
             .setDestinationMACAddress(targetMac)

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/GatewayMacResolverService.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/GatewayMacResolverService.java
index 7c2cfbebae8a3ab589561b4d89ec27bd831f686b..54789bbd7dd1774a936bd84ecada30cb5cdfdf14 100644 (file)
--- a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/GatewayMacResolverService.java
+++ b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/services/arp/GatewayMacResolverService.java
@@ -89,7 +89,7 @@ public class GatewayMacResolverService extends AbstractServiceInstance
                                         implements ConfigInterface, GatewayMacResolver,PacketProcessingListener {
 
     private static final Logger LOG = LoggerFactory.getLogger(GatewayMacResolverService.class);
-    private static final short TABEL_FOR_ARP_FLOW = 0;
+    private static final short TABLE_FOR_ARP_FLOW = 0;
     private static final String ARP_REPLY_TO_CONTROLLER_FLOW_NAME = "GatewayArpReplyRouter";
     private static final int ARP_REPLY_TO_CONTROLLER_FLOW_PRIORITY = 10000;
     private static final Instruction SEND_TO_CONTROLLER_INSTRUCTION;
@@ -157,9 +157,9 @@ public class GatewayMacResolverService extends AbstractServiceInstance
                                     }
 
                                     LOG.debug("Refresh Gateway Mac for gateway {} using source ip {} and mac {} for ARP request",
-                                            gatewayIp.getValue(),gatewayMetaData.getArpRequestSourceIp().getValue(),gatewayMetaData.getArpRequestMacAddress().getValue());
+                                            gatewayIp.getValue(),gatewayMetaData.getArpRequestSourceIp().getValue(),gatewayMetaData.getArpRequestSourceMacAddress().getValue());
 
-                                    sendGatewayArpRequest(externalNetworkBridge,gatewayIp,gatewayMetaData.getArpRequestSourceIp(), gatewayMetaData.getArpRequestMacAddress());
+                                    sendGatewayArpRequest(externalNetworkBridge,gatewayIp,gatewayMetaData.getArpRequestSourceIp(), gatewayMetaData.getArpRequestSourceMacAddress());
                                 }
                             }, 1, TimeUnit.SECONDS);
                         }
@@ -256,16 +256,20 @@ public class GatewayMacResolverService extends AbstractServiceInstance
                 }
                 LOG.debug("Flow to route ARP Reply to Controller installed successfully : {}", flowIid);
 
+                ArpResolverMetadata gatewayArpMetadata = gatewayToArpMetadataMap.get(gatewayIp);
+
                 //cache metadata
-                gatewayToArpMetadataMap.get(gatewayIp).setFlowToRemove(
-                        new RemoveFlowInputBuilder(arpReplyToControllerFlow).setNode(nodeRef).build());
+                gatewayArpMetadata.setFlowToRemove(new RemoveFlowInputBuilder(arpReplyToControllerFlow).setNode(nodeRef).build());
+
+                //get MAC DA for ARP packets
+                MacAddress arpRequestDestMacAddress = gatewayArpMetadata.getArpRequestDestMacAddress();
 
-                //Broadcast ARP request packets
+                //Send ARP request packets
                 for (NodeConnector egressNc : externalNetworkBridge.getNodeConnector()) {
                     KeyedInstanceIdentifier<NodeConnector, NodeConnectorKey> egressNcIid = nodeIid.child(
                             NodeConnector.class, new NodeConnectorKey(egressNc.getId()));
                     ListenableFuture<RpcResult<Void>> futureSendArpResult = arpSender.sendArp(
-                            senderAddress, gatewayIp, egressNcIid);
+                            senderAddress, gatewayIp, arpRequestDestMacAddress, egressNcIid);
                     Futures.addCallback(futureSendArpResult, logResult(gatewayIp, egressNcIid));
                 }
             }
@@ -304,7 +308,7 @@ public class GatewayMacResolverService extends AbstractServiceInstance
     private Flow createArpReplyToControllerFlow(final ArpMessageAddress senderAddress, final Ipv4Address ipForRequestedMac) {
         checkNotNull(senderAddress);
         checkNotNull(ipForRequestedMac);
-        FlowBuilder arpFlow = new FlowBuilder().setTableId(TABEL_FOR_ARP_FLOW)
+        FlowBuilder arpFlow = new FlowBuilder().setTableId(TABLE_FOR_ARP_FLOW)
             .setFlowName(ARP_REPLY_TO_CONTROLLER_FLOW_NAME)
             .setPriority(ARP_REPLY_TO_CONTROLLER_FLOW_PRIORITY)
             .setBufferId(OFConstants.OFP_NO_BUFFER)