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
diff --git a/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java b/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
index 8ed4a61..1762b13 100644
--- a/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
+++ b/core/api/src/main/java/org/onosproject/net/proxyarp/ProxyArpService.java
@@ -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 @@
* @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 @@
* 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.
diff --git a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
index 2bb9c4f..90bc15e 100644
--- a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
+++ b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
@@ -116,7 +116,7 @@
}
@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 @@
}
if (src == null || dst == null) {
- flood(eth);
+ flood(eth, inPort);
return;
}
@@ -263,7 +263,7 @@
}
@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 @@
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 @@
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 @@
* 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;
}
diff --git a/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java b/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
index a80f0a1..a349738f 100644
--- a/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/proxyarp/impl/ProxyArpManagerTest.java
@@ -239,7 +239,7 @@
}
/**
- * 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 @@
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 @@
.andReturn(Sets.newHashSet(host1, host2));
replay(hostService);
- assertTrue(proxyArp.known(IP1));
+ assertTrue(proxyArp.isKnown(IP1));
}
/**
@@ -314,7 +314,7 @@
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 @@
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 @@
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 @@
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 @@
}
});
- 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));