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

protocols/grpc/ctl/src/main/java/org/onosproject/grpc/ctl/AbstractGrpcClientController.java

@@ -101,6 +101,8 @@ public abstract class AbstractGrpcClientController
                     "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 @@ public abstract class AbstractGrpcClientController
         withDeviceLock(() -> {
             final C client = clients.remove(deviceId);
             if (client != null) {
+                log.info("Removing {}...", clientName(deviceId));
                 client.shutdown();
             }
             return null;

protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/client/StreamClientImpl.java

@@ -397,11 +397,16 @@ public final class StreamClientImpl implements P4RuntimeStreamClient {
         void signalClosed() {
             synchronized (this) {
                 final boolean wasOpen = open.getAndSet(false);
-                if (wasOpen) {
+                // FIXME: in case of device disconnection, all clients will
-                    // We lost any valid mastership role.
+                //  signal role NONE, preventing the DeviceManager to mark the
-                    controller.postEvent(new DeviceAgentEvent(
+                //  device as offline, as only the master can do that. We should
-                            DeviceAgentEvent.Type.ROLE_NONE, deviceId));
+                //  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));
+                // }
             }
         }
 

protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/utils/PipeconfHelper.java

@@ -22,7 +22,6 @@ import com.google.common.collect.Maps;
 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 @@ public final class PipeconfHelper {
     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 @@ public final class PipeconfHelper {
      * @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 final class PipeconfHelper {
      */
     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;

providers/general/device/src/main/java/org/onosproject/provider/general/device/impl/GeneralDeviceProvider.java

@@ -670,13 +670,14 @@ public class GeneralDeviceProvider extends AbstractProvider
             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) {