Don't flood ARP packets out the port they came in on.

Also renamed ProxyArpService#known(Ip4Address) to ProxyArpService#isKnown(Ip4Address) Fixes ONOS-722. Change-Id: I136c65e58693926e87b822cb0f4ec1c4ba0e3780

Don't flood ARP packets out the port they came in on.
Also renamed ProxyArpService#known(Ip4Address) to ProxyArpService#isKnown(Ip4Address) Fixes ONOS-722. Change-Id: I136c65e58693926e87b822cb0f4ec1c4ba0e3780
Jonathan Hart · Gerrit Code Review
Commit f84591d3facc79f9e67cc3fd97a3b8ae413a1cd8 f84591d3 1 parent 422916f1
Showing 3 changed files with 26 additions and 22 deletions
core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
--- a/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
View file @f84591d
+++ b/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
View file @f84591d
@@ -15,10 +15,10 @@
  */
 package org.onosproject.net.proxyarp;
-import org.onosproject.net.ConnectPoint;
-import org.onosproject.net.packet.PacketContext;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.Ip4Address;
+import org.onosproject.net.ConnectPoint;
+import org.onosproject.net.packet.PacketContext;
 /**
  * Service for processing arp requests on behalf of applications.
@@ -32,7 +32,7 @@ public interface ProxyArpService {
      * @param addr an IPv4 address
      * @return true if know, false otherwise
      */
-    boolean known(Ip4Address addr);
+    boolean isKnown(Ip4Address addr);
     /**
      * Sends a reply for a given request. If the host is not known then the arp
@@ -48,8 +48,9 @@ public interface ProxyArpService {
      * destination is not known.
      *
      * @param eth an ethernet frame containing an ARP request.
+     * @param inPort the port the request was received on
      */
-    void forward(Ethernet eth);
+    void forward(Ethernet eth, ConnectPoint inPort);
     /**
      * Handles a arp packet.
--- a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
View file @f84591d
+++ b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
View file @f84591d
@@ -116,7 +116,7 @@ public class ProxyArpManager implements ProxyArpService {
     }
     @Override
-    public boolean known(Ip4Address addr) {
+    public boolean isKnown(Ip4Address addr) {
         checkNotNull(addr, MAC_ADDR_NULL);
         Set<Host> hosts = hostService.getHostsByIp(addr);
         return !hosts.isEmpty();
@@ -189,7 +189,7 @@ public class ProxyArpManager implements ProxyArpService {
         }
         if (src == null || dst == null) {
-            flood(eth);
+            flood(eth, inPort);
             return;
         }
@@ -263,7 +263,7 @@ public class ProxyArpManager implements ProxyArpService {
     }
     @Override
-    public void forward(Ethernet eth) {
+    public void forward(Ethernet eth, ConnectPoint inPort) {
         checkNotNull(eth, REQUEST_NULL);
         checkArgument(eth.getEtherType() == Ethernet.TYPE_ARP,
                 REQUEST_NOT_ARP);
@@ -274,7 +274,7 @@ public class ProxyArpManager implements ProxyArpService {
                 VlanId.vlanId(eth.getVlanID())));
         if (h == null) {
-            flood(eth);
+            flood(eth, inPort);
         } else {
             TrafficTreatment.Builder builder = DefaultTrafficTreatment.builder();
             builder.setOutput(h.location().port());
@@ -291,7 +291,7 @@ public class ProxyArpManager implements ProxyArpService {
         if (ethPkt != null && ethPkt.getEtherType() == Ethernet.TYPE_ARP) {
             ARP arp = (ARP) ethPkt.getPayload();
             if (arp.getOpCode() == ARP.OP_REPLY) {
-                forward(ethPkt);
+                forward(ethPkt, context.inPacket().receivedFrom());
             } else if (arp.getOpCode() == ARP.OP_REQUEST) {
                 reply(ethPkt, context.inPacket().receivedFrom());
             }
@@ -305,14 +305,14 @@ public class ProxyArpManager implements ProxyArpService {
      * Flood the arp request at all edges in the network.
      * @param request the arp request.
      */
-    private void flood(Ethernet request) {
+    private void flood(Ethernet request, ConnectPoint inPort) {
         TrafficTreatment.Builder builder = null;
         ByteBuffer buf = ByteBuffer.wrap(request.serialize());
         synchronized (externalPorts) {
             for (Entry<Device, PortNumber> entry : externalPorts.entries()) {
                 ConnectPoint cp = new ConnectPoint(entry.getKey().id(), entry.getValue());
-                if (isOutsidePort(cp)) {
+                if (isOutsidePort(cp) || cp.equals(inPort)) {
                     continue;
                 }
--- a/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
View file @f84591d
+++ b/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
View file @f84591d
@@ -239,7 +239,7 @@ public class ProxyArpManagerTest {
     }
     /**
-     * Tests {@link ProxyArpManager#known(Ip4Address)} in the case where the
+     * Tests {@link ProxyArpManager#isKnown(Ip4Address)} in the case where the
      * IP address is not known.
      * Verifies the method returns false.
      */
@@ -248,11 +248,11 @@ public class ProxyArpManagerTest {
         expect(hostService.getHostsByIp(IP1)).andReturn(Collections.<Host>emptySet());
         replay(hostService);
-        assertFalse(proxyArp.known(IP1));
+        assertFalse(proxyArp.isKnown(IP1));
     }
     /**
-     * Tests {@link ProxyArpManager#known(Ip4Address)} in the case where the
+     * Tests {@link ProxyArpManager#isKnown(Ip4Address)} in the case where the
      * IP address is known.
      * Verifies the method returns true.
      */
@@ -265,7 +265,7 @@ public class ProxyArpManagerTest {
                 .andReturn(Sets.newHashSet(host1, host2));
         replay(hostService);
-        assertTrue(proxyArp.known(IP1));
+        assertTrue(proxyArp.isKnown(IP1));
     }
     /**
@@ -314,7 +314,7 @@ public class ProxyArpManagerTest {
         Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, IP2, IP1);
-        proxyArp.reply(arpRequest, getLocation(5));
+        proxyArp.reply(arpRequest, getLocation(6));
         verifyFlood(arpRequest);
     }
@@ -341,7 +341,7 @@ public class ProxyArpManagerTest {
         Ethernet arpRequest = buildArp(ARP.OP_REQUEST, MAC2, null, IP2, IP1);
-        proxyArp.reply(arpRequest, getLocation(5));
+        proxyArp.reply(arpRequest, getLocation(6));
         verifyFlood(arpRequest);
     }
@@ -435,7 +435,7 @@ public class ProxyArpManagerTest {
         Ethernet arpRequest = buildArp(ARP.OP_REPLY, MAC2, MAC1, IP2, IP1);
-        proxyArp.forward(arpRequest);
+        proxyArp.forward(arpRequest, LOC2);
         assertEquals(1, packetService.packets.size());
         OutboundPacket packet = packetService.packets.get(0);
@@ -455,18 +455,20 @@ public class ProxyArpManagerTest {
         Ethernet arpRequest = buildArp(ARP.OP_REPLY, MAC2, MAC1, IP2, IP1);
-        proxyArp.forward(arpRequest);
+        proxyArp.forward(arpRequest, getLocation(6));
         verifyFlood(arpRequest);
     }
     /**
-     * Verifies that the given packet was flooded out all available edge ports.
+     * Verifies that the given packet was flooded out all available edge ports,
+     * except for the input port.
      *
      * @param packet the packet that was expected to be flooded
      */
     private void verifyFlood(Ethernet packet) {
-        assertEquals(NUM_FLOOD_PORTS, packetService.packets.size());
+        // There should be 1 less than NUM_FLOOD_PORTS; the inPort should be excluded.
+        assertEquals(NUM_FLOOD_PORTS - 1, packetService.packets.size());
         Collections.sort(packetService.packets,
             new Comparator<OutboundPacket>() {
@@ -476,7 +478,8 @@ public class ProxyArpManagerTest {
                 }
             });
-        for (int i = 0; i < NUM_FLOOD_PORTS; i++) {
+
+        for (int i = 0; i < NUM_FLOOD_PORTS - 1; i++) {
             ConnectPoint cp = new ConnectPoint(getDeviceId(NUM_ADDRESS_PORTS + i + 1),
                     PortNumber.portNumber(1));