No need to map table counters in PI pipeline interpreter
This is related to ONOS-7595. In a recent P4Runtime update, it has been
made explicit that tables can support at most 1 direct counter. Hence,
the pipeline interpreter no longer needs to provide a mapping between a
table and one of potentially many counters. If needed, such mapping can
be derived from the pipeline model (i.e. the p4info)
Change-Id: Ibdece52f35a4d187ab9dbeb90f5527b6285e9788
diff --git a/apps/p4-tutorial/pipeconf/src/main/java/org/onosproject/p4tutorial/pipeconf/PipelineInterpreterImpl.java b/apps/p4-tutorial/pipeconf/src/main/java/org/onosproject/p4tutorial/pipeconf/PipelineInterpreterImpl.java
index 47df7f1..a34cada 100644
--- a/apps/p4-tutorial/pipeconf/src/main/java/org/onosproject/p4tutorial/pipeconf/PipelineInterpreterImpl.java
+++ b/apps/p4-tutorial/pipeconf/src/main/java/org/onosproject/p4tutorial/pipeconf/PipelineInterpreterImpl.java
@@ -37,7 +37,6 @@
import org.onosproject.net.pi.model.PiActionId;
import org.onosproject.net.pi.model.PiActionParamId;
import org.onosproject.net.pi.model.PiControlMetadataId;
-import org.onosproject.net.pi.model.PiCounterId;
import org.onosproject.net.pi.model.PiMatchFieldId;
import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.model.PiTableId;
@@ -137,11 +136,6 @@
}
@Override
- public Optional<PiCounterId> mapTableCounter(PiTableId piTableId) {
- return Optional.empty();
- }
-
- @Override
public Collection<PiPacketOperation> mapOutboundPacket(OutboundPacket packet)
throws PiInterpreterException {
diff --git a/core/api/src/main/java/org/onosproject/net/pi/model/PiPipelineInterpreter.java b/core/api/src/main/java/org/onosproject/net/pi/model/PiPipelineInterpreter.java
index ccf2733..a28c5f1 100644
--- a/core/api/src/main/java/org/onosproject/net/pi/model/PiPipelineInterpreter.java
+++ b/core/api/src/main/java/org/onosproject/net/pi/model/PiPipelineInterpreter.java
@@ -85,17 +85,6 @@
throws PiInterpreterException;
/**
- * Returns a PI direct counter ID for the given table to be used to to compute flow entry statistics, if present. If
- * not present, it means that the given table does not offer any counter suitable for the purpose of computing flow
- * rule statistics. Other direct counters might be defined for the given table (check pipeline model), however none
- * of them should be used for flow entry statistics except for this one.
- *
- * @param piTableId table ID
- * @return optional direct counter ID
- */
- Optional<PiCounterId> mapTableCounter(PiTableId piTableId);
-
- /**
* Returns a collection of PI packet operations equivalent to the given outbound packet instance.
*
* @param packet outbound packet
diff --git a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
index 352eca8..00fe9380 100644
--- a/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
+++ b/drivers/p4runtime/src/main/java/org/onosproject/drivers/p4runtime/P4RuntimeFlowRuleProgrammable.java
@@ -29,7 +29,6 @@
import org.onosproject.net.flow.FlowEntry;
import org.onosproject.net.flow.FlowRule;
import org.onosproject.net.flow.FlowRuleProgrammable;
-import org.onosproject.net.pi.model.PiCounterId;
import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.model.PiPipelineModel;
import org.onosproject.net.pi.model.PiTableId;
@@ -87,6 +86,11 @@
private static final String READ_FROM_MIRROR = "tableReadFromMirror";
private static final boolean DEFAULT_READ_FROM_MIRROR = false;
+ // If true, we read counters when reading table entries (if table has
+ // counters). Otherwise, we don't.
+ private static final String SUPPORT_TABLE_COUNTERS = "supportTableCounters";
+ private static final boolean DEFAULT_SUPPORT_TABLE_COUNTERS = true;
+
// If true, we read all direct counters of a table with one request.
// Otherwise, we send as many requests as the number of table entries.
private static final String READ_ALL_DIRECT_COUNTERS = "tableReadAllDirectCounters";
@@ -166,13 +170,8 @@
}
// Read table direct counters (if any).
- final Map<PiTableEntry, PiCounterCellData> counterCellMap;
- if (interpreter.mapTableCounter(piTableId).isPresent()) {
- PiCounterId piCounterId = interpreter.mapTableCounter(piTableId).get();
- counterCellMap = readEntryCounters(piCounterId, installedEntries);
- } else {
- counterCellMap = Collections.emptyMap();
- }
+ final Map<PiTableEntry, PiCounterCellData> counterCellMap =
+ readEntryCounters(installedEntries);
// Forge flow entries with counter values.
for (PiTableEntry installedEntry : installedEntries) {
@@ -391,7 +390,12 @@
}
private Map<PiTableEntry, PiCounterCellData> readEntryCounters(
- PiCounterId counterId, Collection<PiTableEntry> tableEntries) {
+ Collection<PiTableEntry> tableEntries) {
+ if (!driverBoolProperty(SUPPORT_TABLE_COUNTERS,
+ DEFAULT_SUPPORT_TABLE_COUNTERS)) {
+ return Collections.emptyMap();
+ }
+
Collection<PiCounterCellData> cellDatas;
try {
if (driverBoolProperty(READ_ALL_DIRECT_COUNTERS,
@@ -403,6 +407,7 @@
cellDatas = Collections.emptyList();
} else {
Set<PiCounterCellId> cellIds = tableEntries.stream()
+ .filter(e -> tableHasCounter(e.table()))
.map(PiCounterCellId::ofDirect)
.collect(Collectors.toSet());
cellDatas = client.readCounterCells(cellIds, pipeconf).get();
@@ -412,13 +417,18 @@
} catch (InterruptedException | ExecutionException e) {
if (!(e.getCause() instanceof StatusRuntimeException)) {
// gRPC errors are logged in the client.
- log.error("Exception while reading counter '{}' from {}: {}",
- counterId, deviceId, e);
+ log.error("Exception while reading table counters from {}: {}",
+ deviceId, e);
}
return Collections.emptyMap();
}
}
+ private boolean tableHasCounter(PiTableId tableId) {
+ return pipelineModel.table(tableId).isPresent() &&
+ !pipelineModel.table(tableId).get().counters().isEmpty();
+ }
+
enum Operation {
APPLY, REMOVE
}
diff --git a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/BasicInterpreterImpl.java b/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/BasicInterpreterImpl.java
index 5ff38ba..d5e917b 100644
--- a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/BasicInterpreterImpl.java
+++ b/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/BasicInterpreterImpl.java
@@ -32,7 +32,6 @@
import org.onosproject.net.packet.DefaultInboundPacket;
import org.onosproject.net.packet.InboundPacket;
import org.onosproject.net.packet.OutboundPacket;
-import org.onosproject.net.pi.model.PiCounterId;
import org.onosproject.net.pi.model.PiMatchFieldId;
import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.model.PiTableId;
@@ -59,8 +58,6 @@
import static org.onosproject.pipelines.basic.BasicConstants.ACT_PRM_PORT_ID;
import static org.onosproject.pipelines.basic.BasicConstants.ACT_SEND_TO_CPU_ID;
import static org.onosproject.pipelines.basic.BasicConstants.ACT_SET_EGRESS_PORT_ID;
-import static org.onosproject.pipelines.basic.BasicConstants.CNT_TABLE0_ID;
-import static org.onosproject.pipelines.basic.BasicConstants.CNT_WCMP_TABLE_ID;
import static org.onosproject.pipelines.basic.BasicConstants.HDR_ETH_DST_ID;
import static org.onosproject.pipelines.basic.BasicConstants.HDR_ETH_SRC_ID;
import static org.onosproject.pipelines.basic.BasicConstants.HDR_ETH_TYPE_ID;
@@ -71,7 +68,6 @@
import static org.onosproject.pipelines.basic.BasicConstants.PKT_META_INGRESS_PORT_ID;
import static org.onosproject.pipelines.basic.BasicConstants.PORT_BITWIDTH;
import static org.onosproject.pipelines.basic.BasicConstants.TBL_TABLE0_ID;
-import static org.onosproject.pipelines.basic.BasicConstants.TBL_WCMP_TABLE_ID;
/**
* Interpreter implementation for basic.p4.
@@ -83,11 +79,7 @@
new ImmutableBiMap.Builder<Integer, PiTableId>()
.put(0, TBL_TABLE0_ID)
.build();
- private static final ImmutableBiMap<PiTableId, PiCounterId> TABLE_COUNTER_MAP =
- new ImmutableBiMap.Builder<PiTableId, PiCounterId>()
- .put(TBL_TABLE0_ID, CNT_TABLE0_ID)
- .put(TBL_WCMP_TABLE_ID, CNT_WCMP_TABLE_ID)
- .build();
+
private static final ImmutableBiMap<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
new ImmutableBiMap.Builder<Criterion.Type, PiMatchFieldId>()
.put(Criterion.Type.IN_PORT, HDR_IN_PORT_ID)
@@ -143,11 +135,6 @@
}
@Override
- public Optional<PiCounterId> mapTableCounter(PiTableId piTableId) {
- return Optional.ofNullable(TABLE_COUNTER_MAP.get(piTableId));
- }
-
- @Override
public Collection<PiPacketOperation> mapOutboundPacket(OutboundPacket packet)
throws PiInterpreterException {
TrafficTreatment treatment = packet.treatment();
diff --git a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/IntInterpreterImpl.java b/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/IntInterpreterImpl.java
deleted file mode 100644
index ea3c2a7..0000000
--- a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/IntInterpreterImpl.java
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * Copyright 2017-present Open Networking Foundation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.onosproject.pipelines.basic;
-
-import com.google.common.collect.ImmutableBiMap;
-import org.onosproject.net.pi.model.PiCounterId;
-import org.onosproject.net.pi.model.PiTableId;
-
-import java.util.Optional;
-
-import static org.onosproject.pipelines.basic.BasicConstants.CNT_TABLE0_ID;
-import static org.onosproject.pipelines.basic.BasicConstants.TBL_TABLE0_ID;
-import static org.onosproject.pipelines.basic.IntConstants.*;
-
-/**
- * Interpreter implementation for INT pipeline.
- */
-public class IntInterpreterImpl extends BasicInterpreterImpl {
-
- private static final ImmutableBiMap<PiTableId, PiCounterId> TABLE_COUNTER_MAP =
- new ImmutableBiMap.Builder<PiTableId, PiCounterId>()
- .put(TBL_TABLE0_ID, CNT_TABLE0_ID)
- .put(TBL_SET_SOURCE_SINK_ID, CNT_SET_SOURCE_SINK_ID)
- .put(TBL_INT_SOURCE_ID, CNT_INT_SOURCE_ID)
- .put(TBL_INT_INSERT_ID, CNT_INT_INSERT_ID)
- .put(TBL_INT_INST_0003_ID, CNT_INT_INST_0003_ID)
- .put(TBL_INT_INST_0407_ID, CNT_INT_INST_0407_ID)
- .build();
-
- @Override
- public Optional<PiCounterId> mapTableCounter(PiTableId piTableId) {
- return Optional.ofNullable(TABLE_COUNTER_MAP.get(piTableId));
- }
-}
diff --git a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/PipeconfLoader.java b/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/PipeconfLoader.java
index 8b5bf60..4c2364c 100644
--- a/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/PipeconfLoader.java
+++ b/pipelines/basic/src/main/java/org/onosproject/pipelines/basic/PipeconfLoader.java
@@ -100,7 +100,7 @@
return DefaultPiPipeconf.builder()
.withId(INT_PIPECONF_ID)
.withPipelineModel(parseP4Info(p4InfoUrl))
- .addBehaviour(PiPipelineInterpreter.class, IntInterpreterImpl.class)
+ .addBehaviour(PiPipelineInterpreter.class, BasicInterpreterImpl.class)
.addBehaviour(Pipeliner.class, DefaultSingleTablePipeline.class)
.addBehaviour(PortStatisticsDiscovery.class, PortStatisticsDiscoveryImpl.class)
.addExtension(P4_INFO_TEXT, p4InfoUrl)
diff --git a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
index e79cd44..3b0a2f8 100644
--- a/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
+++ b/pipelines/fabric/src/main/java/org/onosproject/pipelines/fabric/FabricInterpreter.java
@@ -29,14 +29,12 @@
import org.onosproject.net.PortNumber;
import org.onosproject.net.device.DeviceService;
import org.onosproject.net.driver.AbstractHandlerBehaviour;
-import org.onosproject.net.driver.Driver;
import org.onosproject.net.flow.TrafficTreatment;
import org.onosproject.net.flow.criteria.Criterion;
import org.onosproject.net.flow.instructions.Instructions;
import org.onosproject.net.packet.DefaultInboundPacket;
import org.onosproject.net.packet.InboundPacket;
import org.onosproject.net.packet.OutboundPacket;
-import org.onosproject.net.pi.model.PiCounterId;
import org.onosproject.net.pi.model.PiMatchFieldId;
import org.onosproject.net.pi.model.PiPipelineInterpreter;
import org.onosproject.net.pi.model.PiTableId;
@@ -133,18 +131,6 @@
.put(FabricConstants.HF_ICMP_ICMP_CODE_ID, Criterion.Type.ICMPV6_CODE)
.build();
- private static final ImmutableBiMap<PiTableId, PiCounterId> TABLE_COUNTER_MAP =
- ImmutableBiMap.<PiTableId, PiCounterId>builder()
- .put(FabricConstants.TBL_FWD_CLASSIFIER_ID, FabricConstants.CNT_FWD_CLASSIFIER_COUNTER_ID)
- .put(FabricConstants.TBL_HASHED_ID, FabricConstants.CNT_HASHED_COUNTER_ID)
- .put(FabricConstants.TBL_INGRESS_PORT_VLAN_ID, FabricConstants.CNT_INGRESS_PORT_VLAN_COUNTER_ID)
- .put(FabricConstants.TBL_SIMPLE_ID, FabricConstants.CNT_SIMPLE_COUNTER_ID)
- .put(FabricConstants.TBL_BRIDGING_ID, FabricConstants.CNT_BRIDGING_COUNTER_ID)
- .put(FabricConstants.TBL_UNICAST_V4_ID, FabricConstants.CNT_UNICAST_V4_COUNTER_ID)
- .put(FabricConstants.TBL_MPLS_ID, FabricConstants.CNT_MPLS_COUNTER_ID)
- .build();
- private static final String SUPPORT_TABLE_COUNTERS_PROP = "supportTableCounters";
-
@Override
public Optional<PiMatchFieldId> mapCriterionType(Criterion.Type type) {
return Optional.ofNullable(CRITERION_MAP.get(type));
@@ -180,18 +166,6 @@
}
}
- @Override
- public Optional<PiCounterId> mapTableCounter(PiTableId piTableId) {
- Driver driver = handler().driver();
- boolean supportTableCounters = Boolean.parseBoolean(driver.getProperty(SUPPORT_TABLE_COUNTERS_PROP));
-
- if (supportTableCounters) {
- return Optional.ofNullable(TABLE_COUNTER_MAP.get(piTableId));
- } else {
- return Optional.empty();
- }
- }
-
private PiPacketOperation createPiPacketOperation(DeviceId deviceId, ByteBuffer data, long portNumber)
throws PiInterpreterException {
PiControlMetadata metadata = createPacketMetadata(portNumber);