Merge pull request #428 from jonohart/master

Work on the ARP module and FlowCache for SDNIP
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..826fd93 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -40,6 +40,7 @@
 import net.onrc.onos.ofcontroller.linkdiscovery.ILinkDiscovery.LDUpdate;
 import net.onrc.onos.ofcontroller.linkdiscovery.ILinkDiscoveryService;
 import net.onrc.onos.ofcontroller.proxyarp.IArpRequester;
+import net.onrc.onos.ofcontroller.proxyarp.IProxyArpService;
 import net.onrc.onos.ofcontroller.proxyarp.ProxyArpManager;
 import net.onrc.onos.ofcontroller.routing.TopoRouteService;
 import net.onrc.onos.ofcontroller.util.DataPath;
@@ -56,7 +57,6 @@
 import org.codehaus.jackson.map.ObjectMapper;
 import org.openflow.protocol.OFFlowMod;
 import org.openflow.protocol.OFMatch;
-import org.openflow.protocol.OFMessage;
 import org.openflow.protocol.OFPacketOut;
 import org.openflow.protocol.OFPort;
 import org.openflow.protocol.OFType;
@@ -76,7 +76,8 @@
 
 public class BgpRoute implements IFloodlightModule, IBgpRouteService, 
 									ITopologyListener, IArpRequester,
-									IOFSwitchListener, ILayer3InfoService {
+									IOFSwitchListener, ILayer3InfoService,
+									IProxyArpService {
 	
 	protected static Logger log = LoggerFactory.getLogger(BgpRoute.class);
 
@@ -139,6 +140,8 @@
 	protected Map<Prefix, Path> prefixToPath;
 	protected Multimap<Prefix, PushedFlowMod> pushedFlows;
 	
+	private FlowCache flowCache;
+	
 	protected volatile Map<Long, ?> topoRouteTopology = null;
 		
 	protected class TopologyChangeDetector implements Runnable {
@@ -232,6 +235,7 @@
 		Map<Class<? extends IFloodlightService>, IFloodlightService> m 
 			= new HashMap<Class<? extends IFloodlightService>, IFloodlightService>();
 		m.put(IBgpRouteService.class, this);
+		m.put(IProxyArpService.class, this);
 		return m;
 	}
 
@@ -262,7 +266,7 @@
 		
 		//TODO We'll initialise this here for now, but it should really be done as
 		//part of the controller core
-		proxyArp = new ProxyArpManager(floodlightProvider, topology, this);
+		proxyArp = new ProxyArpManager(floodlightProvider, topology, this, restApi);
 		
 		linkUpdates = new ArrayList<LDUpdate>();
 		ScheduledExecutorService executor = Executors.newScheduledThreadPool(1);
@@ -278,6 +282,8 @@
 		prefixToPath = new HashMap<Prefix, Path>();
 		pushedFlows = HashMultimap.<Prefix, PushedFlowMod>create();
 		
+		flowCache = new FlowCache(floodlightProvider);
+		
 		bgpUpdatesExecutor = Executors.newSingleThreadExecutor(
 				new ThreadFactoryBuilder().setNameFormat("bgp-updates-%d").build());
 		
@@ -429,7 +435,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 +469,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 +482,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 +539,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();
@@ -546,31 +552,16 @@
 	        actions.add(outputAction);
 	        fm.setActions(actions);
 	        
-	        //Write to switch
-	        IOFSwitch sw = floodlightProvider.getSwitches()
-	        			.get(srcInterface.getDpid());
-	        
-            if (sw == null){
-            	log.warn("Switch not found when pushing flow mod");
-            	continue;
-            }
-            
-            pushedFlows.put(prefix, new PushedFlowMod(sw.getId(), fm));
-            
-            List<OFMessage> msglist = new ArrayList<OFMessage>();
-            msglist.add(fm);
-            try {
-				sw.write(msglist, null);
-				sw.flush();
-				
-				/*
-				 * XXX Rate limit hack!
-				 * This should be solved properly by adding a rate limiting
-				 * layer on top of the switches if we know they need it.
-				 */
+	        pushedFlows.put(prefix, new PushedFlowMod(srcInterface.getDpid(), fm));
+	        flowCache.write(srcInterface.getDpid(), fm);
+
+			/*
+			 * XXX Rate limit hack!
+			 * This should be solved properly by adding a rate limiting
+			 * layer on top of the switches if we know they need it.
+			 */
+	        try {
 				Thread.sleep(1);
-			} catch (IOException e) {
-				log.error("Failure writing flow mod", e);
 			} catch (InterruptedException e) {
 				// TODO handle this properly
 				log.error("Interrupted", e);
@@ -650,24 +641,7 @@
 	}
 	
 	private void sendDeleteFlowMod(OFFlowMod addFlowMod, long dpid) {
-		addFlowMod.setCommand(OFFlowMod.OFPFC_DELETE_STRICT)
-		.setOutPort(OFPort.OFPP_NONE)
-		.setLengthU(OFFlowMod.MINIMUM_LENGTH);
-		
-		addFlowMod.getActions().clear();
-		
-		IOFSwitch sw = floodlightProvider.getSwitches().get(dpid);
-		if (sw == null) {
-        	log.warn("Switch not found when pushing delete flow mod");
-        	return;
-		}
-		
-		try {
-			sw.write(addFlowMod, null);
-			sw.flush();
-		} catch (IOException e) {
-			log.error("Failure writing flow mod", e);
-		}
+		flowCache.delete(dpid, addFlowMod);
 	}
 	
 	//TODO test next-hop changes
@@ -694,8 +668,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 +679,7 @@
 			}
 			
 			//If we know the MAC, lets go ahead and push the paths to this peer
-			calculateAndPushPath(path, MACAddress.valueOf(mac));
+			calculateAndPushPath(path, macAddress);
 		}
 	}
 	
