From 7d9241826a54c27063269efce6919e24f3a1ff3b Mon Sep 17 00:00:00 2001 From: Brady Johnson Date: Mon, 10 Apr 2017 11:27:10 +0200 Subject: [PATCH] BUG 8193 - Fix Netvirt classifier egress service port binding - The Netvirt classifier egress service should bind on egress ports, not ingress ports like it does now. - Since its not possible to know all the possible egress ports to bind on before-hand, the egress classifier service will bind on all switch ports. It will only process NSH packets and return all others to the egress dispatcher. - Also in this patch, when the SFF is on the same bridge as the classifier, the egress classifier will resubmit the packets directly to the SFF instead of going through the ingress dispatcher, since we dont know the correct ingress port to use for the SFF. - Updated OpenFlow13ProviderTest to reflect the change to the egress classifier resubmit. - Changes from code review comments - Final changes to make sure the egress binding is working. Now using getDpnInterfaceList RPC to get the switch ports to bind to. Change-Id: I97bc38722064738ee22b8ddbc7163bc5dc3dd276 Signed-off-by: Brady Johnson --- .../classifier/providers/GeniusProvider.java | 55 ++++++++++++++++--- .../providers/OpenFlow13Provider.java | 8 +-- .../impl/ConfigurationClassifierImpl.java | 7 ++- .../providers/OpenFlow13ProviderTest.java | 2 +- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/GeniusProvider.java b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/GeniusProvider.java index a7ce402157..dd9b9ff000 100644 --- a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/GeniusProvider.java +++ b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/GeniusProvider.java @@ -26,6 +26,9 @@ import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types. import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpidFromInterfaceInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpidFromInterfaceInputBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpidFromInterfaceOutput; +import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpnInterfaceListInput; +import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpnInterfaceListInputBuilder; +import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetDpnInterfaceListOutput; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetEndpointIpForDpnInput; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetEndpointIpForDpnInputBuilder; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetEndpointIpForDpnOutput; @@ -34,6 +37,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpc import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.GetNodeconnectorIdFromInterfaceOutput; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.OdlInterfaceRpcService; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.servicebinding.rev160406.ServiceBindings; +import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.servicebinding.rev160406.ServiceModeEgress; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.servicebinding.rev160406.ServiceModeIngress; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.servicebinding.rev160406.ServiceTypeFlowBased; import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.servicebinding.rev160406.StypeOpenflow; @@ -78,6 +82,7 @@ public class GeniusProvider { NwConstants.SFC_SERVICE_INDEX, NwConstants.INGRESS_SFC_CLASSIFIER_FILTER_TABLE, OpenFlow13Provider.INGRESS_CLASSIFIER_FILTER_COOKIE, + true, interfaceName); } @@ -88,15 +93,16 @@ public class GeniusProvider { NwConstants.EGRESS_SFC_CLASSIFIER_SERVICE_INDEX, NwConstants.EGRESS_SFC_CLASSIFIER_FILTER_TABLE, OpenFlow13Provider.EGRESS_CLASSIFIER_FILTER_COOKIE, + false, interfaceName); } public void unbindPortOnIngressClassifier(String interfaceName) { - unbindService(interfaceName, NwConstants.SFC_CLASSIFIER_INDEX); + unbindService(interfaceName, NwConstants.SFC_CLASSIFIER_INDEX, true); } public void unbindPortOnEgressClassifier(String interfaceName) { - unbindService(interfaceName, NwConstants.EGRESS_SFC_CLASSIFIER_SERVICE_INDEX); + unbindService(interfaceName, NwConstants.EGRESS_SFC_CLASSIFIER_SERVICE_INDEX, false); } public Optional getNodeIdFromLogicalInterface(String logicalInterface) { @@ -225,6 +231,9 @@ public class GeniusProvider { public Optional getEgressVxlanPortForNode(BigInteger dpnId) { List tpList = interfaceMgr.getTunnelPortsOnBridge(dpnId); if (tpList == null) { + // Most likely the bridge doesnt exist for this dpnId + LOG.warn("getEgressVxlanPortForNode Tunnel Port TerminationPoint list not available for dpnId [{}]", + dpnId); return Optional.empty(); } @@ -253,16 +262,43 @@ public class GeniusProvider { } } - LOG.warn("getEgressVxlanPortForNode nothing available for dpnId [{}]", dpnId); + LOG.warn("getEgressVxlanPortForNode no Vxgpe tunnel ports available for dpnId [{}]", dpnId); return Optional.empty(); } + public List getInterfacesFromNode(NodeId nodeId) { + // getPortsOnBridge() only returns Tunnel ports, so instead using getDpnInterfaceList. + GetDpnInterfaceListInputBuilder inputBuilder = new GetDpnInterfaceListInputBuilder(); + inputBuilder.setDpid(BigInteger.valueOf(Long.valueOf(nodeId.getValue().split(":")[1]))); + GetDpnInterfaceListInput input = inputBuilder.build(); + + try { + LOG.debug("getInterfacesFromNode: invoking rpc"); + RpcResult output = + interfaceManagerRpcService.getDpnInterfaceList(input).get(); + if (!output.isSuccessful()) { + LOG.error("getInterfacesFromNode({}) failed: {}", input, output); + return Collections.emptyList(); + } + LOG.debug("getInterfacesFromNode({}) succeeded: {}", input, output); + return output.getResult().getInterfacesList(); + } catch (InterruptedException | ExecutionException e) { + LOG.error("getInterfacesFromNode failed to retrieve target interface name: ", e); + } + + return Collections.emptyList(); + } + private void bindService(short serviceId, String serviceName, int servicePriority, - short serviceDestTable, BigInteger serviceTableCookie, String interfaceName) { + short serviceDestTable, BigInteger serviceTableCookie, + boolean isIngress, String interfaceName) { + ServicesInfoKey servicesInfoKey = isIngress + ? new ServicesInfoKey(interfaceName, ServiceModeIngress.class) : + new ServicesInfoKey(interfaceName, ServiceModeEgress.class); InstanceIdentifier id = InstanceIdentifier.builder(ServiceBindings.class) - .child(ServicesInfo.class, new ServicesInfoKey(interfaceName, ServiceModeIngress.class)) + .child(ServicesInfo.class, servicesInfoKey) .child(BoundServices.class, new BoundServicesKey(serviceId)).build(); StypeOpenflow stypeOpenflow = new StypeOpenflowBuilder().setFlowCookie(serviceTableCookie) @@ -273,16 +309,19 @@ public class GeniusProvider { BoundServices boundServices = new BoundServicesBuilder().setServiceName(serviceName) .setServicePriority(serviceId).setServiceType(ServiceTypeFlowBased.class) .addAugmentation(StypeOpenflow.class, stypeOpenflow).build(); - LOG.info("Binding Service ID [{}] name [{}] priority [{}] table [{}] cookie [{}] interface [{}]", serviceId, serviceName, servicePriority, serviceDestTable, serviceTableCookie, interfaceName); MDSALUtil.syncWrite(this.dataBroker, LogicalDatastoreType.CONFIGURATION, id, boundServices); } - private void unbindService(String interfaceName, short serviceId) { + private void unbindService(String interfaceName, short serviceId, boolean isIngress) { + ServicesInfoKey servicesInfoKey = isIngress + ? new ServicesInfoKey(interfaceName, ServiceModeIngress.class) : + new ServicesInfoKey(interfaceName, ServiceModeEgress.class); + InstanceIdentifier id = InstanceIdentifier.builder(ServiceBindings.class) - .child(ServicesInfo.class, new ServicesInfoKey(interfaceName, ServiceModeIngress.class)) + .child(ServicesInfo.class, servicesInfoKey) .child(BoundServices.class, new BoundServicesKey(serviceId)).build(); LOG.info("Unbinding Service ID [{}] interface [{}]", serviceId, interfaceName); diff --git a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13Provider.java b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13Provider.java index 3157691d67..1655179acf 100644 --- a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13Provider.java +++ b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13Provider.java @@ -12,7 +12,6 @@ import com.google.common.net.InetAddresses; import java.math.BigInteger; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.atomic.AtomicLong; import javax.inject.Singleton; import org.opendaylight.controller.md.sal.binding.api.WriteTransaction; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; @@ -32,8 +31,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node; import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey; import org.opendaylight.yangtools.yang.binding.InstanceIdentifier; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton public class OpenFlow13Provider { @@ -71,8 +68,6 @@ public class OpenFlow13Provider { public static final short NSH_MDTYPE_ONE = 0x01; public static final long DEFAULT_NSH_CONTEXT_VALUE = 0L; private static final int DEFAULT_NETMASK = 32; - private static final Logger LOG = LoggerFactory.getLogger(OpenFlow13Provider.class); - private static AtomicLong flowIdInc = new AtomicLong(); public MatchBuilder getMatchBuilderFromAceMatches(Matches matches) { if (matches == null) { @@ -190,7 +185,6 @@ public class OpenFlow13Provider { actionList.size())); InstructionsBuilder isb = OpenFlow13Utils.wrapActionsIntoApplyActionsInstruction(actionList); - LOG.info("createIngressClassifierAclFlow match.toString() [{}]", match.build().toString()); // The flowIdStr needs to be unique, so the best way to make it unique is to use the match String flowIdStr = INGRESS_CLASSIFIER_ACL_FLOW_NAME + "_" + nodeId.getValue() + match.build().toString(); @@ -318,7 +312,7 @@ public class OpenFlow13Provider { OpenFlow13Utils.addMatchTunIpv4Dst(match, localIpStr, DEFAULT_NETMASK); List actionList = new ArrayList<>(); - actionList.add(OpenFlow13Utils.createActionResubmitTable(NwConstants.LPORT_DISPATCHER_TABLE, + actionList.add(OpenFlow13Utils.createActionResubmitTable(NwConstants.SFC_TRANSPORT_INGRESS_TABLE, actionList.size())); InstructionsBuilder isb = OpenFlow13Utils.wrapActionsIntoApplyActionsInstruction(actionList); diff --git a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/service/domain/impl/ConfigurationClassifierImpl.java b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/service/domain/impl/ConfigurationClassifierImpl.java index 0c0c05c0cd..4cfd64c0d0 100644 --- a/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/service/domain/impl/ConfigurationClassifierImpl.java +++ b/vpnservice/sfc/classifier/impl/src/main/java/org/opendaylight/netvirt/sfc/classifier/service/domain/impl/ConfigurationClassifierImpl.java @@ -148,7 +148,12 @@ public class ConfigurationClassifierImpl implements ClassifierState { }); entries.add(ClassifierEntry.buildNodeEntry(nodeId)); entries.add(ClassifierEntry.buildPathEntry(nodeIdListEntry.getKey(), nsp, destinationIp)); - // TODO get the interfaces of node and add the corresponding egress entries + // Egress services must bind to egress ports. Since we dont know before-hand what + // the egress ports will be, we will bind on all switch ports. If the packet + // doesnt have NSH, it will be returned to the the egress dispatcher table. + List interfaceUuidStrList = geniusProvider.getInterfacesFromNode(nodeId); + interfaceUuidStrList.forEach(interfaceUuidStr -> + entries.add(ClassifierEntry.buildEgressEntry(new InterfaceKey(interfaceUuidStr)))); }); return entries; diff --git a/vpnservice/sfc/classifier/impl/src/test/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13ProviderTest.java b/vpnservice/sfc/classifier/impl/src/test/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13ProviderTest.java index 7686288757..ba00784cd4 100644 --- a/vpnservice/sfc/classifier/impl/src/test/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13ProviderTest.java +++ b/vpnservice/sfc/classifier/impl/src/test/java/org/opendaylight/netvirt/sfc/classifier/providers/OpenFlow13ProviderTest.java @@ -275,7 +275,7 @@ public class OpenFlow13ProviderTest { assertEquals(1, flow.getInstructions().getInstruction().size()); checkActionResubmit(flow.getInstructions().getInstruction().get(0).getInstruction(), - NwConstants.LPORT_DISPATCHER_TABLE); + NwConstants.SFC_TRANSPORT_INGRESS_TABLE); } @Test -- 2.36.6