More improvements and bugfixes in P4Runtime subsystem
Most notably, we fix a bug in which some nodes were not able to find
pipeconf-specific behaviors for a given device. The problem is not
completelly solved but it's mitigated.
There's a race condition caused by the fact that the GDP updates the cfg
with the merged driver name before advertising the device to the core.
Some nodes might receive the cfg update after the device has been
advertised. We mitigate the problem by performing the pipeline deploy
(slow operation) after the cfg update, giving more time for nodes
to catch up. Perhaps we should listen for cfg update events before
advertising the device to the core?
Also:
- NPE when getting P4Runtime client
- Detect if a base driver is already merged in pipeconf manager
- Longer timeouts in P4Runtime driver and protocol (for slow networks)
- Configurable timeout in P4Runtime driver and GDP
- NPE when adding/removing device agent listeners in P4Rtunime handshaker
- Various exceptions due to race conditions in GDP when disconnecting
devices (by serializing disconnect tasks per device)
- NPE when cancelling polling tasks in GDP
- Refactored PipeconfService to distinguish between driver merge,
pipeconf map update, and cfg update (now performed in the GDP)
- Fixed PipeconfManagerTest, not testing driver behaviours
- Use Guava striped locks when possible (more memory-efficient than maps,
and with strict atomicity guarantees w.r.t. to caches).
Change-Id: I30f3887541ba0fd44439a86885e9821ac565b64c
diff --git a/core/net/src/test/java/org/onosproject/net/pi/impl/PiPipeconfManagerTest.java b/core/net/src/test/java/org/onosproject/net/pi/impl/PiPipeconfManagerTest.java
index 4d29e69..a418461 100644
--- a/core/net/src/test/java/org/onosproject/net/pi/impl/PiPipeconfManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/pi/impl/PiPipeconfManagerTest.java
@@ -20,11 +20,11 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
import org.junit.Before;
import org.junit.Test;
import org.onlab.util.ItemNotFoundException;
import org.onosproject.net.DeviceId;
-import org.onosproject.net.behaviour.Pipeliner;
import org.onosproject.net.config.Config;
import org.onosproject.net.config.ConfigApplyDelegate;
import org.onosproject.net.config.ConfigFactory;
@@ -46,9 +46,7 @@
import org.onosproject.net.driver.DriverServiceAdapter;
import org.onosproject.net.pi.model.PiPipeconf;
import org.onosproject.net.pi.model.PiPipeconfId;
-import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.service.PiPipeconfConfig;
-import org.onosproject.pipelines.basic.PipeconfLoader;
import java.io.IOException;
import java.io.InputStream;
@@ -61,6 +59,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.onosproject.pipelines.basic.PipeconfLoader.BASIC_PIPECONF;
/**
@@ -70,15 +69,12 @@
private static final DeviceId DEVICE_ID = DeviceId.deviceId("test:test");
private static final String BASE_DRIVER = "baseDriver";
- private static final Set<Class<? extends Behaviour>> EXPECTED_BEHAVIOURS =
- ImmutableSet.of(DeviceDescriptionDiscovery.class, Pipeliner.class, PiPipelineInterpreter.class);
//Mock util sets and classes
private final NetworkConfigRegistry cfgService = new MockNetworkConfigRegistry();
private final DriverService driverService = new MockDriverService();
private final DriverAdminService driverAdminService = new MockDriverAdminService();
private Driver baseDriver = new MockDriver();
- private String completeDriverName;
private final Set<ConfigFactory> cfgFactories = new HashSet<>();
private final Set<NetworkConfigListener> netCfgListeners = new HashSet<>();
@@ -99,8 +95,7 @@
@Before
public void setUp() throws IOException {
piPipeconfService = new PiPipeconfManager();
- piPipeconf = PipeconfLoader.BASIC_PIPECONF;
- completeDriverName = BASE_DRIVER + ":" + piPipeconf.id();
+ piPipeconf = BASIC_PIPECONF;
piPipeconfService.cfgService = cfgService;
piPipeconfService.driverService = driverService;
piPipeconfService.driverAdminService = driverAdminService;
@@ -147,7 +142,7 @@
@Test
- public void bindToDevice() throws Exception {
+ public void mergeDriver() {
PiPipeconfId piPipeconfId = cfgService.getConfig(DEVICE_ID, PiPipeconfConfig.class).piPipeconfId();
assertEquals(piPipeconf.id(), piPipeconfId);
@@ -158,25 +153,24 @@
assertEquals("Returned PiPipeconf is not correct", piPipeconf,
piPipeconfService.getPipeconf(piPipeconf.id()).get());
- piPipeconfService.bindToDevice(piPipeconfId, DEVICE_ID).whenComplete((booleanResult, ex) -> {
+ String mergedDriverName = piPipeconfService.mergeDriver(DEVICE_ID, piPipeconfId);
- //we assume that the provider is 1 and that it contains 1 driver
- //we also assume that everything after driverAdminService.registerProvider(provider); has been tested.
- assertTrue("Provider should be registered", providers.size() != 0);
+ //we assume that the provider is 1 and that it contains 1 driver
+ //we also assume that everything after driverAdminService.registerProvider(provider); has been tested.
+ assertTrue("Provider should be registered", providers.size() == 1);
- assertTrue("Boolean Result of method should be True", booleanResult);
+ assertTrue("Merged driver name should be valid",
+ mergedDriverName != null && !mergedDriverName.isEmpty());
- providers.forEach(p -> {
- assertTrue("Provider should contain a driver", p.getDrivers().size() != 0);
- p.getDrivers().forEach(driver -> {
- assertEquals("The driver has wrong name", driver.name(), completeDriverName);
- assertEquals("The driver contains wrong behaviours", EXPECTED_BEHAVIOURS, driver.behaviours());
+ DriverProvider provider = providers.iterator().next();
+ assertTrue("Provider should contain one driver", provider.getDrivers().size() == 1);
- });
- });
- }).exceptionally(ex -> {
- throw new IllegalStateException(ex);
- });
+ Driver driver = provider.getDrivers().iterator().next();
+
+ Set<Class<? extends Behaviour>> expectedBehaviours = Sets.newHashSet();
+ expectedBehaviours.addAll(BASIC_PIPECONF.behaviours());
+ expectedBehaviours.addAll(baseDriver.behaviours());
+ assertEquals("The driver contains wrong behaviours", expectedBehaviours, driver.behaviours());
}
private class MockNetworkConfigRegistry extends NetworkConfigRegistryAdapter {
@@ -230,6 +224,11 @@
public void registerProvider(DriverProvider provider) {
providers.add(provider);
}
+
+ @Override
+ public Set<DriverProvider> getProviders() {
+ return providers;
+ }
}
private class MockDelegate implements ConfigApplyDelegate {