@@ -738,7 +712,8 @@
 				return;
 			}
 			
-			pushedFlows.addAll(installPath(shortestPath.flowEntries(), dstMacAddress));
+			List<PushedFlowMod> pushedFlowMods = installPath(shortestPath.flowEntries(), dstMacAddress);
+			pushedFlows.addAll(pushedFlowMods);
 		}
 		
 		path.setFlowMods(pushedFlows);
@@ -778,24 +753,10 @@
             
             fm.setMatch(match);
             
-            IOFSwitch sw = floodlightProvider.getSwitches().get(flowEntry.dpid().value());
+            flowMods.add(new PushedFlowMod(flowEntry.dpid().value(), fm));
             
-            if (sw == null){
-            	log.warn("Switch not found when pushing flow mod");
-            	continue;
-            }
-            
-            flowMods.add(new PushedFlowMod(sw.getId(), fm));
-            
-            List<OFMessage> msglist = new ArrayList<OFMessage>();
-            msglist.add(fm);
-            try {
-				sw.write(msglist, null);
-				sw.flush();
-			} catch (IOException e) {
-				log.error("Failure writing flow mod", e);
-			}
-            
+            flowCache.write(flowEntry.dpid().value(), fm);
+                        
             try {
                 fm = fm.clone();
             } catch (CloneNotSupportedException e1) {
@@ -849,7 +810,7 @@
 	        //Common match fields
 	        forwardMatchSrc.setDataLayerType(Ethernet.TYPE_IPv4);
 	        forwardMatchSrc.setNetworkProtocol(IPv4.PROTOCOL_TCP);
-	        forwardMatchSrc.setTransportDestination(BGP_PORT);
+	        //forwardMatchSrc.setTransportDestination(BGP_PORT);
 	        forwardMatchSrc.setWildcards(forwardMatchSrc.getWildcards() & ~OFMatch.OFPFW_IN_PORT
 	        				& ~OFMatch.OFPFW_DL_TYPE & ~OFMatch.OFPFW_NW_PROTO);
 	        
@@ -931,37 +892,23 @@
 				((OFActionOutput)reverseIcmp.getActions().get(0))
 						.setPort(flowEntry.inPort().value());
 				reverseIcmp.setMatch(reverseIcmpMatch);
-		
-
-				IOFSwitch sw = floodlightProvider.getSwitches().get(flowEntry.dpid().value());
 				
-				if (sw == null) {
-					log.warn("Switch not found when pushing BGP paths");
-					return;
-				}
-				
-				List<OFMessage> msgList = new ArrayList<OFMessage>(2);
-				msgList.add(forwardFlowModSrc);
-				msgList.add(forwardFlowModDst);
-				msgList.add(reverseFlowModSrc);
-				msgList.add(reverseFlowModDst);
-				msgList.add(forwardIcmp);
-				msgList.add(reverseIcmp);
-				
-				try {
-					sw.write(msgList, null);
-					sw.flush();
-				} catch (IOException e) {
-					log.error("Failure writing flow mod", e);
-				}
+				List<OFFlowMod> flowModList = new ArrayList<OFFlowMod>(6);
+				flowModList.add(forwardFlowModSrc);
+				flowModList.add(forwardFlowModDst);
+				flowModList.add(reverseFlowModSrc);
+				flowModList.add(reverseFlowModDst);
+				flowModList.add(forwardIcmp);
+				flowModList.add(reverseIcmp);
+				flowCache.write(flowEntry.dpid().value(), flowModList);
 			}
 		}
 	}
 	
 	@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 +920,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 +932,7 @@
 					}
 				}
 				else {
-					calculateAndPushPath(path, MACAddress.valueOf(macAddress));
+					calculateAndPushPath(path, macAddress);
 					pushedPaths.put(path.getDstIpAddress(), path);
 				}
 			}
