ONOS-7797 Support adding an arbitrary number of members to an existing P4Runtime group
Change-Id: I7b9ed589ca15957ee1b2780934a6da8e998bace3
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 5ee965d..fd2ef27 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
@@ -26,6 +26,7 @@
import org.onosproject.drivers.p4runtime.mirror.P4RuntimeActionProfileMemberMirror;
import org.onosproject.drivers.p4runtime.mirror.P4RuntimeGroupMirror;
import org.onosproject.drivers.p4runtime.mirror.TimedEntry;
+import org.onosproject.net.DefaultAnnotations;
import org.onosproject.net.DeviceId;
import org.onosproject.net.group.DefaultGroup;
import org.onosproject.net.group.DefaultGroupDescription;
@@ -76,6 +77,7 @@
// the ONOS store.
private static final String READ_ACTION_GROUPS_FROM_MIRROR = "actionGroupReadFromMirror";
private static final boolean DEFAULT_READ_ACTION_GROUPS_FROM_MIRROR = false;
+ private static final String MAX_MEM_SIZE = "maxMemSize";
protected GroupStore groupStore;
private P4RuntimeGroupMirror groupMirror;
@@ -84,6 +86,7 @@
// Needed to synchronize operations over the same group.
private static final Striped<Lock> STRIPED_LOCKS = Striped.lock(30);
+ private static final int GROUP_MEMBERS_BUFFER_SIZE = 3;
@Override
protected boolean setupBehaviour() {
@@ -342,20 +345,40 @@
}
private boolean applyGroup(PiActionGroup group, PiActionGroupHandle handle) {
- final P4RuntimeClient.WriteOperationType opType =
+ final int currentMemberSize = group.members().size();
+ if (groupMirror.get(handle) != null) {
+ String maxMemSize = "";
+ if (groupMirror.annotations(handle) != null &&
+ groupMirror.annotations(handle).value(MAX_MEM_SIZE) != null) {
+ maxMemSize = groupMirror.annotations(handle).value(MAX_MEM_SIZE);
+ }
+ if (maxMemSize == "" || currentMemberSize > Integer.parseInt(maxMemSize)) {
+ deleteGroup(group, handle);
+ }
+ }
+
+ P4RuntimeClient.WriteOperationType opType =
groupMirror.get(handle) == null ? INSERT : MODIFY;
+ int currentMaxMemberSize = opType == INSERT ? (currentMemberSize + GROUP_MEMBERS_BUFFER_SIZE) : 0;
+
final boolean success = getFutureWithDeadline(
- client.writeActionGroup(group, opType, pipeconf),
+ client.writeActionGroup(group, opType, pipeconf, currentMaxMemberSize),
"performing action profile group " + opType, false);
if (success) {
groupMirror.put(handle, group);
+ if (opType == INSERT) {
+ groupMirror.putAnnotations(handle, DefaultAnnotations
+ .builder()
+ .set(MAX_MEM_SIZE, Integer.toString(currentMaxMemberSize))
+ .build());
+ }
}
return success;
}
private boolean deleteGroup(PiActionGroup group, PiActionGroupHandle handle) {
final boolean success = getFutureWithDeadline(
- client.writeActionGroup(group, DELETE, pipeconf),
+ client.writeActionGroup(group, DELETE, pipeconf, 0),
"performing action profile group " + DELETE, false);
if (success) {
groupMirror.remove(handle);
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/AbstractDistributedP4RuntimeMirror.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/AbstractDistributedP4RuntimeMirror.java
index 9230547..fc982be 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/AbstractDistributedP4RuntimeMirror.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/AbstractDistributedP4RuntimeMirror.java
@@ -25,6 +25,7 @@
import org.apache.felix.scr.annotations.ReferenceCardinality;
import org.onlab.util.KryoNamespace;
import org.onlab.util.SharedExecutors;
+import org.onosproject.net.Annotations;
import org.onosproject.net.DeviceId;
import org.onosproject.net.pi.runtime.PiEntity;
import org.onosproject.net.pi.runtime.PiHandle;
@@ -69,6 +70,8 @@
private EventuallyConsistentMap<H, TimedEntry<E>> mirrorMap;
+ private EventuallyConsistentMap<H, Annotations> annotationsMap;
+
private final PiPipeconfWatchdogListener pipeconfListener =
new InternalPipeconfWatchdogListener();
@@ -80,6 +83,14 @@
.withSerializer(storeSerializer())
.withTimestampProvider((k, v) -> new WallClockTimestamp())
.build();
+
+ annotationsMap = storageService
+ .<H, Annotations>eventuallyConsistentMapBuilder()
+ .withName(mapName() + "-annotations")
+ .withSerializer(storeSerializer())
+ .withTimestampProvider((k, v) -> new WallClockTimestamp())
+ .build();
+
pipeconfWatchdogService.addListener(pipeconfListener);
log.info("Started");
}
@@ -132,6 +143,20 @@
public void remove(H handle) {
checkNotNull(handle);
mirrorMap.remove(handle);
+ annotationsMap.remove(handle);
+ }
+
+ @Override
+ public void putAnnotations(H handle, Annotations annotations) {
+ checkNotNull(handle);
+ checkNotNull(annotations);
+ annotationsMap.put(handle, annotations);
+ }
+
+ @Override
+ public Annotations annotations(H handle) {
+ checkNotNull(handle);
+ return annotationsMap.get(handle);
}
@Override
@@ -190,7 +215,7 @@
private void removeAll(DeviceId deviceId) {
checkNotNull(deviceId);
Collection<H> handles = getHandlesForDevice(deviceId);
- handles.forEach(mirrorMap::remove);
+ handles.forEach(this::remove);
}
public class InternalPipeconfWatchdogListener implements PiPipeconfWatchdogListener {
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/P4RuntimeMirror.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/P4RuntimeMirror.java
index d1c9cdd..51b31aa 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/P4RuntimeMirror.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/mirror/P4RuntimeMirror.java
@@ -17,6 +17,7 @@
package org.onosproject.drivers.p4runtime.mirror;
import com.google.common.annotations.Beta;
+import org.onosproject.net.Annotations;
import org.onosproject.net.DeviceId;
import org.onosproject.net.pi.runtime.PiEntity;
import org.onosproject.net.pi.runtime.PiHandle;
@@ -74,6 +75,23 @@
void remove(H handle);
/**
+ * Stores the given annotations associating it to the given handle.
+ *
+ * @param handle handle
+ * @param annotations entry
+ */
+ void putAnnotations(H handle, Annotations annotations);
+
+ /**
+ * Returns annotations associated to the given handle, if present, otherwise
+ * null.
+ *
+ * @param handle handle
+ * @return PI table annotations
+ */
+ Annotations annotations(H handle);
+
+ /**
* Synchronizes the state of the given device ID with the given handle map.
*
* @param deviceId device ID
diff --git a/protocols/p4runtime/api/src/main/java/org/onosproject/p4runtime/api/P4RuntimeClient.java b/protocols/p4runtime/api/src/main/java/org/onosproject/p4runtime/api/P4RuntimeClient.java
index f30ec28..f44c629 100644
--- a/protocols/p4runtime/api/src/main/java/org/onosproject/p4runtime/api/P4RuntimeClient.java
+++ b/protocols/p4runtime/api/src/main/java/org/onosproject/p4runtime/api/P4RuntimeClient.java
@@ -212,10 +212,13 @@
* @param group the action group
* @param opType write operation type
* @param pipeconf the pipeconf currently deployed on the device
+ * @param maxMemberSize the maximum number of members that can be added to the group.
+ * This is meaningful only if it's an INSERT operation, otherwise
+ * its value should be 0
* @return true if the operation was successful, false otherwise
*/
CompletableFuture<Boolean> writeActionGroup(
- PiActionGroup group, WriteOperationType opType, PiPipeconf pipeconf);
+ PiActionGroup group, WriteOperationType opType, PiPipeconf pipeconf, int maxMemberSize);
/**
* Dumps all groups currently installed for the given action profile.
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileGroupEncoder.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileGroupEncoder.java
index 709b92e..cb0735d 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileGroupEncoder.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileGroupEncoder.java
@@ -36,8 +36,6 @@
*/
final class ActionProfileGroupEncoder {
- private static final int GROUP_SIZE_ADDITIONAL_MEMBERS = 10;
-
private ActionProfileGroupEncoder() {
// hide default constructor
}
@@ -47,13 +45,14 @@
*
* @param piActionGroup the action profile group
* @param pipeconf the pipeconf
+ * @param maxMemberSize the max member size of action group
* @return a action profile group encoded from PI action group
* @throws P4InfoBrowser.NotFoundException if can't find action profile from
* P4Info browser
* @throws EncodeException if can't find P4Info from
* pipeconf
*/
- static ActionProfileGroup encode(PiActionGroup piActionGroup, PiPipeconf pipeconf)
+ static ActionProfileGroup encode(PiActionGroup piActionGroup, PiPipeconf pipeconf, int maxMemberSize)
throws P4InfoBrowser.NotFoundException, EncodeException {
P4InfoBrowser browser = PipeconfHelper.getP4InfoBrowser(pipeconf);
@@ -78,15 +77,9 @@
actionProfileGroupBuilder.addMembers(member);
});
- // FIXME: ONOS-7797 Make this configurable, or find a different way of
- // supporting group modify. In P4Runtime, group size cannot be modified
- // once the group is created. To allow adding members to an existing
- // group we set max_size to support an additional number of members
- // other than the one already defined in the PI group. Clearly, this
- // will break if we try to add more than GROUP_SIZE_ADDITIONAL_MEMBERS
- // to the same group.
- actionProfileGroupBuilder.setMaxSize(
- piActionGroup.members().size() + GROUP_SIZE_ADDITIONAL_MEMBERS);
+ if (maxMemberSize > 0) {
+ actionProfileGroupBuilder.setMaxSize(maxMemberSize);
+ }
return actionProfileGroupBuilder.build();
}
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeClientImpl.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeClientImpl.java
index a06d67e..821e744 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeClientImpl.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4RuntimeClientImpl.java
@@ -306,8 +306,9 @@
@Override
public CompletableFuture<Boolean> writeActionGroup(PiActionGroup group,
WriteOperationType opType,
- PiPipeconf pipeconf) {
- return supplyInContext(() -> doWriteActionGroup(group, opType, pipeconf),
+ PiPipeconf pipeconf,
+ int maxMemberSize) {
+ return supplyInContext(() -> doWriteActionGroup(group, opType, pipeconf, maxMemberSize),
"writeActionGroup-" + opType.name());
}
@@ -950,10 +951,15 @@
"action profile members");
}
- private boolean doWriteActionGroup(PiActionGroup group, WriteOperationType opType, PiPipeconf pipeconf) {
+ private boolean doWriteActionGroup(PiActionGroup group, WriteOperationType opType, PiPipeconf pipeconf,
+ int maxMemberSize) {
final ActionProfileGroup actionProfileGroup;
+ if (opType == P4RuntimeClient.WriteOperationType.INSERT && maxMemberSize < group.members().size()) {
+ log.warn("Unable to encode group, since group member larger than maximum member size");
+ return false;
+ }
try {
- actionProfileGroup = ActionProfileGroupEncoder.encode(group, pipeconf);
+ actionProfileGroup = ActionProfileGroupEncoder.encode(group, pipeconf, maxMemberSize);
} catch (EncodeException | P4InfoBrowser.NotFoundException e) {
log.warn("Unable to encode group, aborting {} operation: {}", e.getMessage(), opType.name());
return false;
diff --git a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
index 60913ed..36e60ea 100644
--- a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
+++ b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/P4RuntimeGroupTest.java
@@ -161,7 +161,7 @@
@Test
public void testInsertPiActionGroup() throws Exception {
CompletableFuture<Void> complete = p4RuntimeServerImpl.expectRequests(1);
- client.writeActionGroup(GROUP, INSERT, PIPECONF);
+ client.writeActionGroup(GROUP, INSERT, PIPECONF, 3);
complete.get(DEFAULT_TIMEOUT_TIME, TimeUnit.SECONDS);
WriteRequest result = p4RuntimeServerImpl.getWriteReqs().get(0);
assertEquals(1, result.getDeviceId());