More bugfixes in P4Runtime subsystem
- NPE when removing agent listeners
- Don't block netcfg event dispatch thread on GDP
- Avoid unnecessary warn logs during disconnection in GDP
Change-Id: I612f7f7914579eea9ba393e952377a3933d92e8d
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeHandshaker.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeHandshaker.java
index c7e12aa4..c818135 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeHandshaker.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeHandshaker.java
@@ -79,15 +79,15 @@
@Override
public void addDeviceAgentListener(DeviceAgentListener listener) {
- // Don't use controller class variable as it might be uninitialized.
+ // Don't use controller/deviceId class variables as they might be uninitialized.
handler().get(P4RuntimeController.class)
- .addDeviceAgentListener(deviceId, listener);
+ .addDeviceAgentListener(data().deviceId(), listener);
}
@Override
public void removeDeviceAgentListener(DeviceAgentListener listener) {
- // Don't use controller class variable as it might be uninitialized.
+ // Don't use controller/deviceId class variable as they might be uninitialized.
handler().get(P4RuntimeController.class)
- .removeDeviceAgentListener(deviceId, listener);
+ .removeDeviceAgentListener(data().deviceId(), listener);
}
}
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeControllerImpl.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeControllerImpl.java
index b35b999..d8b0cae 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeControllerImpl.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeControllerImpl.java
@@ -223,12 +223,16 @@
@Override
public void addDeviceAgentListener(DeviceId deviceId, DeviceAgentListener listener) {
+ checkNotNull(deviceId, "deviceId cannot be null");
+ checkNotNull(listener, "listener cannot be null");
deviceAgentListeners.putIfAbsent(deviceId, new CopyOnWriteArrayList<>());
deviceAgentListeners.get(deviceId).add(listener);
}
@Override
public void removeDeviceAgentListener(DeviceId deviceId, DeviceAgentListener listener) {
+ checkNotNull(deviceId, "deviceId cannot be null");
+ checkNotNull(listener, "listener cannot be null");
deviceAgentListeners.computeIfPresent(deviceId, (did, listeners) -> {
listeners.remove(listener);
return listeners;
diff --git a/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java b/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java
index b341149..34e707f 100644
--- a/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java
+++ b/providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java
@@ -621,20 +621,24 @@
}
private void doDisconnectDevice(DeviceId deviceId) {
- log.info("Initiating disconnection from {}...", deviceId);
- // Remove from core (if master)
- if (mastershipService.isLocalMaster(deviceId)
- && deviceService.isAvailable(deviceId)) {
+ log.debug("Initiating disconnection from {}...", deviceId);
+ final DeviceHandshaker handshaker = handshakers.remove(deviceId);
+ final boolean isAvailable = deviceService.isAvailable(deviceId);
+ // Signal disconnection to core (if master).
+ if (isAvailable && mastershipService.isLocalMaster(deviceId)) {
providerService.deviceDisconnected(deviceId);
}
+ // Cancel tasks.
cancelStatsPolling(deviceId);
- // Perform disconnection with device.
- final DeviceHandshaker handshaker = handshakers.remove(deviceId);
+ // Disconnect device.
if (handshaker == null) {
- // Gracefully ignore
- log.warn("Missing DeviceHandshaker behavior for {}, " +
- "no guarantees of complete disconnection",
- deviceId);
+ if (isAvailable) {
+ // If not available don't bother logging. We are probably
+ // invoking this method multiple times for the same device.
+ log.warn("Missing DeviceHandshaker behavior for {}, " +
+ "no guarantees of complete disconnection",
+ deviceId);
+ }
return;
}
handshaker.removeDeviceAgentListener(deviceAgentListener);
@@ -706,6 +710,19 @@
@Override
public void event(NetworkConfigEvent event) {
+ connectionExecutor.execute(() -> consumeConfigEvent(event));
+ }
+
+ @Override
+ public boolean isRelevant(NetworkConfigEvent event) {
+ return (event.configClass().equals(GeneralProviderDeviceConfig.class) ||
+ event.configClass().equals(BasicDeviceConfig.class) ||
+ event.configClass().equals(PiPipeconfConfig.class)) &&
+ (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
+ event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED);
+ }
+
+ private void consumeConfigEvent(NetworkConfigEvent event) {
DeviceId deviceId = (DeviceId) event.subject();
//Assuming that the deviceId comes with uri 'device:'
if (notMyScheme(deviceId)) {
@@ -770,15 +787,6 @@
}
return false;
}
-
- @Override
- public boolean isRelevant(NetworkConfigEvent event) {
- return (event.configClass().equals(GeneralProviderDeviceConfig.class) ||
- event.configClass().equals(BasicDeviceConfig.class) ||
- event.configClass().equals(PiPipeconfConfig.class)) &&
- (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
- event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED);
- }
}
private void augmentConfigData(GeneralProviderDeviceConfig providerConfig, DriverData driverData) {