Adding mechanism for device subsystem & providers to trigger device
reconnect in support of ONOS-7645 (device driver change)
- added device listener to OpenFlowDeviceProvider to properly disconnect switch
- removed device listener from OpenFlowControllerImpl
- augmented DriverManager to consult NetworkConfigService as a primary source
Change-Id: I1aa8e9cc7e81ff3af7a72145f4e51f3e32022806
diff --git a/core/api/src/main/java/org/onosproject/net/driver/DriverService.java b/core/api/src/main/java/org/onosproject/net/driver/DriverService.java
index 72a6040..e4432d1 100644
--- a/core/api/src/main/java/org/onosproject/net/driver/DriverService.java
+++ b/core/api/src/main/java/org/onosproject/net/driver/DriverService.java
@@ -40,8 +40,10 @@
Set<Driver> getDrivers(Class<? extends Behaviour> withBehaviour);
/**
- * Returns the driver for the specified device. If the device carries
- * {@code driver} annotation, its value is used to look-up the driver.
+ * Returns the driver for the specified device. If the network configuration
+ * for the specified device carries the {@code driver} property or if the
+ * device carries the {@code driver} annotation, they will be used to look-up
+ * the driver, in respective order.
* Otherwise, the device manufacturer, hardware and software version
* attributes are used to look-up the driver. First using their literal
* values and if no driver is found, using ERE matching against the
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 47d4f91..1dd8f7b 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
@@ -25,6 +25,8 @@
import org.onosproject.net.AbstractProjectableModel;
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
+import org.onosproject.net.config.NetworkConfigService;
+import org.onosproject.net.config.basics.BasicDeviceConfig;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.driver.Behaviour;
import org.onosproject.net.driver.DefaultDriverData;
@@ -66,6 +68,9 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceService deviceService;
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected NetworkConfigService networkConfigService;
+
@Activate
protected void activate() {
AbstractProjectableModel.setDriverService(null, this);
@@ -108,8 +113,28 @@
public Driver getDriver(DeviceId deviceId) {
checkPermission(DRIVER_READ);
+ // Primary source of driver configuration is the network config.
+ BasicDeviceConfig cfg = networkConfigService.getConfig(deviceId, BasicDeviceConfig.class);
+ Driver driver = lookupDriver(cfg != null ? cfg.driver() : null);
+ if (driver != null) {
+ return driver;
+ }
+
+ // Secondary source of the driver selection is driver annotation.
Device device = nullIsNotFound(deviceService.getDevice(deviceId), NO_DEVICE);
- String driverName = device.annotations().value(DRIVER);
+ driver = lookupDriver(device.annotations().value(DRIVER));
+ if (driver != null) {
+ return driver;
+ }
+
+ // Tertiary source of the driver selection is the primordial information
+ // obtained from the device.
+ return nullIsNotFound(getDriver(device.manufacturer(),
+ device.hwVersion(), device.swVersion()),
+ NO_DRIVER);
+ }
+
+ private Driver lookupDriver(String driverName) {
if (driverName != null) {
try {
return getDriver(driverName);
@@ -117,10 +142,7 @@
log.warn("Specified driver {} not found, falling back.", driverName);
}
}
-
- return nullIsNotFound(getDriver(device.manufacturer(),
- device.hwVersion(), device.swVersion()),
- NO_DRIVER);
+ return null;
}
@Override
diff --git a/core/net/src/test/java/org/onosproject/net/meter/impl/MeterManagerTest.java b/core/net/src/test/java/org/onosproject/net/meter/impl/MeterManagerTest.java
index 8de0b5a..3bd5e9b 100644
--- a/core/net/src/test/java/org/onosproject/net/meter/impl/MeterManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/meter/impl/MeterManagerTest.java
@@ -42,6 +42,8 @@
import org.onosproject.net.DeviceId;
import org.onosproject.net.MastershipRole;
import org.onosproject.net.behaviour.MeterQuery;
+import org.onosproject.net.config.NetworkConfigService;
+import org.onosproject.net.config.NetworkConfigServiceAdapter;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.device.DeviceServiceAdapter;
import org.onosproject.net.driver.AbstractHandlerBehaviour;
@@ -214,7 +216,7 @@
deviceService = new TestDeviceService();
//Init step for the driver registry and driver service.
DriverRegistryManager driverRegistry = new DriverRegistryManager();
- driverService = new TestDriverManager(driverRegistry, deviceService);
+ driverService = new TestDriverManager(driverRegistry, deviceService, new NetworkConfigServiceAdapter());
driverRegistry.addDriver(new DefaultDriver("foo", ImmutableList.of(), "",
"", "",
ImmutableMap.of(MeterProgrammable.class,
@@ -519,9 +521,11 @@
}
private class TestDriverManager extends DriverManager {
- TestDriverManager(DriverRegistry registry, DeviceService deviceService) {
+ TestDriverManager(DriverRegistry registry, DeviceService deviceService,
+ NetworkConfigService networkConfigService) {
this.registry = registry;
this.deviceService = deviceService;
+ this.networkConfigService = networkConfigService;
activate();
}
}
diff --git a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
index 5c46d8e..ab7009f 100644
--- a/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
+++ b/protocols/openflow/ctl/src/main/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImpl.java
@@ -28,9 +28,6 @@
import org.apache.felix.scr.annotations.Service;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.core.CoreService;
-import org.onosproject.net.device.DeviceEvent;
-import org.onosproject.net.device.DeviceListener;
-import org.onosproject.net.device.DeviceService;
import org.onosproject.net.driver.DriverService;
import org.onosproject.openflow.controller.DefaultOpenFlowPacketContext;
import org.onosproject.openflow.controller.Dpid;
@@ -77,7 +74,6 @@
import java.util.Collection;
import java.util.Collections;
import java.util.List;
-import java.util.Optional;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
@@ -89,9 +85,6 @@
import java.util.concurrent.locks.ReentrantLock;
import static org.onlab.util.Tools.groupedThreads;
-import static org.onosproject.net.Device.Type.CONTROLLER;
-import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_REMOVED;
-import static org.onosproject.openflow.controller.Dpid.dpid;
@Component(immediate = true)
@@ -114,10 +107,6 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected ComponentConfigService cfgService;
- @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
- protected DeviceService deviceService;
-
-
@Property(name = "openflowPorts", value = DEFAULT_OFPORT,
label = "Port numbers (comma separated) used by OpenFlow protocol; default is 6633,6653")
private String openflowPorts = DEFAULT_OFPORT;
@@ -185,13 +174,11 @@
ArrayListMultimap.create();
private final Controller ctrl = new Controller();
- private InternalDeviceListener listener = new InternalDeviceListener();
@Activate
public void activate(ComponentContext context) {
coreService.registerApplication(APP_ID, this::cleanup);
cfgService.registerProperties(getClass());
- deviceService.addListener(listener);
ctrl.setConfigParams(context.getProperties());
ctrl.start(agent, driverService);
}
@@ -208,7 +195,6 @@
@Deactivate
public void deactivate() {
- deviceService.removeListener(listener);
cleanup();
cfgService.unregisterProperties(getClass(), false);
}
@@ -607,47 +593,6 @@
sw.setRole(role);
}
- class InternalDeviceListener implements DeviceListener {
-
- @Override
- public boolean isRelevant(DeviceEvent event) {
- return event.subject().type() != CONTROLLER && event.type() == DEVICE_REMOVED
- && event.subject().id().uri().getScheme().equals(SCHEME);
- }
-
- @Override
- public void event(DeviceEvent event) {
- switch (event.type()) {
- case DEVICE_ADDED:
- break;
- case DEVICE_AVAILABILITY_CHANGED:
- break;
- case DEVICE_REMOVED:
- // Device administratively removed, disconnect
- Optional.ofNullable(getSwitch(dpid(event.subject().id().uri())))
- .ifPresent(OpenFlowSwitch::disconnectSwitch);
- break;
- case DEVICE_SUSPENDED:
- break;
- case DEVICE_UPDATED:
- break;
- case PORT_ADDED:
- break;
- case PORT_REMOVED:
- break;
- case PORT_STATS_UPDATED:
- break;
- case PORT_UPDATED:
- break;
- default:
- break;
-
- }
-
- }
-
- }
-
/**
* Implementation of an OpenFlow Agent which is responsible for
* keeping track of connected switches and the state in which
diff --git a/protocols/openflow/ctl/src/test/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImplTest.java b/protocols/openflow/ctl/src/test/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImplTest.java
index ca2d55f..f3f7769 100644
--- a/protocols/openflow/ctl/src/test/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImplTest.java
+++ b/protocols/openflow/ctl/src/test/java/org/onosproject/openflow/controller/impl/OpenFlowControllerImplTest.java
@@ -32,7 +32,6 @@
import org.junit.Test;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.core.CoreService;
-import org.onosproject.net.device.DeviceServiceAdapter;
import org.onosproject.openflow.OpenflowSwitchDriverAdapter;
import org.onosproject.openflow.controller.Dpid;
import org.onosproject.openflow.controller.OpenFlowSwitch;
@@ -150,8 +149,6 @@
controller.cfgService = mockConfigService;
replay(mockConfigService);
- controller.deviceService = new DeviceServiceAdapter();
-
ComponentContext mockContext = EasyMock.createMock(ComponentContext.class);
Dictionary<String, Object> properties = new Hashtable<>();
properties.put("openflowPorts",
diff --git a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
index c9f09e5..cce89b0 100644
--- a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
+++ b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
@@ -52,9 +52,12 @@
import org.onosproject.net.device.DefaultPortDescription;
import org.onosproject.net.device.DefaultPortStatistics;
import org.onosproject.net.device.DeviceDescription;
+import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.device.DeviceListener;
import org.onosproject.net.device.DeviceProvider;
import org.onosproject.net.device.DeviceProviderRegistry;
import org.onosproject.net.device.DeviceProviderService;
+import org.onosproject.net.device.DeviceService;
import org.onosproject.net.device.PortDescription;
import org.onosproject.net.device.PortStatistics;
import org.onosproject.net.driver.Driver;
@@ -129,9 +132,11 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Strings.isNullOrEmpty;
import static org.onlab.util.Tools.get;
+import static org.onosproject.net.Device.Type.CONTROLLER;
import static org.onosproject.net.DeviceId.deviceId;
import static org.onosproject.net.Port.Type.COPPER;
import static org.onosproject.net.Port.Type.FIBER;
+import static org.onosproject.net.device.DeviceEvent.Type.DEVICE_REMOVED;
import static org.onosproject.net.optical.device.OchPortHelper.ochPortDescription;
import static org.onosproject.net.optical.device.OduCltPortHelper.oduCltPortDescription;
import static org.onosproject.net.optical.device.OmsPortHelper.omsPortDescription;
@@ -426,11 +431,15 @@
private static final Frequency FREQ4_4 = Frequency.ofGHz(4_400);
private static final long C = 299792458; // speed of light in m/s
+ public static final String SCHEME = "of";
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceProviderRegistry providerRegistry;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+ protected DeviceService deviceService;
+
+ @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected OpenFlowController controller;
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -442,6 +451,7 @@
private DeviceProviderService providerService;
private final InternalDeviceProvider listener = new InternalDeviceProvider();
+ private final InternalDeviceListener deviceListener = new InternalDeviceListener();
private static final String POLL_PROP_NAME = "portStatsPollFrequency";
private static final int POLL_INTERVAL = 5;
@@ -463,13 +473,14 @@
* Creates an OpenFlow device provider.
*/
public OpenFlowDeviceProvider() {
- super(new ProviderId("of", "org.onosproject.provider.openflow"));
+ super(new ProviderId(SCHEME, "org.onosproject.provider.openflow"));
}
@Activate
public void activate(ComponentContext context) {
cfgService.registerProperties(getClass());
providerService = providerRegistry.register(this);
+ deviceService.addListener(deviceListener);
controller.addListener(listener);
controller.addEventListener(listener);
@@ -483,6 +494,7 @@
public void deactivate(ComponentContext context) {
cfgService.unregisterProperties(getClass(), false);
listener.disable();
+ deviceService.removeListener(deviceListener);
controller.removeListener(listener);
providerRegistry.unregister(this);
collectors.values().forEach(PortStatsCollector::stop);
@@ -1520,5 +1532,27 @@
}
}
+ class InternalDeviceListener implements DeviceListener {
+ @Override
+ public boolean isRelevant(DeviceEvent event) {
+ return event.subject().type() != CONTROLLER && event.type() == DEVICE_REMOVED
+ && event.subject().id().uri().getScheme().equals(SCHEME);
+ }
+
+ @Override
+ public void event(DeviceEvent event) {
+ DeviceId deviceId = event.subject().id();
+ Dpid dpid = dpid(deviceId.uri());
+ OpenFlowSwitch sw = controller.getSwitch(dpid);
+ if (sw != null) {
+ LOG.debug("Forcing disconnect for device {}", deviceId);
+ PortStatsCollector portStatsCollector = collectors.remove(dpid);
+ if (portStatsCollector != null) {
+ portStatsCollector.stop();
+ }
+ sw.disconnectSwitch();
+ }
+ }
+ }
}
diff --git a/providers/openflow/device/src/test/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProviderTest.java b/providers/openflow/device/src/test/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProviderTest.java
index 8167b04..a0869c4 100644
--- a/providers/openflow/device/src/test/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProviderTest.java
+++ b/providers/openflow/device/src/test/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProviderTest.java
@@ -32,6 +32,7 @@
import org.onosproject.net.device.DeviceProvider;
import org.onosproject.net.device.DeviceProviderRegistry;
import org.onosproject.net.device.DeviceProviderService;
+import org.onosproject.net.device.DeviceServiceAdapter;
import org.onosproject.net.device.PortDescription;
import org.onosproject.net.device.PortStatistics;
import org.onosproject.net.driver.DriverServiceAdapter;
@@ -93,6 +94,7 @@
provider.controller = controller;
provider.cfgService = new ComponentConfigAdapter();
provider.driverService = new DriverServiceAdapter();
+ provider.deviceService = new DeviceServiceAdapter();
controller.switchMap.put(DPID1, SW1);
provider.activate(null);
assertNotNull("provider should be registered", registry.provider);
diff --git a/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java b/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
index 21c33f2..f68f57d 100644
--- a/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
+++ b/web/gui/src/main/java/org/onosproject/ui/impl/TopologyViewMessageHandlerBase.java
@@ -46,6 +46,7 @@
import org.onosproject.net.config.basics.BasicElementConfig;
import org.onosproject.net.config.basics.BasicHostConfig;
import org.onosproject.net.device.DeviceEvent;
+import org.onosproject.net.driver.Driver;
import org.onosproject.net.host.HostEvent;
import org.onosproject.net.link.LinkEvent;
import org.onosproject.net.provider.ProviderId;
@@ -69,6 +70,7 @@
import java.util.concurrent.ConcurrentHashMap;
import static com.google.common.base.Strings.isNullOrEmpty;
+import static org.onosproject.net.AnnotationKeys.DRIVER;
import static org.onosproject.net.PortNumber.portNumber;
import static org.onosproject.net.config.basics.BasicElementConfig.LOC_TYPE_GEO;
import static org.onosproject.net.config.basics.BasicElementConfig.LOC_TYPE_GRID;
@@ -285,8 +287,10 @@
protected ObjectNode deviceMessage(DeviceEvent event) {
Device device = event.subject();
String uiType = device.annotations().value(AnnotationKeys.UI_TYPE);
+ String driverName = device.annotations().value(DRIVER);
+ Driver driver = driverName == null ? null : services.driver().getDriver(driverName);
String devType = uiType != null ? uiType :
- services.driver().getDriver(device.id()).getProperty(AnnotationKeys.UI_TYPE);
+ (driver != null ? driver.getProperty(AnnotationKeys.UI_TYPE) : null);
if (devType == null) {
devType = device.type().toString().toLowerCase();
}