Merge branch 'arpchanges'
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpPeer.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpPeer.java
index 7425a07..e98c3e8 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpPeer.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpPeer.java
@@ -2,8 +2,6 @@
 
 import java.net.InetAddress;
 
-import net.floodlightcontroller.util.MACAddress;
-
 import org.codehaus.jackson.annotate.JsonProperty;
 
 import com.google.common.net.InetAddresses;
@@ -11,7 +9,6 @@
 public class BgpPeer {
 	private String interfaceName;
 	private InetAddress ipAddress;
-	private MACAddress macAddress;
 	
 	public String getInterfaceName() {
 		return interfaceName;
@@ -30,13 +27,4 @@
 	public void setIpAddress(String ipAddress) {
 		this.ipAddress = InetAddresses.forString(ipAddress);
 	}
-	
-	public MACAddress getMacAddress() {
-		return macAddress;
-	}
-	
-	@JsonProperty("macAddress")
-	public void setMacAddress(String macAddress) {
-		this.macAddress = MACAddress.valueOf(macAddress);
-	}
 }
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 acdf185..9936cb7 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -10,6 +10,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -32,11 +33,14 @@
 import net.floodlightcontroller.routing.Link;
 import net.floodlightcontroller.topology.ITopologyListener;
 import net.floodlightcontroller.topology.ITopologyService;
+import net.floodlightcontroller.util.MACAddress;
+import net.onrc.onos.ofcontroller.bgproute.RibUpdate.Operation;
 import net.onrc.onos.ofcontroller.core.INetMapTopologyService.ITopoLinkService;
 import net.onrc.onos.ofcontroller.core.INetMapTopologyService.ITopoRouteService;
 import net.onrc.onos.ofcontroller.core.internal.TopoLinkServiceImpl;
 import net.onrc.onos.ofcontroller.linkdiscovery.ILinkDiscovery;
 import net.onrc.onos.ofcontroller.linkdiscovery.ILinkDiscovery.LDUpdate;
+import net.onrc.onos.ofcontroller.proxyarp.IArpRequester;
 import net.onrc.onos.ofcontroller.proxyarp.ProxyArpManager;
 import net.onrc.onos.ofcontroller.routing.TopoRouteService;
 import net.onrc.onos.ofcontroller.util.DataPath;
@@ -64,10 +68,15 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Multimaps;
+import com.google.common.collect.SetMultimap;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 public class BgpRoute implements IFloodlightModule, IBgpRouteService, 
-									ITopologyListener, IOFSwitchListener {
+									ITopologyListener, IOFSwitchListener,
+									IArpRequester {
 	
 	protected static Logger log = LoggerFactory.getLogger(BgpRoute.class);
 
@@ -117,6 +126,29 @@
 	protected ArrayList<LDUpdate> linkUpdates;
 	protected SingletonTask topologyChangeDetectorTask;
 	
+	protected SetMultimap<InetAddress, RibUpdate> prefixesWaitingOnArp;
+	protected SetMultimap<InetAddress, PathUpdate> pathsWaitingOnArp;
+	
+	protected Multimap<Prefix, PushedFlowMod> pushedFlows;
+	
+	private class PushedFlowMod {
+		private long dpid;
+		private OFFlowMod flowMod;
+		
+		public PushedFlowMod(long dpid, OFFlowMod flowMod) {
+			this.dpid = dpid;
+			this.flowMod = flowMod;
+		}
+		
+		public long getDpid() {
+			return dpid;
+		}
+		
+		public OFFlowMod getFlowMod() {
+			return flowMod;
+		}
+	}
+	
 	protected class TopologyChangeDetector implements Runnable {
 		@Override
 		public void run() {
@@ -126,9 +158,6 @@
 				ITopoLinkService topoLinkService = new TopoLinkServiceImpl();
 				
 				List<Link> activeLinks = topoLinkService.getActiveLinks();
-				//for (Link l : activeLinks){
-					//log.debug("active link: {}", l);
-				//}
 				
 				Iterator<LDUpdate> it = linkUpdates.iterator();
 				while (it.hasNext()){
@@ -241,6 +270,13 @@
 
 		topoRouteService = new TopoRouteService("");
 		
+		pathsWaitingOnArp = Multimaps.synchronizedSetMultimap(
+				HashMultimap.<InetAddress, PathUpdate>create());
+		prefixesWaitingOnArp = Multimaps.synchronizedSetMultimap(
+				HashMultimap.<InetAddress, RibUpdate>create());
+		
+		pushedFlows = HashMultimap.<Prefix, PushedFlowMod>create();
+		
 		//Read in config values
 		bgpdRestIp = context.getConfigParams(this).get("BgpdRestIp");
 		if (bgpdRestIp == null){
@@ -368,6 +404,7 @@
 
 	}
 	
+	//TODO once the Ptree is object oriented this can go
 	private String getPrefixFromPtree(PtreeNode node){
         InetAddress address = null;
         try {
@@ -428,7 +465,7 @@
 			
 			node.rib = rib;
 			
-			prefixAdded(node);
+			addPrefixFlows(p, rib);
 		} 
 	}
 	
@@ -437,23 +474,29 @@
 		ribUpdates.add(update);
 	}
 	
-	//TODO temporary
-	public void wrapPrefixAdded(RibUpdate update) {
+	public void processRibAdd(RibUpdate update) {
 		Prefix prefix = update.getPrefix();
 		
 		PtreeNode node = ptree.acquire(prefix.getAddress(), prefix.getPrefixLength());
 		
 		if (node.rib != null) {
+			//There was an existing nexthop for this prefix. This update supersedes that,
+			//so we need to remove the old flows for this prefix from the switches
+			deletePrefixFlows(prefix);
+			
+			//Then remove the old nexthop from the Ptree
 			node.rib = null;
 			ptree.delReference(node);
 		}
+		
+		//Put the new nexthop in the Ptree
 		node.rib = update.getRibEntry();
 
-		prefixAdded(node);
+		//Push flows for the new <prefix, nexthop>
+		addPrefixFlows(prefix, update.getRibEntry());
 	}
 	
-	//TODO temporary
-	public void wrapPrefixDeleted(RibUpdate update) {
+	public void processRibDelete(RibUpdate update) {
 		Prefix prefix = update.getPrefix();
 		
 		PtreeNode node = ptree.lookup(prefix.getAddress(), prefix.getPrefixLength());
@@ -466,36 +509,49 @@
 		 * in a non-null node with a null rib. Only a non-null node with a non-null
 		 * rib is an actual prefix in the Ptree.
 		 */
-		if (node != null && node.rib != null){
-			prefixDeleted(node);
-		}
 
 		if (node != null && node.rib != null) {
 			if (update.getRibEntry().equals(node.rib)) {
 				node.rib = null;
-				ptree.delReference(node);					
+				ptree.delReference(node);
+				
+				deletePrefixFlows(update.getPrefix());
 			}
 		}
 	}
 	
-	@Override
+	//TODO compatibility layer, used by beginRouting()
 	public void prefixAdded(PtreeNode node) {
+		Prefix prefix = null;
+		try {
+			prefix = new Prefix(node.key, node.rib.masklen);
+		} catch (UnknownHostException e) {
+			log.error(" ", e);
+		}
+
+		addPrefixFlows(prefix, node.rib);
+	}
+
+	private void addPrefixFlows(Prefix prefix, Rib rib) {
 		if (!topologyReady){
 			return;
 		}
 		
-		String prefix = getPrefixFromPtree(node);
-		
+		//TODO before we do anything, we have to check that the RIB entry is still in the
+		//Ptree because it could have been removed while we were waiting for ARP.
+		//I think we'll have to make prefixAdded and prefixDelete atomic as well
+		//to protect against the prefix getting deleted while where trying to add it
+
 		log.debug("New prefix {} added, next hop {}, routerId {}", 
-				new Object[] {prefix, node.rib.nextHop.toString(), 
-				node.rib.routerId.getHostAddress()});
+				new Object[] {prefix, rib.nextHop.getHostAddress(),
+				rib.routerId.getHostAddress()});
 		
 		//TODO this is wrong, we shouldn't be dealing with BGP peers here.
 		//We need to figure out where the device is attached and what its
 		//mac address is by learning. 
 		//The next hop is not necessarily the peer, and the peer's attachment
 		//point is not necessarily the next hop's attachment point.
-		BgpPeer peer = bgpPeers.get(node.rib.nextHop);
+		BgpPeer peer = bgpPeers.get(rib.nextHop);
 		
 		if (peer == null){
 			//TODO local router isn't in peers list so this will get thrown
@@ -503,11 +559,22 @@
 			
 			//The other scenario is this is a route server route. In that
 			//case the next hop is not in our configuration
-			log.error("Couldn't find next hop router in router {} in config"
-					, node.rib.nextHop.toString());
+			log.error("Couldn't find next hop router in router {} in config",
+					rib.nextHop.getHostAddress());
 			return; //just quit out here? This is probably a configuration error
 		}
 		
+		//Get MAC address for peer from the ARP module
+		//TODO separate out the 'ask for MAC' bit to another method
+		byte[] peerMacAddress = proxyArp.getMacAddress(peer.getIpAddress());
+		if (peerMacAddress == null) {
+			//A RibUpdate is still a nice way to package them up
+			prefixesWaitingOnArp.put(peer.getIpAddress(), 
+					new RibUpdate(Operation.UPDATE, prefix, rib));
+			proxyArp.sendArpRequest(peer.getIpAddress(), this, true);
+			return;
+		}
+		
 		Interface peerInterface = interfaces.get(peer.getInterfaceName());
 
 		//Add a flow to rewrite mac for this prefix to all border switches
@@ -528,9 +595,6 @@
 				return; // just quit here?
 			}
 			
-			//TODO check the shortest path against the cached version we
-			//calculated before. If they don't match up that's a problem
-			
 			//Set up the flow mod
 			OFFlowMod fm =
 	                (OFFlowMod) floodlightProvider.getOFMessageFactory()
@@ -550,27 +614,23 @@
 	        match.setDataLayerType(Ethernet.TYPE_IPv4);
 	        match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_DL_TYPE);
 	        
-	        //match.setDataLayerSource(ingressRouter.getRouterMac().toBytes());
-	        //match.setDataLayerSource(peer.getMacAddress().toBytes());
-	        //match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_DL_SRC);
-
 	        InetAddress address = null;
 	        try {
-				address = InetAddress.getByAddress(node.key);
+	        	address = InetAddress.getByAddress(prefix.getAddress());
 			} catch (UnknownHostException e1) {
 				//Should never happen is the reverse conversion has already been done
 				log.error("Malformed IP address");
 				return;
 			}
 	        
-	        match.setFromCIDR(address.getHostAddress() + "/" + node.rib.masklen, OFMatch.STR_NW_DST);
+	        match.setFromCIDR(address.getHostAddress() + "/" + 
+	        		prefix.getPrefixLength(), OFMatch.STR_NW_DST);
 	        fm.setMatch(match);
 	        
 	        //Set up MAC rewrite action
 	        OFActionDataLayerDestination macRewriteAction = new OFActionDataLayerDestination();
-	        //TODO use ARP module rather than configured mac addresses
 	        //TODO the peer's mac address is not necessarily the next hop's...
-	        macRewriteAction.setDataLayerAddress(peer.getMacAddress().toBytes());
+	        macRewriteAction.setDataLayerAddress(peerMacAddress);
 	        
 	        //Set up output action
 	        OFActionOutput outputAction = new OFActionOutput();
@@ -593,6 +653,9 @@
             	continue;
             }
             
+            //TODO if prefix Added/Deleted are synchronized this shouldn't have to be
+            pushedFlows.put(prefix, new PushedFlowMod(sw.getId(), fm));
+            
             List<OFMessage> msglist = new ArrayList<OFMessage>();
             msglist.add(fm);
             try {
@@ -604,81 +667,47 @@
 		}
 	}
 	
-	//TODO this is largely untested
-	@Override
-	public void prefixDeleted(PtreeNode node) {
+	//TODO test next-hop changes
+	//TODO check delete/add synchronization
+		
+	private void deletePrefixFlows(Prefix prefix) {
 		if (!topologyReady) {
 			return;
 		}
 		
-		String prefix = getPrefixFromPtree(node);
+		log.debug("In deletePrefixFlows for {}", prefix);
 		
-		log.debug("Prefix {} deleted, next hop {}", 
-				prefix, node.rib.nextHop.toString());
+		/*for (Map.Entry<Prefix, PushedFlowMod> entry : pushedFlows.entries()) {
+			log.debug("Pushed flow: {} => {}", entry.getKey(), entry.getValue());
+		}*/
 		
-		//Remove MAC rewriting flows from other border switches
-		BgpPeer peer = bgpPeers.get(node.rib.nextHop);
-		if (peer == null){
-			//either a router server route or local route. Can't handle right now
-			return;
-		}
+		Collection<PushedFlowMod> pushedFlowMods 
+				= pushedFlows.removeAll(prefix);
 		
-		Interface peerInterface = interfaces.get(peer.getInterfaceName());
-		
-		for (Interface srcInterface : interfaces.values()) {
-			if (srcInterface == peerInterface) {
-				continue;
-			}
+		for (PushedFlowMod pfm : pushedFlowMods) {
+			log.debug("Pushing a DELETE flow mod to {}, matches prefix {} with mac-rewrite {}",
+					new Object[] {HexString.toHexString(pfm.getDpid()),
+					pfm.getFlowMod().getMatch().getNetworkDestination() + 
+					pfm.getFlowMod().getMatch().getNetworkDestinationMaskLen(),
+					HexString.toHexString(((OFActionDataLayerDestination)pfm.getFlowMod().getActions().get(0))
+							.getDataLayerAddress())});
 			
-			//Set up the flow mod
-			OFFlowMod fm =
-	                (OFFlowMod) floodlightProvider.getOFMessageFactory()
-	                                              .getMessage(OFType.FLOW_MOD);
+			OFFlowMod fm = pfm.getFlowMod();
 			
-	        fm.setIdleTimeout((short)0)
-	        .setHardTimeout((short)0)
-	        .setBufferId(OFPacketOut.BUFFER_ID_NONE)
-	        .setCookie(MAC_RW_COOKIE)
-	        .setCommand(OFFlowMod.OFPFC_DELETE)
-	        .setOutPort(OFPort.OFPP_NONE)
-	        .setPriority(SDNIP_PRIORITY)
-	        .setLengthU(OFFlowMod.MINIMUM_LENGTH);
-	        		//+ OFActionDataLayerDestination.MINIMUM_LENGTH
-	        		//+ OFActionOutput.MINIMUM_LENGTH);
-	        
-	        OFMatch match = new OFMatch();
-	        match.setDataLayerType(Ethernet.TYPE_IPv4);
-	        match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_DL_TYPE);
-	        
-	        //match.setDataLayerSource(ingressRouter.getRouterMac().toBytes());
-	        //match.setDataLayerSource(peer.getMacAddress().toBytes());
-	        //match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_DL_SRC);
-	        
-	        InetAddress address = null;
-	        try {
-				address = InetAddress.getByAddress(node.key);
-			} catch (UnknownHostException e1) {
-				//Should never happen is the reverse conversion has already been done
-				log.error("Malformed IP address");
-				return;
-			}
-	        
-	        match.setFromCIDR(address.getHostAddress() + "/" + node.rib.masklen, OFMatch.STR_NW_DST);
-	        fm.setMatch(match);
-	        
-	        //Write to switch
-	        IOFSwitch sw = floodlightProvider.getSwitches()
-	        		.get(srcInterface.getDpid());
-	        
-            if (sw == null){
-            	log.warn("Switch not found when pushing flow mod");
+			fm.setCommand(OFFlowMod.OFPFC_DELETE)
+			.setOutPort(OFPort.OFPP_NONE)
+			.setLengthU(OFFlowMod.MINIMUM_LENGTH);
+			
+			fm.getActions().clear();
+			
+			IOFSwitch sw = floodlightProvider.getSwitches().get(pfm.getDpid());
+			if (sw == null) {
+            	log.warn("Switch not found when pushing delete flow mod");
             	continue;
-            }
-            
-            List<OFMessage> msglist = new ArrayList<OFMessage>();
-            msglist.add(fm);
-            try {
-				sw.write(msglist, null);
+			}
+			
+			try {
+				sw.write(fm, null);
 				sw.flush();
 			} catch (IOException e) {
 				log.error("Failure writing flow mod", e);
@@ -699,30 +728,45 @@
 		
 		for (BgpPeer peer : bgpPeers.values()) {
 			Interface peerInterface = interfaces.get(peer.getInterfaceName());
-			//for (Map.Entry<String, Interface> intfEntry : interfaces.entrySet()) {
-			for (Interface srcInterface : interfaces.values()) {
-				//Interface srcInterface = intfEntry.getValue();
-				//if (peer.getInterfaceName().equals(intfEntry.getKey())){
-				if (peer.getInterfaceName().equals(srcInterface.getName())){
-					continue;
-				}
+			
+			//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) {
+				log.debug("Don't know MAC for {}", peer.getIpAddress().getHostAddress());
+				//Put in the pending paths list first
+				pathsWaitingOnArp.put(peer.getIpAddress(),
+						new PathUpdate(peerInterface, peer.getIpAddress()));
 				
-				DataPath shortestPath = topoRouteService.getShortestPath(
-							srcInterface.getSwitchPort(), peerInterface.getSwitchPort()); 
-				
-				if (shortestPath == null){
-					log.debug("Shortest path between {} and {} not found",
-							srcInterface.getSwitchPort(), peerInterface.getSwitchPort());
-					return; // just quit here?
-				}
-				
-				//install flows
-				installPath(shortestPath.flowEntries(), peer);
+				proxyArp.sendArpRequest(peer.getIpAddress(), this, true);
+				continue;
 			}
+			
+			//If we know the MAC, lets go ahead and push the paths to this peer
+			calculateAndPushPath(peerInterface, MACAddress.valueOf(mac));
 		}
 	}
 	
-	private void installPath(List<FlowEntry> flowEntries, BgpPeer peer){
+	private void calculateAndPushPath(Interface dstInterface, MACAddress dstMacAddress) {
+		for (Interface srcInterface : interfaces.values()) {
+			if (dstInterface.equals(srcInterface.getName())){
+				continue;
+			}
+			
+			DataPath shortestPath = topoRouteService.getShortestPath(
+						srcInterface.getSwitchPort(), dstInterface.getSwitchPort()); 
+			
+			if (shortestPath == null){
+				log.debug("Shortest path between {} and {} not found",
+						srcInterface.getSwitchPort(), dstInterface.getSwitchPort());
+				return; // just quit here?
+			}
+			
+			installPath(shortestPath.flowEntries(), dstMacAddress);
+		}
+	}
+	
+	private void installPath(List<FlowEntry> flowEntries, MACAddress dstMacAddress){
 		//Set up the flow mod
 		OFFlowMod fm =
                 (OFFlowMod) floodlightProvider.getOFMessageFactory()
@@ -748,7 +792,7 @@
            
             OFMatch match = new OFMatch();
             //TODO Again using MAC address from configuration
-            match.setDataLayerDestination(peer.getMacAddress().toBytes());
+            match.setDataLayerDestination(dstMacAddress.toBytes());
             match.setWildcards(match.getWildcards() & ~OFMatch.OFPFW_DL_DST);
             ((OFActionOutput) fm.getActions().get(0)).setPort(flowEntry.outPort().value());
             
@@ -904,6 +948,32 @@
 		}
 	}
 	
+	@Override
+	public void arpResponse(InetAddress ipAddress, byte[] macAddress) {
+		log.debug("Received ARP response: {} => {}", ipAddress.getHostAddress(), 
+				MACAddress.valueOf(macAddress).toString());
+		
+		Set<PathUpdate> pathsToPush = pathsWaitingOnArp.removeAll(ipAddress);
+		
+		for (PathUpdate update : pathsToPush) {
+			log.debug("Pushing path to {} at {} on {}", new Object[] {
+					update.getDstIpAddress().getHostAddress(), 
+					MACAddress.valueOf(macAddress),
+					update.getDstInterface().getSwitchPort()});
+			calculateAndPushPath(update.getDstInterface(), 
+					MACAddress.valueOf(macAddress));
+		}
+		
+		Set<RibUpdate> prefixesToPush = prefixesWaitingOnArp.removeAll(ipAddress);
+		
+		for (RibUpdate update : prefixesToPush) {
+			//These will always be adds
+			log.debug("Pushing prefix {} next hop {}", update.getPrefix(), 
+					update.getRibEntry().nextHop.getHostAddress());
+			addPrefixFlows(update.getPrefix(), update.getRibEntry());
+		}
+	}
+	
 	private void beginRouting(){
 		log.debug("Topology is now ready, beginning routing function");
 		setupBgpPaths();
@@ -995,10 +1065,10 @@
 					RibUpdate update = ribUpdates.take();
 					switch (update.getOperation()){
 					case UPDATE:
-						wrapPrefixAdded(update);
+						processRibAdd(update);
 						break;
 					case DELETE:
-						wrapPrefixDeleted(update);
+						processRibDelete(update);
 						break;
 					}
 				} catch (InterruptedException e) {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/Configuration.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/Configuration.java
index c3c8cbb..4b623e4 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/Configuration.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/Configuration.java
@@ -1,5 +1,6 @@
 package net.onrc.onos.ofcontroller.bgproute;
 
+import java.util.Collections;
 import java.util.List;
 
 import org.codehaus.jackson.annotate.JsonProperty;
@@ -11,7 +12,6 @@
 	private List<String> switches;
 	private List<Interface> interfaces;
 	private List<BgpPeer> peers;
-	//private Map<String, GatewayRouter> gateways;
 	
 	public Configuration() {
 		// TODO Auto-generated constructor stub
@@ -36,7 +36,7 @@
 	}
 
 	public List<String> getSwitches() {
-		return switches;
+		return Collections.unmodifiableList(switches);
 	}
 
 	@JsonProperty("switches")
@@ -45,7 +45,7 @@
 	}
 
 	public List<Interface> getInterfaces() {
-		return interfaces;
+		return Collections.unmodifiableList(interfaces);
 	}
 
 	@JsonProperty("interfaces")
@@ -54,7 +54,7 @@
 	}
 	
 	public List<BgpPeer> getPeers() {
-		return peers;
+		return Collections.unmodifiableList(peers);
 	}
 
 	@JsonProperty("bgpPeers")
@@ -62,14 +62,4 @@
 		this.peers = peers;
 	}
 
-	/*
-	public Map<String, GatewayRouter> getGateways() {
-		return gateways;
-	}
-
-	@JsonProperty("gateways")
-	public void setGateways(Map<String, GatewayRouter> gateways) {
-		this.gateways = gateways;
-	}*/
-
 }
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/IBgpRouteService.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/IBgpRouteService.java
index 3dbc940..d865e6e 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/IBgpRouteService.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/IBgpRouteService.java
@@ -21,6 +21,6 @@
 	public void newRibUpdate(RibUpdate update);
 	
 	//TODO This functionality should be provided by some sort of Ptree listener framework
-	public void prefixAdded(PtreeNode node);
-	public void prefixDeleted(PtreeNode node);
+	//public void prefixAdded(PtreeNode node);
+	//public void prefixDeleted(PtreeNode node);
 }
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java
new file mode 100644
index 0000000..1d2a47b
--- /dev/null
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java
@@ -0,0 +1,27 @@
+package net.onrc.onos.ofcontroller.bgproute;
+
+import java.net.InetAddress;
+
+/*
+ * A path is always assumed to be from all other interfaces (external-facing
+ * switchports) to the destination interface.
+ */
+
+public class PathUpdate {
+
+	private Interface dstInterface;
+	private InetAddress dstIpAddress;
+	
+	public PathUpdate(Interface dstInterface, InetAddress dstIpAddress) {
+		this.dstInterface = dstInterface;
+		this.dstIpAddress = dstIpAddress;
+	}
+
+	public Interface getDstInterface() {
+		return dstInterface;
+	}
+
+	public InetAddress getDstIpAddress() {
+		return dstIpAddress;
+	}
+}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/Prefix.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/Prefix.java
index 4d7c53a..54775df 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/Prefix.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/Prefix.java
@@ -37,6 +37,26 @@
 	}
 	
 	@Override
+	public boolean equals(Object other) {
+		if (other == null || !(other instanceof Prefix)) {
+			return false;
+		}
+		
+		Prefix otherPrefix = (Prefix) other;
+		
+		return (address.equals(otherPrefix.address)) && 
+				(prefixLength == otherPrefix.prefixLength);
+	}
+	
+	@Override
+	public int hashCode() {
+		int hash = 17;
+		hash = 31 * hash + prefixLength;
+		hash = 31 * hash + (address == null ? 0 : address.hashCode());
+		return hash;
+	}
+	
+	@Override
 	public String toString() {
 		return address.getHostAddress() + "/" + prefixLength;
 	}
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..f56934d 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,45 +34,55 @@
 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);
 	
 	private final long ARP_ENTRY_TIMEOUT = 600000; //ms (== 10 mins)
 	
-	private final long ARP_REQUEST_TIMEOUT_THREAD_PERIOD = 60000; //ms (== 1 min) 
+	private final long ARP_TIMER_PERIOD = 60000; //ms (== 1 min) 
 			
 	protected IFloodlightProviderService floodlightProvider;
 	protected ITopologyService topology;
 	protected IDeviceService devices;
 	
 	protected Map<InetAddress, ArpTableEntry> arpTable;
-	
-	//protected ConcurrentHashMap<InetAddress, Set<ArpRequest>> arpRequests;
-	protected ConcurrentHashMap<InetAddress, ArpRequest> arpRequests;
+
+	protected SetMultimap<InetAddress, ArpRequest> arpRequests;
 	
 	private class ArpRequest {
-		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.requester = requester;
+			this.retry = retry;
 			this.requestTime = System.currentTimeMillis();
 		}
 		
-		public synchronized void addRequester(IArpRequester requester){
-			requestTime = System.currentTimeMillis();
-			requesters.add(requester);
+		public ArpRequest(ArpRequest old) {
+			this.requester = old.requester;
+			this.retry = old.retry;
+			this.requestTime = System.currentTimeMillis();
 		}
 		
-		public boolean isExpired(){
+		public boolean isExpired() {
 			return (System.currentTimeMillis() - requestTime) 
 					> IProxyArpService.ARP_REQUEST_TIMEOUT;
 		}
 		
-		public synchronized void dispatchReply(byte[] replyMacAddress){
-			for (IArpRequester requester : requesters){
-				requester.arpResponse(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);
 		}
 	}
 	
@@ -85,43 +93,67 @@
 		this.devices = devices;
 		
 		arpTable = new HashMap<InetAddress, ArpTableEntry>();
-		//arpRequests = new ConcurrentHashMap<InetAddress, Set<ArpRequest>>();
-		arpRequests = new ConcurrentHashMap<InetAddress, ArpRequest>();
+
+		arpRequests = Multimaps.synchronizedSetMultimap(
+				HashMultimap.<InetAddress, ArpRequest>create());
 		
-		Timer arpRequestTimeoutTimer = new Timer();
-		arpRequestTimeoutTimer.scheduleAtFixedRate(new TimerTask() {
+		Timer arpTimer = new Timer();
+		arpTimer.scheduleAtFixedRate(new TimerTask() {
 			@Override
 			public void run() {
-				synchronized (arpRequests) {
-					log.debug("Current have {} outstanding requests", 
-							arpRequests.size());
+				doPeriodicArpProcessing();
+			}
+		}, 0, ARP_TIMER_PERIOD);
+	}
+	
+	/*
+	 * Function that runs periodically to manage the asynchronous request mechanism.
+	 * It basically cleans up old ARP requests if we don't get a response for them.
+	 * The caller can designate that a request should be retried indefinitely, and
+	 * this task will handle that as well.
+	 */
+	private void doPeriodicArpProcessing() {
+		SetMultimap<InetAddress, ArpRequest> retryList 
+				= HashMultimap.<InetAddress, ArpRequest>create();
+
+		//Have to synchronize externally on the Multimap while using an iterator,
+		//even though it's a synchronizedMultimap
+		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());
+		
+					it.remove();
 					
-					Iterator<Map.Entry<InetAddress, ArpRequest>> it 
-							= arpRequests.entrySet().iterator();
-					
-					while (it.hasNext()){
-						Map.Entry<InetAddress, ArpRequest> entry
-								= it.next();
-						
-						if (entry.getValue().isExpired()){
-							log.debug("Cleaning expired ARP request for {}", 
-									entry.getKey().getHostAddress());
-							it.remove();
-						}
+					if (request.shouldRetry()) {
+						retryList.put(entry.getKey(), request);
 					}
 				}
 			}
-		}, 0, ARP_REQUEST_TIMEOUT_THREAD_PERIOD);
-	}
-	
-	private void storeRequester(InetAddress address, IArpRequester requester) {
-		synchronized (arpRequests) {
-			if (arpRequests.get(address) == null) {
-				arpRequests.put(address, new ArpRequest());
+		}
+		
+		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));
 			}