@@ -1037,17 +983,7 @@
 		.setLengthU(OFFlowMod.MINIMUM_LENGTH + OFActionOutput.MINIMUM_LENGTH);
 		
 		for (String strdpid : switches){
-			IOFSwitch sw = floodlightProvider.getSwitches().get(HexString.toLong(strdpid));
-			if (sw == null) {
-				log.debug("Couldn't find switch to push ARP flow");
-			}
-			else {
-				try {
-					sw.write(fm, null);
-				} catch (IOException e) {
-					log.warn("Failure writing ARP flow to switch", e);
-				}
-			}
+			flowCache.write(HexString.toLong(strdpid), fm);
 		}
 	}
 	
@@ -1098,22 +1034,13 @@
 		fmBDDP.setPriority(ARP_PRIORITY);
 		fmBDDP.setLengthU(OFFlowMod.MINIMUM_LENGTH + OFActionOutput.MINIMUM_LENGTH);
 		
+		List<OFFlowMod> flowModList = new ArrayList<OFFlowMod>(3); 
+		flowModList.add(fm);
+		flowModList.add(fmLLDP);
+		flowModList.add(fmBDDP);
+		
 		for (String strdpid : switches){
-			IOFSwitch sw = floodlightProvider.getSwitches().get(HexString.toLong(strdpid));
-			if (sw == null) {
-				log.debug("Couldn't find switch to push default deny flow");
-			}
-			else {
-				List<OFMessage> msgList = new ArrayList<OFMessage>();
-				msgList.add(fm);
-				msgList.add(fmLLDP);
-				msgList.add(fmBDDP);
-				try {
-					sw.write(msgList, null);
-				} catch (IOException e) {
-					log.warn("Failure writing default deny flow to switch", e);
-				}
-			}
+			flowCache.write(HexString.toLong(strdpid), flowModList);
 		}
 	}
 	
@@ -1246,6 +1173,8 @@
 		if (!topologyReady) {
 			sw.clearAllFlowMods();
 		}
+		
+		flowCache.switchConnected(sw);
 	}
 
 	@Override
@@ -1299,4 +1228,27 @@
 	public MACAddress getRouterMacAddress() {
 		return bgpdMacAddress;
 	}
+
+	/*
+	 * TODO This is a hack to get the REST API to work for ProxyArpManager.
+	 * The REST API is currently tied to the Floodlight module system and we
+	 * need to separate it to allow ONOS modules to use it. For now we will 
+	 * proxy calls through to the ProxyArpManager (which is not a Floodlight 
+	 * module) through this class which is a module.
+	 */
+	@Override
+	public MACAddress getMacAddress(InetAddress ipAddress) {
+		return proxyArp.getMacAddress(ipAddress);
+	}
+
+	@Override
+	public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
+			boolean retry) {
+		proxyArp.sendArpRequest(ipAddress, requester, retry);		
+	}
+
+	@Override
+	public List<String> getMappings() {
+		return proxyArp.getMappings();
+	}
 }
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/FlowCache.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/FlowCache.java
new file mode 100644
index 0000000..d1cb578
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/FlowCache.java
@@ -0,0 +1,158 @@
+package net.onrc.onos.ofcontroller.bgproute;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import net.floodlightcontroller.core.IFloodlightProviderService;
+import net.floodlightcontroller.core.IOFSwitch;
+
+import org.openflow.protocol.OFFlowMod;
+import org.openflow.protocol.OFMessage;
+import org.openflow.protocol.OFPort;
+import org.openflow.util.HexString;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FlowCache {
+	private static Logger log = LoggerFactory.getLogger(FlowCache.class);
+	
+	private IFloodlightProviderService floodlightProvider;
+	
+	private Map<Long, List<OFFlowMod>> flowCache;
+	
+	private Comparator<OFFlowMod> cookieComparator = new Comparator<OFFlowMod>() {
+		@Override
+		public int compare(OFFlowMod fm1, OFFlowMod fm2) {
+			long difference = fm2.getCookie() - fm1.getCookie(); 
+			
+			if (difference > 0) {
+				return 1;
+			}
+			else if (difference < 0) {
+				return -1;
+			}
+			else {
+				return 0;
+			}
+		}
+	};
+	
+	public FlowCache(IFloodlightProviderService floodlightProvider) {
+		this.floodlightProvider = floodlightProvider;
+		
+		flowCache = new HashMap<Long, List<OFFlowMod>>();
+	}
+
+	public synchronized void write(long dpid, OFFlowMod flowMod) {
+		List<OFFlowMod> flowModList = new ArrayList<OFFlowMod>(1);
+		flowModList.add(flowMod);
+		write(dpid, flowModList);
+	}
+	
+	public synchronized void write(long dpid, List<OFFlowMod> flowMods) {
+		ensureCacheForSwitch(dpid);
+		
+		List<OFFlowMod> clones = new ArrayList<OFFlowMod>(flowMods.size());
+		
+		//Somehow the OFFlowMods we get passed in will change later on.
+		//No idea how this happens, but we can just clone to prevent problems
+		try {
+			for (OFFlowMod fm : flowMods) {
+				clones.add(fm.clone());
+			}
+		} catch (CloneNotSupportedException e) {
+			log.debug("Clone exception", e);
+		}
+		
+		flowCache.get(dpid).addAll(clones);
+		
+		IOFSwitch sw = floodlightProvider.getSwitches().get(dpid);
+		
+		if (sw == null) {
+			log.debug("Switch not found when writing flow mods");
+			return;
+		}
+
+		List<OFMessage> msgList = new ArrayList<OFMessage>(clones.size());
+		msgList.addAll(clones);
+		
+		try {
+			sw.write(msgList, null);
+		} catch (IOException e) {
+			log.error("Error writing to switch", e);
+		}
+		
+
+	}
+	
+	public synchronized void delete(long dpid, OFFlowMod flowMod) {
+		List<OFFlowMod> flowModList = new ArrayList<OFFlowMod>(1);
+		flowModList.add(flowMod);
+		delete(dpid, flowModList);
+	}
+	
+	public synchronized void delete(long dpid, List<OFFlowMod> flowMods) {
+		ensureCacheForSwitch(dpid);
+		
+		//Remove the flow mods from the cache first before we alter them
+		flowCache.get(dpid).removeAll(flowMods);
+		
+		//Alter the original flow mods to make them delete flow mods
+		for (OFFlowMod fm : flowMods) {
+			fm.setCommand(OFFlowMod.OFPFC_DELETE_STRICT)
+			.setOutPort(OFPort.OFPP_NONE)
+			.setLengthU(OFFlowMod.MINIMUM_LENGTH);
+			
+			fm.getActions().clear();
+		}
+		
+		IOFSwitch sw = floodlightProvider.getSwitches().get(dpid);
+		if (sw == null) {
+			log.debug("Switch not found when writing flow mods");
+			return;
+		}
+		
+		List<OFMessage> msgList = new ArrayList<OFMessage>(flowMods.size());
+		msgList.addAll(flowMods);
+		
+		try {
+			sw.write(msgList, null);
+		} catch (IOException e) {
+			log.error("Error writing to switch", e);
+		}
+	}
+	
+	//TODO can the Prontos handle being sent all flow mods in one message?
+	public synchronized void switchConnected(IOFSwitch sw) {
+		log.debug("Switch connected: {}", sw);
+		
+		ensureCacheForSwitch(sw.getId());
+		
+		List<OFFlowMod> flowMods = flowCache.get(sw.getId());
+
+		Collections.sort(flowMods, cookieComparator);
+		
+		sw.clearAllFlowMods();
+		
+		List<OFMessage> messages = new ArrayList<OFMessage>(flowMods.size());
+		messages.addAll(flowMods);
+		
+		try {
+			sw.write(messages, null);
+		} catch (IOException e) {
+			log.error("Failure writing flow mods to switch {}",
+					HexString.toHexString(sw.getId()));
+		}		
+	}
+	
+	private void ensureCacheForSwitch(long dpid) {
+		if (!flowCache.containsKey(dpid)) {
+			flowCache.put(dpid, new ArrayList<OFFlowMod>());
+		}
+	}
+}
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..83a3b55
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCache.java
@@ -0,0 +1,99 @@
+package net.onrc.onos.ofcontroller.proxyarp;
+
+import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+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.
+ */
+class ArpCache {
+	private final static Logger log = LoggerFactory.getLogger(ArpCache.class);
+	
+	private final static 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 void setTimeLastSeen(long time){
+			timeLastSeen = time;
+		}
+		
+		public boolean isExpired() {
+			return System.currentTimeMillis() - timeLastSeen > ARP_ENTRY_TIMEOUT;
+		}
+	}
+
+	ArpCache() {
+		arpCache = new HashMap<InetAddress, ArpCacheEntry>();
+	}
+
+	synchronized MACAddress lookup(InetAddress ipAddress){	
+		ArpCacheEntry arpEntry = arpCache.get(ipAddress);
+		
+		if (arpEntry == null){
+			return null;
+		}
+		
+		if (arpEntry.isExpired()) {
+			//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();
+	}
+
+	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));
+		}
+	}
+	
+	synchronized List<String> getMappings() {
+		List<String> result = new ArrayList<String>(arpCache.size());
+		
+		for (Map.Entry<InetAddress, ArpCacheEntry> entry : arpCache.entrySet()) {
+			result.add(entry.getKey().getHostAddress() + " => " + 
+					entry.getValue().getMacAddress().toString() + 
+					(entry.getValue().isExpired()?" : EXPIRED":" : VALID"));
+		}
+		
+		return result;
+	}
+}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCacheResource.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCacheResource.java
new file mode 100644
index 0000000..252e66e
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpCacheResource.java
@@ -0,0 +1,18 @@
+package net.onrc.onos.ofcontroller.proxyarp;
+
+import java.util.List;
+
+import org.restlet.resource.Get;
+import org.restlet.resource.ServerResource;
+
+public class ArpCacheResource extends ServerResource {
+
+	@Get("json")
+	public List<String> getArpCache() {
+		IProxyArpService arp = (IProxyArpService) getContext().getAttributes().
+				get(IProxyArpService.class.getCanonicalName());
+		
+		return arp.getMappings();
+	}
+
+}
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/ArpWebRoutable.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpWebRoutable.java
new file mode 100644
index 0000000..eefa2db
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ArpWebRoutable.java
@@ -0,0 +1,22 @@
+package net.onrc.onos.ofcontroller.proxyarp;
+
+import net.floodlightcontroller.restserver.RestletRoutable;
+
+import org.restlet.Context;
+import org.restlet.Restlet;
+import org.restlet.routing.Router;
+
+public class ArpWebRoutable implements RestletRoutable {
+
+	@Override
+	public Restlet getRestlet(Context context) {
+		Router router = new Router(context);
+		router.attach("/cache/json", ArpCacheResource.class);
+		return router;
+	}
+
+	@Override
+	public String basePath() {
+		return "/wm/arp";
+	}
+}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
deleted file mode 100644
index 1474d02..0000000
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/HostArpRequester.java
+++ /dev/null
@@ -1,28 +0,0 @@
-package net.onrc.onos.ofcontroller.proxyarp;
-
-import java.net.InetAddress;
-
-import net.floodlightcontroller.packet.ARP;
-
-public class HostArpRequester implements IArpRequester {
-
-	private IProxyArpService arpService;
-	private ARP arpRequest;
-	private long dpid;
-	private short port;
-	
-	public HostArpRequester(IProxyArpService arpService, ARP arpRequest, 
-			long dpid, short port) {
-		
-		this.arpService = arpService;
-		this.arpRequest = arpRequest;
-		this.dpid = dpid;
-		this.port = port;
-	}
-
-	@Override
-	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 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..97844d3 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/IProxyArpService.java
@@ -1,30 +1,20 @@
 package net.onrc.onos.ofcontroller.proxyarp;
 
 import java.net.InetAddress;
+import java.util.List;
 
-import net.floodlightcontroller.packet.ARP;
+import net.floodlightcontroller.core.module.IFloodlightService;
+import net.floodlightcontroller.util.MACAddress;
 
-public interface IProxyArpService {
-	
-	public final int ARP_REQUEST_TIMEOUT = 2000; //ms
-	
+//Extends IFloodlightService so we can access it from REST API resources
+public interface IProxyArpService extends IFloodlightService{
 	/**
-	 * Tell the IProxyArpService to send an ARP reply with the targetMac to 
-	 * the host on the specified switchport.
-	 * @param arpRequest
-	 * @param dpid
-	 * @param port
-	 * @param targetMac
-	 */
-	public void sendArpReply(ARP arpRequest, long dpid, short port, byte[] targetMac);
-	
-	/**
-	 * Returns the mac address if there is a valid entry in the cache.
+	 * Returns the MAC address if there is a valid entry in the cache.
 	 * Otherwise returns null.
 	 * @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.
@@ -32,8 +22,13 @@
 	 * @param ipAddress
 	 * @param requester
 	 * @param retry Whether to keep sending requests until the MAC is learnt
-	 * @return
 	 */
 	public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
 			boolean retry);
+	
+	/**
+	 * Returns a snapshot of the entire ARP cache.
+	 * @return
+	 */
+	public List<String> getMappings();
 }
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..b6a9591 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;
@@ -20,6 +19,7 @@
 import net.floodlightcontroller.packet.ARP;
 import net.floodlightcontroller.packet.Ethernet;
 import net.floodlightcontroller.packet.IPv4;
+import net.floodlightcontroller.restserver.IRestApiService;
 import net.floodlightcontroller.topology.ITopologyService;
 import net.floodlightcontroller.util.MACAddress;
 import net.onrc.onos.ofcontroller.bgproute.ILayer3InfoService;
@@ -40,25 +40,25 @@
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.SetMultimap;
 
-//TODO REST API to inspect ARP table
 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 static final int ARP_REQUEST_TIMEOUT = 2000; //ms
 			
-	private IFloodlightProviderService floodlightProvider;
-	private ITopologyService topology;
-	private ILayer3InfoService layer3;
+	private final IFloodlightProviderService floodlightProvider;
+	private final ITopologyService topology;
+	private final ILayer3InfoService layer3;
+	private final IRestApiService restApi;
 	
-	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){
@@ -74,32 +74,52 @@
 		}
 		
 		public boolean isExpired() {
-			return (System.currentTimeMillis() - requestTime) 
-					> IProxyArpService.ARP_REQUEST_TIMEOUT;
+			return (System.currentTimeMillis() - requestTime) > ARP_REQUEST_TIMEOUT;
 		}
 		
 		public boolean shouldRetry() {
 			return retry;
 		}
 		
