[SDFAB-522] Fix port type for pair ports
Additionally standardize the usage of the 64 bits carried in the metadata instruction
Change-Id: I023ddd7d86cfe55f3816f63aaa932803c0626df4
diff --git a/api/src/main/java/org/onosproject/segmentrouting/metadata/SRObjectiveMetadata.java b/api/src/main/java/org/onosproject/segmentrouting/metadata/SRObjectiveMetadata.java
new file mode 100644
index 0000000..27a857c
--- /dev/null
+++ b/api/src/main/java/org/onosproject/segmentrouting/metadata/SRObjectiveMetadata.java
@@ -0,0 +1,149 @@
+/*
+ * Copyright 2021-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.segmentrouting.metadata;
+
+import org.onosproject.net.flow.criteria.Criterion;
+import org.onosproject.net.flow.criteria.MetadataCriterion;
+import org.onosproject.net.flow.instructions.Instructions;
+import org.onosproject.net.flowobjective.FilteringObjective;
+import org.onosproject.net.flowobjective.ForwardingObjective;
+import org.onosproject.net.flowobjective.Objective;
+
+/**
+ * Defines the SegmentRouting metadata extensions.
+ */
+public final class SRObjectiveMetadata {
+ // Helper class
+ private SRObjectiveMetadata() {
+ }
+
+ //////////////////////////////////////////////////////////////////////////////
+ // 64 .... 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 //
+ // X X X X X X X X X X X X X X X X X X X X X 1 1 1 1 1 //
+ //////////////////////////////////////////////////////////////////////////////
+ // Metadata instruction is used as 8 byte sequence to carry up to 64 metadata
+
+ // FIXME We are assuming SR as the only app programming this meta.
+ // SDFAB-530 to get rid of this limitation
+
+ /**
+ * SR is setting this metadata when a double tagged filtering objective is removed
+ * and no other hosts is sharing the same input port. Thus, termination mac entries
+ * can be removed together with the vlan table entries.
+ *
+ * See org.onosproject.segmentrouting.RoutingRulePopulator#buildDoubleTaggedFilteringObj()
+ * See org.onosproject.segmentrouting.RoutingRulePopulator#processDoubleTaggedFilter()
+ */
+ public static final long CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES = 1L;
+
+ /**
+ * SR is setting this metadata when an interface config update has been performed
+ * and thus termination mac entries should not be removed.
+ *
+ * See org.onosproject.segmentrouting.RoutingRulePopulator#processSinglePortFiltersInternal
+ */
+ public static final long INTERFACE_CONFIG_UPDATE = 1L << 1;
+
+ /**
+ * SR is setting this metadata to signal the driver when the config is for the pair port,
+ * i.e. ports connecting two leaves.
+ *
+ * See org.onosproject.segmentrouting.RoutingRulePopulator#portType
+ */
+ public static final long PAIR_PORT = 1L << 2;
+
+ /**
+ * SR is setting this metadata to signal the driver when the config is for an edge port,
+ * i.e. ports facing an host.
+ *
+ * See org.onosproject.segmentrouting.policy.impl.PolicyManager#trafficMatchFwdObjective
+ * See org.onosproject.segmentrouting.RoutingRulePopulator#portType
+ */
+ public static final long EDGE_PORT = 1L << 3;
+
+ /**
+ * SR is setting this metadata to signal the driver when the config is for an infra port,
+ * i.e. ports connecting a leaf with a spine.
+ */
+ public static final long INFRA_PORT = 1L << 4;
+
+ private static final long METADATA_MASK = 0x1FL;
+
+ /**
+ * Check metadata passed from SegmentRouting app.
+ *
+ * @param obj the objective containing the metadata
+ * @return true if the objective contains valid metadata, false otherwise
+ */
+ public static boolean isValidSrMetadata(Objective obj) {
+ long meta = 0;
+ if (obj instanceof FilteringObjective) {
+ FilteringObjective filtObj = (FilteringObjective) obj;
+ if (filtObj.meta() == null) {
+ return true;
+ }
+ Instructions.MetadataInstruction metaIns = filtObj.meta().writeMetadata();
+ if (metaIns == null) {
+ return true;
+ }
+ meta = metaIns.metadata() & metaIns.metadataMask();
+ } else if (obj instanceof ForwardingObjective) {
+ ForwardingObjective fwdObj = (ForwardingObjective) obj;
+ if (fwdObj.meta() == null) {
+ return true;
+ }
+ MetadataCriterion metaCrit = (MetadataCriterion) fwdObj.meta().getCriterion(Criterion.Type.METADATA);
+ if (metaCrit == null) {
+ return true;
+ }
+ meta = metaCrit.metadata();
+ }
+ return meta != 0 && (meta ^ METADATA_MASK) <= METADATA_MASK;
+ }
+
+ /**
+ * Verify if a given flag has been set into the metadata.
+ *
+ * @param obj the objective containing the metadata
+ * @param flag the flag to verify
+ * @return true if the flag is set, false otherwise
+ */
+ public static boolean isSrMetadataSet(Objective obj, long flag) {
+ long meta = 0;
+ if (obj instanceof FilteringObjective) {
+ FilteringObjective filtObj = (FilteringObjective) obj;
+ if (filtObj.meta() == null) {
+ return false;
+ }
+ Instructions.MetadataInstruction metaIns = filtObj.meta().writeMetadata();
+ if (metaIns == null) {
+ return false;
+ }
+ meta = metaIns.metadata() & metaIns.metadataMask();
+ } else if (obj instanceof ForwardingObjective) {
+ ForwardingObjective fwdObj = (ForwardingObjective) obj;
+ if (fwdObj.meta() == null) {
+ return false;
+ }
+ MetadataCriterion metaCrit = (MetadataCriterion) fwdObj.meta().getCriterion(Criterion.Type.METADATA);
+ if (metaCrit == null) {
+ return false;
+ }
+ meta = metaCrit.metadata();
+ }
+ return (meta & flag) == flag;
+ }
+}
diff --git a/api/src/main/java/org/onosproject/segmentrouting/metadata/package-info.java b/api/src/main/java/org/onosproject/segmentrouting/metadata/package-info.java
new file mode 100644
index 0000000..6ce5528
--- /dev/null
+++ b/api/src/main/java/org/onosproject/segmentrouting/metadata/package-info.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright 2018-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.
+ */
+
+/**
+ * SegmentRouting metadata API.
+ */
+package org.onosproject.segmentrouting.metadata;
\ No newline at end of file
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index e933d3e..5e29dde 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -79,6 +79,11 @@
import static org.onlab.packet.ICMP6.ROUTER_ADVERTISEMENT;
import static org.onlab.packet.ICMP6.ROUTER_SOLICITATION;
import static org.onlab.packet.IPv6.PROTOCOL_ICMP6;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.INFRA_PORT;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.INTERFACE_CONFIG_UPDATE;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.PAIR_PORT;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.EDGE_PORT;
/**
* Populator of segment routing flow rules.
@@ -93,10 +98,6 @@
private DeviceConfiguration config;
private RouteSimplifierUtils routeSimplifierUtils;
- // used for signalling the driver to remove vlan table and tmac entry also
- private static final long CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES = 1;
- // used for signalling the driver when not remove tmac entries
- private static final long INTERFACE_CONFIG_UPDATE = 2;
private static final long METADATA_MASK = 0xffffffffffffffffL;
/**
@@ -819,12 +820,8 @@
* @param routerIp the router ip representing the destination switch
* @return a collection of fwdobjective
*/
- private Collection<ForwardingObjective> handleMpls(
- DeviceId targetSwId,
- DeviceId destSwId,
- Set<DeviceId> nextHops,
- int segmentId,
- IpAddress routerIp,
+ private Collection<ForwardingObjective> handleMpls(DeviceId targetSwId, DeviceId destSwId,
+ Set<DeviceId> nextHops, int segmentId, IpAddress routerIp,
boolean isMplsBos) {
TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
@@ -847,20 +844,13 @@
+ "label {} in switch {} with pop to next-hops {}",
segmentId, targetSwId, nextHops);
ForwardingObjective.Builder fwdObjNoBosBuilder =
- getMplsForwardingObjective(targetSwId,
- nextHops,
- true,
- isMplsBos,
- metabuilder.build(),
- routerIp,
- segmentId,
- destSwId);
+ getMplsForwardingObjective(targetSwId, nextHops, true, isMplsBos,
+ metabuilder.build(), routerIp, segmentId, destSwId);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
return Collections.emptyList();
}
fwdObjBuilders.add(fwdObjNoBosBuilder);
-
} else {
// next hop is not destination, irrespective of the number of next
// hops (1 or more) -- SR CONTINUE case (swap with self)
@@ -868,20 +858,13 @@
+ "label {} in switch {} without pop to next-hops {}",
segmentId, targetSwId, nextHops);
ForwardingObjective.Builder fwdObjNoBosBuilder =
- getMplsForwardingObjective(targetSwId,
- nextHops,
- false,
- isMplsBos,
- metabuilder.build(),
- routerIp,
- segmentId,
- destSwId);
+ getMplsForwardingObjective(targetSwId, nextHops, false, isMplsBos,
+ metabuilder.build(), routerIp, segmentId, destSwId);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
return Collections.emptyList();
}
fwdObjBuilders.add(fwdObjNoBosBuilder);
-
}
List<ForwardingObjective> fwdObjs = Lists.newArrayList();
@@ -895,11 +878,9 @@
.withFlag(ForwardingObjective.Flag.SPECIFIC);
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) ->
- log.debug("MPLS rule {} for SID {} populated in dev:{} ",
+ (objective) -> log.debug("MPLS rule {} for SID {} populated in dev:{} ",
objective.id(), segmentId, targetSwId),
- (objective, error) ->
- log.warn("Failed to populate MPLS rule {} for SID {}: {} in dev:{}",
+ (objective, error) -> log.warn("Failed to populate MPLS rule {} for SID {}: {} in dev:{}",
objective.id(), segmentId, error, targetSwId));
ForwardingObjective fob = fwdObjBuilder.add(context);
@@ -923,15 +904,9 @@
* @param destSw the destination sw
* @return the mpls forwarding objective builder
*/
- private ForwardingObjective.Builder getMplsForwardingObjective(
- DeviceId targetSw,
- Set<DeviceId> nextHops,
- boolean phpRequired,
- boolean isBos,
- TrafficSelector meta,
- IpAddress routerIp,
- int segmentId,
- DeviceId destSw) {
+ private ForwardingObjective.Builder getMplsForwardingObjective(DeviceId targetSw, Set<DeviceId> nextHops,
+ boolean phpRequired, boolean isBos, TrafficSelector meta,
+ IpAddress routerIp, int segmentId, DeviceId destSw) {
ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
.builder().withFlag(ForwardingObjective.Flag.SPECIFIC);
@@ -996,7 +971,7 @@
return null;
} else {
log.debug("nextObjId found:{} for mpls rule on device:{} to ds:{}",
- nextId, targetSw, ds);
+ nextId, targetSw, ds);
}
fwdBuilder.nextStep(nextId);
@@ -1162,8 +1137,11 @@
boolean pushVlan, VlanId vlanId,
boolean doTMAC, boolean update) {
MacAddress deviceMac;
+ long metadata = 0;
+ boolean isPairPort;
try {
deviceMac = config.getDeviceMac(deviceId);
+ isPairPort = portnum.equals(config.getPairLocalPort(deviceId));
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Processing SinglePortFilters aborted");
return null;
@@ -1194,16 +1172,20 @@
if (noMoreEnabledPort(deviceId, vlanId)) {
tBuilder.wipeDeferred();
}
-
// NOTE: Some switch hardware share the same tmac flow among different vlans.
// We use this metadata to let the driver know that there is still a vlan
// configuration associated to that port
if (update) {
- tBuilder.writeMetadata(INTERFACE_CONFIG_UPDATE, METADATA_MASK);
+ metadata = metadata | INTERFACE_CONFIG_UPDATE;
+ }
+ // NOTE: Metadata to signal the driver the port type
+ metadata = metadata | portType(VlanId.NONE, vlanId, isPairPort);
+ // NOTE: metadata equals 0 is rejected by the driver
+ if (metadata > 0) {
+ tBuilder.writeMetadata(metadata, METADATA_MASK);
}
fob.withMeta(tBuilder.build());
-
fob.permit().fromApp(srManager.appId);
return fob;
}
@@ -1222,9 +1204,8 @@
// We should trigger the removal of double tagged rules only when removing
// the filtering objective and no other hosts are connected to the same device port.
boolean cleanupDoubleTaggedRules = !anyDoubleTaggedHost(deviceId, portNum) && !install;
- FilteringObjective.Builder fob = buildDoubleTaggedFilteringObj(deviceId, portNum,
- outerVlan, innerVlan,
- cleanupDoubleTaggedRules);
+ FilteringObjective.Builder fob = buildDoubleTaggedFilteringObj(deviceId, portNum, outerVlan,
+ innerVlan, cleanupDoubleTaggedRules);
if (fob == null) {
// error encountered during build
return;
@@ -1264,8 +1245,11 @@
VlanId outerVlan, VlanId innerVlan,
boolean cleanupDoubleTaggedRules) {
MacAddress deviceMac;
+ long metadata = 0;
+ boolean isPairPort;
try {
deviceMac = config.getDeviceMac(deviceId);
+ isPairPort = portNum.equals(config.getPairLocalPort(deviceId));
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Processing DoubleTaggedFilters aborted");
return null;
@@ -1282,13 +1266,16 @@
// Pop outer vlan
tBuilder.popVlan();
- // special metadata for driver
+ // NOTE: Special metadata for driver to signal when clean up double tagged entries
if (cleanupDoubleTaggedRules) {
- tBuilder.writeMetadata(CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES, METADATA_MASK);
- } else {
- tBuilder.writeMetadata(0, METADATA_MASK);
+ metadata = metadata | CLEANUP_DOUBLE_TAGGED_HOST_ENTRIES;
}
-
+ // NOTE: Metadata to signal the port type to the driver
+ metadata = metadata | portType(innerVlan, outerVlan, isPairPort);
+ // NOTE: metadata equals 0 is rejected by the driver
+ if (metadata > 0) {
+ tBuilder.writeMetadata(metadata, METADATA_MASK);
+ }
// NOTE: Some switch hardware share the same filtering flow among different ports.
// We use this metadata to let the driver know that there is no more enabled port
// within the same VLAN on this device.
@@ -1297,11 +1284,31 @@
}
fob.withMeta(tBuilder.build());
-
fob.permit().fromApp(srManager.appId);
return fob;
}
+ // Returns port type based on the input parameters
+ private long portType(VlanId innerVlanId, VlanId outerVlanId, boolean isPairPort) {
+ if (isPairPort) {
+ return PAIR_PORT;
+ }
+
+ // Look at the vlan config to determine if it is an INFRA or an EDGE port
+ boolean outerVlanValid = outerVlanId != null && !outerVlanId.equals(VlanId.NONE);
+ boolean innerVlanValid = innerVlanId != null && !innerVlanId.equals(VlanId.NONE);
+
+ // double tagged interfaces are always edge ports. Edge ports do not match on the
+ // transport vlans (default internal vlan and pw transport vlan)
+ if ((innerVlanValid && outerVlanValid) ||
+ (outerVlanValid && !outerVlanId.equals(srManager.getDefaultInternalVlan()) &&
+ !outerVlanId.equals(srManager.getPwTransportVlan()))) {
+ return EDGE_PORT;
+ }
+
+ return INFRA_PORT;
+ }
+
/**
* Creates packet requests to punt all IP packets for the router.
* @param deviceId the switch dpid for the router
@@ -1409,7 +1416,6 @@
void manageSingleIpPunts(DeviceId deviceId, IpAddress ipAddress, boolean request) {
TrafficSelector.Builder sbuilder = buildIpSelectorFromIpAddress(ipAddress);
Optional<DeviceId> optDeviceId = Optional.of(deviceId);
-
if (request) {
srManager.packetService.requestPackets(sbuilder.build(),
PacketPriority.CONTROL, srManager.appId, optDeviceId);
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
index 6bc2522..7d5af6a 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
@@ -92,6 +92,7 @@
import java.util.stream.Collectors;
import static org.onlab.util.Tools.groupedThreads;
+import static org.onosproject.segmentrouting.metadata.SRObjectiveMetadata.EDGE_PORT;
import static org.slf4j.LoggerFactory.getLogger;
/**
@@ -110,9 +111,6 @@
private static final Set<PolicyType> SUPPORTED_POLICIES = ImmutableSet.of(
PolicyType.DROP, PolicyType.REDIRECT);
- // Driver should use this meta to match ig_port_type field in the ACL table
- private static final long EDGE_PORT = 1;
-
// Policy/TrafficMatch store related objects. We use these consistent maps to keep track of the
// lifecycle of a policy/traffic match. These are decomposed in multiple operations which have
// to be performed on multiple devices in order to have a policy/traffic match in ADDED state.
@@ -881,6 +879,7 @@
private ForwardingObjective.Builder trafficMatchFwdObjective(TrafficMatch trafficMatch, PolicyType policyType) {
TrafficSelector.Builder metaBuilder = DefaultTrafficSelector.builder(trafficMatch.trafficSelector());
+ // Driver should use this meta to match ig_port_type field in the ACL table
if (policyType == PolicyType.REDIRECT) {
metaBuilder.matchMetadata(EDGE_PORT);
}