Do not insert or delete default action entries in P4Runtime
Spec says:
the default entry for a table is always set. It can be set at
compile-time by the P4 programmer - or defaults to NoAction (which is a
no-op) otherwise - and assuming it is not declared as const, can be
modified by the P4Runtime client. Because the default entry is always
set, we do not allow INSERT and DELETE updates on the default entry and
the P4Runtime server must return an INVALID_ARGUMENT error code if the
client attempts one.
With this patch we convert insert or delete operations into modify ones
(unless specified by a driver property, to support non-compliant devices).
For delete, we use the interpreter to suggest a default action that is
the same as the one when the pipeline was originally deployed.
Also, we introduce the capability of synchronizing the device mirror
with the device state.
Change-Id: I3758fc11780eb0f1cf4ed5a295bd98b54b182e29
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 886a39d..562f0ae 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
@@ -136,15 +136,18 @@
PiPipeconf pipeconf);
/**
- * Dumps all entries currently installed in the given table, for the given
- * pipeconf.
+ * Dumps all entries currently installed in the given tables, for the given
+ * pipeconf. If defaultEntries is set to true only the default action
+ * entries will be returned, otherwise non-default entries will be
+ * considered.
*
- * @param tableId table identifier
+ * @param tableIds table identifiers
+ * @param defaultEntries true to read default entries, false for non-default
* @param pipeconf pipeconf currently deployed on the device
* @return completable future of a collection of table entries
*/
- CompletableFuture<Collection<PiTableEntry>> dumpTable(
- PiTableId tableId, PiPipeconf pipeconf);
+ CompletableFuture<Collection<PiTableEntry>> dumpTables(
+ Set<PiTableId> tableIds, boolean defaultEntries, PiPipeconf pipeconf);
/**
* Dumps entries from all tables, for the given pipeconf.
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 0bd45d3..e020ed5 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
@@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import io.grpc.Context;
@@ -264,13 +265,15 @@
}
@Override
- public CompletableFuture<Collection<PiTableEntry>> dumpTable(PiTableId piTableId, PiPipeconf pipeconf) {
- return supplyInContext(() -> doDumpTable(piTableId, pipeconf), "dumpTable-" + piTableId);
+ public CompletableFuture<Collection<PiTableEntry>> dumpTables(
+ Set<PiTableId> piTableIds, boolean defaultEntries, PiPipeconf pipeconf) {
+ return supplyInContext(() -> doDumpTables(piTableIds, defaultEntries, pipeconf),
+ "dumpTables-" + piTableIds.hashCode());
}
@Override
public CompletableFuture<Collection<PiTableEntry>> dumpAllTables(PiPipeconf pipeconf) {
- return supplyInContext(() -> doDumpTable(null, pipeconf), "dumpAllTables");
+ return supplyInContext(() -> doDumpTables(null, false, pipeconf), "dumpAllTables");
}
@Override
@@ -515,43 +518,53 @@
return write(updateMsgs, piTableEntries, opType, "table entry");
}
- private Collection<PiTableEntry> doDumpTable(PiTableId piTableId, PiPipeconf pipeconf) {
+ private Collection<PiTableEntry> doDumpTables(
+ Set<PiTableId> piTableIds, boolean defaultEntries, PiPipeconf pipeconf) {
- log.debug("Dumping table {} from {} (pipeconf {})...", piTableId, deviceId, pipeconf.id());
+ log.debug("Dumping tables {} from {} (pipeconf {})...",
+ piTableIds, deviceId, pipeconf.id());
- int tableId;
- if (piTableId == null) {
+ Set<Integer> tableIds = Sets.newHashSet();
+ if (piTableIds == null) {
// Dump all tables.
- tableId = 0;
+ tableIds.add(0);
} else {
P4InfoBrowser browser = PipeconfHelper.getP4InfoBrowser(pipeconf);
if (browser == null) {
log.warn("Unable to get a P4Info browser for pipeconf {}", pipeconf);
return Collections.emptyList();
}
- try {
- tableId = browser.tables().getByName(piTableId.id()).getPreamble().getId();
- } catch (P4InfoBrowser.NotFoundException e) {
- log.warn("Unable to dump table: {}", e.getMessage());
- return Collections.emptyList();
- }
+ piTableIds.forEach(piTableId -> {
+ try {
+ tableIds.add(browser.tables().getByName(piTableId.id()).getPreamble().getId());
+ } catch (P4InfoBrowser.NotFoundException e) {
+ log.warn("Unable to dump table {}: {}", piTableId, e.getMessage());
+ }
+ });
}
- ReadRequest requestMsg = ReadRequest.newBuilder()
- .setDeviceId(p4DeviceId)
- .addEntities(Entity.newBuilder()
- .setTableEntry(TableEntry.newBuilder()
- .setTableId(tableId)
- .build())
- .build())
- .build();
+ if (tableIds.isEmpty()) {
+ return Collections.emptyList();
+ }
+
+ ReadRequest.Builder requestMsgBuilder = ReadRequest.newBuilder()
+ .setDeviceId(p4DeviceId);
+ tableIds.forEach(tableId -> requestMsgBuilder.addEntities(
+ Entity.newBuilder()
+ .setTableEntry(
+ TableEntry.newBuilder()
+ .setTableId(tableId)
+ .setIsDefaultAction(defaultEntries)
+ .build())
+ .build())
+ .build());
Iterator<ReadResponse> responses;
try {
- responses = blockingStub.read(requestMsg);
+ responses = blockingStub.read(requestMsgBuilder.build());
} catch (StatusRuntimeException e) {
checkGrpcException(e);
- log.warn("Unable to dump table {} from {}: {}", piTableId, deviceId, e.getMessage());
+ log.warn("Unable to dump tables from {}: {}", deviceId, e.getMessage());
return Collections.emptyList();
}
@@ -564,7 +577,8 @@
.map(Entity::getTableEntry)
.collect(Collectors.toList());
- log.debug("Retrieved {} entries from table {} on {}...", tableEntryMsgs.size(), piTableId, deviceId);
+ log.debug("Retrieved {} entries from {} tables on {}...",
+ tableEntryMsgs.size(), tableIds.size(), deviceId);
return TableEntryEncoder.decode(tableEntryMsgs, pipeconf);
}
@@ -1034,8 +1048,8 @@
if (errors.isEmpty()) {
final String description = ex.getStatus().getDescription();
log.warn("Unable to {} {} {}(s) on {}: {}",
- opType.name(), writeEntities.size(), entryType, deviceId,
- ex.getStatus().getCode().name(),
+ opType.name(), writeEntities.size(), entryType, deviceId,
+ ex.getStatus().getCode().name(),
description == null ? "" : " - " + description);
return;
}
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/TableEntryEncoder.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/TableEntryEncoder.java
index 505b4cb..53fcf2a 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/TableEntryEncoder.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/TableEntryEncoder.java
@@ -263,7 +263,9 @@
piTableEntryBuilder.forTable(PiTableId.of(tableInfo.getPreamble().getName()));
// Priority.
- piTableEntryBuilder.withPriority(tableEntryMsg.getPriority());
+ if (tableEntryMsg.getPriority() > 0) {
+ piTableEntryBuilder.withPriority(tableEntryMsg.getPriority());
+ }
// Controller metadata (cookie)
piTableEntryBuilder.withCookie(tableEntryMsg.getControllerMetadata());
diff --git a/protocols/p4runtime/model/src/main/java/org/onosproject/p4runtime/model/P4TableModel.java b/protocols/p4runtime/model/src/main/java/org/onosproject/p4runtime/model/P4TableModel.java
index 77f55ec..5afad85 100644
--- a/protocols/p4runtime/model/src/main/java/org/onosproject/p4runtime/model/P4TableModel.java
+++ b/protocols/p4runtime/model/src/main/java/org/onosproject/p4runtime/model/P4TableModel.java
@@ -48,7 +48,7 @@
private final boolean supportAging;
private final ImmutableMap<PiMatchFieldId, PiMatchFieldModel> matchFields;
private final ImmutableMap<PiActionId, PiActionModel> actions;
- private final PiActionModel defaultAction;
+ private final PiActionModel constDefaultAction;
private final boolean hasDefaultMutableParams;
private final boolean isConstTable;
@@ -58,7 +58,7 @@
ImmutableMap<PiMeterId, PiMeterModel> meters, boolean supportAging,
ImmutableMap<PiMatchFieldId, PiMatchFieldModel> matchFields,
ImmutableMap<PiActionId, PiActionModel> actions,
- PiActionModel defaultAction, boolean hasDefaultMutableParams,
+ PiActionModel constDefaultAction, boolean hasDefaultMutableParams,
boolean isConstTable) {
this.id = id;
this.tableType = tableType;
@@ -69,7 +69,7 @@
this.supportAging = supportAging;
this.matchFields = matchFields;
this.actions = actions;
- this.defaultAction = defaultAction;
+ this.constDefaultAction = constDefaultAction;
this.hasDefaultMutableParams = hasDefaultMutableParams;
this.isConstTable = isConstTable;
}
@@ -120,8 +120,8 @@
}
@Override
- public Optional<PiActionModel> defaultAction() {
- return Optional.ofNullable(defaultAction);
+ public Optional<PiActionModel> constDefaultAction() {
+ return Optional.ofNullable(constDefaultAction);
}
@Override
@@ -148,7 +148,7 @@
public int hashCode() {
return Objects.hash(id, tableType, actionProfile, maxSize, counters,
meters, supportAging, matchFields, actions,
- defaultAction, hasDefaultMutableParams);
+ constDefaultAction, hasDefaultMutableParams);
}
@Override
@@ -169,7 +169,7 @@
&& Objects.equals(this.supportAging, other.supportAging)
&& Objects.equals(this.matchFields, other.matchFields)
&& Objects.equals(this.actions, other.actions)
- && Objects.equals(this.defaultAction, other.defaultAction)
+ && Objects.equals(this.constDefaultAction, other.constDefaultAction)
&& Objects.equals(this.hasDefaultMutableParams, other.hasDefaultMutableParams);
}
}
diff --git a/protocols/p4runtime/model/src/test/java/org/onosproject/p4runtime/model/P4InfoParserTest.java b/protocols/p4runtime/model/src/test/java/org/onosproject/p4runtime/model/P4InfoParserTest.java
index e23a49c..3a803d4 100644
--- a/protocols/p4runtime/model/src/test/java/org/onosproject/p4runtime/model/P4InfoParserTest.java
+++ b/protocols/p4runtime/model/src/test/java/org/onosproject/p4runtime/model/P4InfoParserTest.java
@@ -193,7 +193,7 @@
wcmpTableModel.actions(), IsIterableContainingInAnyOrder.containsInAnyOrder(
setEgressPortAction, noAction));
- PiActionModel table0DefaultAction = table0Model.defaultAction().orElse(null);
+ PiActionModel table0DefaultAction = table0Model.constDefaultAction().orElse(null);
new EqualsTester().addEqualityGroup(table0DefaultAction, dropAction).testEquals();