Few core changes in preparation of P4Runtime subsystem refactoring
Refactoring aims at providing a more robust connection procedure of
P4Runtime-based devices.
Refactoring requires devices to be registered to the core with
available=false (i.e. offline), and marked online only when the P4
pipeline has been deployed to the device. Also, it leaves the duty of
handling pipeconf-specific drivers to the driver subsystem, instead of
having the GeneralDeviceProvider take care of it, which resulted to be
brittle.
Changes include:
- Choose not to mark device online when re-asserting mastership role by
declaring special device annotation.
- Lookup drivers in device projectable model unsing driver service
device-based method, which is consistent with the way other subsystems
look for device drivers.
- In DriverManager, when looking for device drivers, consider the case
of pipeconf-programmable devices, in which case a special "merged"
driver needs to instantiated at runtime.
- In PacketManager, push flow objectives only if device is available
(i.e. after pipeline has been deployed).
Change-Id: I80f78a4f40730a06cb82fa55fefab08a91a89268
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index b0f64f9..2161f7d 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -35,6 +35,7 @@
import org.onosproject.mastership.MastershipService;
import org.onosproject.mastership.MastershipTerm;
import org.onosproject.mastership.MastershipTermService;
+import org.onosproject.net.AnnotationKeys;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.Device;
import org.onosproject.net.Device.Type;
@@ -823,6 +824,12 @@
return (cfg == null || cfg.isAllowed());
}
+ private boolean canMarkOnline(Device device) {
+ final boolean providerMarkOnline = Boolean.parseBoolean(
+ device.annotations().value(AnnotationKeys.PROVIDER_MARK_ONLINE));
+ return !providerMarkOnline;
+ }
+
// Applies the specified role to the device; ignores NONE
/**
@@ -884,7 +891,7 @@
switch (myNextRole) {
case MASTER:
final Device device = getDevice(did);
- if ((device != null) && !isAvailable(did)) {
+ if (device != null && !isAvailable(did) && canMarkOnline(device)) {
post(store.markOnline(did));
}
// TODO: should apply role only if there is mismatch
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 1dd8f7b..f2ece6b 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
@@ -36,6 +36,8 @@
import org.onosproject.net.driver.DriverListener;
import org.onosproject.net.driver.DriverRegistry;
import org.onosproject.net.driver.DriverService;
+import org.onosproject.net.pi.model.PiPipeconfId;
+import org.onosproject.net.pi.service.PiPipeconfService;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -71,6 +73,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected NetworkConfigService networkConfigService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected PiPipeconfService pipeconfService;
+
@Activate
protected void activate() {
AbstractProjectableModel.setDriverService(null, this);
@@ -113,9 +118,25 @@
public Driver getDriver(DeviceId deviceId) {
checkPermission(DRIVER_READ);
+ Driver driver;
+
// Primary source of driver configuration is the network config.
+ if (pipeconfService.ofDevice(deviceId).isPresent()) {
+ // Device has pipeconf associated, look for merged driver.
+ // Implementation of PiPipeconfService is expected to look for a
+ // base driver in network config.
+ PiPipeconfId pipeconfId = pipeconfService.ofDevice(deviceId).get();
+ String mergedDriver = pipeconfService.getMergedDriver(deviceId, pipeconfId);
+ driver = mergedDriver != null ? lookupDriver(mergedDriver) : null;
+ if (driver != null) {
+ return driver;
+ } else {
+ log.error("Merged driver for {} with pipeconf {} not found, falling back.",
+ deviceId, pipeconfId);
+ }
+ }
BasicDeviceConfig cfg = networkConfigService.getConfig(deviceId, BasicDeviceConfig.class);
- Driver driver = lookupDriver(cfg != null ? cfg.driver() : null);
+ driver = lookupDriver(cfg != null ? cfg.driver() : null);
if (driver != null) {
return driver;
}
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 6be6c54..ac5c732 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
@@ -309,6 +309,10 @@
return;
}
+ if (!deviceService.isAvailable(device.id())) {
+ return;
+ }
+
ForwardingObjective forwarding = createBuilder(request)
.add(new ObjectiveContext() {
@Override
diff --git a/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfManager.java b/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfManager.java
index 5be4a26..0e2ea82 100644
--- a/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfManager.java
+++ b/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfManager.java
@@ -27,7 +27,6 @@
import org.apache.felix.scr.annotations.ReferenceCardinality;
import org.apache.felix.scr.annotations.Service;
import org.onlab.util.ItemNotFoundException;
-import org.onosproject.cluster.ClusterService;
import org.onosproject.net.DeviceId;
import org.onosproject.net.config.ConfigFactory;
import org.onosproject.net.config.NetworkConfigRegistry;
@@ -40,7 +39,6 @@
import org.onosproject.net.driver.DriverEvent;
import org.onosproject.net.driver.DriverListener;
import org.onosproject.net.driver.DriverProvider;
-import org.onosproject.net.driver.DriverService;
import org.onosproject.net.pi.model.PiPipeconf;
import org.onosproject.net.pi.model.PiPipeconfId;
import org.onosproject.net.pi.service.PiPipeconfConfig;
@@ -81,17 +79,11 @@
protected NetworkConfigRegistry cfgService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected DriverService driverService;
-
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DriverAdminService driverAdminService;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
private PiPipeconfMappingStore pipeconfMappingStore;
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected ClusterService clusterService;
-
// Registered pipeconf are replicated through the app subsystem and
// registered on app activated events. Hence, there should be no need of
// distributing this map.
@@ -117,7 +109,7 @@
@Activate
public void activate() {
cfgService.registerConfigFactory(configFactory);
- driverService.addListener(driverListener);
+ driverAdminService.addListener(driverListener);
checkMissingMergedDrivers();
log.info("Started");
}
@@ -127,12 +119,11 @@
public void deactivate() {
executor.shutdown();
cfgService.unregisterConfigFactory(configFactory);
- driverService.removeListener(driverListener);
+ driverAdminService.removeListener(driverListener);
pipeconfs.clear();
missingMergedDrivers.clear();
cfgService = null;
driverAdminService = null;
- driverService = null;
log.info("Stopped");
}
@@ -170,11 +161,17 @@
@Override
public void bindToDevice(PiPipeconfId pipeconfId, DeviceId deviceId) {
+ PiPipeconfId existingPipeconfId = pipeconfMappingStore.getPipeconfId(deviceId);
+ if (existingPipeconfId != null && !existingPipeconfId.equals(pipeconfId)) {
+ log.error("Cannot set binding for {} to {} as one already exists ({})",
+ deviceId, pipeconfId, existingPipeconfId);
+ return;
+ }
pipeconfMappingStore.createOrUpdateBinding(deviceId, pipeconfId);
}
@Override
- public String mergeDriver(DeviceId deviceId, PiPipeconfId pipeconfId) {
+ public String getMergedDriver(DeviceId deviceId, PiPipeconfId pipeconfId) {
log.debug("Starting device driver merge of {} with {}...", deviceId, pipeconfId);
final BasicDeviceConfig basicDeviceConfig = cfgService.getConfig(
deviceId, BasicDeviceConfig.class);
@@ -298,7 +295,7 @@
private Driver getDriver(String name) {
try {
- return driverService.getDriver(name);
+ return driverAdminService.getDriver(name);
} catch (ItemNotFoundException e) {
return null;
}