From e9261c974468eebf9f2dabc99b73fb2f7080755d Mon Sep 17 00:00:00 2001 From: pierventre Date: Fri, 27 Aug 2021 13:12:06 +0200 Subject: [PATCH] [SDFAB-522] Fix port type for pair ports Additionally standardize the usage of the 64 bits carried in the metadata instruction Change-Id: I023ddd7d86cfe55f3816f63aaa932803c0626df4 --- .../metadata/SRObjectiveMetadata.java | 149 ++++++++++++++++++ .../segmentrouting/metadata/package-info.java | 20 +++ .../segmentrouting/RoutingRulePopulator.java | 116 +++++++------- .../policy/impl/PolicyManager.java | 5 +- 4 files changed, 232 insertions(+), 58 deletions(-) create mode 100644 api/src/main/java/org/onosproject/segmentrouting/metadata/SRObjectiveMetadata.java create mode 100644 api/src/main/java/org/onosproject/segmentrouting/metadata/package-info.java 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 @@ public class RoutingRulePopulator { 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 @@ boolean populateMplsRule(DeviceId targetSwId, DeviceId destSwId, * @param routerIp the router ip representing the destination switch * @return a collection of fwdobjective */ - private Collection handleMpls( - DeviceId targetSwId, - DeviceId destSwId, - Set nextHops, - int segmentId, - IpAddress routerIp, + private Collection handleMpls(DeviceId targetSwId, DeviceId destSwId, + Set nextHops, int segmentId, IpAddress routerIp, boolean isMplsBos) { TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder(); @@ -847,20 +844,13 @@ private Collection handleMpls( + "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 @@ private Collection handleMpls( + "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 fwdObjs = Lists.newArrayList(); @@ -895,11 +878,9 @@ private Collection handleMpls( .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 @@ private Collection handleMpls( * @param destSw the destination sw * @return the mpls forwarding objective builder */ - private ForwardingObjective.Builder getMplsForwardingObjective( - DeviceId targetSw, - Set nextHops, - boolean phpRequired, - boolean isBos, - TrafficSelector meta, - IpAddress routerIp, - int segmentId, - DeviceId destSw) { + private ForwardingObjective.Builder getMplsForwardingObjective(DeviceId targetSw, Set 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 @@ private ForwardingObjective.Builder getMplsForwardingObjective( 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 @@ private FilteringObjective.Builder buildFilteringObjective(DeviceId deviceId, Po 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 @@ private FilteringObjective.Builder buildFilteringObjective(DeviceId deviceId, Po 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 @@ void processDoubleTaggedFilter(DeviceId deviceId, PortNumber portNum, VlanId out // 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 @@ private FilteringObjective.Builder buildDoubleTaggedFilteringObj(DeviceId device 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 @@ private FilteringObjective.Builder buildDoubleTaggedFilteringObj(DeviceId device // 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 @@ private FilteringObjective.Builder buildDoubleTaggedFilteringObj(DeviceId device } 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 manageIpPunts(DeviceId deviceId, boolean request) { void manageSingleIpPunts(DeviceId deviceId, IpAddress ipAddress, boolean request) { TrafficSelector.Builder sbuilder = buildIpSelectorFromIpAddress(ipAddress); Optional 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 @@ public class PolicyManager implements PolicyService { private static final Set 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 void removeOperations(PolicyId policyId, Optional traffi 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); }