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 <brady.allen.johnson@ericsson.com>

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 a7ce4021571840bace71a41a3f69866323fd9364..dd9b9ff00026fd0b17c47a06ad56551d2e9f3b80 100644 (file)
--- 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<NodeId> getNodeIdFromLogicalInterface(String logicalInterface) {
@@ -225,6 +231,9 @@ public class GeniusProvider {
     public Optional<Long> getEgressVxlanPortForNode(BigInteger dpnId) {
         List<OvsdbTerminationPointAugmentation> 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<String> 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<GetDpnInterfaceListOutput> 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<BoundServices> 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<BoundServices> 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 3157691d672334c4525de3db8a43f76ad95dfbe3..1655179acfe86829ce77ed4be1dfdf350cde0a25 100644 (file)
--- 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<Action> 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 0c0c05c0cd62eea24d817edc2eb85371bdc4b968..4cfd64c0d01d821678118a7509cae3c9cfe94097 100644 (file)
--- 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<String> 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 7686288757453bc38144bebdeea45d83962cf972..ba00784cd4b8a5631097e2f22c27b08d4f232f55 100644 (file)
--- 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