-		public void dispatchReply(InetAddress ipAddress, byte[] replyMacAddress) {
+		public void dispatchReply(InetAddress ipAddress, MACAddress replyMacAddress) {
 			requester.arpResponse(ipAddress, replyMacAddress);
 		}
 	}
 	
+	private class HostArpRequester implements IArpRequester {
+		private final ARP arpRequest;
+		private final long dpid;
+		private final short port;
+		
+		public HostArpRequester(ARP arpRequest, long dpid, short port) {
+			this.arpRequest = arpRequest;
+			this.dpid = dpid;
+			this.port = port;
+		}
+
+		@Override
+		public void arpResponse(InetAddress ipAddress, MACAddress macAddress) {
+			ProxyArpManager.this.sendArpReply(arpRequest, dpid, port, macAddress);
+		}
+	}
+	
 	public ProxyArpManager(IFloodlightProviderService floodlightProvider,
-				ITopologyService topology, ILayer3InfoService layer3){
+				ITopologyService topology, ILayer3InfoService layer3,
+				IRestApiService restApi){
 		this.floodlightProvider = floodlightProvider;
 		this.topology = topology;
 		this.layer3 = layer3;
+		this.restApi = restApi;
 		
-		arpTable = new HashMap<InetAddress, ArpTableEntry>();
+		arpCache = new ArpCache();
 
 		arpRequests = Multimaps.synchronizedSetMultimap(
 				HashMultimap.<InetAddress, ArpRequest>create());
 	}
 	
 	public void startUp() {
+		restApi.addRestletRoutable(new ArpWebRoutable());
+		
 		Timer arpTimer = new Timer("arp-processing");
 		arpTimer.scheduleAtFixedRate(new TimerTask() {
 			@Override
@@ -204,7 +224,7 @@
 		return Command.CONTINUE;
 	}
 	
-	protected void handleArpRequest(IOFSwitch sw, OFPacketIn pi, ARP arp) {
+	private void handleArpRequest(IOFSwitch sw, OFPacketIn pi, ARP arp) {
 		if (log.isTraceEnabled()) {
 			log.trace("ARP request received for {}", 
 					inetAddressToString(arp.getTargetProtocolAddress()));
@@ -226,20 +246,20 @@
 						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(
-					new HostArpRequester(this, arp, sw.getId(), pi.getInPort()), false));
+					new HostArpRequester(arp, sw.getId(), pi.getInPort()), false));
 						
 			//Flood the request out edge ports
 			sendArpRequestToSwitches(target, pi.getPacketData(), sw.getId(), pi.getInPort());
@@ -249,15 +269,15 @@
 			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);
 		}
 	}
 	
