Improved ARP interface to other app modules and changed SDNIP full-mesh paths between peers to use learned MAC addresses
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
index 20c6a28..1474d02 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
@@ -1,5 +1,7 @@
package net.onrc.onos.ofcontroller.proxyarp;
+import java.net.InetAddress;
+
import net.floodlightcontroller.packet.ARP;
public class HostArpRequester implements IArpRequester {
@@ -8,7 +10,6 @@
private ARP arpRequest;
private long dpid;
private short port;
- //private long requestTime; //in ms
public HostArpRequester(IProxyArpService arpService, ARP arpRequest,
long dpid, short port) {
@@ -17,12 +18,11 @@
this.arpRequest = arpRequest;
this.dpid = dpid;
this.port = port;
- //this.requestTime = System.currentTimeMillis();
}
@Override
- public void arpResponse(byte[] mac) {
- arpService.sendArpReply(arpRequest, dpid, port, mac);
+ public void arpResponse(InetAddress ipAddress, byte[] macAddress) {
+ arpService.sendArpReply(arpRequest, dpid, port, macAddress);
}
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java
index 2a74944..90da2ba 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java
@@ -1,5 +1,7 @@
package net.onrc.onos.ofcontroller.proxyarp;
+import java.net.InetAddress;
+
public interface IArpRequester {
- public void arpResponse(byte[] mac);
+ public void arpResponse(InetAddress ipAddress, byte[] macAddress);
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
index 4632aba..2bb32f4 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
@@ -29,13 +29,11 @@
/**
* Tell the IProxyArpService to send an ARP request for the IP address.
* The request will be broadcast out all edge ports in the network.
- * As an optimization, the IProxyArpService will first check its cache and
- * return the MAC address if it is already known. If not, the request will be
- * sent and the callback will be called when the MAC address is known
- * (or if the request times out).
* @param ipAddress
* @param requester
+ * @param retry Whether to keep sending requests until the MAC is learnt
* @return
*/
- public byte[] sendArpRequest(InetAddress ipAddress, IArpRequester requester);
+ public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
+ boolean retry);
}
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 5c2a2b9..1c9e929 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
@@ -6,14 +6,12 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Timer;
import java.util.TimerTask;
-import java.util.concurrent.ConcurrentHashMap;
import net.floodlightcontroller.core.FloodlightContext;
import net.floodlightcontroller.core.IFloodlightProviderService;
@@ -36,6 +34,10 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimaps;
+import com.google.common.collect.SetMultimap;
+
public class ProxyArpManager implements IProxyArpService, IOFMessageListener {
private static Logger log = LoggerFactory.getLogger(ProxyArpManager.class);
@@ -50,31 +52,53 @@
protected Map<InetAddress, ArpTableEntry> arpTable;
//protected ConcurrentHashMap<InetAddress, Set<ArpRequest>> arpRequests;
- protected ConcurrentHashMap<InetAddress, ArpRequest> arpRequests;
+ //protected ConcurrentHashMap<InetAddress, ArpRequest> arpRequests;
+ protected SetMultimap<InetAddress, ArpRequest> arpRequests;
private class ArpRequest {
- private Set<IArpRequester> requesters;
+ //private Set<IArpRequester> requesters;
+ private IArpRequester requester;
+ private boolean retry;
private long requestTime;
- public ArpRequest(){
- this.requesters = new HashSet<IArpRequester>();
+ public ArpRequest(IArpRequester requester, boolean retry){
+ //this.requesters = new HashSet<IArpRequester>();
+ this.requester = requester;
+ this.retry = retry;
this.requestTime = System.currentTimeMillis();
}
+ public ArpRequest(ArpRequest old) {
+ this.requester = old.requester;
+ this.retry = old.retry;
+ this.requestTime = System.currentTimeMillis();
+ }
+
+ /*
public synchronized void addRequester(IArpRequester requester){
requestTime = System.currentTimeMillis();
requesters.add(requester);
}
+ */
- public boolean isExpired(){
+ public boolean isExpired() {
return (System.currentTimeMillis() - requestTime)
> IProxyArpService.ARP_REQUEST_TIMEOUT;
}
- public synchronized void dispatchReply(byte[] replyMacAddress){
+ public boolean shouldRetry() {
+ return retry;
+ }
+
+ public synchronized void dispatchReply(InetAddress ipAddress, byte[] replyMacAddress) {
+ log.debug("Dispatching reply for {} to {}", ipAddress.getHostAddress(),
+ requester);
+ requester.arpResponse(ipAddress, replyMacAddress);
+ /*
for (IArpRequester requester : requesters){
- requester.arpResponse(replyMacAddress);
+ requester.arpResponse(ipAddress, replyMacAddress);
}
+ */
}
}
@@ -86,12 +110,15 @@
arpTable = new HashMap<InetAddress, ArpTableEntry>();
//arpRequests = new ConcurrentHashMap<InetAddress, Set<ArpRequest>>();
- arpRequests = new ConcurrentHashMap<InetAddress, ArpRequest>();
+ //arpRequests = new ConcurrentHashMap<InetAddress, ArpRequest>();
+ arpRequests = Multimaps.synchronizedSetMultimap(
+ HashMultimap.<InetAddress, ArpRequest>create());
Timer arpRequestTimeoutTimer = new Timer();
arpRequestTimeoutTimer.scheduleAtFixedRate(new TimerTask() {
@Override
public void run() {
+ /*
synchronized (arpRequests) {
log.debug("Current have {} outstanding requests",
arpRequests.size());
@@ -110,11 +137,63 @@
}
}
}
+ */
+
+ //List<ArpRequest> retryList = new ArrayList<ArpRequest>();
+ SetMultimap<InetAddress, ArpRequest> retryList
+ = HashMultimap.<InetAddress, ArpRequest>create();
+
+ //Have to synchronize externally on the Multimap while using an iterator.
+ //But because we're using a synchronizedMultiMap we don't have to
+ //synchronize when adding.
+ synchronized (arpRequests) {
+ log.debug("Current have {} outstanding requests",
+ arpRequests.size());
+
+ Iterator<Map.Entry<InetAddress, ArpRequest>> it
+ = arpRequests.entries().iterator();
+
+ while (it.hasNext()) {
+ Map.Entry<InetAddress, ArpRequest> entry
+ = it.next();
+ ArpRequest request = entry.getValue();
+ if (request.isExpired()) {
+ log.debug("Cleaning expired ARP request for {}",
+ entry.getKey().getHostAddress());
+ //TODO retry here? Caller should specify whether we retry or not
+ //because we obviously won't retry received packets, only
+ //requests from other app modules.
+ it.remove();
+
+ //retryList.add(new ArpRequest(request));
+ if (request.shouldRetry()) {
+ retryList.put(entry.getKey(), request);
+ }
+ }
+ }
+ }
+
+ for (Map.Entry<InetAddress, Collection<ArpRequest>> entry
+ : retryList.asMap().entrySet()) {
+
+ InetAddress address = entry.getKey();
+
+ log.debug("Resending ARP request for {}", address.getHostAddress());
+
+ sendArpRequestForAddress(address);
+
+ for (ArpRequest request : entry.getValue()) {
+ arpRequests.put(address, new ArpRequest(request));
+ }
+ }
}
}, 0, ARP_REQUEST_TIMEOUT_THREAD_PERIOD);
}
- private void storeRequester(InetAddress address, IArpRequester requester) {
+ private void storeRequester(InetAddress address, IArpRequester requester,
+ boolean retry) {
+ arpRequests.put(address, new ArpRequest(requester, retry));
+ /*
synchronized (arpRequests) {
if (arpRequests.get(address) == null) {
arpRequests.put(address, new ArpRequest());
@@ -123,6 +202,7 @@
request.addRequester(requester);
}
+ */
}
@Override
@@ -190,11 +270,22 @@
return;
}
- storeRequester(target, new HostArpRequester(this, arp, sw.getId(),
- pi.getInPort()));
+ //storeRequester(target, new HostArpRequester(this, arp, sw.getId(),
+ // pi.getInPort()));
+ boolean shouldBroadcastRequest = false;
+ synchronized (arpRequests) {
+ if (!arpRequests.containsKey(target)) {
+ shouldBroadcastRequest = true;
+ }
+ arpRequests.put(target, new ArpRequest(
+ new HostArpRequester(this, arp, sw.getId(), pi.getInPort()), false));
+ }
+
//Flood the request out edge ports
- broadcastArpRequestOutEdge(pi.getPacketData(), sw.getId(), pi.getInPort());
+ if (shouldBroadcastRequest) {
+ broadcastArpRequestOutEdge(pi.getPacketData(), sw.getId(), pi.getInPort());
+ }
}
else {
//We know the address, so send a reply
@@ -205,8 +296,10 @@
}
protected void handleArpReply(IOFSwitch sw, OFPacketIn pi, ARP arp){
- log.debug("ARP reply recieved for {}",
- bytesToStringAddr(arp.getSenderProtocolAddress()));
+ log.debug("ARP reply recieved for {}, is {}, on {}/{}", new Object[] {
+ bytesToStringAddr(arp.getSenderProtocolAddress()),
+ HexString.toHexString(arp.getSenderHardwareAddress()),
+ HexString.toHexString(sw.getId()), pi.getInPort()});
updateArpTable(arp);
@@ -218,6 +311,18 @@
return;
}
+ Set<ArpRequest> requests = arpRequests.get(addr);
+
+ //Synchronize on the Multimap while using an iterator for one of the sets
+ synchronized (arpRequests) {
+ Iterator<ArpRequest> it = requests.iterator();
+ while (it.hasNext()) {
+ ArpRequest request = it.next();
+ it.remove();
+ request.dispatchReply(addr, arp.getSenderHardwareAddress());
+ }
+ }
+ /*
ArpRequest request = null;
synchronized (arpRequests) {
request = arpRequests.get(addr);
@@ -226,8 +331,9 @@
}
}
if (request != null && !request.isExpired()) {
- request.dispatchReply(arp.getSenderHardwareAddress());
+ request.dispatchReply(addr, arp.getSenderHardwareAddress());
}
+ */
/*
Set<ArpRequest> requests = arpRequests.get(addr);
@@ -256,12 +362,14 @@
ArpTableEntry arpEntry = arpTable.get(addr);
if (arpEntry == null){
+ log.debug("MAC for {} unknown", bytesToStringAddr(ipAddress));
return null;
}
if (System.currentTimeMillis() - arpEntry.getTimeLastSeen()
> ARP_ENTRY_TIMEOUT){
//Entry has timed out so we'll remove it and return null
+ log.debug("Timing out old ARP entry for {}", bytesToStringAddr(ipAddress));
arpTable.remove(addr);
return null;
}
@@ -294,7 +402,8 @@
private void sendArpRequestForAddress(InetAddress ipAddress) {
byte[] zeroIpv4 = {0x0, 0x0, 0x0, 0x0};
byte[] zeroMac = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0};
- byte[] broadcastMac = {(byte)0xff, (byte)0xff, (byte)0xff, (byte)0xff,
+ byte[] bgpdMac = {0x0, 0x0, 0x0, 0x0, 0x0, 0x01};
+ byte[] broadcastMac = {(byte)0xff, (byte)0xff, (byte)0xff,
(byte)0xff, (byte)0xff, (byte)0xff};
ARP arpRequest = new ARP();
@@ -304,14 +413,15 @@
.setHardwareAddressLength((byte)Ethernet.DATALAYER_ADDRESS_LENGTH)
.setProtocolAddressLength((byte)4) //can't find the constant anywhere
.setOpCode(ARP.OP_REQUEST)
- .setSenderHardwareAddress(zeroMac)
+ .setSenderHardwareAddress(bgpdMac)
.setSenderProtocolAddress(zeroIpv4)
.setTargetHardwareAddress(zeroMac)
.setTargetProtocolAddress(ipAddress.getAddress());
Ethernet eth = new Ethernet();
- eth.setDestinationMACAddress(arpRequest.getSenderHardwareAddress())
- .setSourceMACAddress(broadcastMac)
+ //eth.setDestinationMACAddress(arpRequest.getSenderHardwareAddress())
+ eth.setSourceMACAddress(bgpdMac)
+ .setDestinationMACAddress(broadcastMac)
.setEtherType(Ethernet.TYPE_ARP)
.setPayload(arpRequest);
@@ -433,21 +543,23 @@
else return addr.getHostAddress();
}
-
+ @Override
public byte[] getMacAddress(InetAddress ipAddress) {
return lookupArpTable(ipAddress.getAddress());
}
-
- public byte[] sendArpRequest(InetAddress ipAddress, IArpRequester requester) {
+
+ @Override
+ public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
+ boolean retry) {
+ /*
byte[] lookupMac;
if ((lookupMac = lookupArpTable(ipAddress.getAddress())) == null) {
return lookupMac;
}
+ */
+
+ storeRequester(ipAddress, requester, retry);
sendArpRequestForAddress(ipAddress);
-
- storeRequester(ipAddress, requester);
-
- return null;
}
}