ONOS-7078 Fixed inconsistencies when encoding/decoding P4Runtime msgs
Now P4InfoBrowser permits looking up entities by name only, not alias.
Applications should use names as defined in the P4Info when
creating PI IDs (e.g. PiCounterId). However, to avoid breaking support
with BMv2-based pipeline models, when referring to header fields in
tables, i.e. match fields, application should drop any scope identifier
from field names, e.g. "hdr.ethernet.src_addr" should be referred by
applications as "ethernet.src_addr". Such inconsistency will be fixed
with ONOS-7066.
Change-Id: I4d6dceadd233a293b845dba84e62a49680ac930b
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 c3783c1..eb43f4c 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
@@ -58,7 +58,7 @@
PiActionProfileId piActionProfileId = piActionGroup.actionProfileId();
P4InfoOuterClass.ActionProfile actionProfile = browser.actionProfiles()
- .getByNameOrAlias(piActionProfileId.id());
+ .getByName(piActionProfileId.id());
int actionProfileId = actionProfile.getPreamble().getId();
ActionProfileGroup.Builder actionProfileGroupBuilder = ActionProfileGroup.newBuilder()
.setGroupId(piActionGroup.id().id())
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileMemberEncoder.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileMemberEncoder.java
index 878ede6..2f08a59 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileMemberEncoder.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/ActionProfileMemberEncoder.java
@@ -73,7 +73,7 @@
// action profile id
P4InfoOuterClass.ActionProfile actionProfile =
- browser.actionProfiles().getByNameOrAlias(group.actionProfileId().id());
+ browser.actionProfiles().getByName(group.actionProfileId().id());
int actionProfileId = actionProfile.getPreamble().getId();
actionProfileMemberBuilder.setActionProfileId(actionProfileId);
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/CounterEntryCodec.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/CounterEntryCodec.java
index 025a5a3..c22d612 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/CounterEntryCodec.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/CounterEntryCodec.java
@@ -122,7 +122,7 @@
// Encode PI cell ID into entity message and add to read request.
switch (cellId.type()) {
case INDIRECT:
- counterId = browser.counters().getByNameOrAlias(cellId.counterId().name()).getPreamble().getId();
+ counterId = browser.counters().getByName(cellId.counterId().id()).getPreamble().getId();
PiIndirectCounterCellId indCellId = (PiIndirectCounterCellId) cellId;
entity = Entity.newBuilder().setCounterEntry(CounterEntry.newBuilder()
.setCounterId(counterId)
@@ -131,7 +131,7 @@
.build();
break;
case DIRECT:
- counterId = browser.directCounters().getByNameOrAlias(cellId.counterId().name()).getPreamble().getId();
+ counterId = browser.directCounters().getByName(cellId.counterId().id()).getPreamble().getId();
PiDirectCounterCellId dirCellId = (PiDirectCounterCellId) cellId;
DirectCounterEntry.Builder entryBuilder = DirectCounterEntry.newBuilder().setCounterId(counterId);
if (!dirCellId.tableEntry().equals(PiTableEntry.EMTPY)) {
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4InfoBrowser.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4InfoBrowser.java
index 177c381..48b1476 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4InfoBrowser.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/P4InfoBrowser.java
@@ -75,8 +75,12 @@
EntityBrowser<MatchField> matchFieldBrowser = new EntityBrowser<>(format(
"match field for table '%s'", tableName));
entity.getMatchFieldsList().forEach(m -> {
- String alias = extractMatchFieldSimpleName(m.getName());
- matchFieldBrowser.add(m.getName(), alias, m.getId(), m);
+ // FIXME: nasty hack needed to provide compatibility with BMv2-based pipeline models.
+ // Indeed in the BMv2 JSON header fields have format like "ethernet.srd_addr", while in P4Info
+ // the same will be "hdr.ethernet.srd_addr".
+ // To be removed when ONOS-7066 will be implemented.
+ String simpleName = extractMatchFieldSimpleName(m.getName());
+ matchFieldBrowser.add(simpleName, null, m.getId(), m);
});
matchFields.put(tableId, matchFieldBrowser);
});
@@ -121,7 +125,7 @@
});
}
- private String extractMatchFieldSimpleName(String name) {
+ static String extractMatchFieldSimpleName(String name) {
// Removes the leading "hdr." or other scope identifier.
// E.g.: "hdr.ethernet.etherType" becomes "ethernet.etherType"
String[] pieces = name.split("\\.");
@@ -255,7 +259,7 @@
private String entityName;
private final Map<String, T> names = Maps.newHashMap();
- private final Map<String, T> aliases = Maps.newHashMap();
+ private final Map<String, String> aliasToNames = Maps.newHashMap();
private final Map<Integer, T> ids = Maps.newHashMap();
private EntityBrowser(String entityName) {
@@ -277,7 +281,7 @@
names.put(name, entity);
ids.put(id, entity);
if (alias != null && !alias.isEmpty()) {
- aliases.put(alias, entity);
+ aliasToNames.put(alias, name);
}
}
@@ -303,33 +307,25 @@
}
/**
- * Returns the entity identified by the given name or alias, if present, otherwise, throws an exception.
+ * Returns the entity identified by the given name, if present, otherwise, throws an exception.
*
* @param name entity name or alias
* @return entity message
* @throws NotFoundException if the entity cannot be found
*/
- T getByNameOrAlias(String name) throws NotFoundException {
+ T getByName(String name) throws NotFoundException {
if (hasName(name)) {
return names.get(name);
- } else if (hasAlias(name)) {
- return aliases.get(name);
} else {
- throw new NotFoundException(entityName, name);
+ final String hint = aliasToNames.containsKey(name)
+ ? format("Did you mean '%s'? Make sure to use entity names in PI IDs, not aliases",
+ aliasToNames.get(name))
+ : "";
+ throw new NotFoundException(entityName, name, hint);
}
}
/**
- * Returns true if the P4Info defines an entity with such alias, false otherwise.
- *
- * @param alias entity alias
- * @return boolean
- */
- boolean hasAlias(String alias) {
- return aliases.containsKey(alias);
- }
-
- /**
* Returns true if the P4Info defines an entity with such id, false otherwise.
*
* @param id entity id
@@ -359,8 +355,9 @@
*/
public static final class NotFoundException extends Exception {
- NotFoundException(String entityName, String key) {
- super(format("No such %s in P4Info with %name or alias '%s'", entityName, key));
+ NotFoundException(String entityName, String key, String hint) {
+ super(format(
+ "No such %s in P4Info with name '%s'%s", entityName, key, hint.isEmpty() ? "" : " (" + hint + ")"));
}
NotFoundException(String entityName, int id) {
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 a579e36..3dd5fcd 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
@@ -389,7 +389,7 @@
P4InfoBrowser browser = PipeconfHelper.getP4InfoBrowser(pipeconf);
int tableId;
try {
- tableId = browser.tables().getByNameOrAlias(piTableId.id()).getPreamble().getId();
+ tableId = browser.tables().getByName(piTableId.id()).getPreamble().getId();
} catch (P4InfoBrowser.NotFoundException e) {
log.warn("Unable to dump table: {}", e.getMessage());
return Collections.emptyList();
@@ -558,7 +558,7 @@
try {
actionProfileId = browser
.actionProfiles()
- .getByNameOrAlias(piActionProfileId.id())
+ .getByName(piActionProfileId.id())
.getPreamble()
.getId();
} catch (P4InfoBrowser.NotFoundException e) {
diff --git a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/PacketIOCodec.java b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/PacketIOCodec.java
index 2ffe31b..03cdb1e 100644
--- a/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/PacketIOCodec.java
+++ b/protocols/p4runtime/ctl/src/main/java/org/onosproject/p4runtime/ctl/PacketIOCodec.java
@@ -73,7 +73,7 @@
//Get the packet out controller packet metadata
P4InfoOuterClass.ControllerPacketMetadata controllerPacketMetadata =
- browser.controllerPacketMetadatas().getByNameOrAlias(PACKET_OUT);
+ browser.controllerPacketMetadatas().getByName(PACKET_OUT);
PacketOut.Builder packetOutBuilder = PacketOut.newBuilder();
//outer controller packet metadata id
@@ -94,7 +94,7 @@
try {
//get each metadata id
int metadataId = browser.packetMetadatas(controllerPacketMetadataId)
- .getByNameOrAlias(metadata.id().name()).getId();
+ .getByName(metadata.id().name()).getId();
//Add the metadata id and it's data the packet out
return PacketMetadata.newBuilder()
@@ -127,7 +127,7 @@
List<PiPacketMetadata> packetMetadatas;
try {
- int controllerPacketMetadataId = browser.controllerPacketMetadatas().getByNameOrAlias(PACKET_IN)
+ int controllerPacketMetadataId = browser.controllerPacketMetadatas().getByName(PACKET_IN)
.getPreamble().getId();
packetMetadatas = decodePacketMetadataIn(packetIn.getMetadataList(), browser,
controllerPacketMetadataId);
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 3890848..4c192da 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
@@ -61,7 +61,6 @@
final class TableEntryEncoder {
private static final Logger log = getLogger(TableEntryEncoder.class);
- private static final String HEADER_PREFIX = "hdr.";
private static final String VALUE_OF_PREFIX = "value of ";
private static final String MASK_OF_PREFIX = "mask of ";
private static final String HIGH_RANGE_VALUE_OF_PREFIX = "high range value of ";
@@ -196,8 +195,7 @@
P4InfoBrowser browser = PipeconfHelper.getP4InfoBrowser(pipeconf);
TableEntry.Builder tableEntryMsgBuilder = TableEntry.newBuilder();
- //FIXME this throws some kind of NPE
- P4InfoOuterClass.Table tableInfo = browser.tables().getByNameOrAlias(tableId.id());
+ P4InfoOuterClass.Table tableInfo = browser.tables().getByName(tableId.id());
// Table id.
tableEntryMsgBuilder.setTableId(tableInfo.getPreamble().getId());
@@ -215,8 +213,7 @@
TableEntry.Builder tableEntryMsgBuilder = TableEntry.newBuilder();
- //FIXME this throws some kind of NPE
- P4InfoOuterClass.Table tableInfo = browser.tables().getByNameOrAlias(piTableEntry.table().id());
+ P4InfoOuterClass.Table tableInfo = browser.tables().getByName(piTableEntry.table().id());
// Table id.
tableEntryMsgBuilder.setTableId(tableInfo.getPreamble().getId());
@@ -282,7 +279,7 @@
// FIXME: check how field names for stacked headers are constructed in P4Runtime.
String fieldName = piFieldMatch.fieldId().id();
int tableId = tableInfo.getPreamble().getId();
- P4InfoOuterClass.MatchField matchFieldInfo = browser.matchFields(tableId).getByNameOrAlias(fieldName);
+ P4InfoOuterClass.MatchField matchFieldInfo = browser.matchFields(tableId).getByName(fieldName);
String entityName = format("field match '%s' of table '%s'",
matchFieldInfo.getName(), tableInfo.getPreamble().getName());
int fieldId = matchFieldInfo.getId();
@@ -384,9 +381,12 @@
int tableId = tableInfo.getPreamble().getId();
String fieldMatchName = browser.matchFields(tableId).getById(fieldMatchMsg.getFieldId()).getName();
- if (fieldMatchName.startsWith(HEADER_PREFIX)) {
- fieldMatchName = fieldMatchName.substring(HEADER_PREFIX.length());
- }
+
+ // FIXME: nasty hack needed to provide compatibility with BMv2-based pipeline models.
+ // Indeed in the BMv2 JSON header fields have format like "ethernet.srd_addr", while in P4Info
+ // the same will be "hdr.ethernet.srd_addr".
+ // To be removed when ONOS-7066 will be implemented.
+ fieldMatchName = P4InfoBrowser.extractMatchFieldSimpleName(fieldMatchName);
// FIXME: Add support for decoding of stacked header names.
String[] pieces = fieldMatchName.split("\\.");
@@ -469,13 +469,13 @@
static Action encodePiAction(PiAction piAction, P4InfoBrowser browser)
throws P4InfoBrowser.NotFoundException, EncodeException {
- int actionId = browser.actions().getByNameOrAlias(piAction.id().name()).getPreamble().getId();
+ int actionId = browser.actions().getByName(piAction.id().name()).getPreamble().getId();
Action.Builder actionMsgBuilder =
Action.newBuilder().setActionId(actionId);
for (PiActionParam p : piAction.parameters()) {
- P4InfoOuterClass.Action.Param paramInfo = browser.actionParams(actionId).getByNameOrAlias(p.id().name());
+ P4InfoOuterClass.Action.Param paramInfo = browser.actionParams(actionId).getByName(p.id().name());
ByteString paramValue = ByteString.copyFrom(p.value().asReadOnlyBuffer());
assertSize(format("param '%s' of action '%s'", p.id(), piAction.id()),
paramValue, paramInfo.getBitwidth());
diff --git a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/TableEntryEncoderTest.java b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/TableEntryEncoderTest.java
index b434b4d..80127ff 100644
--- a/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/TableEntryEncoderTest.java
+++ b/protocols/p4runtime/ctl/src/test/java/org/onosproject/p4runtime/ctl/TableEntryEncoderTest.java
@@ -112,8 +112,8 @@
assertThat(browser.tables().hasName(TABLE_0), is(true));
assertThat(browser.actions().hasName(SET_EGRESS_PORT), is(true));
- int tableId = browser.tables().getByNameOrAlias(TABLE_0).getPreamble().getId();
- int actionId = browser.actions().getByNameOrAlias(SET_EGRESS_PORT).getPreamble().getId();
+ int tableId = browser.tables().getByName(TABLE_0).getPreamble().getId();
+ int actionId = browser.actions().getByName(SET_EGRESS_PORT).getPreamble().getId();
assertThat(browser.matchFields(tableId).hasName(STANDARD_METADATA + "." + INGRESS_PORT), is(true));
assertThat(browser.actionParams(actionId).hasName(PORT), is(true));
@@ -139,7 +139,7 @@
.testEquals();
// Table ID.
- int p4InfoTableId = browser.tables().getByNameOrAlias(tableId.id()).getPreamble().getId();
+ int p4InfoTableId = browser.tables().getByName(tableId.id()).getPreamble().getId();
int encodedTableId = tableEntryMsg.getTableId();
assertThat(encodedTableId, is(p4InfoTableId));
@@ -150,12 +150,12 @@
Action actionMsg = tableEntryMsg.getAction().getAction();
// Action ID.
- int p4InfoActionId = browser.actions().getByNameOrAlias(outActionId.name()).getPreamble().getId();
+ int p4InfoActionId = browser.actions().getByName(outActionId.name()).getPreamble().getId();
int encodedActionId = actionMsg.getActionId();
assertThat(encodedActionId, is(p4InfoActionId));
// Action param ID.
- int p4InfoActionParamId = browser.actionParams(p4InfoActionId).getByNameOrAlias(portParamId.name()).getId();
+ int p4InfoActionParamId = browser.actionParams(p4InfoActionId).getByName(portParamId.name()).getId();
int encodedActionParamId = actionMsg.getParams(0).getParamId();
assertThat(encodedActionParamId, is(p4InfoActionParamId));