Fix for ARPs being broadcast out the input port
diff --git a/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java b/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
index 430d05d..bd51fdc 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/forwarding/Forwarding.java
@@ -233,7 +233,8 @@
// actually ARP before broadcasting, so we can trick it into sending
// our non-ARP packets.
// TODO This should be refactored later to account for the new use case.
- datagrid.sendArpRequest(ArpMessage.newRequest(targetAddress, eth.serialize()));
+ datagrid.sendArpRequest(ArpMessage.newRequest(targetAddress, eth.serialize(),
+ sw.getId(), pi.getInPort()));
}
private void handlePacketIn(IOFSwitch sw, OFPacketIn pi, Ethernet eth) {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpMessage.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpMessage.java
index ee8f23d..44b9ea0 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpMessage.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpMessage.java
@@ -4,38 +4,40 @@
import java.net.InetAddress;
import net.floodlightcontroller.util.MACAddress;
+// TODO This is getting very messy!!! Needs refactoring
public class ArpMessage implements Serializable {
- /**
- *
- */
private static final long serialVersionUID = 1L;
private final Type type;
private final InetAddress forAddress;
private final byte[] packetData;
- //ARP reply message needs MAC info
+ // ARP reply message needs MAC info
private final MACAddress mac;
- //only send the ARP request message to the device attachment needs the attachment switch and port.
+ // Only send the ARP request message to the device attachment needs the
+ // attachment switch and port.
private final long outSwitch;
private final short outPort;
-
+ private final long inSwitch;
+ private final short inPort;
public enum Type {
REQUEST,
REPLY
}
- private ArpMessage(Type type, InetAddress address, byte[] eth) {
- // TODO Auto-generated constructor stub
+ private ArpMessage(Type type, InetAddress address, byte[] eth,
+ long outSwitch, short outPort, long inSwitch, short inPort) {
this.type = type;
this.forAddress = address;
this.packetData = eth;
this.mac = null;
this.outSwitch = -1;
this.outPort = -1;
+ this.inSwitch = inSwitch;
+ this.inPort = inPort;
}
private ArpMessage(Type type, InetAddress address) {
@@ -46,6 +48,8 @@
this.outSwitch = -1;
this.outPort = -1;
+ this.inSwitch = -1;
+ this.inPort = -1;
}
// the ARP reply message with MAC
private ArpMessage(Type type, InetAddress address, MACAddress mac) {
@@ -55,6 +59,9 @@
this.mac = mac;
this.outSwitch = -1;
this.outPort = -1;
+
+ this.inSwitch = -1;
+ this.inPort = -1;
}
// construct ARP request message with attachment switch and port
@@ -66,24 +73,32 @@
this.mac = null;
this.outSwitch = outSwitch;
this.outPort = outPort;
+
+ this.inSwitch = -1;
+ this.inPort = -1;
}
- public static ArpMessage newRequest(InetAddress forAddress, byte[] arpRequest) {
- return new ArpMessage(Type.REQUEST, forAddress, arpRequest);
+ // TODO Awful quick fix - caller has to supply dummy outSwitch and outPort
+ public static ArpMessage newRequest(InetAddress forAddress, byte[] arpRequest,
+ long outSwitch, short outPort, long inSwitch, short inPort) {
+ return new ArpMessage(Type.REQUEST, forAddress, arpRequest,
+ outSwitch, outPort, inSwitch, inPort);
}
public static ArpMessage newReply(InetAddress forAddress) {
return new ArpMessage(Type.REPLY, forAddress);
}
+
//ARP reply message with MAC
public static ArpMessage newReply(InetAddress forAddress, MACAddress mac) {
return new ArpMessage(Type.REPLY, forAddress, mac);
-
}
- //ARP reqsuest message with attachment switch and port
- public static ArpMessage newRequest(InetAddress forAddress, byte[] arpRequest, long outSwitch, short outPort ) {
- return new ArpMessage(Type.REQUEST, forAddress, arpRequest, outSwitch, outPort);
-
+
+ //ARP request message with attachment switch and port
+ public static ArpMessage newRequest(InetAddress forAddress,
+ byte[] arpRequest, long outSwitch, short outPort ) {
+ return new ArpMessage(Type.REQUEST, forAddress, arpRequest, outSwitch,
+ outPort);
}
public Type getType() {
@@ -97,6 +112,7 @@
public byte[] getPacket() {
return packetData;
}
+
public MACAddress getMAC() {
return mac;
}
@@ -108,5 +124,12 @@
public short getOutPort() {
return outPort;
}
+
+ public long getInSwitch() {
+ return inSwitch;
+ }
+ public short getInPort() {
+ return inPort;
+ }
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
index eadbbdd..5d09f96 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
@@ -363,7 +363,7 @@
if(!outPorts.iterator().hasNext()){
log.debug("outPort : null");
- sendToOtherNodes(eth, pi);
+ sendToOtherNodes(eth, sw.getId(), pi);
}else{
for (IPortObject portObject : outPorts) {
@@ -391,7 +391,7 @@
log.debug("The Device info in DB is {} for IP {}", targetDevice, inetAddressToString(arp.getTargetProtocolAddress()));
// We don't know the device so broadcast the request out
- sendToOtherNodes(eth, pi);
+ sendToOtherNodes(eth, sw.getId(), pi);
}
}
@@ -516,7 +516,7 @@
}
}
- private void sendToOtherNodes(Ethernet eth, OFPacketIn pi) {
+ private void sendToOtherNodes(Ethernet eth, long inSwitchId, OFPacketIn pi) {
ARP arp = (ARP) eth.getPayload();
if (log.isTraceEnabled()) {
@@ -532,7 +532,8 @@
return;
}
- datagrid.sendArpRequest(ArpMessage.newRequest(targetAddress, eth.serialize()));
+ datagrid.sendArpRequest(ArpMessage.newRequest(targetAddress, eth.serialize(),
+ -1L, (short)-1, inSwitchId, pi.getInPort()));
}
//hazelcast to other ONOS instances to send the ARP packet out on outPort of outSwitch
private void sendToOtherNodes(Ethernet eth, OFPacketIn pi, long outSwitch, short outPort) {
@@ -628,7 +629,8 @@
}
}
- private void broadcastArpRequestOutMyEdge(byte[] arpRequest) {
+ private void broadcastArpRequestOutMyEdge(byte[] arpRequest,
+ long inSwitch, short inPort) {
List<SwitchPort> switchPorts = new ArrayList<SwitchPort>();
for (IOFSwitch sw : floodlightProvider.getSwitches().values()) {
@@ -648,9 +650,17 @@
for (IPortObject portObject : ports) {
if (!portObject.getLinkedPorts().iterator().hasNext()) {
+ short portNumber = portObject.getNumber();
+
+ if (sw.getId() == inSwitch && portNumber == inPort) {
+ // This is the port that the ARP message came in,
+ // so don't broadcast out this port
+ continue;
+ }
+
switchPorts.add(new SwitchPort(new Dpid(sw.getId()),
- new Port(portObject.getNumber())));
- actions.add(new OFActionOutput(portObject.getNumber()));
+ new Port(portNumber)));
+ actions.add(new OFActionOutput(portNumber));
}
}
@@ -814,7 +824,8 @@
switch (arpMessage.getType()){
case REQUEST:
if(arpMessage.getOutSwitch() == -1 || arpMessage.getOutPort() == -1){
- broadcastArpRequestOutMyEdge(arpMessage.getPacket());
+ broadcastArpRequestOutMyEdge(arpMessage.getPacket(),
+ arpMessage.getInSwitch(), arpMessage.getInPort());
}else{
sendArpRequestOutPort(arpMessage.getPacket(),arpMessage.getOutSwitch(),arpMessage.getOutPort());
log.debug("OutSwitch in ARP request message is: {}; OutPort in ARP request message is: {}",arpMessage.getOutSwitch(),arpMessage.getOutPort());