Updated SDN-IP to use the Intent framework batch-based intents.
Also, created a local cache of IPv4-to-MAC address mapping,
to avoid relatively costly hostService.getHostsByIp() calls
per added route.
Change-Id: I8abed378985708e883fd99e85c54b01f38756515
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
index ac69faa..858f064 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/IntentSynchronizer.java
@@ -26,6 +26,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.Semaphore;
+import org.apache.commons.lang3.tuple.Pair;
import org.onlab.onos.core.ApplicationId;
import org.onlab.onos.net.flow.criteria.Criteria.IPCriterion;
import org.onlab.onos.net.flow.criteria.Criterion;
@@ -116,7 +117,7 @@
//
log.debug("SDN-IP Intent Synchronizer shutdown: " +
"withdrawing all intents...");
- IntentOperations.Builder builder = IntentOperations.builder();
+ IntentOperations.Builder builder = IntentOperations.builder(appId);
for (Intent intent : intentService.getIntents()) {
// Skip the intents from other applications
if (!intent.appId().equals(appId)) {
@@ -234,51 +235,84 @@
}
/**
- * Submits a multi-point-to-single-point intent.
+ * Updates multi-point-to-single-point route intents.
*
- * @param prefix the IPv4 matching prefix for the intent to submit
- * @param intent the intent to submit
+ * @param submitIntents the intents to submit
+ * @param withdrawPrefixes the IPv4 matching prefixes for the intents
+ * to withdraw
*/
- void submitRouteIntent(Ip4Prefix prefix,
- MultiPointToSinglePointIntent intent) {
- synchronized (this) {
- MultiPointToSinglePointIntent oldIntent =
- routeIntents.put(prefix, intent);
+ void updateRouteIntents(
+ Collection<Pair<Ip4Prefix, MultiPointToSinglePointIntent>> submitIntents,
+ Collection<Ip4Prefix> withdrawPrefixes) {
- if (isElectedLeader && isActivatedLeader) {
- if (oldIntent != null) {
- //
- // TODO: Short-term solution to explicitly withdraw
- // instead of using "replace" operation.
- //
- log.debug("SDN-IP Withdrawing old intent: {}", oldIntent);
- intentService.withdraw(oldIntent);
+ //
+ // NOTE: Semantically, we MUST withdraw existing intents before
+ // submitting new intents.
+ //
+ synchronized (this) {
+ MultiPointToSinglePointIntent intent;
+
+ log.debug("SDN-IP submitting intents = {} withdrawing = {}",
+ submitIntents.size(), withdrawPrefixes.size());
+
+ //
+ // Prepare the Intent batch operations for the intents to withdraw
+ //
+ IntentOperations.Builder withdrawBuilder =
+ IntentOperations.builder(appId);
+ for (Ip4Prefix prefix : withdrawPrefixes) {
+ intent = routeIntents.remove(prefix);
+ if (intent == null) {
+ log.debug("SDN-IP No intent in routeIntents to delete " +
+ "for prefix: {}", prefix);
+ continue;
}
- log.debug("SDN-IP Submitting intent: {}", intent);
- intentService.submit(intent);
- }
- }
- }
-
- /**
- * Withdraws a multi-point-to-single-point intent.
- *
- * @param prefix the IPv4 matching prefix for the intent to withdraw.
- */
- void withdrawRouteIntent(Ip4Prefix prefix) {
- synchronized (this) {
- MultiPointToSinglePointIntent intent =
- routeIntents.remove(prefix);
-
- if (intent == null) {
- log.debug("SDN-IP no intent in routeIntents to delete for " +
- "prefix: {}", prefix);
- return;
+ if (isElectedLeader && isActivatedLeader) {
+ log.debug("SDN-IP Withdrawing intent: {}", intent);
+ withdrawBuilder.addWithdrawOperation(intent.id());
+ }
}
+ //
+ // Prepare the Intent batch operations for the intents to submit
+ //
+ IntentOperations.Builder submitBuilder =
+ IntentOperations.builder(appId);
+ for (Pair<Ip4Prefix, MultiPointToSinglePointIntent> pair :
+ submitIntents) {
+ Ip4Prefix prefix = pair.getLeft();
+ intent = pair.getRight();
+ MultiPointToSinglePointIntent oldIntent =
+ routeIntents.put(prefix, intent);
+ if (isElectedLeader && isActivatedLeader) {
+ if (oldIntent != null) {
+ //
+ // TODO: Short-term solution to explicitly withdraw
+ // instead of using "replace" operation.
+ //
+ log.debug("SDN-IP Withdrawing old intent: {}",
+ oldIntent);
+ withdrawBuilder.addWithdrawOperation(oldIntent.id());
+ }
+ log.debug("SDN-IP Submitting intent: {}", intent);
+ submitBuilder.addSubmitOperation(intent);
+ }
+ }
+
+ //
+ // Submit the Intent operations
+ //
if (isElectedLeader && isActivatedLeader) {
- log.debug("SDN-IP Withdrawing intent: {}", intent);
- intentService.withdraw(intent);
+ IntentOperations intentOperations = withdrawBuilder.build();
+ if (!intentOperations.operations().isEmpty()) {
+ log.debug("SDN-IP Withdrawing intents executed");
+ intentService.execute(intentOperations);
+ }
+ intentOperations = submitBuilder.build();
+ if (!intentOperations.operations().isEmpty()) {
+ log.debug("SDN-IP Submitting intents executed");
+ intentService.execute(intentOperations);
+ }
}
}
}
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/RouteListener.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/RouteListener.java
index 0058e25..cf573a0 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/RouteListener.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/RouteListener.java
@@ -15,6 +15,8 @@
*/
package org.onlab.onos.sdnip;
+import java.util.Collection;
+
/**
* An interface to receive route updates from route providers.
*/
@@ -22,7 +24,7 @@
/**
* Receives a route update from a route provider.
*
- * @param routeUpdate the updated route information
+ * @param routeUpdates the collection with updated route information
*/
- public void update(RouteUpdate routeUpdate);
+ public void update(Collection<RouteUpdate> routeUpdates);
}
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
index f26aab3..2935427 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/Router.java
@@ -20,12 +20,15 @@
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
+import org.apache.commons.lang3.tuple.Pair;
import org.onlab.onos.core.ApplicationId;
import org.onlab.onos.net.ConnectPoint;
import org.onlab.onos.net.Host;
@@ -74,11 +77,14 @@
private InvertedRadixTree<RouteEntry> bgpRoutes;
// Stores all incoming route updates in a queue.
- private final BlockingQueue<RouteUpdate> routeUpdates;
+ private final BlockingQueue<Collection<RouteUpdate>> routeUpdatesQueue;
// The Ip4Address is the next hop address of each route update.
private final SetMultimap<Ip4Address, RouteEntry> routesWaitingOnArp;
+ // The IPv4 address to MAC address mapping
+ private final Map<Ip4Address, MacAddress> ip2Mac;
+
private final ApplicationId appId;
private final IntentSynchronizer intentSynchronizer;
private final HostService hostService;
@@ -110,9 +116,10 @@
bgpRoutes = new ConcurrentInvertedRadixTree<>(
new DefaultByteArrayNodeFactory());
- routeUpdates = new LinkedBlockingQueue<>();
+ routeUpdatesQueue = new LinkedBlockingQueue<>();
routesWaitingOnArp = Multimaps.synchronizedSetMultimap(
HashMultimap.<Ip4Address, RouteEntry>create());
+ ip2Mac = new ConcurrentHashMap<>();
bgpUpdatesExecutor = Executors.newSingleThreadExecutor(
new ThreadFactoryBuilder()
@@ -146,19 +153,18 @@
// Cleanup all local state
bgpRoutes = new ConcurrentInvertedRadixTree<>(
new DefaultByteArrayNodeFactory());
- routeUpdates.clear();
+ routeUpdatesQueue.clear();
routesWaitingOnArp.clear();
+ ip2Mac.clear();
}
}
@Override
- public void update(RouteUpdate routeUpdate) {
- log.debug("Received new route update: {}", routeUpdate);
-
+ public void update(Collection<RouteUpdate> routeUpdates) {
try {
- routeUpdates.put(routeUpdate);
+ routeUpdatesQueue.put(routeUpdates);
} catch (InterruptedException e) {
- log.debug("Interrupted while putting on routeUpdates queue", e);
+ log.debug("Interrupted while putting on routeUpdatesQueue", e);
Thread.currentThread().interrupt();
}
}
@@ -171,18 +177,9 @@
try {
while (!interrupted) {
try {
- RouteUpdate update = routeUpdates.take();
- switch (update.type()) {
- case UPDATE:
- processRouteAdd(update.routeEntry());
- break;
- case DELETE:
- processRouteDelete(update.routeEntry());
- break;
- default:
- log.error("Unknown update Type: {}", update.type());
- break;
- }
+ Collection<RouteUpdate> routeUpdates =
+ routeUpdatesQueue.take();
+ processRouteUpdates(routeUpdates);
} catch (InterruptedException e) {
log.debug("Interrupted while taking from updates queue", e);
interrupted = true;
@@ -198,98 +195,137 @@
}
/**
+ * Processes route updates.
+ *
+ * @param routeUpdates the route updates to process
+ */
+ void processRouteUpdates(Collection<RouteUpdate> routeUpdates) {
+ synchronized (this) {
+ Collection<Pair<Ip4Prefix, MultiPointToSinglePointIntent>>
+ submitIntents = new LinkedList<>();
+ Collection<Ip4Prefix> withdrawPrefixes = new LinkedList<>();
+ MultiPointToSinglePointIntent intent;
+
+ for (RouteUpdate update : routeUpdates) {
+ switch (update.type()) {
+ case UPDATE:
+ intent = processRouteAdd(update.routeEntry(),
+ withdrawPrefixes);
+ if (intent != null) {
+ submitIntents.add(Pair.of(update.routeEntry().prefix(),
+ intent));
+ }
+ break;
+ case DELETE:
+ processRouteDelete(update.routeEntry(), withdrawPrefixes);
+ break;
+ default:
+ log.error("Unknown update Type: {}", update.type());
+ break;
+ }
+ }
+
+ intentSynchronizer.updateRouteIntents(submitIntents,
+ withdrawPrefixes);
+ }
+ }
+
+ /**
* Processes adding a route entry.
* <p>
- * Put new route entry into the radix tree. If there was an existing
- * next hop for this prefix, but the next hop was different, then execute
- * deleting old route entry. If the next hop is the SDN domain, we do not
- * handle it at the moment. Otherwise, execute adding a route.
+ * The route entry is added to the radix tree. If there was an existing
+ * next hop for this prefix, but the next hop was different, then the
+ * old route entry is deleted.
* </p>
- *
- * @param routeEntry the route entry to add
- */
- void processRouteAdd(RouteEntry routeEntry) {
- synchronized (this) {
- log.debug("Processing route add: {}", routeEntry);
-
- Ip4Prefix prefix = routeEntry.prefix();
- Ip4Address nextHop = null;
- RouteEntry foundRouteEntry =
- bgpRoutes.put(RouteEntry.createBinaryString(prefix),
- routeEntry);
- if (foundRouteEntry != null) {
- nextHop = foundRouteEntry.nextHop();
- }
-
- if (nextHop != null && !nextHop.equals(routeEntry.nextHop())) {
- // 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
- executeRouteDelete(routeEntry);
- }
- if (nextHop != null && nextHop.equals(routeEntry.nextHop())) {
- return;
- }
-
- if (routeEntry.nextHop().equals(LOCAL_NEXT_HOP)) {
- // Route originated by SDN domain
- // We don't handle these at the moment
- log.debug("Own route {} to {}",
- routeEntry.prefix(), routeEntry.nextHop());
- return;
- }
-
- executeRouteAdd(routeEntry);
- }
- }
-
- /**
- * Executes adding a route entry.
* <p>
- * Find out the egress Interface and MAC address of next hop router for
- * this route entry. If the MAC address can not be found in ARP cache,
- * then this prefix will be put in routesWaitingOnArp queue. Otherwise,
- * new route intent will be created and installed.
+ * NOTE: Currently, we don't handle routes if the next hop is within the
+ * SDN domain.
* </p>
*
* @param routeEntry the route entry to add
+ * @param withdrawPrefixes the collection of accumulated prefixes whose
+ * intents will be withdrawn
+ * @return the corresponding intent that should be submitted, or null
*/
- private void executeRouteAdd(RouteEntry routeEntry) {
- log.debug("Executing route add: {}", routeEntry);
+ private MultiPointToSinglePointIntent processRouteAdd(
+ RouteEntry routeEntry,
+ Collection<Ip4Prefix> withdrawPrefixes) {
+ log.debug("Processing route add: {}", routeEntry);
- // Monitor the IP address so we'll get notified of updates to the MAC
- // address.
+ Ip4Prefix prefix = routeEntry.prefix();
+ Ip4Address nextHop = null;
+ RouteEntry foundRouteEntry =
+ bgpRoutes.put(RouteEntry.createBinaryString(prefix),
+ routeEntry);
+ if (foundRouteEntry != null) {
+ nextHop = foundRouteEntry.nextHop();
+ }
+
+ if (nextHop != null && !nextHop.equals(routeEntry.nextHop())) {
+ // 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
+ withdrawPrefixes.add(routeEntry.prefix());
+ }
+ if (nextHop != null && nextHop.equals(routeEntry.nextHop())) {
+ return null;
+ }
+
+ if (routeEntry.nextHop().equals(LOCAL_NEXT_HOP)) {
+ // Route originated by SDN domain
+ // We don't handle these at the moment
+ log.debug("Own route {} to {}",
+ routeEntry.prefix(), routeEntry.nextHop());
+ return null;
+ }
+
+ //
+ // Find the MAC address of next hop router for this route entry.
+ // If the MAC address can not be found in ARP cache, then this prefix
+ // will be put in routesWaitingOnArp queue. Otherwise, generate
+ // a new route intent.
+ //
+
+ // Monitor the IP address for updates of the MAC address
hostService.startMonitoringIp(routeEntry.nextHop());
- // See if we know the MAC address of the next hop
- MacAddress nextHopMacAddress = null;
- Set<Host> hosts = hostService.getHostsByIp(routeEntry.nextHop());
- if (!hosts.isEmpty()) {
- // TODO how to handle if multiple hosts are returned?
- nextHopMacAddress = hosts.iterator().next().mac();
+ // Check if we know the MAC address of the next hop
+ MacAddress nextHopMacAddress = ip2Mac.get(routeEntry.nextHop());
+ if (nextHopMacAddress == null) {
+ Set<Host> hosts = hostService.getHostsByIp(routeEntry.nextHop());
+ if (!hosts.isEmpty()) {
+ // TODO how to handle if multiple hosts are returned?
+ nextHopMacAddress = hosts.iterator().next().mac();
+ }
+ if (nextHopMacAddress != null) {
+ ip2Mac.put(routeEntry.nextHop(), nextHopMacAddress);
+ }
}
-
if (nextHopMacAddress == null) {
routesWaitingOnArp.put(routeEntry.nextHop(), routeEntry);
- return;
+ return null;
}
-
- addRouteIntentToNextHop(routeEntry.prefix(),
- routeEntry.nextHop(),
- nextHopMacAddress);
+ return generateRouteIntent(routeEntry.prefix(), routeEntry.nextHop(),
+ nextHopMacAddress);
}
/**
- * Adds a route intent given a prefix and a next hop IP address. This
- * method will find the egress interface for the intent.
+ * Generates a route intent for a prefix, the next hop IP address, and
+ * the next hop MAC address.
+ * <p/>
+ * This method will find the egress interface for the intent.
+ * Intent will match dst IP prefix and rewrite dst MAC address at all other
+ * border switches, then forward packets according to dst MAC address.
*
* @param prefix IP prefix of the route to add
* @param nextHopIpAddress IP address of the next hop
* @param nextHopMacAddress MAC address of the next hop
+ * @return the generated intent, or null if no intent should be submitted
*/
- private void addRouteIntentToNextHop(Ip4Prefix prefix,
- Ip4Address nextHopIpAddress,
- MacAddress nextHopMacAddress) {
+ private MultiPointToSinglePointIntent generateRouteIntent(
+ Ip4Prefix prefix,
+ Ip4Address nextHopIpAddress,
+ MacAddress nextHopMacAddress) {
// Find the attachment point (egress interface) of the next hop
Interface egressInterface;
@@ -308,30 +344,17 @@
if (egressInterface == null) {
log.warn("No outgoing interface found for {}",
nextHopIpAddress);
- return;
+ return null;
}
}
- doAddRouteIntent(prefix, egressInterface, nextHopMacAddress);
- }
-
- /**
- * Installs a route intent for a prefix.
- * <p/>
- * Intent will match dst IP prefix and rewrite dst MAC address at all other
- * border switches, then forward packets according to dst MAC address.
- *
- * @param prefix IP prefix from route
- * @param egressInterface egress Interface connected to next hop router
- * @param nextHopMacAddress MAC address of next hop router
- */
- private void doAddRouteIntent(Ip4Prefix prefix, Interface egressInterface,
- MacAddress nextHopMacAddress) {
- log.debug("Adding intent for prefix {}, next hop mac {}",
- prefix, nextHopMacAddress);
-
+ //
+ // Generate the intent itself
+ //
Set<ConnectPoint> ingressPorts = new HashSet<>();
ConnectPoint egressPort = egressInterface.connectPoint();
+ log.debug("Generating intent for prefix {}, next hop mac {}",
+ prefix, nextHopMacAddress);
for (Interface intf : interfaceService.getInterfaces()) {
if (!intf.connectPoint().equals(egressInterface.connectPoint())) {
@@ -351,51 +374,39 @@
.setEthDst(nextHopMacAddress)
.build();
- MultiPointToSinglePointIntent intent =
- new MultiPointToSinglePointIntent(appId, selector, treatment,
- ingressPorts, egressPort);
-
- intentSynchronizer.submitRouteIntent(prefix, intent);
+ return new MultiPointToSinglePointIntent(appId, selector, treatment,
+ ingressPorts, egressPort);
}
/**
- * Executes deleting a route entry.
+ * Processes the deletion of a route entry.
* <p>
- * Removes prefix from radix tree, and if successful, then try to delete
- * the related intent.
+ * The prefix for the routing entry is removed from radix tree.
+ * If the operation is successful, the prefix is added to the collection
+ * of prefixes whose intents that will be withdrawn.
* </p>
*
* @param routeEntry the route entry to delete
+ * @param withdrawPrefixes the collection of accumulated prefixes whose
+ * intents will be withdrawn
*/
- void processRouteDelete(RouteEntry routeEntry) {
- synchronized (this) {
- log.debug("Processing route delete: {}", routeEntry);
- Ip4Prefix prefix = routeEntry.prefix();
+ private void processRouteDelete(RouteEntry routeEntry,
+ Collection<Ip4Prefix> withdrawPrefixes) {
+ log.debug("Processing route delete: {}", routeEntry);
+ Ip4Prefix prefix = routeEntry.prefix();
- if (bgpRoutes.remove(RouteEntry.createBinaryString(prefix))) {
- //
- // Only delete flows if an entry was actually removed from the
- // tree. If no entry was removed, the <prefix, nexthop> wasn't
- // there so it's probably already been removed and we don't
- // need to do anything.
- //
- executeRouteDelete(routeEntry);
- }
-
- routesWaitingOnArp.remove(routeEntry.nextHop(), routeEntry);
- // TODO cancel the request in the ARP manager as well
+ if (bgpRoutes.remove(RouteEntry.createBinaryString(prefix))) {
+ //
+ // Only withdraw intents if an entry was actually removed from the
+ // tree. If no entry was removed, the <prefix, nexthop> wasn't
+ // there so it's probably already been removed and we don't
+ // need to do anything.
+ //
+ withdrawPrefixes.add(routeEntry.prefix());
}
- }
- /**
- * Executed deleting a route entry.
- *
- * @param routeEntry the route entry to delete
- */
- private void executeRouteDelete(RouteEntry routeEntry) {
- log.debug("Executing route delete: {}", routeEntry);
-
- intentSynchronizer.withdrawRouteIntent(routeEntry.prefix());
+ routesWaitingOnArp.remove(routeEntry.nextHop(), routeEntry);
+ // TODO cancel the request in the ARP manager as well
}
/**
@@ -418,6 +429,9 @@
// while we're pushing intents. If the tree changes, the
// tree and intents could get out of sync.
synchronized (this) {
+ Collection<Pair<Ip4Prefix, MultiPointToSinglePointIntent>>
+ submitIntents = new LinkedList<>();
+ MultiPointToSinglePointIntent intent;
Set<RouteEntry> routesToPush =
routesWaitingOnArp.removeAll(ipAddress);
@@ -429,18 +443,30 @@
RouteEntry foundRouteEntry =
bgpRoutes.getValueForExactKey(binaryString);
if (foundRouteEntry != null &&
- foundRouteEntry.nextHop().equals(routeEntry.nextHop())) {
+ foundRouteEntry.nextHop().equals(routeEntry.nextHop())) {
// We only push prefix flows if the prefix is still in the
// radix tree and the next hop is the same as our
// update.
// The prefix could have been removed while we were waiting
// for the ARP, or the next hop could have changed.
- addRouteIntentToNextHop(prefix, ipAddress, macAddress);
+ intent = generateRouteIntent(prefix, ipAddress,
+ macAddress);
+ if (intent != null) {
+ submitIntents.add(Pair.of(prefix, intent));
+ }
} else {
log.debug("{} has been revoked before the MAC was resolved",
- routeEntry);
+ routeEntry);
}
}
+
+ if (!submitIntents.isEmpty()) {
+ Collection<Ip4Prefix> withdrawPrefixes = new LinkedList<>();
+ intentSynchronizer.updateRouteIntents(submitIntents,
+ withdrawPrefixes);
+ }
+
+ ip2Mac.put(ipAddress, macAddress);
}
}
@@ -469,9 +495,13 @@
class InternalHostListener implements HostListener {
@Override
public void event(HostEvent event) {
- if (event.type() == HostEvent.Type.HOST_ADDED ||
- event.type() == HostEvent.Type.HOST_UPDATED) {
- Host host = event.subject();
+ log.debug("Received HostEvent {}", event);
+
+ Host host = event.subject();
+ switch (event.type()) {
+ case HOST_ADDED:
+ // FALLTHROUGH
+ case HOST_UPDATED:
for (IpAddress ip : host.ipAddresses()) {
Ip4Address ip4Address = ip.getIp4Address();
if (ip4Address == null) {
@@ -480,6 +510,19 @@
}
updateMac(ip4Address, host.mac());
}
+ break;
+ case HOST_REMOVED:
+ for (IpAddress ip : host.ipAddresses()) {
+ Ip4Address ip4Address = ip.getIp4Address();
+ if (ip4Address == null) {
+ // TODO: For now we support only IPv4
+ continue;
+ }
+ ip2Mac.remove(ip4Address);
+ }
+ break;
+ default:
+ break;
}
}
}
diff --git a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSessionManager.java b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSessionManager.java
index 1ef19c6..c66d2a1 100644
--- a/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSessionManager.java
+++ b/apps/sdnip/src/main/java/org/onlab/onos/sdnip/bgp/BgpSessionManager.java
@@ -22,6 +22,7 @@
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Collection;
+import java.util.LinkedList;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
@@ -248,18 +249,28 @@
synchronized void routeUpdates(BgpSession bgpSession,
Collection<BgpRouteEntry> addedBgpRouteEntries,
Collection<BgpRouteEntry> deletedBgpRouteEntries) {
+ Collection<RouteUpdate> routeUpdates = new LinkedList<>();
+ RouteUpdate routeUpdate;
+
if (isShutdown) {
return; // Ignore any leftover updates if shutdown
}
// Process the deleted route entries
for (BgpRouteEntry bgpRouteEntry : deletedBgpRouteEntries) {
- processDeletedRoute(bgpSession, bgpRouteEntry);
+ routeUpdate = processDeletedRoute(bgpSession, bgpRouteEntry);
+ if (routeUpdate != null) {
+ routeUpdates.add(routeUpdate);
+ }
}
// Process the added/updated route entries
for (BgpRouteEntry bgpRouteEntry : addedBgpRouteEntries) {
- processAddedRoute(bgpSession, bgpRouteEntry);
+ routeUpdate = processAddedRoute(bgpSession, bgpRouteEntry);
+ if (routeUpdate != null) {
+ routeUpdates.add(routeUpdate);
+ }
}
+ routeListener.update(routeUpdates);
}
/**
@@ -268,9 +279,11 @@
* @param bgpSession the BGP session the route entry update was
* received on
* @param bgpRouteEntry the added/updated route entry
+ * @return the result route update that should be forwarded to the
+ * Route Listener, or null if no route update should be forwarded
*/
- private void processAddedRoute(BgpSession bgpSession,
- BgpRouteEntry bgpRouteEntry) {
+ private RouteUpdate processAddedRoute(BgpSession bgpSession,
+ BgpRouteEntry bgpRouteEntry) {
RouteUpdate routeUpdate;
BgpRouteEntry bestBgpRouteEntry =
bgpRoutes.get(bgpRouteEntry.prefix());
@@ -284,9 +297,7 @@
bgpRoutes.put(bgpRouteEntry.prefix(), bgpRouteEntry);
routeUpdate =
new RouteUpdate(RouteUpdate.Type.UPDATE, bgpRouteEntry);
- // Forward the result route updates to the Route Listener
- routeListener.update(routeUpdate);
- return;
+ return routeUpdate;
}
//
@@ -296,7 +307,7 @@
//
if (bestBgpRouteEntry.getBgpSession() !=
bgpRouteEntry.getBgpSession()) {
- return;
+ return null; // Nothing to do
}
// Find the next best route
@@ -315,8 +326,7 @@
bgpRoutes.put(bestBgpRouteEntry.prefix(), bestBgpRouteEntry);
routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
bestBgpRouteEntry);
- // Forward the result route updates to the Route Listener
- routeListener.update(routeUpdate);
+ return routeUpdate;
}
/**
@@ -325,9 +335,11 @@
* @param bgpSession the BGP session the route entry update was
* received on
* @param bgpRouteEntry the deleted route entry
+ * @return the result route update that should be forwarded to the
+ * Route Listener, or null if no route update should be forwarded
*/
- private void processDeletedRoute(BgpSession bgpSession,
- BgpRouteEntry bgpRouteEntry) {
+ private RouteUpdate processDeletedRoute(BgpSession bgpSession,
+ BgpRouteEntry bgpRouteEntry) {
RouteUpdate routeUpdate;
BgpRouteEntry bestBgpRouteEntry =
bgpRoutes.get(bgpRouteEntry.prefix());
@@ -340,7 +352,7 @@
// because we need to check whether this is same object.
//
if (bgpRouteEntry != bestBgpRouteEntry) {
- return; // Nothing to do
+ return null; // Nothing to do
}
//
@@ -353,9 +365,7 @@
bestBgpRouteEntry);
routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
bestBgpRouteEntry);
- // Forward the result route updates to the Route Listener
- routeListener.update(routeUpdate);
- return;
+ return routeUpdate;
}
//
@@ -364,8 +374,7 @@
bgpRoutes.remove(bgpRouteEntry.prefix());
routeUpdate = new RouteUpdate(RouteUpdate.Type.DELETE,
bgpRouteEntry);
- // Forward the result route updates to the Route Listener
- routeListener.update(routeUpdate);
+ return routeUpdate;
}
/**
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
index 2cb74f5..d0c0aeb 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/IntentSyncTest.java
@@ -325,13 +325,13 @@
.andReturn(IntentState.WITHDRAWING).anyTimes();
expect(intentService.getIntents()).andReturn(intents).anyTimes();
- IntentOperations.Builder builder = IntentOperations.builder(null); //FIXME null
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
builder.addWithdrawOperation(intent2.id());
builder.addWithdrawOperation(intent4.id());
intentService.execute(TestIntentServiceHelper.eqExceptId(
builder.build()));
- builder = IntentOperations.builder(null); //FIXME null
+ builder = IntentOperations.builder(APPID);
builder.addSubmitOperation(intent3);
builder.addSubmitOperation(intent4Update);
builder.addSubmitOperation(intent6);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
index 0c39582..46824ca 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/PeerConnectivityManagerTest.java
@@ -566,7 +566,7 @@
reset(intentService);
// Setup the expected intents
- IntentOperations.Builder builder = IntentOperations.builder(null); //FIXME null
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
for (Intent intent : intentList) {
builder.addSubmitOperation(intent);
}
@@ -601,9 +601,8 @@
replay(configInfoService);
reset(intentService);
- IntentOperations.Builder builder = IntentOperations.builder(null); //FIXME null
- intentService.execute(TestIntentServiceHelper.eqExceptId(
- builder.build()));
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ intentService.execute(builder.build());
replay(intentService);
peerConnectivityManager.start();
verify(intentService);
@@ -627,9 +626,8 @@
replay(configInfoService);
reset(intentService);
- IntentOperations.Builder builder = IntentOperations.builder(null); //FIXME null
- intentService.execute(TestIntentServiceHelper.eqExceptId(
- builder.build()));
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ intentService.execute(builder.build());
replay(intentService);
peerConnectivityManager.start();
verify(intentService);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
index f15d50f..e877f34 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTest.java
@@ -25,6 +25,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -50,6 +51,7 @@
import org.onlab.onos.net.host.HostService;
import org.onlab.onos.net.host.InterfaceIpAddress;
import org.onlab.onos.net.intent.Intent;
+import org.onlab.onos.net.intent.IntentOperations;
import org.onlab.onos.net.intent.IntentService;
import org.onlab.onos.net.intent.MultiPointToSinglePointIntent;
import org.onlab.onos.net.intent.AbstractIntentTest;
@@ -234,7 +236,7 @@
* This method tests adding a route entry.
*/
@Test
- public void testProcessRouteAdd() throws TestUtilsException {
+ public void testRouteAdd() throws TestUtilsException {
// Construct a route entry
RouteEntry routeEntry = new RouteEntry(
Ip4Prefix.valueOf("1.1.1.0/24"),
@@ -261,13 +263,19 @@
// Set up test expectation
reset(intentService);
- intentService.submit(TestIntentServiceHelper.eqExceptId(intent));
+ // Setup the expected intents
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addSubmitOperation(intent);
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call the processRouteAdd() method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteAdd(routeEntry);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
+ routeEntry);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
// Verify
assertEquals(router.getRoutes().size(), 1);
@@ -289,32 +297,16 @@
@Test
public void testRouteUpdate() throws TestUtilsException {
// Firstly add a route
- testProcessRouteAdd();
+ testRouteAdd();
+
+ Intent addedIntent =
+ intentSynchronizer.getRouteIntents().iterator().next();
// Construct the existing route entry
RouteEntry routeEntry = new RouteEntry(
Ip4Prefix.valueOf("1.1.1.0/24"),
Ip4Address.valueOf("192.168.10.1"));
- // Construct the existing MultiPointToSinglePointIntent intent
- TrafficSelector.Builder selectorBuilder =
- DefaultTrafficSelector.builder();
- selectorBuilder.matchEthType(Ethernet.TYPE_IPV4).matchIPDst(
- routeEntry.prefix());
-
- TrafficTreatment.Builder treatmentBuilder =
- DefaultTrafficTreatment.builder();
- treatmentBuilder.setEthDst(MacAddress.valueOf("00:00:00:00:00:01"));
-
- Set<ConnectPoint> ingressPoints = new HashSet<ConnectPoint>();
- ingressPoints.add(SW2_ETH1);
- ingressPoints.add(SW3_ETH1);
-
- MultiPointToSinglePointIntent intent =
- new MultiPointToSinglePointIntent(APPID,
- selectorBuilder.build(), treatmentBuilder.build(),
- ingressPoints, SW1_ETH1);
-
// Start to construct a new route entry and new intent
RouteEntry routeEntryUpdate = new RouteEntry(
Ip4Prefix.valueOf("1.1.1.0/24"),
@@ -343,14 +335,23 @@
// Set up test expectation
reset(intentService);
- intentService.withdraw(TestIntentServiceHelper.eqExceptId(intent));
- intentService.submit(TestIntentServiceHelper.eqExceptId(intentNew));
+ // Setup the expected intents
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addWithdrawOperation(addedIntent.id());
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
+ builder = IntentOperations.builder(APPID);
+ builder.addSubmitOperation(intentNew);
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call the processRouteAdd() method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteAdd(routeEntryUpdate);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
+ routeEntryUpdate);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
// Verify
assertEquals(router.getRoutes().size(), 1);
@@ -368,43 +369,33 @@
* This method tests deleting a route entry.
*/
@Test
- public void testProcessRouteDelete() throws TestUtilsException {
+ public void testRouteDelete() throws TestUtilsException {
// Firstly add a route
- testProcessRouteAdd();
+ testRouteAdd();
+
+ Intent addedIntent =
+ intentSynchronizer.getRouteIntents().iterator().next();
// Construct the existing route entry
RouteEntry routeEntry = new RouteEntry(
Ip4Prefix.valueOf("1.1.1.0/24"),
Ip4Address.valueOf("192.168.10.1"));
- // Construct the existing MultiPointToSinglePointIntent intent
- TrafficSelector.Builder selectorBuilder =
- DefaultTrafficSelector.builder();
- selectorBuilder.matchEthType(Ethernet.TYPE_IPV4).matchIPDst(
- routeEntry.prefix());
-
- TrafficTreatment.Builder treatmentBuilder =
- DefaultTrafficTreatment.builder();
- treatmentBuilder.setEthDst(MacAddress.valueOf("00:00:00:00:00:01"));
-
- Set<ConnectPoint> ingressPoints = new HashSet<ConnectPoint>();
- ingressPoints.add(SW2_ETH1);
- ingressPoints.add(SW3_ETH1);
-
- MultiPointToSinglePointIntent intent =
- new MultiPointToSinglePointIntent(APPID,
- selectorBuilder.build(), treatmentBuilder.build(),
- ingressPoints, SW1_ETH1);
-
// Set up expectation
reset(intentService);
- intentService.withdraw(TestIntentServiceHelper.eqExceptId(intent));
+ // Setup the expected intents
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addWithdrawOperation(addedIntent.id());
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call route deleting method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteDelete(routeEntry);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.DELETE,
+ routeEntry);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
// Verify
assertEquals(router.getRoutes().size(), 0);
@@ -428,10 +419,12 @@
reset(intentService);
replay(intentService);
- // Call the processRouteAdd() method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteAdd(routeEntry);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
+ routeEntry);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
// Verify
assertEquals(router.getRoutes().size(), 1);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
index 0e1226e..46bcdd3 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/RouterTestWithAsyncArp.java
@@ -24,6 +24,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -50,6 +51,7 @@
import org.onlab.onos.net.host.HostService;
import org.onlab.onos.net.host.InterfaceIpAddress;
import org.onlab.onos.net.intent.Intent;
+import org.onlab.onos.net.intent.IntentOperations;
import org.onlab.onos.net.intent.IntentService;
import org.onlab.onos.net.intent.MultiPointToSinglePointIntent;
import org.onlab.onos.net.intent.AbstractIntentTest;
@@ -196,7 +198,7 @@
* This method tests adding a route entry.
*/
@Test
- public void testProcessRouteAdd() throws TestUtilsException {
+ public void testRouteAdd() throws TestUtilsException {
// Construct a route entry
RouteEntry routeEntry = new RouteEntry(
@@ -214,13 +216,18 @@
replay(hostService);
reset(intentService);
- intentService.submit(TestIntentServiceHelper.eqExceptId(intent));
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addSubmitOperation(intent);
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call the processRouteAdd() method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteAdd(routeEntry);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
+ routeEntry);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
Host host = new DefaultHost(ProviderId.NONE, HostId.NONE,
MacAddress.valueOf("00:00:00:00:00:01"), VlanId.NONE,
@@ -299,14 +306,22 @@
replay(hostService);
reset(intentService);
- intentService.withdraw(TestIntentServiceHelper.eqExceptId(intent));
- intentService.submit(TestIntentServiceHelper.eqExceptId(intentNew));
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addWithdrawOperation(intent.id());
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
+ builder = IntentOperations.builder(APPID);
+ builder.addSubmitOperation(intentNew);
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call the processRouteAdd() method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteAdd(routeEntryUpdate);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.UPDATE,
+ routeEntryUpdate);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
Host host = new DefaultHost(ProviderId.NONE, HostId.NONE,
MacAddress.valueOf("00:00:00:00:00:02"), VlanId.NONE,
@@ -334,7 +349,7 @@
* This method tests deleting a route entry.
*/
@Test
- public void testProcessRouteDelete() throws TestUtilsException {
+ public void testRouteDelete() throws TestUtilsException {
// Construct the existing route entry
RouteEntry routeEntry = new RouteEntry(
@@ -351,13 +366,18 @@
// Set up expectation
reset(intentService);
- intentService.withdraw(TestIntentServiceHelper.eqExceptId(intent));
+ IntentOperations.Builder builder = IntentOperations.builder(APPID);
+ builder.addWithdrawOperation(intent.id());
+ intentService.execute(TestIntentServiceHelper.eqExceptId(
+ builder.build()));
replay(intentService);
- // Call route deleting method in Router class
+ // Call the processRouteUpdates() method in Router class
intentSynchronizer.leaderChanged(true);
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
- router.processRouteDelete(routeEntry);
+ RouteUpdate routeUpdate = new RouteUpdate(RouteUpdate.Type.DELETE,
+ routeEntry);
+ router.processRouteUpdates(Collections.<RouteUpdate>singletonList(routeUpdate));
// Verify
assertEquals(router.getRoutes().size(), 0);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
index ab26766..4dfe3cc 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/SdnIpTest.java
@@ -236,9 +236,7 @@
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
// Add route updates
- for (RouteUpdate update : routeUpdates) {
- router.processRouteAdd(update.routeEntry());
- }
+ router.processRouteUpdates(routeUpdates);
latch.await(5000, TimeUnit.MILLISECONDS);
@@ -304,17 +302,19 @@
TestUtils.setField(intentSynchronizer, "isActivatedLeader", true);
// Send the add updates first
- for (RouteUpdate update : routeUpdates) {
- router.processRouteAdd(update.routeEntry());
- }
+ router.processRouteUpdates(routeUpdates);
// Give some time to let the intents be submitted
installCount.await(5000, TimeUnit.MILLISECONDS);
// Send the DELETE updates
+ List<RouteUpdate> deleteRouteUpdates = new ArrayList<>();
for (RouteUpdate update : routeUpdates) {
- router.processRouteDelete(update.routeEntry());
+ RouteUpdate deleteUpdate = new RouteUpdate(RouteUpdate.Type.DELETE,
+ update.routeEntry());
+ deleteRouteUpdates.add(deleteUpdate);
}
+ router.processRouteUpdates(deleteRouteUpdates);
deleteCount.await(5000, TimeUnit.MILLISECONDS);
diff --git a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/bgp/BgpSessionManagerTest.java b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/bgp/BgpSessionManagerTest.java
index 5f8c075..8905289 100644
--- a/apps/sdnip/src/test/java/org/onlab/onos/sdnip/bgp/BgpSessionManagerTest.java
+++ b/apps/sdnip/src/test/java/org/onlab/onos/sdnip/bgp/BgpSessionManagerTest.java
@@ -82,7 +82,7 @@
*/
private class DummyRouteListener implements RouteListener {
@Override
- public void update(RouteUpdate routeUpdate) {
+ public void update(Collection<RouteUpdate> routeUpdate) {
// Nothing to do
}
}