-			ArpRequest request = arpRequests.get(address);
-							
-			request.addRequester(requester);
 		}
 	}
 	
@@ -190,23 +222,32 @@
 				return;
 			}
 			
-			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
 			log.debug("Sending reply of {}", MACAddress.valueOf(mac).toString());
-			//sendArpReply(arp, pi, mac, sw);
 			sendArpReply(arp, sw.getId(), pi.getInPort(), mac);
 		}
 	}
 	
 	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,30 +259,17 @@
 			return;
 		}
 		
-		ArpRequest request = null;
-		synchronized (arpRequests) {
-			request = arpRequests.get(addr);
-			if (request != null) {
-				arpRequests.remove(addr);
-			}
-		}
-		if (request != null && !request.isExpired()) {
-			request.dispatchReply(arp.getSenderHardwareAddress());
-		}
-		
-		/*
 		Set<ArpRequest> requests = arpRequests.get(addr);
-		if (requests != null){
-			
-			synchronized (requests) {
-				for (ArpRequest request : requests) {
-					if (!request.isExpired()){
-						request.getRequester().arpResponse(
-								arp.getSenderHardwareAddress());
-					}
-				}
+		
+		//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());
 			}
-		}*/
+		}
 	}
 
 	private synchronized byte[] lookupArpTable(byte[] ipAddress){
@@ -256,12 +284,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;
 		}
