Removed peer MAC addresses from the config.json file and cleaned up a lot of old code
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 7ceb3f3..9936cb7 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -126,8 +126,6 @@
protected ArrayList<LDUpdate> linkUpdates;
protected SingletonTask topologyChangeDetectorTask;
- //protected HashMap<InetAddress, RibUpdate> prefixesWaitingOnArp;
- //protected HashMap<InetAddress, PathUpdate> pathsWaitingOnArp;
protected SetMultimap<InetAddress, RibUpdate> prefixesWaitingOnArp;
protected SetMultimap<InetAddress, PathUpdate> pathsWaitingOnArp;
@@ -406,6 +404,7 @@
}
+ //TODO once the Ptree is object oriented this can go
private String getPrefixFromPtree(PtreeNode node){
InetAddress address = null;
try {
@@ -466,7 +465,6 @@
node.rib = rib;
- //prefixAdded(node);
addPrefixFlows(p, rib);
}
}
@@ -495,7 +493,6 @@
node.rib = update.getRibEntry();
//Push flows for the new <prefix, nexthop>
- //prefixAdded(node);
addPrefixFlows(prefix, update.getRibEntry());
}
@@ -512,16 +509,12 @@
* 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);
- //prefixDeleted(update);
deletePrefixFlows(update.getPrefix());
}
}
@@ -529,23 +522,16 @@
//TODO compatibility layer, used by beginRouting()
public void prefixAdded(PtreeNode node) {
- //String strPrefix = getPrefixFromPtree(node);
-
Prefix prefix = null;
try {
prefix = new Prefix(node.key, node.rib.masklen);
} catch (UnknownHostException e) {
log.error(" ", e);
}
-
- //RibUpdate update = new RibUpdate(Operation.UPDATE, prefix, node.rib);
-
- //addPrefixFlows(update);
-
+
addPrefixFlows(prefix, node.rib);
}
- //private void addPrefixFlows(RibUpdate update) {
private void addPrefixFlows(Prefix prefix, Rib rib) {
if (!topologyReady){
return;
@@ -557,8 +543,6 @@
//to protect against the prefix getting deleted while where trying to add it
log.debug("New prefix {} added, next hop {}, routerId {}",
- //new Object[] {update.getPrefix(), update.getRibEntry().nextHop.getHostAddress(),
- //update.getRibEntry().routerId.getHostAddress()});
new Object[] {prefix, rib.nextHop.getHostAddress(),
rib.routerId.getHostAddress()});
@@ -567,7 +551,6 @@
//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(update.getRibEntry().nextHop);
BgpPeer peer = bgpPeers.get(rib.nextHop);
if (peer == null){
@@ -577,7 +560,6 @@
//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",
- //update.getRibEntry().nextHop.getHostAddress());
rib.nextHop.getHostAddress());
return; //just quit out here? This is probably a configuration error
}
@@ -632,13 +614,8 @@
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(update.getPrefix().getAddress());
address = InetAddress.getByAddress(prefix.getAddress());
} catch (UnknownHostException e1) {
//Should never happen is the reverse conversion has already been done
@@ -646,16 +623,13 @@
return;
}
- //match.setFromCIDR(address.getHostAddress() + "/" + node.rib.masklen, OFMatch.STR_NW_DST);
match.setFromCIDR(address.getHostAddress() + "/" +
- //update.getPrefix().getPrefixLength(), OFMatch.STR_NW_DST);
prefix.getPrefixLength(), OFMatch.STR_NW_DST);
fm.setMatch(match);
//Set up MAC rewrite action
OFActionDataLayerDestination macRewriteAction = new OFActionDataLayerDestination();
//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
@@ -680,7 +654,6 @@
}
//TODO if prefix Added/Deleted are synchronized this shouldn't have to be
- //pushedFlows.put(update.getPrefix(), new PushedFlowMod(sw.getId(), fm));
pushedFlows.put(prefix, new PushedFlowMod(sw.getId(), fm));
List<OFMessage> msglist = new ArrayList<OFMessage>();
@@ -696,27 +669,19 @@
//TODO test next-hop changes
//TODO check delete/add synchronization
-
- /*
- private void prefixDeleted(RibUpdate update) {
- deletePrefix(update.getPrefix());
- }
- */
private void deletePrefixFlows(Prefix prefix) {
if (!topologyReady) {
return;
}
- //log.debug("In prefixDeleted for {}", update.getPrefix());
- log.debug("In prefixDeleted for {}", prefix);
+ log.debug("In deletePrefixFlows for {}", prefix);
/*for (Map.Entry<Prefix, PushedFlowMod> entry : pushedFlows.entries()) {
log.debug("Pushed flow: {} => {}", entry.getKey(), entry.getValue());
}*/
Collection<PushedFlowMod> pushedFlowMods
- //= pushedFlows.removeAll(update.getPrefix());
= pushedFlows.removeAll(prefix);
for (PushedFlowMod pfm : pushedFlowMods) {
@@ -750,86 +715,6 @@
}
}
- /*public void prefixDeleted(PtreeNode node) {
- if (!topologyReady) {
- return;
- }
-
- String prefix = getPrefixFromPtree(node);
-
- log.debug("Prefix {} deleted, next hop {}",
- prefix, node.rib.nextHop.toString());
-
- //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;
- }
-
- Interface peerInterface = interfaces.get(peer.getInterfaceName());
-
- for (Interface srcInterface : interfaces.values()) {
- if (srcInterface == peerInterface) {
- continue;
- }
-
- //Set up the flow mod
- OFFlowMod fm =
- (OFFlowMod) floodlightProvider.getOFMessageFactory()
- .getMessage(OFType.FLOW_MOD);
-
- 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");
- continue;
- }
-
- 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);
- }
- }
- }*/
-
/*
* On startup we need to calculate a full mesh of paths between all gateway
* switches
@@ -877,7 +762,6 @@
return; // just quit here?
}
- //install flows
installPath(shortestPath.flowEntries(), dstMacAddress);
}
}
@@ -1083,7 +967,7 @@
Set<RibUpdate> prefixesToPush = prefixesWaitingOnArp.removeAll(ipAddress);
for (RibUpdate update : prefixesToPush) {
- //These must always be adds
+ //These will always be adds
log.debug("Pushing prefix {} next hop {}", update.getPrefix(),
update.getRibEntry().nextHop.getHostAddress());
addPrefixFlows(update.getPrefix(), update.getRibEntry());
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java
index 61d50a4..1d2a47b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/PathUpdate.java
@@ -9,25 +9,14 @@
public class PathUpdate {
- //private Set<Interface> srcInterfaces;
private Interface dstInterface;
private InetAddress dstIpAddress;
- public PathUpdate(//Collection<Interface> srcInterfaces,
- Interface dstInterface, InetAddress dstIpAddress) {
+ public PathUpdate(Interface dstInterface, InetAddress dstIpAddress) {
this.dstInterface = dstInterface;
this.dstIpAddress = dstIpAddress;
-
- //this.srcInterfaces = new HashSet<Interface>(srcInterfaces.size());
- //for (Interface intf : srcInterfaces) {
- // this.srcInterfaces.add(intf);
- //}
}
- //public Set<Interface> getSrcInterfaces() {
- // return Collections.unmodifiableSet(srcInterfaces);
- //}
-
public Interface getDstInterface() {
return dstInterface;
}
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 112fb98..f56934d 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/proxyarp/ProxyArpManager.java
@@ -43,26 +43,22 @@
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(IArpRequester requester, boolean retry){
- //this.requesters = new HashSet<IArpRequester>();
this.requester = requester;
this.retry = retry;
this.requestTime = System.currentTimeMillis();
@@ -74,13 +70,6 @@
this.requestTime = System.currentTimeMillis();
}
- /*
- public synchronized void addRequester(IArpRequester requester){
- requestTime = System.currentTimeMillis();
- requesters.add(requester);
- }
- */
-
public boolean isExpired() {
return (System.currentTimeMillis() - requestTime)
> IProxyArpService.ARP_REQUEST_TIMEOUT;
@@ -94,11 +83,6 @@
log.debug("Dispatching reply for {} to {}", ipAddress.getHostAddress(),
requester);
requester.arpResponse(ipAddress, replyMacAddress);
- /*
- for (IArpRequester requester : requesters){
- requester.arpResponse(ipAddress, replyMacAddress);
- }
- */
}
}
@@ -109,100 +93,68 @@
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());
-
- 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();
- }
- }
- }
- */
-
- //List<ArpRequest> retryList = new ArrayList<ArpRequest>();
- SetMultimap<InetAddress, ArpRequest> retryList
- = HashMultimap.<InetAddress, ArpRequest>create();
-
- //Have to synchronize externally on the Multimap while using an iterator.
- //But because we're using a synchronizedMultiMap we don't have to
- //synchronize when adding.
- synchronized (arpRequests) {
- log.debug("Current have {} outstanding requests",
- arpRequests.size());
-
- Iterator<Map.Entry<InetAddress, ArpRequest>> it
- = arpRequests.entries().iterator();
-
- while (it.hasNext()) {
- Map.Entry<InetAddress, ArpRequest> entry
- = it.next();
- ArpRequest request = entry.getValue();
- if (request.isExpired()) {
- log.debug("Cleaning expired ARP request for {}",
- entry.getKey().getHostAddress());
- //TODO retry here? Caller should specify whether we retry or not
- //because we obviously won't retry received packets, only
- //requests from other app modules.
- it.remove();
-
- //retryList.add(new ArpRequest(request));
- if (request.shouldRetry()) {
- retryList.put(entry.getKey(), request);
- }
- }
- }
- }
-
- for (Map.Entry<InetAddress, Collection<ArpRequest>> entry
- : retryList.asMap().entrySet()) {
-
- InetAddress address = entry.getKey();
-
- log.debug("Resending ARP request for {}", address.getHostAddress());
-
- sendArpRequestForAddress(address);
-
- for (ArpRequest request : entry.getValue()) {
- arpRequests.put(address, new ArpRequest(request));
- }
- }
+ doPeriodicArpProcessing();
}
- }, 0, ARP_REQUEST_TIMEOUT_THREAD_PERIOD);
+ }, 0, ARP_TIMER_PERIOD);
}
- private void storeRequester(InetAddress address, IArpRequester requester,
- boolean retry) {
- arpRequests.put(address, new ArpRequest(requester, retry));
- /*
+ /*
+ * 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) {
- if (arpRequests.get(address) == null) {
- arpRequests.put(address, new ArpRequest());
+ 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();
+
+ if (request.shouldRetry()) {
+ retryList.put(entry.getKey(), request);
+ }
+ }
}
- ArpRequest request = arpRequests.get(address);
-
- request.addRequester(requester);
}
- */
+
+ 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));
+ }
+ }
}
@Override
@@ -270,9 +222,6 @@
return;
}
- //storeRequester(target, new HostArpRequester(this, arp, sw.getId(),
- // pi.getInPort()));
-
boolean shouldBroadcastRequest = false;
synchronized (arpRequests) {
if (!arpRequests.containsKey(target)) {
@@ -290,7 +239,6 @@
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);
}
}
@@ -322,32 +270,6 @@
request.dispatchReply(addr, arp.getSenderHardwareAddress());
}
}
- /*
- ArpRequest request = null;
- synchronized (arpRequests) {
- request = arpRequests.get(addr);
- if (request != null) {
- arpRequests.remove(addr);
- }
- }
- if (request != null && !request.isExpired()) {
- request.dispatchReply(addr, 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());
- }
- }
- }
- }*/
}
private synchronized byte[] lookupArpTable(byte[] ipAddress){
@@ -429,7 +351,6 @@
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();
@@ -457,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);
@@ -479,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())
@@ -494,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();
@@ -552,14 +469,8 @@
@Override
public void sendArpRequest(InetAddress ipAddress, IArpRequester requester,
boolean retry) {
- /*
- byte[] lookupMac;
- if ((lookupMac = lookupArpTable(ipAddress.getAddress())) == null) {
- return lookupMac;
- }
- */
-
- storeRequester(ipAddress, requester, retry);
+ arpRequests.put(ipAddress, new ArpRequest(requester, retry));
+ //storeRequester(ipAddress, requester, retry);
sendArpRequestForAddress(ipAddress);
}