Merge branch 'master' into dev/murrelet
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterId.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterId.java
index dcc7c02..015d86b 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterId.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterId.java
@@ -18,6 +18,7 @@
import com.google.common.annotations.Beta;
import com.google.common.base.Objects;
+import org.onlab.util.Identifier;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -26,12 +27,15 @@
* Identifier of a counter of a protocol-independent pipeline.
*/
@Beta
-public final class PiCounterId {
+public final class PiCounterId extends Identifier<String> {
+ private final String scope;
private final String name;
private final PiCounterType type;
- private PiCounterId(String name, PiCounterType type) {
+ private PiCounterId(String scope, String name, PiCounterType type) {
+ super((!scope.isEmpty() ? scope + "." : "") + name);
+ this.scope = scope;
this.name = name;
this.type = type;
}
@@ -47,7 +51,33 @@
checkNotNull(name);
checkNotNull(type);
checkArgument(!name.isEmpty(), "Name can't be empty");
- return new PiCounterId(name, type);
+ return new PiCounterId("", name, type);
+ }
+
+ /**
+ * Returns a counter identifier for the given scope, name, and type.
+ *
+ * @param scope counter scope
+ * @param name counter name
+ * @param type counter type
+ * @return counter identifier
+ */
+ public static PiCounterId of(String scope, String name, PiCounterType type) {
+ checkNotNull(scope);
+ checkNotNull(name);
+ checkNotNull(type);
+ checkArgument(!scope.isEmpty(), "Scope can't be empty");
+ checkArgument(!name.isEmpty(), "Name can't be empty");
+ return new PiCounterId(scope, name, type);
+ }
+
+ /**
+ * Returns the scope of the counter.
+ *
+ * @return counter scope
+ */
+ public String scope() {
+ return this.scope;
}
/**
@@ -77,17 +107,12 @@
return false;
}
PiCounterId that = (PiCounterId) o;
- return Objects.equal(name, that.name) &&
+ return Objects.equal(id(), that.id()) &&
type == that.type;
}
@Override
public int hashCode() {
- return Objects.hashCode(name, type);
- }
-
- @Override
- public String toString() {
- return type.name().toLowerCase() + ":" + name;
+ return Objects.hashCode(id(), type);
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHeaderFieldId.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHeaderFieldId.java
index 6af08ef..99f7e75 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHeaderFieldId.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiHeaderFieldId.java
@@ -26,6 +26,10 @@
*/
public final class PiHeaderFieldId extends Identifier<String> {
+ // FIXME: this abstraction is brittle and we should drop any string-composition logic.
+ // e.g. in P4_14 there is no scope for match fields.
+ // In light of ONOS-7066, the best solution seems to have IDs defined as arbitrary
+ // strings equal to the entity names defined in the P4Info.
private final String headerName;
private final String fieldName;
private final int index;
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/DefaultP4PortStatisticsDiscovery.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/DefaultP4PortStatisticsDiscovery.java
index b127855..081e0f1 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/DefaultP4PortStatisticsDiscovery.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/DefaultP4PortStatisticsDiscovery.java
@@ -42,8 +42,21 @@
public class DefaultP4PortStatisticsDiscovery extends AbstractP4RuntimeHandlerBehaviour
implements PortStatisticsDiscovery {
- private static final PiCounterId INGRESS_COUNTER_ID = PiCounterId.of("ingress_port_counter", INDIRECT);
- private static final PiCounterId EGRESS_COUNTER_ID = PiCounterId.of("egress_port_counter", INDIRECT);
+ // FIXME: hard-coding the scope here will break support for the P4_14 version of the program.
+ // With P4_14, counter names in the generated P4Info won't have any scope.
+ // A solution could be that of dynamically building counter IDs based on the P4Info (as in DefaultP4Interpreter).
+ private static final String DEFAULT_SCOPE = "port_counters_control";
+ private static final String INGRESS_COUNTER = "ingress_port_counter";
+ private static final String EGRESS_COUNTER = "egress_port_counter";
+
+ /**
+ * Returns the scope string to be used for the counter IDs.
+ *
+ * @return scope string
+ */
+ public String scope() {
+ return DEFAULT_SCOPE;
+ }
@Override
public Collection<PortStatistics> discoverPortStatistics() {
@@ -52,6 +65,9 @@
return Collections.emptyList();
}
+ final PiCounterId ingressCounterId = PiCounterId.of(scope(), INGRESS_COUNTER, INDIRECT);
+ final PiCounterId egressCounterId = PiCounterId.of(scope(), EGRESS_COUNTER, INDIRECT);
+
Map<Long, DefaultPortStatistics.Builder> portStatBuilders = Maps.newHashMap();
deviceService.getPorts(deviceId)
@@ -63,8 +79,8 @@
Set<PiCounterCellId> counterCellIds = Sets.newHashSet();
portStatBuilders.keySet().forEach(p -> {
// Counter cell/index = port number.
- counterCellIds.add(PiIndirectCounterCellId.of(INGRESS_COUNTER_ID, p));
- counterCellIds.add(PiIndirectCounterCellId.of(EGRESS_COUNTER_ID, p));
+ counterCellIds.add(PiIndirectCounterCellId.of(ingressCounterId, p));
+ counterCellIds.add(PiIndirectCounterCellId.of(egressCounterId, p));
});
Collection<PiCounterCellData> counterEntryResponse;
@@ -87,10 +103,10 @@
return;
}
DefaultPortStatistics.Builder statsBuilder = portStatBuilders.get(indCellId.index());
- if (counterData.cellId().counterId().equals(INGRESS_COUNTER_ID)) {
+ if (counterData.cellId().counterId().equals(ingressCounterId)) {
statsBuilder.setPacketsReceived(counterData.packets());
statsBuilder.setBytesReceived(counterData.bytes());
- } else if (counterData.cellId().counterId().equals(EGRESS_COUNTER_ID)) {
+ } else if (counterData.cellId().counterId().equals(egressCounterId)) {
statsBuilder.setPacketsSent(counterData.packets());
statsBuilder.setBytesSent(counterData.bytes());
} else {
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 6cd2a97..e05f62d 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
@@ -406,7 +406,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();
@@ -575,7 +575,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));