Bumped supported commit of P4Runtime and BMv2
Includes fixes for:
- ONOS-7593: Support for indirect resource Index type
- ONOS-7595: Removed ID from direct resources
- P4Runtime requires unset bits to be 0 in ternary field matches
- Incorrect parsing of flow rule byte counters
- Full entity names in P4Info with top-level control block (fixed only
for basic.p4, other programs need to be re-compiled and PI IDs in
respective pipeconf changed)
Change-Id: Ia19aa949c02e363a550e692915c6d6516a2d13d7
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterCellId.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterCellId.java
index da8a882..4b9a70e 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterCellId.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiCounterCellId.java
@@ -35,8 +35,8 @@
private final long index;
private final PiTableEntry tableEntry;
- private PiCounterCellId(PiCounterId counterId, PiCounterType counterType, long index,
- PiTableEntry tableEntry) {
+ private PiCounterCellId(PiCounterId counterId, PiCounterType counterType,
+ long index, PiTableEntry tableEntry) {
this.counterId = counterId;
this.counterType = counterType;
this.index = index;
@@ -44,7 +44,9 @@
}
/**
- * Returns the identifier of the counter instance where this cell is contained.
+ * Returns the identifier of the counter instance where this cell is
+ * contained. Meaningful only if the counter is of type {@link
+ * PiCounterType#INDIRECT}.
*
* @return counter identifier
*/
@@ -62,8 +64,8 @@
}
/**
- * Returns the counter index to which this cell ID is associated. Meaningful only if the counter is of type {@link
- * PiCounterType#INDIRECT}.
+ * Returns the counter index to which this cell ID is associated. Meaningful
+ * only if the counter is of type {@link PiCounterType#INDIRECT}.
*
* @return counter index
*/
@@ -72,8 +74,9 @@
}
/**
- * Returns the table entry to which this cell ID is associated. Meaningful only if the counter is of type {@link
- * PiCounterType#DIRECT}, otherwise returns null.
+ * Returns the table entry to which this cell ID is associated. Meaningful
+ * only if the counter is of type {@link PiCounterType#DIRECT}, otherwise
+ * returns null.
*
* @return PI table entry or null
*/
@@ -82,16 +85,15 @@
}
/**
- * Return a direct counter cell ID for the given counter ID and table entry.
+ * Return a direct counter cell ID for the given counter ID and table
+ * entry.
*
- * @param counterId counter ID
* @param tableEntry table entry
* @return counter cell ID
*/
- public static PiCounterCellId ofDirect(PiCounterId counterId, PiTableEntry tableEntry) {
- checkNotNull(counterId);
+ public static PiCounterCellId ofDirect(PiTableEntry tableEntry) {
checkNotNull(tableEntry);
- return new PiCounterCellId(counterId, PiCounterType.DIRECT, -1, tableEntry);
+ return new PiCounterCellId(null, PiCounterType.DIRECT, -1, tableEntry);
}
/**
@@ -129,7 +131,8 @@
@Override
public String toString() {
- return counterId.toString() + ':'
- + (counterType == PiCounterType.DIRECT ? tableEntry.toString() : String.valueOf(index));
+ return counterType == PiCounterType.DIRECT
+ ? tableEntry.toString()
+ : counterId.toString() + ':' + String.valueOf(index);
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellId.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellId.java
index 09cf7bf..34af8bf 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellId.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiMeterCellId.java
@@ -46,6 +46,7 @@
/**
* Returns the identifier of the meter instance where this cell is contained.
+ * Meaningful only if the meter is of type {@link PiMeterType#DIRECT}, otherwise returns null.
*
* @return meter identifier
*/
@@ -90,14 +91,12 @@
/**
* Return a direct meter cell ID for the given meter ID and table entry.
*
- * @param meterId meter ID
* @param tableEntry table entry
* @return meter cell ID
*/
- public static PiMeterCellId ofDirect(PiMeterId meterId, PiTableEntry tableEntry) {
- checkNotNull(meterId);
+ public static PiMeterCellId ofDirect(PiTableEntry tableEntry) {
checkNotNull(tableEntry);
- return new PiMeterCellId(meterId, PiMeterType.DIRECT, -1, tableEntry);
+ return new PiMeterCellId(null, PiMeterType.DIRECT, -1, tableEntry);
}
/**
@@ -135,7 +134,8 @@
@Override
public String toString() {
- return meterId.toString() + ':'
- + (meterType == PiMeterType.DIRECT ? tableEntry.toString() : String.valueOf(index));
+ return meterType == PiMeterType.DIRECT
+ ? tableEntry.toString()
+ : meterId.toString() + ':' + String.valueOf(index);
}
}
diff --git a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
index 18b31f2..7b8797a 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/runtime/PiTableEntry.java
@@ -32,8 +32,6 @@
@Beta
public final class PiTableEntry implements PiEntity {
- public static final PiTableEntry EMTPY = new PiTableEntry();
-
private static final int NO_PRIORITY = -1;
private static final double NO_TIMEOUT = -1;
@@ -44,15 +42,6 @@
private final int priority;
private final double timeout;
- private PiTableEntry() {
- this.tableId = null;
- this.matchKey = null;
- this.tableAction = null;
- this.cookie = 0;
- this.priority = NO_PRIORITY;
- this.timeout = NO_TIMEOUT;
- }
-
private PiTableEntry(PiTableId tableId, PiMatchKey matchKey,
PiTableAction tableAction, long cookie, int priority, double timeout) {
this.tableId = tableId;
diff --git a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiCounterCellDataTest.java b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiCounterCellDataTest.java
index 4104050..6676470 100644
--- a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiCounterCellDataTest.java
+++ b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiCounterCellDataTest.java
@@ -19,7 +19,6 @@
import com.google.common.testing.EqualsTester;
import org.junit.Test;
import org.onosproject.net.pi.model.PiActionId;
-import org.onosproject.net.pi.model.PiCounterId;
import org.onosproject.net.pi.model.PiTableId;
import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable;
@@ -30,9 +29,6 @@
*/
public class PiCounterCellDataTest {
- private static final PiCounterId PI_COUNTER_ID_1 = PiCounterId.of("Name1");
- private static final PiCounterId PI_COUNTER_ID_2 = PiCounterId.of("Name2");
-
private static final PiTableEntry PI_TABLE_ENTRY_1 = PiTableEntry.builder()
.forTable(PiTableId.of("T10"))
.withCookie(0xac)
@@ -49,9 +45,9 @@
.build();
private static final PiCounterCellId PI_COUNTER_CELL_ID_1 =
- PiCounterCellId.ofDirect(PI_COUNTER_ID_1, PI_TABLE_ENTRY_1);
+ PiCounterCellId.ofDirect(PI_TABLE_ENTRY_1);
private static final PiCounterCellId PI_COUNTER_CELL_ID_2 =
- PiCounterCellId.ofDirect(PI_COUNTER_ID_2, PI_TABLE_ENTRY_2);
+ PiCounterCellId.ofDirect(PI_TABLE_ENTRY_2);
private static final long PACKETS_1 = 10;
private static final long PACKETS_2 = 20;
@@ -83,4 +79,4 @@
.addEqualityGroup(PI_COUNTER_CELL_DATA_2)
.testEquals();
}
-}
\ No newline at end of file
+}
diff --git a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiTableEntryTest.java b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiTableEntryTest.java
index 0b451e8..087eb77 100644
--- a/core/api/src/test/java/org/onosproject/net/pi/runtime/PiTableEntryTest.java
+++ b/core/api/src/test/java/org/onosproject/net/pi/runtime/PiTableEntryTest.java
@@ -81,16 +81,6 @@
}
/**
- * Tests equality of the empty table entry.
- */
- @Test
- public void testEmptyEquals() {
- new EqualsTester()
- .addEqualityGroup(PiTableEntry.EMTPY, PiTableEntry.EMTPY)
- .testEquals();
- }
-
- /**
* Tests creation of a DefaultFlowRule using a FlowRule constructor.
*/
@Test
diff --git a/core/net/src/main/java/org/onosproject/net/pi/impl/PiFlowRuleTranslatorImpl.java b/core/net/src/main/java/org/onosproject/net/pi/impl/PiFlowRuleTranslatorImpl.java
index 85c763c..f22fdde 100644
--- a/core/net/src/main/java/org/onosproject/net/pi/impl/PiFlowRuleTranslatorImpl.java
+++ b/core/net/src/main/java/org/onosproject/net/pi/impl/PiFlowRuleTranslatorImpl.java
@@ -59,6 +59,7 @@
import static java.lang.String.format;
import static org.onlab.util.ImmutableByteSequence.ByteSequenceTrimException;
+import static org.onlab.util.ImmutableByteSequence.prefixOnes;
import static org.onosproject.net.flow.criteria.Criterion.Type.PROTOCOL_INDEPENDENT;
import static org.onosproject.net.pi.impl.CriterionTranslatorHelper.translateCriterion;
import static org.onosproject.net.pi.impl.PiUtils.getInterpreterOrNull;
@@ -128,7 +129,17 @@
}
if (needPriority) {
- tableEntryBuilder.withPriority(rule.priority());
+ // In the P4 world 0 is the highest priority, in ONOS the lowest one.
+ // FIXME: move priority conversion to the P4Runtime driver
+ final int newPriority;
+ if (rule.priority() > MAX_PI_PRIORITY) {
+ log.warn("Flow rule priority too big, setting translated priority to max value {}: {}",
+ MAX_PI_PRIORITY, rule);
+ newPriority = 0;
+ } else {
+ newPriority = MAX_PI_PRIORITY - rule.priority();
+ }
+ tableEntryBuilder.withPriority(newPriority);
}
if (!rule.isPermanent()) {
@@ -290,7 +301,6 @@
PiMatchFieldId fieldId = fieldModel.id();
int bitWidth = fieldModel.bitWidth();
- int fieldByteWidth = (int) Math.ceil((double) bitWidth / 8);
Optional<Criterion.Type> criterionType =
interpreter == null
@@ -304,17 +314,8 @@
// Can ignore if the match is ternary or LPM.
switch (fieldModel.matchType()) {
case TERNARY:
- // Wildcard the whole field.
- fieldMatches.put(fieldId, new PiTernaryFieldMatch(
- fieldId,
- ImmutableByteSequence.ofZeros(fieldByteWidth),
- ImmutableByteSequence.ofZeros(fieldByteWidth)));
- break;
case LPM:
- // LPM with prefix 0
- fieldMatches.put(fieldId, new PiLpmFieldMatch(fieldId,
- ImmutableByteSequence.ofZeros(fieldByteWidth),
- 0));
+ // Skip field.
break;
// FIXME: Can we handle the case of RANGE or VALID match?
default:
@@ -404,7 +405,9 @@
/*
Here we try to be robust against wrong size fields with the goal of having PiCriterion independent of the
pipeline model. We duplicate the field match, fitting the byte sequences to the bit-width specified in the
- model. This operation is expensive when performed for each field match of each flow rule, but should be
+ model. We also normalize ternary (and LPM) field matches by setting to 0 unused bits, as required by P4Runtime.
+
+ These operations are expensive when performed for each field match of each flow rule, but should be
mitigated by the translation cache provided by PiFlowRuleTranslationServiceImpl.
*/
@@ -414,9 +417,12 @@
return new PiExactFieldMatch(fieldMatch.fieldId(),
((PiExactFieldMatch) fieldMatch).value().fit(modelBitWidth));
case TERNARY:
- return new PiTernaryFieldMatch(fieldMatch.fieldId(),
- ((PiTernaryFieldMatch) fieldMatch).value().fit(modelBitWidth),
- ((PiTernaryFieldMatch) fieldMatch).mask().fit(modelBitWidth));
+ PiTernaryFieldMatch ternField = (PiTernaryFieldMatch) fieldMatch;
+ ImmutableByteSequence ternMask = ternField.mask().fit(modelBitWidth);
+ ImmutableByteSequence ternValue = ternField.value()
+ .fit(modelBitWidth)
+ .bitwiseAnd(ternMask);
+ return new PiTernaryFieldMatch(fieldMatch.fieldId(), ternValue, ternMask);
case LPM:
PiLpmFieldMatch lpmfield = (PiLpmFieldMatch) fieldMatch;
if (lpmfield.prefixLength() > modelBitWidth) {
@@ -424,9 +430,13 @@
"Invalid prefix length for LPM field '%s', found %d but field has bit-width %d",
fieldMatch.fieldId(), lpmfield.prefixLength(), modelBitWidth));
}
+ ImmutableByteSequence lpmMask = prefixOnes(modelBitWidth * Byte.SIZE,
+ lpmfield.prefixLength());
+ ImmutableByteSequence lpmValue = lpmfield.value()
+ .fit(modelBitWidth)
+ .bitwiseAnd(lpmMask);
return new PiLpmFieldMatch(fieldMatch.fieldId(),
- lpmfield.value().fit(modelBitWidth),
- lpmfield.prefixLength());
+ lpmValue, lpmfield.prefixLength());
case RANGE:
return new PiRangeFieldMatch(fieldMatch.fieldId(),
((PiRangeFieldMatch) fieldMatch).lowValue().fit(modelBitWidth),
diff --git a/core/net/src/test/java/org/onosproject/net/pi/impl/PiTranslatorServiceTest.java b/core/net/src/test/java/org/onosproject/net/pi/impl/PiTranslatorServiceTest.java
index 9af6745..e55ec25 100644
--- a/core/net/src/test/java/org/onosproject/net/pi/impl/PiTranslatorServiceTest.java
+++ b/core/net/src/test/java/org/onosproject/net/pi/impl/PiTranslatorServiceTest.java
@@ -182,7 +182,6 @@
.addEqualityGroup(entry1, entry2)
.testEquals();
- int numMatchParams = pipeconf.pipelineModel().table(TBL_TABLE0_ID).get().matchFields().size();
// parse values stored in entry1
PiTernaryFieldMatch inPortParam = (PiTernaryFieldMatch) entry1.matchKey().fieldMatch(HDR_IN_PORT_ID).get();
PiTernaryFieldMatch ethDstParam = (PiTernaryFieldMatch) entry1.matchKey().fieldMatch(HDR_ETH_DST_ID).get();
@@ -191,10 +190,6 @@
Optional<Double> expectedTimeout = pipeconf.pipelineModel().table(TBL_TABLE0_ID).get().supportsAging()
? Optional.of((double) rule1.timeout()) : Optional.empty();
- // check that the number of parameters in the entry is the same as the number of table keys
- assertThat("Incorrect number of match parameters",
- entry1.matchKey().fieldMatches().size(), is(equalTo(numMatchParams)));
-
// check that values stored in entry are the same used for the flow rule
assertThat("Incorrect inPort match param value",
inPortParam.value().asReadOnlyBuffer().getShort(), is(equalTo(inPort)));