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/api/src/main/java/org/onosproject/net/driver/DefaultDriverProviderService.java b/core/api/src/main/java/org/onosproject/net/driver/DefaultDriverProviderService.java
index 2b7eeff..153c7c5 100644
--- a/core/api/src/main/java/org/onosproject/net/driver/DefaultDriverProviderService.java
+++ b/core/api/src/main/java/org/onosproject/net/driver/DefaultDriverProviderService.java
@@ -17,7 +17,7 @@
package org.onosproject.net.driver;
/**
- * Service capable of providing a set of default drivers.
+ * Service representing availability of default drivers.
*/
-public interface DefaultDriverProviderService extends DriverProvider {
+public interface DefaultDriverProviderService {
}
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());
diff --git a/drivers/src/main/java/org/onosproject/driver/pipeline/DefaultDrivers.java b/drivers/src/main/java/org/onosproject/driver/pipeline/DefaultDrivers.java
index bfa9c32..977a959 100644
--- a/drivers/src/main/java/org/onosproject/driver/pipeline/DefaultDrivers.java
+++ b/drivers/src/main/java/org/onosproject/driver/pipeline/DefaultDrivers.java
@@ -18,9 +18,11 @@
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Reference;
+import org.apache.felix.scr.annotations.ReferenceCardinality;
import org.apache.felix.scr.annotations.Service;
import org.onosproject.net.driver.DefaultDriverProviderService;
-import org.onosproject.net.driver.Driver;
+import org.onosproject.net.driver.DriverAdminService;
import org.onosproject.net.driver.DriverProvider;
import org.onosproject.net.driver.XmlDriverLoader;
import org.slf4j.Logger;
@@ -28,7 +30,6 @@
import java.io.IOException;
import java.io.InputStream;
-import java.util.Set;
/**
* Bootstrap for built in drivers.
@@ -43,12 +44,16 @@
private DriverProvider provider;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected DriverAdminService driverAdminService;
+
@Activate
protected void activate() {
ClassLoader classLoader = getClass().getClassLoader();
try {
InputStream stream = classLoader.getResourceAsStream(DRIVERS_XML);
provider = new XmlDriverLoader(classLoader).loadDrivers(stream);
+ driverAdminService.registerProvider(provider);
} catch (IOException e) {
log.error("Unable to load default drivers", e);
}
@@ -57,11 +62,8 @@
@Deactivate
protected void deactivate() {
+ driverAdminService.unregisterProvider(provider);
log.info("Stopped");
}
- @Override
- public Set<Driver> getDrivers() {
- return provider.getDrivers();
- }
}
diff --git a/providers/openflow/app/features.xml b/providers/openflow/app/features.xml
index ef95892..01d5e33 100644
--- a/providers/openflow/app/features.xml
+++ b/providers/openflow/app/features.xml
@@ -23,7 +23,6 @@
<bundle>mvn:${project.groupId}/onos-of-api/${project.version}</bundle>
<bundle>mvn:${project.groupId}/onos-of-drivers/${project.version}</bundle>
<bundle>mvn:${project.groupId}/onos-of-ctl/${project.version}</bundle>
- <bundle>mvn:${project.groupId}/onos-drivers/${project.version}</bundle>
<bundle>mvn:${project.groupId}/onos-lldp-provider/${project.version}</bundle>
<bundle>mvn:${project.groupId}/onos-host-provider/${project.version}</bundle>