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/conf/cassandra-repair.sh b/conf/cassandra-repair.sh
new file mode 100755
index 0000000..2c2638e
--- /dev/null
+++ b/conf/cassandra-repair.sh
@@ -0,0 +1 @@
+<cassandra_dir>/bin/nodetool repair
diff --git a/conf/logback-deployment.xml b/conf/logback-deployment.xml
new file mode 100644
index 0000000..3877f65
--- /dev/null
+++ b/conf/logback-deployment.xml
@@ -0,0 +1,37 @@
+<configuration scan="true" debug="true">
+ <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+ <encoder>
+ <pattern>%level [%logger:%thread] %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <appender name="FILE" class="ch.qos.logback.core.FileAppender">
+ <file>./onos-logs/onos.ubuntu.log</file>
+ <encoder>
+ <pattern>%date %level [%thread] %logger{10} [%file:%line] %msg%n</pattern>
+ </encoder>
+ </appender>
+
+ <appender name="ROLLINGFILE" class="ch.qos.logback.core.rolling.RollingFileAppender">
+ <file>./onos-logs/onos.ubuntu.log</file>
+ <rollingPolicy class="ch.qos.logback.core.rolling.TimeBasedRollingPolicy">
+ <!-- Roll over to a new log file every day -->
+ <fileNamePattern>./onos-logs/onos.ubuntu.%d{yyyy-MM-dd}.log</fileNamePattern>
+ <!-- Keep 10 days worth of logs -->
+ <maxHistory>10</maxHistory>
+ </rollingPolicy>
+
+ <encoder>
+ <pattern>%date %level [%thread] %logger{10} [%file:%line] %msg%n</pattern>
+ </encoder>
+
+ </appender>
+
+ <logger name="org" level="WARN"/>
+ <logger name="LogService" level="WARN"/> <!-- Restlet access logging -->
+ <logger name="net.floodlightcontroller.logging" level="WARN"/>
+
+ <root level="DEBUG">
+ <appender-ref ref="ROLLINGFILE" />
+ </root>
+</configuration>
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
index 960d001..c70a2ea 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -429,7 +429,7 @@
InetAddress dstIpAddress = rib.getNextHop();
//See if we know the MAC address of the next hop
- byte[] nextHopMacAddress = proxyArp.getMacAddress(rib.getNextHop());
+ MACAddress nextHopMacAddress = proxyArp.getMacAddress(rib.getNextHop());
//Find the attachment point (egress interface) of the next hop
Interface egressInterface = null;
@@ -463,7 +463,7 @@
Path path = pushedPaths.get(dstIpAddress);
if (path == null) {
path = new Path(egressInterface, dstIpAddress);
- calculateAndPushPath(path, MACAddress.valueOf(nextHopMacAddress));
+ calculateAndPushPath(path, nextHopMacAddress);
pushedPaths.put(dstIpAddress, path);
}
@@ -476,9 +476,9 @@
}
}
- private void addPrefixFlows(Prefix prefix, Interface egressInterface, byte[] nextHopMacAddress) {
+ private void addPrefixFlows(Prefix prefix, Interface egressInterface, MACAddress nextHopMacAddress) {
log.debug("Adding flows for prefix {} added, next hop mac {}",
- prefix, HexString.toHexString(nextHopMacAddress));
+ prefix, nextHopMacAddress);
//We only need one flow mod per switch, so pick one interface on each switch
Map<Long, Interface> srcInterfaces = new HashMap<Long, Interface>();
@@ -533,7 +533,7 @@
//Set up MAC rewrite action
OFActionDataLayerDestination macRewriteAction = new OFActionDataLayerDestination();
- macRewriteAction.setDataLayerAddress(nextHopMacAddress);
+ macRewriteAction.setDataLayerAddress(nextHopMacAddress.toBytes());
//Set up output action
OFActionOutput outputAction = new OFActionOutput();
@@ -694,8 +694,8 @@
//See if we know the MAC address of the peer. If not we can't
//do anything until we learn it
- byte[] mac = proxyArp.getMacAddress(peer.getIpAddress());
- if (mac == null) {
+ MACAddress macAddress = proxyArp.getMacAddress(peer.getIpAddress());
+ if (macAddress == null) {
log.debug("Don't know MAC for {}", peer.getIpAddress().getHostAddress());
//Put in the pending paths list first
pathsWaitingOnArp.put(peer.getIpAddress(), path);
@@ -705,7 +705,7 @@
}
//If we know the MAC, lets go ahead and push the paths to this peer
- calculateAndPushPath(path, MACAddress.valueOf(mac));
+ calculateAndPushPath(path, macAddress);
}
}
@@ -959,9 +959,9 @@
}
@Override
- public void arpResponse(InetAddress ipAddress, byte[] macAddress) {
- log.debug("Received ARP response: {} => {}", ipAddress.getHostAddress(),
- MACAddress.valueOf(macAddress).toString());
+ public void arpResponse(InetAddress ipAddress, MACAddress macAddress) {
+ log.debug("Received ARP response: {} => {}",
+ ipAddress.getHostAddress(), macAddress);
/*
* We synchronize on this to prevent changes to the ptree while we're pushing
@@ -973,8 +973,7 @@
if (path != null) {
log.debug("Pushing path to {} at {} on {}", new Object[] {
- path.getDstIpAddress().getHostAddress(),
- MACAddress.valueOf(macAddress),
+ path.getDstIpAddress().getHostAddress(), macAddress,
path.getDstInterface().getSwitchPort()});
//These paths should always be to BGP peers. Paths to non-peers are
//handled once the first prefix is ready to push
@@ -986,7 +985,7 @@
}
}
else {
- calculateAndPushPath(path, MACAddress.valueOf(macAddress));
+ calculateAndPushPath(path, macAddress);
pushedPaths.put(path.getDstIpAddress(), path);
}
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCache.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCache.java
new file mode 100644
index 0000000..8b9c65a
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCache.java
@@ -0,0 +1,87 @@
+package net.onrc.onos.ofcontroller.proxyarp;
+
+import java.net.InetAddress;
+import java.util.HashMap;
+import java.util.Map;
+
+import net.floodlightcontroller.util.MACAddress;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Implements a basic ARP cache which maps IPv4 addresses to MAC addresses.
+ * Mappings time out after a short period of time (currently 1 min). We don't
+ * try and refresh the mapping before the entry times out because as a controller
+ * we don't know if the mapping is still needed.
+ *
+ */
+
+/* TODO clean out old ARP entries out of the cache periodically. We currently
+ * don't do this which means the cache memory size will never decrease. We already
+ * have a periodic thread that can be used to do this in ProxyArpManager.
+ */
+public class ArpCache {
+ private final static Logger log = LoggerFactory.getLogger(ArpCache.class);
+
+ private final long ARP_ENTRY_TIMEOUT = 60000; //ms (1 min)
+
+ //Protected by locking on the ArpCache object
+ private final Map<InetAddress, ArpCacheEntry> arpCache;
+
+ private static class ArpCacheEntry {
+ private final MACAddress macAddress;
+ private long timeLastSeen;
+
+ public ArpCacheEntry(MACAddress macAddress) {
+ this.macAddress = macAddress;
+ this.timeLastSeen = System.currentTimeMillis();
+ }
+
+ public MACAddress getMacAddress() {
+ return macAddress;
+ }
+
+ public long getTimeLastSeen() {
+ return timeLastSeen;
+ }
+
+ public void setTimeLastSeen(long time){
+ timeLastSeen = time;
+ }
+ }
+
+ public ArpCache() {
+ arpCache = new HashMap<InetAddress, ArpCacheEntry>();
+ }
+
+ public synchronized MACAddress lookup(InetAddress ipAddress){
+ ArpCacheEntry arpEntry = arpCache.get(ipAddress);
+
+ 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 {}", ipAddress.getHostAddress());
+
+ arpCache.remove(ipAddress);
+ return null;
+ }
+
+ return arpEntry.getMacAddress();
+ }
+
+ public synchronized void update(InetAddress ipAddress, MACAddress macAddress){
+ ArpCacheEntry arpEntry = arpCache.get(ipAddress);
+
+ if (arpEntry != null && arpEntry.getMacAddress().equals(macAddress)){
+ arpEntry.setTimeLastSeen(System.currentTimeMillis());
+ }
+ else {
+ arpCache.put(ipAddress, new ArpCacheEntry(macAddress));
+ }
+ }
+}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpTableEntry.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpTableEntry.java
deleted file mode 100644
index 5830cfd..0000000
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpTableEntry.java
+++ /dev/null
@@ -1,27 +0,0 @@
-package net.onrc.onos.ofcontroller.proxyarp;
-
-
-public class ArpTableEntry {
-
- private byte[] macAddress;
- private long timeLastSeen;
-
- public ArpTableEntry(byte[] macAddress, long timeLastSeen) {
- this.macAddress = macAddress;
- this.timeLastSeen = timeLastSeen;
- }
-
- public byte[] getMacAddress() {
- return macAddress;
- }
-
- public long getTimeLastSeen() {
- return timeLastSeen;
- }
-
- public void setTimeLastSeen(long time){
- //TODO thread safety issues?
- timeLastSeen = time;
- }
-
-}
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 1474d02..fd30574 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
@@ -3,6 +3,7 @@
import java.net.InetAddress;
import net.floodlightcontroller.packet.ARP;
+import net.floodlightcontroller.util.MACAddress;
public class HostArpRequester implements IArpRequester {
@@ -21,7 +22,7 @@
}
@Override
- public void arpResponse(InetAddress ipAddress, byte[] macAddress) {
+ public void arpResponse(InetAddress ipAddress, MACAddress 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 90da2ba..66a17a2 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IArpRequester.java
@@ -2,6 +2,20 @@
import java.net.InetAddress;
+import net.floodlightcontroller.util.MACAddress;
+
+/**
+ * Callback interface for modules using the {@link IProxyArpService} to
+ * send ARP requests.
+ *
+ */
public interface IArpRequester {
- public void arpResponse(InetAddress ipAddress, byte[] macAddress);
+ /**
+ * Callback method that will be called by the {@link IProxyArpService}
+ * when it receives a reply for a request previously submitted by this
+ * {@code IArpRequester}.
+ * @param ipAddress The IP address than an ARP request was sent for
+ * @param macAddress The MAC address mapped to the requested IP address
+ */
+ public void arpResponse(InetAddress ipAddress, MACAddress 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 2bb32f4..7dde89b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
@@ -3,6 +3,7 @@
import java.net.InetAddress;
import net.floodlightcontroller.packet.ARP;
+import net.floodlightcontroller.util.MACAddress;
public interface IProxyArpService {
@@ -16,7 +17,7 @@
* @param port
* @param targetMac
*/
- public void sendArpReply(ARP arpRequest, long dpid, short port, byte[] targetMac);
+ public void sendArpReply(ARP arpRequest, long dpid, short port, MACAddress targetMac);
/**
* Returns the mac address if there is a valid entry in the cache.
@@ -24,7 +25,7 @@
* @param ipAddress
* @return
*/
- public byte[] getMacAddress(InetAddress ipAddress);
+ public MACAddress getMacAddress(InetAddress ipAddress);
/**
* Tell the IProxyArpService to send an ARP request for the IP address.
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
diff --git a/start-onos.sh b/start-onos.sh
index a35d181..14adfb0 100755
--- a/start-onos.sh
+++ b/start-onos.sh
@@ -71,7 +71,8 @@
done
# Create a logback file if required
- cat <<EOF_LOGBACK >${ONOS_LOGBACK}
+ if [ ! -f ${ONOS_LOGBACK} ]; then
+ cat <<EOF_LOGBACK >${ONOS_LOGBACK}
<configuration scan="true" debug="true">
<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
<encoder>
@@ -95,6 +96,7 @@
</root>
</configuration>
EOF_LOGBACK
+ fi
# Run floodlight
echo "Starting ONOS controller ..."