Various fixes in preparation of Stratum demo at ONS 2019

- Do not read counters with table entries for Barefoot drivers
- If driver behavior setup fails, log which operation we are aborting
- Remove unnecessary setup steps in Stratum-related drivers
- Always get clients by their key in gRPC-based drivers
- Log when P4Runtime group operation fails because of missing group in
store
- Fix polling of table entry counters for P4Runtime driver

Change-Id: Ic9bf19b76d8cb5a191aec24852af4410fea8b998
diff --git a/drivers/barefoot/src/main/resources/barefoot-drivers.xml b/drivers/barefoot/src/main/resources/barefoot-drivers.xml
index d712fb4..51fd29d 100644
--- a/drivers/barefoot/src/main/resources/barefoot-drivers.xml
+++ b/drivers/barefoot/src/main/resources/barefoot-drivers.xml
@@ -19,6 +19,7 @@
             swVersion="1.0" extends="p4runtime">
         <behaviour api="org.onosproject.net.behaviour.PiPipelineProgrammable"
                    impl="org.onosproject.drivers.barefoot.TofinoPipelineProgrammable"/>
+        <property name="tableReadCountersWithTableEntries">false</property>
         <property name="tableDeleteBeforeUpdate">true</property>
     </driver>
 
@@ -27,8 +28,9 @@
         <behaviour api="org.onosproject.net.behaviour.PiPipelineProgrammable"
                    impl="org.onosproject.drivers.barefoot.TofinoPipelineProgrammable"/>
         <!-- The current version of p4lang/PI used in Stratum does not
-        support reading default  table entries -->
+        support reading default table entries -->
         <property name="supportDefaultTableEntry">false</property>
+        <property name="tableReadCountersWithTableEntries">false</property>
     </driver>
 
 </drivers>
diff --git a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/AbstractGnmiHandlerBehaviour.java b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/AbstractGnmiHandlerBehaviour.java
index 3774bfb..72233fc 100644
--- a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/AbstractGnmiHandlerBehaviour.java
+++ b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/AbstractGnmiHandlerBehaviour.java
@@ -20,7 +20,6 @@
 import org.onosproject.gnmi.api.GnmiClient;
 import org.onosproject.gnmi.api.GnmiClientKey;
 import org.onosproject.gnmi.api.GnmiController;
-import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.config.NetworkConfigService;
 import org.onosproject.net.config.basics.BasicDeviceConfig;
@@ -40,18 +39,14 @@
     protected final Logger log = LoggerFactory.getLogger(getClass());
     protected DeviceId deviceId;
     protected DeviceService deviceService;
-    protected Device device;
-    protected GnmiController controller;
     protected GnmiClient client;
 
-    protected boolean setupBehaviour() {
+    protected boolean setupBehaviour(String opName) {
         deviceId = handler().data().deviceId();
         deviceService = handler().get(DeviceService.class);
-        controller = handler().get(GnmiController.class);
-        client = controller.getClient(deviceId);
-
+        client = getClientByKey();
         if (client == null) {
-            log.warn("Unable to find client for {}, aborting operation", deviceId);
+            log.warn("Missing client for {}, aborting {}", deviceId, opName);
             return false;
         }
 
diff --git a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiDeviceDescriptionDiscovery.java b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiDeviceDescriptionDiscovery.java
index ce134ad..a85cdfb 100644
--- a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiDeviceDescriptionDiscovery.java
+++ b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiDeviceDescriptionDiscovery.java
@@ -76,7 +76,7 @@
 
     @Override
     public List<PortDescription> discoverPortDetails() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("discoverPortDetails()")) {
             return Collections.emptyList();
         }
         log.debug("Discovering port details on device {}", handler().data().deviceId());
diff --git a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiPortStatisticsDiscovery.java b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiPortStatisticsDiscovery.java
index a41144e..fa7231f 100644
--- a/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiPortStatisticsDiscovery.java
+++ b/drivers/gnmi/src/main/java/org/onosproject/drivers/gnmi/OpenConfigGnmiPortStatisticsDiscovery.java
@@ -50,7 +50,7 @@
 
     @Override
     public Collection<PortStatistics> discoverPortStatistics() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("discoverPortStatistics()")) {
             return Collections.emptyList();
         }
 
