Merge pull request #437 from jonohart/master
Sequence number ordering for BGP RIB sync protocol
diff --git a/pom.xml b/pom.xml
index ae020cf..002ba60 100644
--- a/pom.xml
+++ b/pom.xml
@@ -166,6 +166,14 @@
</execution>
</executions>
</plugin>
+ <plugin>
+ <artifactId>maven-assembly-plugin</artifactId>
+ <configuration>
+ <descriptorRefs>
+ <descriptorRef>jar-with-dependencies</descriptorRef>
+ </descriptorRefs>
+ </configuration>
+ </plugin>
</plugins>
</build>
<!-- for getting visualization reporting -->
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 14ecf85..79cca4b 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRoute.java
@@ -80,72 +80,72 @@
IOFSwitchListener, ILayer3InfoService,
IProxyArpService {
- protected static Logger log = LoggerFactory.getLogger(BgpRoute.class);
+ private static Logger log = LoggerFactory.getLogger(BgpRoute.class);
- protected IFloodlightProviderService floodlightProvider;
- protected ITopologyService topologyService;
- protected ITopologyNetService topologyNetService;
- protected ILinkDiscoveryService linkDiscoveryService;
- protected IRestApiService restApi;
+ private IFloodlightProviderService floodlightProvider;
+ private ITopologyService topologyService;
+ private ITopologyNetService topologyNetService;
+ private ILinkDiscoveryService linkDiscoveryService;
+ private IRestApiService restApi;
- protected ProxyArpManager proxyArp;
+ private ProxyArpManager proxyArp;
- protected IPatriciaTrie<RibEntry> ptree;
- protected IPatriciaTrie<Interface> interfacePtrie;
- protected BlockingQueue<RibUpdate> ribUpdates;
+ private IPatriciaTrie<RibEntry> ptree;
+ private IPatriciaTrie<Interface> interfacePtrie;
+ private BlockingQueue<RibUpdate> ribUpdates;
- protected String bgpdRestIp;
- protected String routerId;
- protected String configFilename = "config.json";
+ private String bgpdRestIp;
+ private String routerId;
+ private String configFilename = "config.json";
//We need to identify our flows somehow. But like it says in LearningSwitch.java,
//the controller/OS should hand out cookie IDs to prevent conflicts.
- protected final long APP_COOKIE = 0xa0000000000000L;
+ private final long APP_COOKIE = 0xa0000000000000L;
//Cookie for flows that do L2 forwarding within SDN domain to egress routers
- protected final long L2_FWD_COOKIE = APP_COOKIE + 1;
+ private final long L2_FWD_COOKIE = APP_COOKIE + 1;
//Cookie for flows in ingress switches that rewrite the MAC address
- protected final long MAC_RW_COOKIE = APP_COOKIE + 2;
+ private final long MAC_RW_COOKIE = APP_COOKIE + 2;
//Cookie for flows that setup BGP paths
- protected final long BGP_COOKIE = APP_COOKIE + 3;
+ private final long BGP_COOKIE = APP_COOKIE + 3;
//Forwarding uses priority 0, and the mac rewrite entries in ingress switches
//need to be higher priority than this otherwise the rewrite may not get done
- protected final short SDNIP_PRIORITY = 10;
- protected final short ARP_PRIORITY = 20;
+ private final short SDNIP_PRIORITY = 10;
+ private final short ARP_PRIORITY = 20;
- protected final short BGP_PORT = 179;
+ private final short BGP_PORT = 179;
- protected final int TOPO_DETECTION_WAIT = 2; //seconds
+ private final int TOPO_DETECTION_WAIT = 2; //seconds
//Configuration stuff
- protected List<String> switches;
- protected Map<String, Interface> interfaces;
- protected Map<InetAddress, BgpPeer> bgpPeers;
- protected SwitchPort bgpdAttachmentPoint;
- protected MACAddress bgpdMacAddress;
+ private List<String> switches;
+ private Map<String, Interface> interfaces;
+ private Map<InetAddress, BgpPeer> bgpPeers;
+ private SwitchPort bgpdAttachmentPoint;
+ private MACAddress bgpdMacAddress;
//True when all switches have connected
- protected volatile boolean switchesConnected = false;
+ private volatile boolean switchesConnected = false;
//True when we have a full mesh of shortest paths between gateways
- protected volatile boolean topologyReady = false;
+ private volatile boolean topologyReady = false;
- protected ArrayList<LDUpdate> linkUpdates;
- protected SingletonTask topologyChangeDetectorTask;
+ private ArrayList<LDUpdate> linkUpdates;
+ private SingletonTask topologyChangeDetectorTask;
- protected SetMultimap<InetAddress, RibUpdate> prefixesWaitingOnArp;
+ private SetMultimap<InetAddress, RibUpdate> prefixesWaitingOnArp;
- protected Map<InetAddress, Path> pathsWaitingOnArp;
+ private Map<InetAddress, Path> pathsWaitingOnArp;
- protected ExecutorService bgpUpdatesExecutor;
+ private ExecutorService bgpUpdatesExecutor;
- protected Map<InetAddress, Path> pushedPaths;
- protected Map<Prefix, Path> prefixToPath;
- protected Multimap<Prefix, PushedFlowMod> pushedFlows;
+ private Map<InetAddress, Path> pushedPaths;
+ private Map<Prefix, Path> prefixToPath;
+ private Multimap<Prefix, PushedFlowMod> pushedFlows;
private FlowCache flowCache;
- protected volatile Topology topology = null;
+ private volatile Topology topology = null;
- protected class TopologyChangeDetector implements Runnable {
+ private class TopologyChangeDetector implements Runnable {
@Override
public void run() {
log.debug("Running topology change detection task");
@@ -484,7 +484,7 @@
}
private void addPrefixFlows(Prefix prefix, Interface egressInterface, MACAddress nextHopMacAddress) {
- log.debug("Adding flows for prefix {} added, next hop mac {}",
+ log.debug("Adding flows for prefix {}, next hop mac {}",
prefix, nextHopMacAddress);
//We only need one flow mod per switch, so pick one interface on each switch
@@ -1121,10 +1121,22 @@
RibUpdate update = ribUpdates.take();
switch (update.getOperation()){
case UPDATE:
- processRibAdd(update);
+ if (validateUpdate(update)) {
+ processRibAdd(update);
+ }
+ else {
+ log.debug("Rib UPDATE out of order: {} via {}",
+ update.getPrefix(), update.getRibEntry().getNextHop());
+ }
break;
case DELETE:
- processRibDelete(update);
+ if (validateUpdate(update)) {
+ processRibDelete(update);
+ }
+ else {
+ log.debug("Rib DELETE out of order: {} via {}",
+ update.getPrefix(), update.getRibEntry().getNextHop());
+ }
break;
}
} catch (InterruptedException e) {
@@ -1140,6 +1152,40 @@
}
}
}
+
+ private boolean validateUpdate(RibUpdate update) {
+ RibEntry newEntry = update.getRibEntry();
+ RibEntry oldEntry = ptree.lookup(update.getPrefix());
+
+ //If there is no existing entry we must assume this is the most recent
+ //update. However this might not always be the case as we might have a
+ //POST then DELETE reordering.
+ //if (oldEntry == null || !newEntry.getNextHop().equals(oldEntry.getNextHop())) {
+ if (oldEntry == null) {
+ return true;
+ }
+
+ // This handles the case where routes are gathered in the initial
+ // request because they don't have sequence number info
+ if (newEntry.getSysUpTime() == -1 && newEntry.getSequenceNum() == -1) {
+ return true;
+ }
+
+ if (newEntry.getSysUpTime() > oldEntry.getSysUpTime()) {
+ return true;
+ }
+ else if (newEntry.getSysUpTime() == oldEntry.getSysUpTime()) {
+ if (newEntry.getSequenceNum() > oldEntry.getSequenceNum()) {
+ return true;
+ }
+ else {
+ return false;
+ }
+ }
+ else {
+ return false;
+ }
+ }
@Override
public void topologyChanged() {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteResource.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteResource.java
index 8403f71..c1f0cd6 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteResource.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteResource.java
@@ -68,19 +68,27 @@
IBgpRouteService bgpRoute = (IBgpRouteService) getContext().getAttributes().
get(IBgpRouteService.class.getCanonicalName());
+ String strSysuptime = (String) getRequestAttributes().get("sysuptime");
+ String strSequence = (String) getRequestAttributes().get("sequence");
String routerId = (String) getRequestAttributes().get("routerid");
String prefix = (String) getRequestAttributes().get("prefix");
String mask = (String) getRequestAttributes().get("mask");
String nexthop = (String) getRequestAttributes().get("nexthop");
String capability = (String) getRequestAttributes().get("capability");
+
+ log.debug("sysuptime: {}", strSysuptime);
+ log.debug("sequence: {}", strSequence);
String reply = "";
if (capability == null) {
// this is a prefix add
Prefix p;
+ long sysUpTime, sequenceNum;
try {
p = new Prefix(prefix, Integer.valueOf(mask));
+ sysUpTime = Long.parseLong(strSysuptime);
+ sequenceNum = Long.parseLong(strSequence);
} catch (NumberFormatException e) {
reply = "[POST: mask format is wrong]";
log.info(reply);
@@ -91,7 +99,7 @@
return reply + "\n";
}
- RibEntry rib = new RibEntry(routerId, nexthop);
+ RibEntry rib = new RibEntry(routerId, nexthop, sysUpTime, sequenceNum);
bgpRoute.newRibUpdate(new RibUpdate(Operation.UPDATE, p, rib));
@@ -117,19 +125,27 @@
IBgpRouteService bgpRoute = (IBgpRouteService)getContext().getAttributes().
get(IBgpRouteService.class.getCanonicalName());
+ String strSysuptime = (String) getRequestAttributes().get("sysuptime");
+ String strSequence = (String) getRequestAttributes().get("sequence");
String routerId = (String) getRequestAttributes().get("routerid");
String prefix = (String) getRequestAttributes().get("prefix");
String mask = (String) getRequestAttributes().get("mask");
String nextHop = (String) getRequestAttributes().get("nexthop");
String capability = (String) getRequestAttributes().get("capability");
+ log.debug("sysuptime: {}", strSysuptime);
+ log.debug("sequence: {}", strSequence);
+
String reply = "";
if (capability == null) {
// this is a prefix delete
Prefix p;
+ long sysUpTime, sequenceNum;
try {
p = new Prefix(prefix, Integer.valueOf(mask));
+ sysUpTime = Long.parseLong(strSysuptime);
+ sequenceNum = Long.parseLong(strSequence);
} catch (NumberFormatException e) {
reply = "[DELE: mask format is wrong]";
log.info(reply);
@@ -140,7 +156,7 @@
return reply + "\n";
}
- RibEntry r = new RibEntry(routerId, nextHop);
+ RibEntry r = new RibEntry(routerId, nextHop, sysUpTime, sequenceNum);
bgpRoute.newRibUpdate(new RibUpdate(Operation.DELETE, p, r));
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteWebRoutable.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteWebRoutable.java
index 669c385..26971b0 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteWebRoutable.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/BgpRouteWebRoutable.java
@@ -12,7 +12,7 @@
Router router = new Router(context);
router.attach("/json", BgpRouteResource.class);
router.attach("/rib/{dest}", BgpRouteResource.class);
- router.attach("/{routerid}/{prefix}/{mask}/{nexthop}", BgpRouteResource.class);
+ router.attach("/{sysuptime}/{sequence}/{routerid}/{prefix}/{mask}/{nexthop}", BgpRouteResource.class);
router.attach("/{routerid}/{prefix}/{mask}/{nexthop}/synch", BgpRouteResourceSynch.class);
router.attach("/{routerid}/{capability}", BgpRouteResource.class);
return router;
diff --git a/src/main/java/net/onrc/onos/ofcontroller/bgproute/RibEntry.java b/src/main/java/net/onrc/onos/ofcontroller/bgproute/RibEntry.java
index 1520c60..ccf8951 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/bgproute/RibEntry.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/bgproute/RibEntry.java
@@ -7,21 +7,57 @@
public class RibEntry {
private final InetAddress routerId;
private final InetAddress nextHop;
+
+ /*
+ * Store the sequence number information provided on the update here for
+ * now. I think this *should* really be in the RibUpdate, and we should
+ * store RibUpdates in the Ptrie. But, that's a bigger change to change
+ * what the Ptrie stores.
+ */
+ private final long sysUpTime;
+ private final long sequenceNum;
+
+ /*
+ * Marker for RibEntries where we don't have sequence number info.
+ * The user of this class should make sure they don't check this data
+ * if they don't provide it.
+ */
+ private final static long NULL_TIME = -1;
public RibEntry(InetAddress routerId, InetAddress nextHop) {
this.routerId = routerId;
this.nextHop = nextHop;
+ sequenceNum = NULL_TIME;
+ sysUpTime = NULL_TIME;
}
public RibEntry(String routerId, String nextHop) {
this.routerId = InetAddresses.forString(routerId);
this.nextHop = InetAddresses.forString(nextHop);
+ sequenceNum = NULL_TIME;
+ sysUpTime = NULL_TIME;
+ }
+
+ public RibEntry(String routerId, String nextHop, long sysUpTime
+ , long sequenceNum) {
+ this.routerId = InetAddresses.forString(routerId);
+ this.nextHop = InetAddresses.forString(nextHop);
+ this.sequenceNum = sequenceNum;
+ this.sysUpTime = sysUpTime;
}
public InetAddress getNextHop() {
return nextHop;
}
+ public long getSysUpTime() {
+ return sysUpTime;
+ }
+
+ public long getSequenceNum() {
+ return sequenceNum;
+ }
+
@Override
public boolean equals(Object other) {
if (other == null || !(other instanceof RibEntry)) {