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 47df7f188e..a34cada6a2 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.packet.OutboundPacket; 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; @@ -136,11 +135,6 @@ public final class PipelineInterpreterImpl extends AbstractHandlerBehaviour impl } } - @Override - public Optional mapTableCounter(PiTableId piTableId) { - return Optional.empty(); - } - @Override public Collection 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 ccf27338af..a28c5f19c9 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 @@ -84,17 +84,6 @@ public interface PiPipelineInterpreter extends HandlerBehaviour { PiAction mapTreatment(TrafficTreatment treatment, PiTableId piTableId) 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 mapTableCounter(PiTableId piTableId); - /** * Returns a collection of PI packet operations equivalent to the given outbound packet instance. * 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 352eca87d8..00fe938030 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.DefaultFlowEntry; 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 @@ public class P4RuntimeFlowRuleProgrammable 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 @@ public class P4RuntimeFlowRuleProgrammable } // Read table direct counters (if any). - final Map counterCellMap; - if (interpreter.mapTableCounter(piTableId).isPresent()) { - PiCounterId piCounterId = interpreter.mapTableCounter(piTableId).get(); - counterCellMap = readEntryCounters(piCounterId, installedEntries); - } else { - counterCellMap = Collections.emptyMap(); - } + final Map counterCellMap = + readEntryCounters(installedEntries); // Forge flow entries with counter values. for (PiTableEntry installedEntry : installedEntries) { @@ -391,7 +390,12 @@ public class P4RuntimeFlowRuleProgrammable } private Map readEntryCounters( - PiCounterId counterId, Collection tableEntries) { + Collection tableEntries) { + if (!driverBoolProperty(SUPPORT_TABLE_COUNTERS, + DEFAULT_SUPPORT_TABLE_COUNTERS)) { + return Collections.emptyMap(); + } + Collection cellDatas; try { if (driverBoolProperty(READ_ALL_DIRECT_COUNTERS, @@ -403,6 +407,7 @@ public class P4RuntimeFlowRuleProgrammable cellDatas = Collections.emptyList(); } else { Set cellIds = tableEntries.stream() + .filter(e -> tableHasCounter(e.table())) .map(PiCounterCellId::ofDirect) .collect(Collectors.toSet()); cellDatas = client.readCounterCells(cellIds, pipeconf).get(); @@ -412,13 +417,18 @@ public class P4RuntimeFlowRuleProgrammable } 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 5ff38baaf7..d5e917b4df 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.flow.instructions.Instruction; 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_NOACTION_ID; 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_EGRESS_POR 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 @@ public class BasicInterpreterImpl extends AbstractHandlerBehaviour new ImmutableBiMap.Builder() .put(0, TBL_TABLE0_ID) .build(); - private static final ImmutableBiMap TABLE_COUNTER_MAP = - new ImmutableBiMap.Builder() - .put(TBL_TABLE0_ID, CNT_TABLE0_ID) - .put(TBL_WCMP_TABLE_ID, CNT_WCMP_TABLE_ID) - .build(); + private static final ImmutableBiMap CRITERION_MAP = new ImmutableBiMap.Builder() .put(Criterion.Type.IN_PORT, HDR_IN_PORT_ID) @@ -142,11 +134,6 @@ public class BasicInterpreterImpl extends AbstractHandlerBehaviour } } - @Override - public Optional mapTableCounter(PiTableId piTableId) { - return Optional.ofNullable(TABLE_COUNTER_MAP.get(piTableId)); - } - @Override public Collection mapOutboundPacket(OutboundPacket packet) throws PiInterpreterException { 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 ea3c2a7bf0..0000000000 --- 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 TABLE_COUNTER_MAP = - new ImmutableBiMap.Builder() - .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 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 8b5bf604d2..4c2364c278 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 @@ public final class PipeconfLoader { 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 e79cd44405..3b0a2f84c0 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.Port; 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 @@ public class FabricInterpreter extends AbstractHandlerBehaviour .put(FabricConstants.HF_ICMP_ICMP_CODE_ID, Criterion.Type.ICMPV6_CODE) .build(); - private static final ImmutableBiMap TABLE_COUNTER_MAP = - ImmutableBiMap.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 mapCriterionType(Criterion.Type type) { return Optional.ofNullable(CRITERION_MAP.get(type)); @@ -180,18 +166,6 @@ public class FabricInterpreter extends AbstractHandlerBehaviour } } - @Override - public Optional 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);