-	protected void handleArpReply(IOFSwitch sw, OFPacketIn pi, ARP arp){
+	private void handleArpReply(IOFSwitch sw, OFPacketIn pi, ARP arp){
 		if (log.isTraceEnabled()) {
 			log.trace("ARP reply recieved: {} => {}, on {}/{}", new Object[] { 
 					inetAddressToString(arp.getSenderProtocolAddress()),
@@ -265,18 +285,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 +313,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);
 		}
 	}
 	
@@ -501,24 +473,11 @@
 		}
 	}
 	
-	private String inetAddressToString(byte[] bytes) {
-		try {
-			return InetAddress.getByAddress(bytes).getHostAddress();
-		} catch (UnknownHostException e) {
-			log.debug("Invalid IP address", e);
-			return "";
-		}
-	}
-	
-	/*
-	 * IProxyArpService methods
-	 */
-	
-	public void sendArpReply(ARP arpRequest, long dpid, short port, byte[] targetMac) {
+	private 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 +487,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);
 		
@@ -569,10 +528,23 @@
 			log.error("Failure writing packet out to switch", e);
 		}
 	}
+	
+	private String inetAddressToString(byte[] bytes) {
+		try {
+			return InetAddress.getByAddress(bytes).getHostAddress();
+		} catch (UnknownHostException e) {
+			log.debug("Invalid IP address", e);
+			return "";
+		}
+	}
+	
+	/*
+	 * IProxyArpService methods
+	 */
 
 	@Override
-	public byte[] getMacAddress(InetAddress ipAddress) {
-		return lookupArpTable(ipAddress.getAddress());
+	public MACAddress getMacAddress(InetAddress ipAddress) {
+		return arpCache.lookup(ipAddress);
 	}
 
 	@Override
@@ -585,4 +557,9 @@
 			sendArpRequestForAddress(ipAddress);
 		}
 	}
+	
+	@Override
+	public List<String> getMappings() {
+		return arpCache.getMappings();
+	}
 }
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 ..."