@@ -292,9 +322,11 @@
 	}
 	
 	private void sendArpRequestForAddress(InetAddress ipAddress) {
+		//TODO what should the sender IP address be? Probably not 0.0.0.0
 		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,21 +336,21 @@
 			.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);
 		
 		broadcastArpRequestOutEdge(eth.serialize(), 0, OFPort.OFPP_NONE.getValue());
 	}
 	
-	//private void broadcastArpRequestOutEdge(OFPacketIn pi, long inSwitch, short inPort){
 	private void broadcastArpRequestOutEdge(byte[] arpRequest, long inSwitch, short inPort) {
 		for (IOFSwitch sw : floodlightProvider.getSwitches().values()){
 			Collection<Short> enabledPorts = sw.getEnabledPortNumbers();
@@ -346,7 +378,7 @@
 				}
 				
 				actions.add(new OFActionOutput(portNum));
-				log.debug("Broadcasting out {}/{}", HexString.toHexString(sw.getId()), portNum);
+				//log.debug("Broadcasting out {}/{}", HexString.toHexString(sw.getId()), portNum);
 			}
 			
 			po.setActions(actions);
@@ -368,14 +400,12 @@
 	}
 	
 	public void sendArpReply(ARP arpRequest, long dpid, short port, byte[] targetMac) {
-	//private void sendArpReply(ARP arpRequest, OFPacketIn pi, byte[] macRequested, IOFSwitch sw){
 		ARP arpReply = new ARP();
 		arpReply.setHardwareType(ARP.HW_TYPE_ETHERNET)
 			.setProtocolType(ARP.PROTO_TYPE_IP)
 			.setHardwareAddressLength((byte)Ethernet.DATALAYER_ADDRESS_LENGTH)
 			.setProtocolAddressLength((byte)4) //can't find the constant anywhere
 			.setOpCode(ARP.OP_REPLY)
-			//.setSenderHardwareAddress(macRequested)
 			.setSenderHardwareAddress(targetMac)
 			.setSenderProtocolAddress(arpRequest.getTargetProtocolAddress())
 			.setTargetHardwareAddress(arpRequest.getSenderHardwareAddress())
@@ -383,13 +413,11 @@
 		
 		Ethernet eth = new Ethernet();
 		eth.setDestinationMACAddress(arpRequest.getSenderHardwareAddress())
-			//.setSourceMACAddress(macRequested)
 			.setSourceMACAddress(targetMac)
 			.setEtherType(Ethernet.TYPE_ARP)
 			.setPayload(arpReply);
 		
 		List<OFAction> actions = new ArrayList<OFAction>();
-		//actions.add(new OFActionOutput(pi.getInPort()));
 		actions.add(new OFActionOutput(port));
 		
 		OFPacketOut po = new OFPacketOut();
@@ -433,21 +461,17 @@
 		else return addr.getHostAddress();
 	}
 	
-	
+	@Override
 	public byte[] getMacAddress(InetAddress ipAddress) {
 		return lookupArpTable(ipAddress.getAddress());
 	}
-	
-	public byte[] sendArpRequest(InetAddress ipAddress, IArpRequester requester) {
-		byte[] lookupMac;
-		if ((lookupMac = lookupArpTable(ipAddress.getAddress())) == null) {
-			return lookupMac;
-		}
+
+	@Override
+	public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
+			boolean retry) {
+		arpRequests.put(ipAddress, new ArpRequest(requester, retry));
+		//storeRequester(ipAddress, requester, retry);
 		
 		sendArpRequestForAddress(ipAddress);
-		
-		storeRequester(ipAddress, requester);
-		
-		return null;
 	}
 }