diff --git a/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/AbstractGnoiHandlerBehaviour.java b/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/AbstractGnoiHandlerBehaviour.java
index 23181b4..84043a8 100644
--- a/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/AbstractGnoiHandlerBehaviour.java
+++ b/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/AbstractGnoiHandlerBehaviour.java
@@ -20,11 +20,9 @@
 import org.onosproject.gnoi.api.GnoiClient;
 import org.onosproject.gnoi.api.GnoiClientKey;
 import org.onosproject.gnoi.api.GnoiController;
-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.AbstractHandlerBehaviour;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -39,19 +37,14 @@
 
     protected final Logger log = LoggerFactory.getLogger(getClass());
     protected DeviceId deviceId;
-    protected DeviceService deviceService;
-    protected Device device;
-    protected GnoiController controller;
     protected GnoiClient client;
 
-    protected boolean setupBehaviour() {
+    protected boolean setupBehaviour(String opName) {
         deviceId = handler().data().deviceId();
-        deviceService = handler().get(DeviceService.class);
-        controller = handler().get(GnoiController.class);
-        client = controller.getClient(deviceId);
+        client = getClientByKey();
 
         if (client == null) {
-            log.warn("Unable to find client for {}, aborting operation", deviceId);
+            log.warn("Missing client for {}, aborting {}", deviceId, opName);
             return false;
         }
 
diff --git a/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/GnoiBasicSystemOperationsImpl.java b/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/GnoiBasicSystemOperationsImpl.java
index 411a8bf..b979546 100644
--- a/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/GnoiBasicSystemOperationsImpl.java
+++ b/drivers/gnoi/src/main/java/org/onosproject/drivers/gnoi/GnoiBasicSystemOperationsImpl.java
@@ -16,17 +16,18 @@
 
 package org.onosproject.drivers.gnoi;
 
+import gnoi.system.SystemOuterClass.RebootMethod;
 import gnoi.system.SystemOuterClass.RebootRequest;
 import gnoi.system.SystemOuterClass.RebootResponse;
-import gnoi.system.SystemOuterClass.RebootMethod;
 import org.onosproject.net.behaviour.BasicSystemOperations;
-import java.util.concurrent.CompletableFuture;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.concurrent.CompletableFuture;
+
 /**
- * Implementation of the BasicSystemOperations behavior for gNOI-enabled devices.
+ * Implementation of the BasicSystemOperations behavior for gNOI-enabled
+ * devices.
  */
 public class GnoiBasicSystemOperationsImpl
         extends AbstractGnoiHandlerBehaviour implements BasicSystemOperations {
@@ -36,12 +37,13 @@
 
     @Override
     public CompletableFuture<Boolean> reboot() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("reboot()")) {
             return CompletableFuture.completedFuture(false);
         }
 
         final RebootRequest.Builder requestMsg = RebootRequest.newBuilder().setMethod(RebootMethod.COLD);
-        final CompletableFuture<Boolean> future = client.reboot(requestMsg.build())
+
+        return client.reboot(requestMsg.build())
                 .handle((response, error) -> {
                     if (error == null) {
                         log.debug("gNOI reboot() for device {} returned {}", deviceId, response);
@@ -51,16 +53,14 @@
                         return false;
                     }
                 });
-
-        return future;
     }
 
     @Override
     public CompletableFuture<Long> time() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("time()")) {
             return CompletableFuture.completedFuture(0L);
         }
-        final CompletableFuture<Long> future = client.time()
+        return client.time()
                 .handle((response, error) -> {
                     if (error == null) {
                         log.debug("gNOI time() for device {} returned {}", deviceId.uri(), response.getTime());
@@ -70,7 +70,5 @@
                         return 0L;
                     }
                 });
-
-        return future;
     }
 }
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/AbstractP4RuntimeHandlerBehaviour.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/AbstractP4RuntimeHandlerBehaviour.java
index 5326861..c71907b 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/AbstractP4RuntimeHandlerBehaviour.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/AbstractP4RuntimeHandlerBehaviour.java
@@ -47,43 +47,29 @@
 
     // Initialized by setupBehaviour()
     protected DeviceId deviceId;
