Clean up PipelineInterpreter API by removing ambiguous methods

Such as mapping from PiMatchFieldId to Criterion.Type. This should not
be required since the only translation happening is from north
(Criterion.Type) to south (PiMatchFieldId).

Change-Id: I204e0bd66b3996fd60bc11d4241e8a0408e11582
This commit is contained in:
Carmelo Cascone 2019-04-17 20:05:21 -07:00
parent adb89058b4
commit 33f36a0ec7
5 changed files with 56 additions and 100 deletions

View File

@ -16,9 +16,8 @@
package org.onosproject.p4tutorial.pipeconf;
import com.google.common.collect.BiMap;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Ethernet;
@ -50,6 +49,7 @@ import org.onosproject.net.pi.runtime.PiPacketOperation;
import java.nio.ByteBuffer;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import static java.lang.String.format;
@ -98,13 +98,13 @@ public final class PipelineInterpreterImpl
private static final PiActionParamId ACT_PARAM_ID_PORT =
PiActionParamId.of("port");
private static final BiMap<Integer, PiTableId> TABLE_MAP =
new ImmutableBiMap.Builder<Integer, PiTableId>()
private static final Map<Integer, PiTableId> TABLE_MAP =
new ImmutableMap.Builder<Integer, PiTableId>()
.put(0, TABLE_L2_FWD_ID)
.build();
private static final BiMap<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
new ImmutableBiMap.Builder<Criterion.Type, PiMatchFieldId>()
private static final Map<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
ImmutableMap.<Criterion.Type, PiMatchFieldId>builder()
.put(Criterion.Type.IN_PORT, INGRESS_PORT_ID)
.put(Criterion.Type.ETH_DST, ETH_DST_ID)
.put(Criterion.Type.ETH_SRC, ETH_SRC_ID)
@ -116,21 +116,11 @@ public final class PipelineInterpreterImpl
return Optional.ofNullable(CRITERION_MAP.get(type));
}
@Override
public Optional<Criterion.Type> mapPiMatchFieldId(PiMatchFieldId headerFieldId) {
return Optional.ofNullable(CRITERION_MAP.inverse().get(headerFieldId));
}
@Override
public Optional<PiTableId> mapFlowRuleTableId(int flowRuleTableId) {
return Optional.ofNullable(TABLE_MAP.get(flowRuleTableId));
}
@Override
public Optional<Integer> mapPiTableId(PiTableId piTableId) {
return Optional.ofNullable(TABLE_MAP.inverse().get(piTableId));
}
@Override
public PiAction mapTreatment(TrafficTreatment treatment, PiTableId piTableId)
throws PiInterpreterException {

View File

@ -46,16 +46,6 @@ public interface PiPipelineInterpreter extends HandlerBehaviour {
*/
Optional<PiMatchFieldId> mapCriterionType(Criterion.Type type);
/**
* Returns the criterion type that is equivalent to the given PI match field
* ID, if present. If not present, it means that the given match field is
* not supported by this interpreter.
*
* @param fieldId match field ID
* @return optional criterion type
*/
Optional<Criterion.Type> mapPiMatchFieldId(PiMatchFieldId fieldId);
/**
* Returns a PI table ID equivalent to the given numeric table ID (as in
* {@link org.onosproject.net.flow.FlowRule#tableId()}). If not present, it
@ -71,18 +61,6 @@ public interface PiPipelineInterpreter extends HandlerBehaviour {
// specific PiTableId even when mapping to a single table.
Optional<PiTableId> mapFlowRuleTableId(int flowRuleTableId);
/**
* Returns an integer table ID equivalent to the given PI table ID. If not
* present, it means that the given PI table ID cannot be mapped to any
* integer table ID, because such mapping would be meaningless or because
* such PI table ID is not defined by the pipeline model.
*
* @param piTableId PI table ID
* @return numeric table ID
*/
// FIXME: as above
Optional<Integer> mapPiTableId(PiTableId piTableId);
/**
* Returns an action of a PI pipeline that is functionally equivalent to the
* given traffic treatment for the given table.

View File

@ -54,7 +54,6 @@ import org.slf4j.LoggerFactory;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.StringJoiner;
@ -80,7 +79,8 @@ final class PiFlowRuleTranslatorImpl {
}
/**
* Returns a PI table entry equivalent to the given flow rule, for the given pipeconf and device.
* Returns a PI table entry equivalent to the given flow rule, for the given
* pipeconf and device.
*
* @param rule flow rule
* @param pipeconf pipeconf
@ -148,7 +148,7 @@ final class PiFlowRuleTranslatorImpl {
tableEntryBuilder.withTimeout((double) rule.timeout());
} else {
log.debug("Flow rule is temporary, but table '{}' doesn't support " +
"aging, translating to permanent.", tableModel.id());
"aging, translating to permanent.", tableModel.id());
}
}
@ -158,16 +158,19 @@ final class PiFlowRuleTranslatorImpl {
/**
* Returns a PI action equivalent to the given treatment, optionally using the given interpreter. This method also
* checks that the produced PI table action is suitable for the given table ID and pipeline model. If suitable, the
* returned action instance will have parameters well-sized, according to the table model.
* Returns a PI action equivalent to the given treatment, optionally using
* the given interpreter. This method also checks that the produced PI table
* action is suitable for the given table ID and pipeline model. If
* suitable, the returned action instance will have parameters well-sized,
* according to the table model.
*
* @param treatment traffic treatment
* @param interpreter interpreter
* @param tableId PI table ID
* @param pipelineModel pipeline model
* @return PI table action
* @throws PiTranslationException if the treatment cannot be translated or if the PI action is not suitable for the
* @throws PiTranslationException if the treatment cannot be translated or
* if the PI action is not suitable for the
* given pipeline model
*/
static PiTableAction translateTreatment(TrafficTreatment treatment, PiPipelineInterpreter interpreter,
@ -185,7 +188,8 @@ final class PiFlowRuleTranslatorImpl {
}
/**
* Builds a PI action out of the given treatment, optionally using the given interpreter.
* Builds a PI action out of the given treatment, optionally using the given
* interpreter.
*/
private static PiTableAction buildAction(TrafficTreatment treatment, PiPipelineInterpreter interpreter,
PiTableId tableId)
@ -285,8 +289,9 @@ final class PiFlowRuleTranslatorImpl {
}
/**
* Builds a collection of PI field matches out of the given selector, optionally using the given interpreter. The
* field matches returned are guaranteed to be compatible for the given table model.
* Builds a collection of PI field matches out of the given selector,
* optionally using the given interpreter. The field matches returned are
* guaranteed to be compatible for the given table model.
*/
private static Collection<PiFieldMatch> translateFieldMatches(PiPipelineInterpreter interpreter,
TrafficSelector selector, PiTableModel tableModel)
@ -312,18 +317,41 @@ final class PiFlowRuleTranslatorImpl {
Set<PiMatchFieldId> usedPiCriterionFields = Sets.newHashSet();
Set<PiMatchFieldId> ignoredPiCriterionFields = Sets.newHashSet();
Map<PiMatchFieldId, Criterion> criterionMap = Maps.newHashMap();
if (interpreter != null) {
// NOTE: if two criterion types map to the same match field ID, and
// those two criterion types are present in the selector, this won't
// work. This is unlikely to happen since those cases should be
// mutually exclusive:
// e.g. ICMPV6_TYPE -> metadata.my_normalized_icmp_type
// ICMPV4_TYPE -> metadata.my_normalized_icmp_type
// A packet can be either ICMPv6 or ICMPv4 but not both.
selector.criteria()
.stream()
.map(Criterion::type)
.filter(t -> t != PROTOCOL_INDEPENDENT)
.forEach(t -> {
PiMatchFieldId mfid = interpreter.mapCriterionType(t)
.orElse(null);
if (mfid != null) {
if (criterionMap.containsKey(mfid)) {
log.warn("Detected criterion mapping " +
"conflict for PiMatchFieldId {}",
mfid);
}
criterionMap.put(mfid, selector.getCriterion(t));
}
});
}
for (PiMatchFieldModel fieldModel : tableModel.matchFields()) {
PiMatchFieldId fieldId = fieldModel.id();
int bitWidth = fieldModel.bitWidth();
Optional<Criterion.Type> criterionType =
interpreter == null
? Optional.empty()
: interpreter.mapPiMatchFieldId(fieldId);
Criterion criterion = criterionType.map(selector::getCriterion).orElse(null);
Criterion criterion = criterionMap.get(fieldId);
if (!piCriterionFields.containsKey(fieldId) && criterion == null) {
// Neither a field in PiCriterion is available nor a Criterion mapping is possible.

View File

@ -16,8 +16,8 @@
package org.onosproject.pipelines.basic;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.onlab.packet.DeserializationException;
import org.onlab.packet.Ethernet;
import org.onlab.util.ImmutableByteSequence;
@ -45,6 +45,7 @@ import org.onosproject.net.pi.runtime.PiPacketOperation;
import java.nio.ByteBuffer;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import static java.lang.String.format;
@ -80,12 +81,12 @@ public class BasicInterpreterImpl extends AbstractHandlerBehaviour
private static final int PORT_BITWIDTH = 9;
private static final ImmutableBiMap<Integer, PiTableId> TABLE_MAP =
new ImmutableBiMap.Builder<Integer, PiTableId>()
private static final Map<Integer, PiTableId> TABLE_MAP =
new ImmutableMap.Builder<Integer, PiTableId>()
.put(0, BasicConstants.INGRESS_TABLE0_CONTROL_TABLE0)
.build();
private static final ImmutableBiMap<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
new ImmutableBiMap.Builder<Criterion.Type, PiMatchFieldId>()
private static final Map<Criterion.Type, PiMatchFieldId> CRITERION_MAP =
new ImmutableMap.Builder<Criterion.Type, PiMatchFieldId>()
.put(Criterion.Type.IN_PORT, HDR_STANDARD_METADATA_INGRESS_PORT)
.put(Criterion.Type.ETH_DST, HDR_HDR_ETHERNET_DST_ADDR)
.put(Criterion.Type.ETH_SRC, HDR_HDR_ETHERNET_SRC_ADDR)
@ -235,18 +236,8 @@ public class BasicInterpreterImpl extends AbstractHandlerBehaviour
return Optional.ofNullable(CRITERION_MAP.get(type));
}
@Override
public Optional<Criterion.Type> mapPiMatchFieldId(PiMatchFieldId headerFieldId) {
return Optional.ofNullable(CRITERION_MAP.inverse().get(headerFieldId));
}
@Override
public Optional<PiTableId> mapFlowRuleTableId(int flowRuleTableId) {
return Optional.ofNullable(TABLE_MAP.get(flowRuleTableId));
}
@Override
public Optional<Integer> mapPiTableId(PiTableId piTableId) {
return Optional.ofNullable(TABLE_MAP.inverse().get(piTableId));
}
}

View File

@ -99,25 +99,6 @@ public class FabricInterpreter extends AbstractFabricHandlerBehavior
.put(Criterion.Type.ICMPV6_CODE, FabricConstants.HDR_ICMP_CODE)
.build();
private static final ImmutableMap<PiMatchFieldId, Criterion.Type> INVERSE_CRITERION_MAP =
ImmutableMap.<PiMatchFieldId, Criterion.Type>builder()
.put(FabricConstants.HDR_IG_PORT, Criterion.Type.IN_PORT)
.put(FabricConstants.HDR_ETH_DST, Criterion.Type.ETH_DST_MASKED)
.put(FabricConstants.HDR_ETH_SRC, Criterion.Type.ETH_SRC_MASKED)
.put(FabricConstants.HDR_ETH_TYPE, Criterion.Type.ETH_TYPE)
.put(FabricConstants.HDR_MPLS_LABEL, Criterion.Type.MPLS_LABEL)
.put(FabricConstants.HDR_VLAN_ID, Criterion.Type.VLAN_VID)
.put(FabricConstants.HDR_IPV4_DST, Criterion.Type.IPV4_DST)
.put(FabricConstants.HDR_IPV4_SRC, Criterion.Type.IPV4_SRC)
.put(FabricConstants.HDR_IPV6_DST, Criterion.Type.IPV6_DST)
// FIXME: might be incorrect if we inverse the map....
.put(FabricConstants.HDR_L4_SPORT, Criterion.Type.UDP_SRC)
.put(FabricConstants.HDR_L4_DPORT, Criterion.Type.UDP_DST)
.put(FabricConstants.HDR_IP_PROTO, Criterion.Type.IP_PROTO)
.put(FabricConstants.HDR_ICMP_TYPE, Criterion.Type.ICMPV6_TYPE)
.put(FabricConstants.HDR_ICMP_CODE, Criterion.Type.ICMPV6_CODE)
.build();
private static final PiAction NOP = PiAction.builder()
.withId(FabricConstants.NOP).build();
@ -131,11 +112,6 @@ public class FabricInterpreter extends AbstractFabricHandlerBehavior
return Optional.ofNullable(CRITERION_MAP.get(type));
}
@Override
public Optional<Criterion.Type> mapPiMatchFieldId(PiMatchFieldId fieldId) {
return Optional.ofNullable(INVERSE_CRITERION_MAP.get(fieldId));
}
@Override
public Optional<PiTableId> mapFlowRuleTableId(int flowRuleTableId) {
// The only use case for Index ID->PiTableId is when using the single
@ -143,13 +119,6 @@ public class FabricInterpreter extends AbstractFabricHandlerBehavior
return Optional.empty();
}
@Override
public Optional<Integer> mapPiTableId(PiTableId piTableId) {
// The only use case for Index ID->PiTableId is when using the single
// table pipeliner. fabric.p4 is never used with such pipeliner.
return Optional.empty();
}
@Override
public PiAction mapTreatment(TrafficTreatment treatment, PiTableId piTableId)
throws PiInterpreterException {