Refactored the ARP cache logic out of ProxyArpManager and into its own class to make the code clearer. Moved to using MACAddress objects in the ProxyArpManager APIs and implementation which prevents us having to pass byte arrays around and are safer as MACAddress prevents a rogue client changing the MAC address for everyone
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 6e12f28..fe16144 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
@@ -5,7 +5,6 @@
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -44,21 +43,19 @@
public class ProxyArpManager implements IProxyArpService, IOFMessageListener {
private final static Logger log = LoggerFactory.getLogger(ProxyArpManager.class);
- private final long ARP_ENTRY_TIMEOUT = 600000; //ms (== 10 mins)
-
private final long ARP_TIMER_PERIOD = 60000; //ms (== 1 min)
- private IFloodlightProviderService floodlightProvider;
- private ITopologyService topology;
- private ILayer3InfoService layer3;
+ private final IFloodlightProviderService floodlightProvider;
+ private final ITopologyService topology;
+ private final ILayer3InfoService layer3;
- private Map<InetAddress, ArpTableEntry> arpTable;
+ private final ArpCache arpCache;
- private SetMultimap<InetAddress, ArpRequest> arpRequests;
+ private final SetMultimap<InetAddress, ArpRequest> arpRequests;
- private class ArpRequest {
+ private static class ArpRequest {
private final IArpRequester requester;
- private boolean retry;
+ private final boolean retry;
private long requestTime;
public ArpRequest(IArpRequester requester, boolean retry){
@@ -82,7 +79,7 @@
return retry;
}
- public void dispatchReply(InetAddress ipAddress, byte[] replyMacAddress) {
+ public void dispatchReply(InetAddress ipAddress, MACAddress replyMacAddress) {
requester.arpResponse(ipAddress, replyMacAddress);
}
}
@@ -93,7 +90,7 @@
this.topology = topology;
this.layer3 = layer3;
- arpTable = new HashMap<InetAddress, ArpTableEntry>();
+ arpCache = new ArpCache();
arpRequests = Multimaps.synchronizedSetMultimap(
HashMultimap.<InetAddress, ArpRequest>create());
@@ -226,16 +223,16 @@
target.getHostAddress(), layer3.getRouterMacAddress());
sendArpReply(arp, sw.getId(), pi.getInPort(),
- layer3.getRouterMacAddress().toBytes());
+ layer3.getRouterMacAddress());
}
return;
}
- byte[] mac = lookupArpTable(arp.getTargetProtocolAddress());
+ MACAddress macAddress = arpCache.lookup(target);
- if (mac == null){
- //Mac address is not in our arp table.
+ if (macAddress == null){
+ //MAC address is not in our ARP cache.
//Record where the request came from so we know where to send the reply
arpRequests.put(target, new ArpRequest(
@@ -249,11 +246,11 @@
if (log.isTraceEnabled()) {
log.trace("Sending reply: {} => {} to host at {}/{}", new Object [] {
inetAddressToString(arp.getTargetProtocolAddress()),
- MACAddress.valueOf(mac).toString(),
+ macAddress.toString(),
HexString.toHexString(sw.getId()), pi.getInPort()});
}
- sendArpReply(arp, sw.getId(), pi.getInPort(), mac);
+ sendArpReply(arp, sw.getId(), pi.getInPort(), macAddress);
}
}
@@ -265,18 +262,20 @@
HexString.toHexString(sw.getId()), pi.getInPort()});
}
- updateArpTable(arp);
-
- //See if anyone's waiting for this ARP reply
- InetAddress addr;
+ InetAddress senderIpAddress;
try {
- addr = InetAddress.getByAddress(arp.getSenderProtocolAddress());
+ senderIpAddress = InetAddress.getByAddress(arp.getSenderProtocolAddress());
} catch (UnknownHostException e) {
log.debug("Invalid address in ARP reply", e);
return;
}
- Set<ArpRequest> requests = arpRequests.get(addr);
+ MACAddress senderMacAddress = MACAddress.valueOf(arp.getSenderHardwareAddress());
+
+ arpCache.update(senderIpAddress, senderMacAddress);
+
+ //See if anyone's waiting for this ARP reply
+ Set<ArpRequest> requests = arpRequests.get(senderIpAddress);
//Synchronize on the Multimap while using an iterator for one of the sets
List<ArpRequest> requestsToSend = new ArrayList<ArpRequest>(requests.size());
@@ -291,57 +290,7 @@
//Don't hold an ARP lock while dispatching requests
for (ArpRequest request : requestsToSend) {
- request.dispatchReply(addr, arp.getSenderHardwareAddress());
- }
- }
-
- private synchronized byte[] lookupArpTable(byte[] ipAddress){
- InetAddress addr;
- try {
- addr = InetAddress.getByAddress(ipAddress);
- } catch (UnknownHostException e) {
- log.debug("Unable to create InetAddress", e);
- return null;
- }
-
- ArpTableEntry arpEntry = arpTable.get(addr);
-
- if (arpEntry == null){
- return null;
- }
-
- if (System.currentTimeMillis() - arpEntry.getTimeLastSeen()
- > ARP_ENTRY_TIMEOUT){
- //Entry has timed out so we'll remove it and return null
- log.trace("Removing expired ARP entry for {}",
- inetAddressToString(ipAddress));
-
- arpTable.remove(addr);
- return null;
- }
-
- return arpEntry.getMacAddress();
- }
-
- private synchronized void updateArpTable(ARP arp){
- InetAddress addr;
- try {
- addr = InetAddress.getByAddress(arp.getSenderProtocolAddress());
- } catch (UnknownHostException e) {
- log.debug("Unable to create InetAddress", e);
- return;
- }
-
- ArpTableEntry arpEntry = arpTable.get(addr);
-
- if (arpEntry != null
- && arpEntry.getMacAddress() == arp.getSenderHardwareAddress()){
- arpEntry.setTimeLastSeen(System.currentTimeMillis());
- }
- else {
- arpTable.put(addr,
- new ArpTableEntry(arp.getSenderHardwareAddress(),
- System.currentTimeMillis()));
+ request.dispatchReply(senderIpAddress, senderMacAddress);
}
}
@@ -514,11 +463,12 @@
* IProxyArpService methods
*/
- public void sendArpReply(ARP arpRequest, long dpid, short port, byte[] targetMac) {
+ @Override
+ public void sendArpReply(ARP arpRequest, long dpid, short port, MACAddress targetMac) {
if (log.isTraceEnabled()) {
log.trace("Sending reply {} => {} to {}", new Object[] {
inetAddressToString(arpRequest.getTargetProtocolAddress()),
- HexString.toHexString(targetMac),
+ targetMac,
inetAddressToString(arpRequest.getSenderProtocolAddress())});
}
@@ -528,14 +478,14 @@
.setHardwareAddressLength((byte)Ethernet.DATALAYER_ADDRESS_LENGTH)
.setProtocolAddressLength((byte)IPv4.ADDRESS_LENGTH)
.setOpCode(ARP.OP_REPLY)
- .setSenderHardwareAddress(targetMac)
+ .setSenderHardwareAddress(targetMac.toBytes())
.setSenderProtocolAddress(arpRequest.getTargetProtocolAddress())
.setTargetHardwareAddress(arpRequest.getSenderHardwareAddress())
.setTargetProtocolAddress(arpRequest.getSenderProtocolAddress());
Ethernet eth = new Ethernet();
eth.setDestinationMACAddress(arpRequest.getSenderHardwareAddress())
- .setSourceMACAddress(targetMac)
+ .setSourceMACAddress(targetMac.toBytes())
.setEtherType(Ethernet.TYPE_ARP)
.setPayload(arpReply);
@@ -571,8 +521,8 @@
}
@Override
- public byte[] getMacAddress(InetAddress ipAddress) {
- return lookupArpTable(ipAddress.getAddress());
+ public MACAddress getMacAddress(InetAddress ipAddress) {
+ return arpCache.lookup(ipAddress);
}
@Override