-    protected DeviceService deviceService;
-    protected Device device;
-    protected P4RuntimeController controller;
     protected PiPipeconf pipeconf;
     protected P4RuntimeClient client;
     protected PiTranslationService translationService;
 
     /**
      * Initializes this behaviour attributes. Returns true if the operation was
-     * successful, false otherwise. This method assumes that the P4Runtime
-     * controller already has a client for this device and that the device has
-     * been created in the core.
+     * successful, false otherwise.
      *
+     * @param opName name of the operation
      * @return true if successful, false otherwise
      */
-    protected boolean setupBehaviour() {
+    protected boolean setupBehaviour(String opName) {
         deviceId = handler().data().deviceId();
 
-        deviceService = handler().get(DeviceService.class);
-        device = deviceService.getDevice(deviceId);
-        if (device == null) {
-            log.warn("Unable to find device with id {}", deviceId);
-            return false;
-        }
-
-        controller = handler().get(P4RuntimeController.class);
-        client = controller.getClient(deviceId);
+        client = getClientByKey();
         if (client == null) {
-            log.warn("Unable to find client for {}", deviceId);
+            log.warn("Missing client for {}, aborting {}", deviceId, opName);
             return false;
         }
 
         PiPipeconfService piPipeconfService = handler().get(PiPipeconfService.class);
         if (!piPipeconfService.getPipeconf(deviceId).isPresent()) {
-            log.warn("Unable to get assigned pipeconf for {} (mapping " +
-                             "missing in PiPipeconfService)",
-                     deviceId);
+            log.warn("Missing pipeconf for {}, cannot perform {}", deviceId, opName);
             return false;
         }
         pipeconf = piPipeconfService.getPipeconf(deviceId).get();
@@ -100,6 +86,11 @@
      * @return interpreter or null
      */
     PiPipelineInterpreter getInterpreter() {
+        final Device device = handler().get(DeviceService.class).getDevice(deviceId);
+        if (device == null) {
+            log.warn("Unable to find device {}, cannot get interpreter", deviceId);
+            return null;
+        }
         if (!device.is(PiPipelineInterpreter.class)) {
             log.warn("Unable to get interpreter for {}, missing behaviour",
                      deviceId);
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeActionGroupProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeActionGroupProgrammable.java
index 9603241..d14b917 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeActionGroupProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeActionGroupProgrammable.java
@@ -81,8 +81,8 @@
     private PiGroupTranslator groupTranslator;
 
     @Override
-    protected boolean setupBehaviour() {
-        if (!super.setupBehaviour()) {
+    protected boolean setupBehaviour(String opName) {
+        if (!super.setupBehaviour(opName)) {
             return false;
         }
         groupMirror = this.handler().get(P4RuntimeActionProfileGroupMirror.class);
@@ -95,7 +95,7 @@
     @Override
     public void performGroupOperation(DeviceId deviceId,
                                       GroupOperations groupOps) {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("performGroupOperation()")) {
             return;
         }
 
@@ -107,8 +107,8 @@
                     // GroupDescription.
                     Group groupOnStore = groupStore.getGroup(deviceId, op.groupId());
                     if (groupOnStore == null) {
-                        log.warn("Unable to find group {} in store, aborting {} operation",
-                                 op.groupId(), op.opType());
+                        log.warn("Unable to find group {} in store, aborting {} operation [{}]",
+                                 op.groupId(), op.opType(), op);
                         return;
                     }
                     GroupDescription groupDesc = new DefaultGroupDescription(
@@ -121,7 +121,7 @@
 
     @Override
     public Collection<Group> getGroups() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("getGroups()")) {
             return Collections.emptyList();
         }
 
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
index a4ae0ef..d55dd71 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
@@ -108,9 +108,9 @@
     private PiFlowRuleTranslator translator;
 
     @Override
-    protected boolean setupBehaviour() {
+    protected boolean setupBehaviour(String opName) {
 
-        if (!super.setupBehaviour()) {
+        if (!super.setupBehaviour(opName)) {
             return false;
         }
 
@@ -123,7 +123,7 @@
     @Override
     public Collection<FlowEntry> getFlowEntries() {
 
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("getFlowEntries()")) {
             return Collections.emptyList();
         }
 
@@ -149,12 +149,13 @@
             return Collections.emptyList();
         }
 
-        final Map<PiTableEntry, PiCounterCellData> counterCellMap =
+        final Map<PiTableEntryHandle, PiCounterCellData> counterCellMap =
                 readEntryCounters(deviceEntries);
         // Forge flow entries with counter values.
         for (PiTableEntry entry : deviceEntries) {
+            final PiTableEntryHandle handle = entry.handle(deviceId);
             final FlowEntry flowEntry = forgeFlowEntry(
-                    entry, counterCellMap.get(entry));
+                    entry, handle, counterCellMap.get(handle));
             if (flowEntry == null) {
                 // Entry is on device but unknown to translation service or
                 // device mirror. Inconsistent. Mark for removal.
@@ -228,8 +229,8 @@
     }
 
     private FlowEntry forgeFlowEntry(PiTableEntry entry,
+                                     PiTableEntryHandle handle,
                                      PiCounterCellData cellData) {
-        final PiTableEntryHandle handle = entry.handle(deviceId);
         final Optional<PiTranslatedEntity<FlowRule, PiTableEntry>>
                 translatedEntity = translator.lookup(handle);
         final TimedEntry<PiTableEntry> timedEntry = tableMirror.get(handle);
@@ -262,14 +263,14 @@
     private Collection<FlowEntry> getFlowEntriesFromMirror() {
         return tableMirror.getAll(deviceId).stream()
                 .map(timedEntry -> forgeFlowEntry(
-                        timedEntry.entry(), null))
+                        timedEntry.entry(), timedEntry.entry().handle(deviceId), null))
                 .filter(Objects::nonNull)
                 .collect(Collectors.toList());
     }
 
     private Collection<FlowRule> processFlowRules(Collection<FlowRule> rules,
                                                   Operation driverOperation) {
-        if (!setupBehaviour() || rules.isEmpty()) {
+        if (!setupBehaviour("processFlowRules()") || rules.isEmpty()) {
             return Collections.emptyList();
         }
         // Created batched write request.
@@ -459,52 +460,38 @@
                 originalDefaultEntry.action().equals(entry.action());
     }
 
-    private Map<PiTableEntry, PiCounterCellData> readEntryCounters(
+    private Map<PiTableEntryHandle, PiCounterCellData> readEntryCounters(
             Collection<PiTableEntry> tableEntries) {
+
         if (!driverBoolProperty(SUPPORT_TABLE_COUNTERS,
                                 DEFAULT_SUPPORT_TABLE_COUNTERS)
                 || tableEntries.isEmpty()) {
             return Collections.emptyMap();
         }
 
-        final Map<PiTableEntry, PiCounterCellData> cellDataMap = Maps.newHashMap();
-
-        // We expect the server to return table entries with counter data (if
-        // the table supports counter). Here we extract such counter data and we
-        // determine if there are missing counter cells (if, for example, the
-        // serves does not support returning counter data with table entries)
-        final Set<PiHandle> missingCellHandles = tableEntries.stream()
-                .map(t -> {
-                    if (t.counter() != null) {
-                        // Counter data found in table entry.
-                        cellDataMap.put(t, t.counter());
-                        return null;
-                    } else {
-                        return t;
-                    }
-                })
-                .filter(Objects::nonNull)
-                // Ignore for default entries and for tables without counters.
-                .filter(e -> !e.isDefaultAction())
-                .filter(e -> tableHasCounter(e.table()))
-                .map(PiCounterCellId::ofDirect)
-                .map(id -> PiCounterCellHandle.of(deviceId, id))
-                .collect(Collectors.toSet());
-        // We might be sending a large read request (for thousands or more
-        // of counter cell handles). We request the driver to vet this
-        // operation via driver property.
-        if (!missingCellHandles.isEmpty()
-                && !driverBoolProperty(READ_COUNTERS_WITH_TABLE_ENTRIES,
-                                       DEFAULT_READ_COUNTERS_WITH_TABLE_ENTRIES)) {
-            client.read(pipeconf)
-                    .handles(missingCellHandles)
+        if (driverBoolProperty(READ_COUNTERS_WITH_TABLE_ENTRIES,
+                               DEFAULT_READ_COUNTERS_WITH_TABLE_ENTRIES)) {
+            return tableEntries.stream()
+                    .filter(t -> t.counter() != null)
+                    .collect(Collectors.toMap(
+                            t -> t.handle(deviceId), PiTableEntry::counter));
+        } else {
+            final Set<PiHandle> cellHandles = tableEntries.stream()
+                    .filter(e -> !e.isDefaultAction())
+                    .filter(e -> tableHasCounter(e.table()))
+                    .map(PiCounterCellId::ofDirect)
+                    .map(id -> PiCounterCellHandle.of(deviceId, id))
+                    .collect(Collectors.toSet());
+            // FIXME: We might be sending a very large read request...
+            return client.read(pipeconf)
+                    .handles(cellHandles)
                     .submitSync()
                     .all(PiCounterCell.class).stream()
                     .filter(c -> c.cellId().counterType().equals(PiCounterType.DIRECT))
-                    .forEach(c -> cellDataMap.put(c.cellId().tableEntry(), c.data()));
+                    .collect(Collectors.toMap(
+                            c -> c.cellId().tableEntry().handle(deviceId),
+                            PiCounterCell::data));
         }
-
-        return cellDataMap;
     }
 
     private boolean tableHasCounter(PiTableId tableId) {
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 60b0dbc..64611fe 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
@@ -120,12 +120,12 @@
 
     @Override
     public void roleChanged(MastershipRole newRole) {
+        if (!setupBehaviour("roleChanged()")) {
+            return;
+        }
         if (newRole.equals(MastershipRole.NONE)) {
-            final P4RuntimeClient client = getClientByKey();
-            if (client != null) {
-                log.info("Notified role NONE, closing session...");
-                client.closeSession();
-            }
+            log.info("Notified role NONE, closing session...");
+            client.closeSession();
         } else {
             throw new UnsupportedOperationException(
                     "Use preference-based way for setting MASTER or STANDBY roles");
@@ -134,18 +134,19 @@
 
     @Override
     public void roleChanged(int preference, long term) {
-        if (setupBehaviour()) {
-            final int clusterSize = handler().get(ClusterService.class)
-                    .getNodes().size();
-            if (clusterSize > MAX_CLUSTER_SIZE) {
-                throw new IllegalStateException(
-                        "Cluster too big! Maz size supported is " + MAX_CLUSTER_SIZE);
-            }
-            BigInteger electionId = BigInteger.valueOf(term)
-                    .multiply(BigInteger.valueOf(MAX_CLUSTER_SIZE))
-                    .subtract(BigInteger.valueOf(preference));
-            client.setMastership(preference == 0, electionId);
+        if (!setupBehaviour("roleChanged()")) {
+            return;
         }
+        final int clusterSize = handler().get(ClusterService.class)
+                .getNodes().size();
+        if (clusterSize > MAX_CLUSTER_SIZE) {
+            throw new IllegalStateException(
+                    "Cluster too big! Maz size supported is " + MAX_CLUSTER_SIZE);
+        }
+        BigInteger electionId = BigInteger.valueOf(term)
+                .multiply(BigInteger.valueOf(MAX_CLUSTER_SIZE))
+                .subtract(BigInteger.valueOf(preference));
+        client.setMastership(preference == 0, electionId);
     }
 
     @Override
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
index c9c436d..73faf45 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMeterProgrammable.java
@@ -66,8 +66,8 @@
     private PiPipelineModel pipelineModel;
 
     @Override
-    protected boolean setupBehaviour() {
-        if (!super.setupBehaviour()) {
+    protected boolean setupBehaviour(String opName) {
+        if (!super.setupBehaviour(opName)) {
             return false;
         }
 
@@ -80,7 +80,7 @@
     @Override
     public CompletableFuture<Boolean> performMeterOperation(MeterOperation meterOp) {
 
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("performMeterOperation()")) {
             return CompletableFuture.completedFuture(false);
         }
 
@@ -118,7 +118,7 @@
     @Override
     public CompletableFuture<Collection<Meter>> getMeters() {
 
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("getMeters()")) {
             return CompletableFuture.completedFuture(Collections.emptyList());
         }
 
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMulticastGroupProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMulticastGroupProgrammable.java
index fcb578d..80c771e 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMulticastGroupProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeMulticastGroupProgrammable.java
@@ -62,8 +62,8 @@
     private PiMulticastGroupTranslator mcGroupTranslator;
 
     @Override
-    protected boolean setupBehaviour() {
-        if (!super.setupBehaviour()) {
+    protected boolean setupBehaviour(String opName) {
+        if (!super.setupBehaviour(opName)) {
             return false;
         }
         mcGroupMirror = this.handler().get(P4RuntimeMulticastGroupMirror.class);
@@ -74,20 +74,25 @@
 
     @Override
     public void performGroupOperation(DeviceId deviceId, GroupOperations groupOps) {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("performGroupOperation()")) {
             return;
         }
         groupOps.operations().stream()
                 .filter(op -> op.groupType().equals(GroupDescription.Type.ALL))
                 .forEach(op -> {
                     final Group group = groupStore.getGroup(deviceId, op.groupId());
+                    if (group == null) {
+                        log.warn("Unable to find group {} in store, aborting {} operation [{}]",
+                                 op.groupId(), op.opType(), op);
+                        return;
+                    }
                     processMcGroupOp(group, op.opType());
                 });
     }
 
     @Override
     public Collection<Group> getGroups() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("getGroups()")) {
             return Collections.emptyList();
         }
         return ImmutableList.copyOf(getMcGroups());
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimePacketProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimePacketProgrammable.java
index 73acf98..9010c51 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimePacketProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimePacketProgrammable.java
@@ -33,7 +33,7 @@
     @Override
     public void emit(OutboundPacket packet) {
 
-        if (!this.setupBehaviour()) {
+        if (!this.setupBehaviour("emit()")) {
             return;
         }
 
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeTableStatisticsDiscovery.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeTableStatisticsDiscovery.java
index f130732..1b390ef 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeTableStatisticsDiscovery.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeTableStatisticsDiscovery.java
@@ -17,23 +17,23 @@
 
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.behaviour.TableStatisticsDiscovery;
-import org.onosproject.net.flow.FlowRuleService;
-import org.onosproject.net.flow.FlowEntry;
-import org.onosproject.net.flow.TableId;
-import org.onosproject.net.flow.IndexTableId;
-import org.onosproject.net.flow.TableStatisticsEntry;
 import org.onosproject.net.flow.DefaultTableStatisticsEntry;
-import org.onosproject.net.pi.model.PiPipelineModel;
+import org.onosproject.net.flow.FlowEntry;
+import org.onosproject.net.flow.FlowRuleService;
+import org.onosproject.net.flow.IndexTableId;
+import org.onosproject.net.flow.TableId;
+import org.onosproject.net.flow.TableStatisticsEntry;
 import org.onosproject.net.pi.model.PiPipelineInterpreter;
+import org.onosproject.net.pi.model.PiPipelineModel;
 import org.onosproject.net.pi.model.PiTableId;
 import org.onosproject.net.pi.model.PiTableModel;
 
-import java.util.List;
+import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Map;
 import java.util.HashMap;
 import java.util.Iterator;
-import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
 
 import static com.google.common.collect.Lists.newArrayList;
 
@@ -45,7 +45,7 @@
 
     @Override
     public List<TableStatisticsEntry> getTableStatistics() {
-        if (!setupBehaviour()) {
+        if (!setupBehaviour("getTableStatistics()")) {
             return Collections.emptyList();
         }
         FlowRuleService flowService = handler().get(FlowRuleService.class);
diff --git a/drivers/stratum/src/main/java/org/onosproject/drivers/stratum/StratumDeviceDescriptionDiscovery.java b/drivers/stratum/src/main/java/org/onosproject/drivers/stratum/StratumDeviceDescriptionDiscovery.java
index f2e6e8d..dbcb193 100644
--- a/drivers/stratum/src/main/java/org/onosproject/drivers/stratum/StratumDeviceDescriptionDiscovery.java
+++ b/drivers/stratum/src/main/java/org/onosproject/drivers/stratum/StratumDeviceDescriptionDiscovery.java
@@ -66,8 +66,8 @@
                         .set(AnnotationKeys.PROTOCOL, format(
                                 "%s, %s, %s",
                                 p4Descr.annotations().value(AnnotationKeys.PROTOCOL),
-                                gnoiDescr.annotations().value(AnnotationKeys.PROTOCOL),
-                                gnmiDescr.annotations().value(AnnotationKeys.PROTOCOL)))
+                                gnmiDescr.annotations().value(AnnotationKeys.PROTOCOL),
+                                gnoiDescr.annotations().value(AnnotationKeys.PROTOCOL)))
                         .build());
     }