Reworked the dependency on default drivers. Driver manager must NOT depend on these.
Instead, it is ok for the flow manager to have a dependency on the drivers and go fully active/dormant when the default drivers arrive/depart.
Removed inclusion of the onos-drivers bundle as part of the onos-openflow app as this caused an unwanted dependency.
Change-Id: I614290277d1621c8243c0c19e5d79273f2168016
diff --git a/core/net/src/main/java/org/onosproject/net/driver/impl/DriverManager.java b/core/net/src/main/java/org/onosproject/net/driver/impl/DriverManager.java
index c3160be..5f7e2e2 100644
--- a/core/net/src/main/java/org/onosproject/net/driver/impl/DriverManager.java
+++ b/core/net/src/main/java/org/onosproject/net/driver/impl/DriverManager.java
@@ -31,7 +31,6 @@
import org.onosproject.net.driver.DefaultDriverData;
import org.onosproject.net.driver.DefaultDriverHandler;
import org.onosproject.net.driver.DefaultDriverProvider;
-import org.onosproject.net.driver.DefaultDriverProviderService;
import org.onosproject.net.driver.Driver;
import org.onosproject.net.driver.DriverAdminService;
import org.onosproject.net.driver.DriverHandler;
@@ -63,21 +62,16 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceService deviceService;
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected DefaultDriverProviderService defaultDriverService;
-
private Set<DriverProvider> providers = Sets.newConcurrentHashSet();
private Map<String, Driver> driverByKey = Maps.newConcurrentMap();
@Activate
protected void activate() {
- registerProvider(defaultDriverService);
log.info("Started");
}
@Deactivate
protected void deactivate() {
- unregisterProvider(defaultDriverService);
log.info("Stopped");
}
diff --git a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
index 129b6c2..f84c24f 100644
--- a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
+++ b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
@@ -27,7 +27,6 @@
import org.onlab.osgi.ServiceDirectory;
import org.onlab.util.ItemNotFoundException;
import org.onosproject.cluster.ClusterService;
-import org.onosproject.cluster.NodeId;
import org.onosproject.mastership.MastershipEvent;
import org.onosproject.mastership.MastershipListener;
import org.onosproject.mastership.MastershipService;
@@ -37,6 +36,7 @@
import org.onosproject.net.device.DeviceEvent;
import org.onosproject.net.device.DeviceListener;
import org.onosproject.net.device.DeviceService;
+import org.onosproject.net.driver.DefaultDriverProviderService;
import org.onosproject.net.driver.DriverHandler;
import org.onosproject.net.driver.DriverService;
import org.onosproject.net.flow.FlowRuleService;
@@ -56,8 +56,8 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import static java.util.concurrent.Executors.newFixedThreadPool;
import static org.onlab.util.Tools.groupedThreads;
/**
@@ -96,6 +96,12 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected FlowObjectiveStore flowObjectiveStore;
+ // Note: This must remain an optional dependency to allow re-install of default drivers.
+ // Note: For now disabled until we can move to OPTIONAL_UNARY dependency
+ // @Reference(cardinality = ReferenceCardinality.OPTIONAL_UNARY, policy = ReferencePolicy.DYNAMIC)
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected DefaultDriverProviderService defaultDriverService;
+
private final FlowObjectiveStoreDelegate delegate = new InternalStoreDelegate();
private final Map<DeviceId, DriverHandler> driverHandlers = Maps.newConcurrentMap();
@@ -107,20 +113,14 @@
protected ServiceDirectory serviceDirectory = new DefaultServiceDirectory();
- private NodeId localNode;
-
- private Map<Integer, Set<PendingNext>> pendingForwards =
- Maps.newConcurrentMap();
+ private Map<Integer, Set<PendingNext>> pendingForwards = Maps.newConcurrentMap();
private ExecutorService executorService;
@Activate
protected void activate() {
- executorService = Executors.newFixedThreadPool(
- 4, groupedThreads("onos/objective-installer", "%d"));
-
+ executorService = newFixedThreadPool(4, groupedThreads("onos/objective-installer", "%d"));
flowObjectiveStore.setDelegate(delegate);
- localNode = clusterService.getLocalNode().id();
mastershipService.addListener(mastershipListener);
deviceService.addListener(deviceListener);
deviceService.getDevices().forEach(device -> setupPipelineHandler(device.id()));
@@ -132,10 +132,39 @@
flowObjectiveStore.unsetDelegate(delegate);
mastershipService.removeListener(mastershipListener);
deviceService.removeListener(deviceListener);
+ executorService.shutdown();
+ pipeliners.clear();
+ driverHandlers.clear();
log.info("Stopped");
}
/**
+ * Hook for binding the optional default driver providers.
+ *
+ * @param service arriving default driver provider service
+ */
+ // Note: For now disabled until we can move to OPTIONAL_UNARY dependency
+ protected void xbindDefaultDriverService(DefaultDriverProviderService service) {
+ log.info("Detected default drivers... going active");
+ defaultDriverService = service;
+ deviceService.getDevices().forEach(device -> setupPipelineHandler(device.id()));
+ }
+
+ /**
+ * Hook for unbinding the optional default driver providers.
+ *
+ * @param service departing default driver provider service
+ */
+ // Note: For now disabled until we can move to OPTIONAL_UNARY dependency
+ protected void xunbindDefaultDriverService(DefaultDriverProviderService service) {
+ log.info("Lost default drivers... going dormant");
+ defaultDriverService = null;
+ pipeliners.clear();
+ driverHandlers.clear();
+ }
+
+
+ /**
* Task that passes the flow objective down to the driver. The task will
* make a few attempts to find the appropriate driver, then eventually give
* up and report an error if no suitable driver could be found.
@@ -187,13 +216,10 @@
}
@Override
- public void forward(DeviceId deviceId,
- ForwardingObjective forwardingObjective) {
-
+ public void forward(DeviceId deviceId, ForwardingObjective forwardingObjective) {
if (queueObjective(deviceId, forwardingObjective)) {
return;
}
-
executorService.submit(new ObjectiveInstaller(deviceId, forwardingObjective));
}
@@ -212,7 +238,7 @@
flowObjectiveStore.getNextGroup(fwd.nextId()) == null) {
log.trace("Queuing forwarding objective for nextId {}", fwd.nextId());
if (pendingForwards.putIfAbsent(fwd.nextId(),
- Sets.newHashSet(new PendingNext(deviceId, fwd))) != null) {
+ Sets.newHashSet(new PendingNext(deviceId, fwd))) != null) {
Set<PendingNext> pending = pendingForwards.get(fwd.nextId());
pending.add(new PendingNext(deviceId, fwd));
}
@@ -228,6 +254,11 @@
}
private void setupPipelineHandler(DeviceId deviceId) {
+ if (defaultDriverService == null) {
+ // We're not ready to go to work yet.
+ return;
+ }
+
// Attempt to lookup the handler in the cache
DriverHandler handler = driverHandlers.get(deviceId);
if (handler == null) {
diff --git a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
index d669d72..27763f9 100644
--- a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
+++ b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
@@ -78,9 +78,6 @@
private CoreService coreService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- private FlowObjectiveService objectiveService;
-
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
private DeviceService deviceService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -89,6 +86,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
private PacketStore store;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ private FlowObjectiveService objectiveService;
+
private final DeviceListener deviceListener = new InternalDeviceListener();
private final Map<Integer, PacketProcessor> processors = new ConcurrentHashMap<>();
@@ -100,6 +100,7 @@
appId = coreService.getAppId(CoreService.CORE_APP_NAME);
store.setDelegate(delegate);
deviceService.addListener(deviceListener);
+ // TODO: Should we request packets for all existing devices? I believe we should.
log.info("Started");
}
@@ -250,8 +251,7 @@
/**
* Internal callback from the packet store.
*/
- private class InternalStoreDelegate
- implements PacketStoreDelegate {
+ private class InternalStoreDelegate implements PacketStoreDelegate {
@Override
public void notify(PacketEvent event) {
localEmit(event.subject());