Minor fixes for P4Runtime devices
- Push ports before device is marked online
- Do not notify role NONE on device disconnection (otherwise the
DeviceManager won't mark the device as offline if there's not a master)
- Detect changes in the pipeconf extensions when reloading a pipeconf
Change-Id: I1779987da1269ec98c71b2ccda7bb579be5bf3f0
diff --git a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
index 9fb3625..5b3153c 100644
--- a/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
+++ b/protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java
@@ -101,6 +101,8 @@
"A %s client already exists for %s", serviceName, deviceId));
}
+ log.info("Creating {}...", clientName(deviceId));
+
final C client;
try {
client = createClientInstance(deviceId, channel);
@@ -133,6 +135,7 @@
withDeviceLock(() -> {
final C client = clients.remove(deviceId);
if (client != null) {
+ log.info("Removing {}...", clientName(deviceId));
client.shutdown();
}
return null;
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/StreamClientImpl.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/StreamClientImpl.java
index a14f960..cf3cda0 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/StreamClientImpl.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/StreamClientImpl.java
@@ -397,11 +397,16 @@
void signalClosed() {
synchronized (this) {
final boolean wasOpen = open.getAndSet(false);
- if (wasOpen) {
- // We lost any valid mastership role.
- controller.postEvent(new DeviceAgentEvent(
- DeviceAgentEvent.Type.ROLE_NONE, deviceId));
- }
+ // FIXME: in case of device disconnection, all clients will
+ // signal role NONE, preventing the DeviceManager to mark the
+ // device as offline, as only the master can do that. We should
+ // change the DeviceManager. For now, we disable signaling role
+ // NONE.
+ // if (wasOpen) {
+ // // We lost any valid mastership role.
+ // controller.postEvent(new DeviceAgentEvent(
+ // DeviceAgentEvent.Type.ROLE_NONE, deviceId));
+ // }
}
}
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/utils/PipeconfHelper.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/utils/PipeconfHelper.java
index dec7438..f5afd41 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/utils/PipeconfHelper.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/utils/PipeconfHelper.java
@@ -22,7 +22,6 @@
import com.google.protobuf.ExtensionRegistry;
import com.google.protobuf.TextFormat;
import org.onosproject.net.pi.model.PiPipeconf;
-import org.onosproject.net.pi.model.PiPipeconfId;
import org.slf4j.Logger;
import p4.config.v1.P4InfoOuterClass.P4Info;
@@ -44,10 +43,10 @@
private static final int P4INFO_BROWSER_EXPIRE_TIME_IN_MIN = 10;
private static final Logger log = getLogger(PipeconfHelper.class);
- private static final Cache<PiPipeconfId, P4InfoBrowser> BROWSERS = CacheBuilder.newBuilder()
+ private static final Cache<Long, P4InfoBrowser> BROWSERS = CacheBuilder.newBuilder()
.expireAfterAccess(P4INFO_BROWSER_EXPIRE_TIME_IN_MIN, TimeUnit.MINUTES)
.build();
- private static final Map<PiPipeconfId, P4Info> P4INFOS = Maps.newConcurrentMap();
+ private static final Map<Long, P4Info> P4INFOS = Maps.newConcurrentMap();
private PipeconfHelper() {
// hide.
@@ -61,7 +60,7 @@
* @return P4Info or null
*/
public static P4Info getP4Info(PiPipeconf pipeconf) {
- return P4INFOS.computeIfAbsent(pipeconf.id(), piPipeconfId -> {
+ return P4INFOS.computeIfAbsent(pipeconf.fingerprint(), piPipeconfId -> {
if (!pipeconf.extension(P4_INFO_TEXT).isPresent()) {
log.warn("Missing P4Info extension in pipeconf {}", pipeconf.id());
return null;
@@ -90,7 +89,7 @@
*/
public static P4InfoBrowser getP4InfoBrowser(PiPipeconf pipeconf) {
try {
- return BROWSERS.get(pipeconf.id(), () -> {
+ return BROWSERS.get(pipeconf.fingerprint(), () -> {
P4Info p4info = PipeconfHelper.getP4Info(pipeconf);
if (p4info == null) {
return null;
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 46fb7f1..e716f56 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
@@ -670,13 +670,14 @@
return;
}
assertConfig(deviceId);
- providerService.deviceConnected(deviceId, getDeviceDescription(
- deviceId, available));
if (available) {
- // Push port descriptions.
+ // Push port descriptions. If marking online, make sure to update
+ // ports before other subsystems pick up the device event.
final List<PortDescription> ports = getPortDetails(deviceId);
providerService.updatePorts(deviceId, ports);
}
+ providerService.deviceConnected(deviceId, getDeviceDescription(
+ deviceId, available));
}
private boolean probeAvailability(DeviceHandshaker handshaker) {