commit 95308287cf713896edc00f83c840d5058d3e14b5
author Carmelo Cascone <carmelo@opennetworking.org> Mon Mar 18 17:18:04 2019 -0700
committer Carmelo Cascone <carmelo@opennetworking.org> Tue Mar 19 19:51:43 2019 +0000
tree e0e3ab70ea774337d43391a69053f730a9bbbc14
parent 67a317d0f0c942c0a97c803f1eb8a8aa32924f46

Fix pipeline not marked as READY after device reboot

Since Stratum persists pipeline config across reboots, the
GeneralDeviceProvider was marking the device available as the device
had a pipeline config set right after the connection open event, but we
had to wait for the periodic PipeconfWatchdog check to mark the pipeline
as READY. Now we trigger a PipeconfWatchdog check after every device
availability change event.

Change-Id: I11a6f52ff5ea5304aa26dbe39786a25055b828aa

core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfWatchdogManager.java

diff --git a/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfWatchdogManager.java b/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfWatchdogManager.java
index eafb778..0503d9a 100644
--- a/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfWatchdogManager.java
+++ b/core/net/src/main/java/org/onosproject/net/pi/impl/PiPipeconfWatchdogManager.java
@@ -55,18 +55,15 @@
 import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.slf4j.Logger;
 
-import java.util.Collections;
 import java.util.Dictionary;
 import java.util.Map;
 import java.util.Timer;
 import java.util.TimerTask;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.locks.Lock;
 
+import static java.util.Collections.singleton;
 import static org.onlab.util.Tools.groupedThreads;
 import static org.onosproject.net.OsgiPropertyConstants.PWM_PROBE_INTERVAL;
 import static org.onosproject.net.OsgiPropertyConstants.PWM_PROBE_INTERVAL_DEFAULT;
@@ -91,9 +88,6 @@
     private final Logger log = getLogger(getClass());
 
     private static final long SECONDS = 1000L;
-    // Long enough to allow for network delay (e.g. to transfer large pipeline
-    // binaries over slow network).
-    private static final long PIPECONF_SET_TIMEOUT = 60; // Seconds.
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY)
     private PiPipeconfMappingStore pipeconfMappingStore;
@@ -185,7 +179,7 @@
     public void triggerProbe(DeviceId deviceId) {
         final Device device = deviceService.getDevice(deviceId);
         if (device != null) {
-            filterAndTriggerTasks(Collections.singleton(device));
+            filterAndTriggerTasks(singleton(device));
         }
     }
 
@@ -206,8 +200,7 @@
             }
 
             final PiPipeconfId pipeconfId = pipeconfMappingStore.getPipeconfId(device.id());
-            if (pipeconfId == null
-                    || !device.is(PiPipelineProgrammable.class)) {
+            if (pipeconfId == null || !device.is(PiPipelineProgrammable.class)) {
                 return;
             }
 
@@ -256,21 +249,7 @@
                       pipeconf.id(), device.id());
             return true;
         }
-        try {
-            return pipelineProg.setPipeconf(pipeconf)
-                    .get(PIPECONF_SET_TIMEOUT, TimeUnit.SECONDS);
-        } catch (InterruptedException e) {
-            log.error("Thread interrupted while setting pipeconf on {}",
-                      device.id());
-            Thread.currentThread().interrupt();
-        } catch (ExecutionException e) {
-            log.error("Exception while setting pipeconf on {}",
-                      device.id(), e.getCause());
-        } catch (TimeoutException e) {
-            log.error("Operation TIMEOUT while setting pipeconf on {}",
-                      device.id());
-        }
-        return false;
+        return Futures.getUnchecked(pipelineProg.setPipeconf(pipeconf));
     }
 
     private Runnable withLock(Runnable task, Object object) {
@@ -307,7 +286,7 @@
     }
 
     private void startProbeTask() {
-        synchronized (timer) {
+        synchronized (this) {
             log.info("Starting pipeline probe thread with {} seconds interval...", probeInterval);
             task = new InternalTimerTask();
             timer.scheduleAtFixedRate(task, probeInterval * SECONDS,
@@ -317,7 +296,7 @@
 
 
     private void stopProbeTask() {
-        synchronized (timer) {
+        synchronized (this) {
             log.info("Stopping pipeline probe thread...");
             task.cancel();
             task = null;
@@ -326,7 +305,7 @@
 
 
     private synchronized void rescheduleProbeTask() {
-        synchronized (timer) {
+        synchronized (this) {
             stopProbeTask();
             startProbeTask();
         }
@@ -353,6 +332,14 @@
                 case DEVICE_AVAILABILITY_CHANGED:
                     if (!deviceService.isAvailable(device.id())) {
                         signalStatusUnknown(device.id());
+                    } else {
+                        // The GeneralDeviceProvider marks online devices that
+                        // have ANY pipeline config set. Here we make sure the
+                        // one configured in the pipeconf service is the
+                        // expected one. Clearly, it would be better to let the
+                        // GDP do this check and avoid sending twice the same
+                        // message to the switch.
+                        filterAndTriggerTasks(singleton(device));
                     }
                     break;
                 case DEVICE_REMOVED: