Corrections in mac handling for logical sff

Included changes:
- nextHop: inner dst mac address was being changed instead of
     the one in the eth encapsulation header (fixed). Also, added
     the src mac address, so the packet can be correctly forwarded
     (by switching src and dst mac) after SF processing
- changed src mac address at chain egress so the packet won't
     be dropped because of bad src mac at subsequent pipeline
     stages
- Table 0 (classifier) table is no longer written by Logical
     transport processor in any circumstance (logical SFF is
     always used in cooperation with Genius -> Genius owns
     table 0 always (it is used for service binding)
- Some test improvements

Change-Id: I1f344604801f0855cc35ea27885de91427766f24
Signed-off-by: Diego Granados <diego.jesus.granados.lopez@ericsson.com>

diff --git a/sfc-genius/src/main/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtils.java b/sfc-genius/src/main/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtils.java
index 8ad75e4380a5c50d9125f2e5d4653a4f456bbc5e..bd1b273c6e26d499798bd5a3270d2ce879e51f6c 100644 (file)
--- a/sfc-genius/src/main/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtils.java
+++ b/sfc-genius/src/main/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtils.java
@@ -59,6 +59,24 @@ public class SfcGeniusDataUtils {
                 .findFirst();
     }
 
+    /**
+     * Fetches the MAC address for the switch port to which a given SF is connected
+     *
+     * @param ifName    the name of the neutron port to which the SF is connected
+     * @return          the MAC address used by the SFF port to which
+     *      the SF is connected, when available
+     */
+    public static Optional<MacAddress> getServiceFunctionForwarderPortMacAddress(String ifName) {
+
+        Interface theIf = SfcGeniusUtilsDataGetter.getServiceFunctionAttachedInterfaceState(ifName)
+                        .orElseThrow(() -> new RuntimeException("Interface is not present in the OPERATIONAL DS"));
+        if (theIf.getPhysAddress() == null) {
+            throw new RuntimeException(
+                    "Interface is present in the OPER DS, but it doesn't have a mac address");
+        }
+        return Optional.of(new MacAddress(theIf.getPhysAddress().getValue()));
+    }
+
     /**
      * Checks if a given Service Function has logical interfaces attached to it
      * @param sf    The service function we want to check

diff --git a/sfc-genius/src/test/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtilsTest.java b/sfc-genius/src/test/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtilsTest.java
index ffdfe4235d92515933a66ed4c8c33d2cc66b216d..990e0c14d8eeb2c5b55d1076772cb8eee80cb0c5 100644 (file)
--- a/sfc-genius/src/test/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtilsTest.java
+++ b/sfc-genius/src/test/java/org/opendaylight/sfc/genius/util/SfcGeniusDataUtilsTest.java
@@ -154,6 +154,43 @@ public class SfcGeniusDataUtilsTest {
         SfcGeniusDataUtils.getServiceFunctionMacAddress(ifName);
     }
 
+    /**
+     * Positive test
+     *
+     */
+    @Test
+    public void readRemoteMacAddress() {
+        Optional<MacAddress> mac = SfcGeniusDataUtils.getServiceFunctionForwarderPortMacAddress(ifName);
+        Assert.assertTrue(mac.isPresent());
+    }
+
+    /**
+     * Negative test when the interface does not exist in the CONFIG DS
+     *
+     */
+    @Test(expected = RuntimeException.class)
+    public void readRemoteMacAddressInterfaceDoesNotExist() {
+        PowerMockito.when(SfcGeniusUtilsDataGetter.getServiceFunctionAttachedInterfaceState(ifName)).thenReturn(null);
+        SfcGeniusDataUtils.getServiceFunctionForwarderPortMacAddress(ifName);
+    }
+
+    /**
+     * Negative test when the interface exists, but does not have a physical address
+     */
+    @Test(expected = RuntimeException.class)
+    public void readRemoteMacAddressPhysicalAddressDoesNotExists() {
+        PowerMockito.when(SfcGeniusUtilsDataGetter.getServiceFunctionAttachedInterfaceState(ifName))
+        .thenReturn(Optional.of(new InterfaceBuilder()
+                .setKey(new InterfaceKey(logicalIfName))
+                .setPhysAddress(null)
+                .setLowerLayerIf(new ArrayList<String>() {{
+                    add("openflow:79268612506848:1");
+                }})
+                .setType(L2vlan.class)
+                .build()));
+        SfcGeniusDataUtils.getServiceFunctionForwarderPortMacAddress(ifName);
+    }
+
     /**
      * Negative test when the interface features in the CONFIG DS
      * but does not feature in the OPERATIONAL DS

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
index df61fdfd6f677cdf809ee2324e78f4a81bd1d751..10ed73e5366a019313608844e5610a223a4a8c22 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerImpl.java
@@ -21,6 +21,7 @@ import org.opendaylight.sfc.genius.util.appcoexistence.SfcTableIndexMapper;
 import org.opendaylight.sfc.ofrenderer.sfg.GroupBucketInfo;
 import org.opendaylight.sfc.sfc_ovs.provider.SfcOvsUtil;
 import org.opendaylight.sfc.util.openflow.SfcOpenflowUtils;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.GroupActionCaseBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.group.action._case.GroupActionBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
@@ -799,21 +800,29 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
     }
 
     /**
-     * Configure the NshEth Next Hop by matching on the NSH pathId and
-     * index stored in the NSH header.
+     * Configure the NshEth Next Hop by matching on the NSH pathId and index
+     * stored in the NSH header.
      *
-     * @param sffNodeName - the SFF to write the flow to
-     * @param dstMac - the destination Mac
-     * @param nsp - NSH Service Path to match on
-     * @param nsi - NSH Index to match on
+     * @param sffNodeName
+     *            - the SFF to write the flow to
+     * @param srcMac
+     *            - the source Mac
+     * @param dstMac
+     *            - the destination Mac
+     * @param nsp
+     *            - NSH Service Path to match on
+     * @param nsi
+     *            - NSH Index to match on
      */
     @Override
-    public void configureNshEthNextHopFlow(String sffNodeName, String dstMac, long nsp, short nsi) {
+    public void configureNshEthNextHopFlow(String sffNodeName, String srcMac, String dstMac, long nsp, short nsi) {
         MatchBuilder match = SfcOpenflowUtils.getNshMatches(nsp, nsi);
-
         int order = 0;
         List<Action> actionList = new ArrayList<>();
-        actionList.add(SfcOpenflowUtils.createActionSetDlDst(dstMac, order++));
+        actionList.add(SfcOpenflowUtils.createActionNxLoadEncapEthSrc(srcMac,
+                order++));
+        actionList.add(SfcOpenflowUtils.createActionNxLoadEncapEthDst(dstMac,
+                order++));
 
         FlowBuilder nextHopFlow = configureNextHopFlow(match, actionList);
         sfcOfFlowWriter.writeFlow(flowRspId, sffNodeName, nextHopFlow);
@@ -1079,12 +1088,18 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
      * @param sffNodeName - the SFF to write the flow to
      * @param nshNsp - the NSH Service Path to match on
      * @param nshNsi - the NSH Service Index to match on
+     * @param macAddress - mac address to set as source mac address after removing
+     *    Eth-NSH encapsulation
      */
     @Override
-    public void configureNshEthLastHopTransportEgressFlow(final String sffNodeName, final long nshNsp, final short nshNsi) {
+    public void configureNshEthLastHopTransportEgressFlow(final String sffNodeName, final long nshNsp, final short nshNsi,
+            MacAddress macAddress) {
         MatchBuilder match = SfcOpenflowUtils.getNshMatches(nshNsp, nshNsi);
 
-        // On the last hop just remove nsh header and resubmit to the dispatcher table
+        // On the last hop:
+        // 1. remove nsh header
+        // 2. Change src mac to the mac of the last SF
+        // 3. resubmit to the dispatcher table
         int order = 0;
         List<Action> actionList = new ArrayList<>();
 
@@ -1092,6 +1107,11 @@ public class SfcOfFlowProgrammerImpl implements SfcOfFlowProgrammerInterface {
         Action popNsh = SfcOpenflowUtils.createActionNxPopNsh(order++);
         actionList.add(popNsh);
 
+        // Change source address
+        Action changeSourceMac = SfcOpenflowUtils
+                .createActionSetDlSrc(macAddress.getValue(), order++);
+        actionList.add(changeSourceMac);
+
         // Proceed with other services
         actionList.add(SfcOpenflowUtils.createActionResubmitTable(NwConstants.LPORT_DISPATCHER_TABLE, order++));
 

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerInterface.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerInterface.java
index 85a1de439bb4cc2e815edb24d036add79b2a1e7b..10b3eceb895ff695da52e6963195fef142c9ee2d 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerInterface.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowProgrammerInterface.java
@@ -14,6 +14,7 @@ import java.util.concurrent.ExecutionException;
 
 import org.opendaylight.sfc.genius.util.appcoexistence.SfcTableIndexMapper;
 import org.opendaylight.sfc.ofrenderer.sfg.GroupBucketInfo;
+import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
 
@@ -102,7 +103,23 @@ public interface SfcOfFlowProgrammerInterface {
     public void configureNshVxgpeNextHopFlow(final String sffNodeName, final String dstIp, final long nsp,
             final short nsi);
 
-    public void configureNshEthNextHopFlow(final String sffNodeName, final String dstMac, final long nsp,
+    /**
+     * Configure nsh next hop flow
+     *
+     * @param sffNodeName
+     *            The openflow node name
+     * @param srcMac
+     *            MAC address used by the openflow port to which the SF is
+     *            connected
+     * @param dstMac
+     *            MAC address used by the SF
+     * @param nsp
+     *            the NSH NSP
+     * @param nsi
+     *            the NSH NSI
+     */
+    public void configureNshEthNextHopFlow(final String sffNodeName,
+            final String srcMac, final String dstMac, final long nsp,
             final short nsi);
 
     //
@@ -161,12 +178,22 @@ public interface SfcOfFlowProgrammerInterface {
 
     /**
      * Used by logical sff processor in order to write chain egress flows
-     * @param sffNodeName    last openflow node in the chain
-     * @param nshNsp         nsp for the match
-     * @param nshNsi         nsi for the match
+     *
+     * @param sffNodeName
+     *            last openflow node in the chain
+     * @param nshNsp
+     *            nsp for the match
+     * @param nshNsi
+     *            nsi for the match
+     * @param macAddress
+     *            the mac address to set as source address at chain egress time
+     *            (if not set, the src mac address after decapsulation would be
+     *            the one set before the chain was executed (at classification
+     *            time), and the packet would be dropped at subsequent pipeline
+     *            processing)
      */
     public void configureNshEthLastHopTransportEgressFlow(String sffNodeName,
-            long nshNsp, short nshNsi);
+            long nshNsp, short nshNsi, MacAddress macAddress);
 
     /**
      * Configure transport egress flows, using a list of externally provided actions

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowWriterImpl.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowWriterImpl.java
index a88dcc26b51b269f0ad57c57040701de81e6b98c..992632175e5e478970fb8de5dddaf9ca300580b7 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowWriterImpl.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/openflow/SfcOfFlowWriterImpl.java
@@ -117,6 +117,7 @@ public class SfcOfFlowWriterImpl implements SfcOfFlowWriterInterface {
             this.flowsToWrite.addAll(flowsToWrite);
         }
 
+        @Override
         public void run(){
             WriteTransaction trans = OpendaylightSfc.getOpendaylightSfcObj().getDataProvider().newWriteOnlyTransaction();
 
@@ -156,6 +157,7 @@ public class SfcOfFlowWriterImpl implements SfcOfFlowWriterInterface {
             this.flowsToDelete.addAll(flowsToDelete);
         }
 
+        @Override
         public void run(){
 
             WriteTransaction writeTx = OpendaylightSfc.getOpendaylightSfcObj().getDataProvider().newWriteOnlyTransaction();

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfRspProcessor.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfRspProcessor.java
index 352310c1f073a28f55b71f134ae5060aa01dde9c..af8446350ca45420d16d86dabfb8c2818f3f27de 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfRspProcessor.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfRspProcessor.java
@@ -132,7 +132,7 @@ public class SfcOfRspProcessor {
                 LOG.debug("build flows of entry: {}", entry);
                 // The flows created by initializeSff dont belong to any particular RSP
                 sfcOfFlowProgrammer.setFlowRspId(SFC_FLOWS);
-                initializeSff(entry);
+                initializeSff(entry, transportProcessor);
                 sfcOfFlowProgrammer.setFlowRspId(rsp.getPathId());
                 configureTransportIngressFlows(entry, sffGraph, transportProcessor);
                 configurePathMapperFlows(entry, sffGraph, transportProcessor);
@@ -469,8 +469,10 @@ public class SfcOfRspProcessor {
      * Initialize the SFF by creating the match any flows, if not already created.
      *
      * @param entry - contains the SFF and RSP id
+     * @param transportProcessor the transport processor to use when initialization
+     * flows are transport-dependent
      */
-    private void initializeSff(SffGraph.SffGraphEntry entry) {
+    private void initializeSff(SffGraph.SffGraphEntry entry, SfcRspTransportProcessorBase transportProcessor) {
         if (entry.getDstSff().equals(SffGraph.EGRESS)) {
             return;
         }
@@ -483,7 +485,7 @@ public class SfcOfRspProcessor {
         NodeId sffNodeId = new NodeId(sffNodeName);
         if (!getSffInitialized(sffNodeId)) {
             LOG.debug("Initializing SFF [{}] node [{}]", entry.getDstSff().getValue(), sffNodeName);
-            this.sfcOfFlowProgrammer.configureClassifierTableMatchAny(sffNodeName);
+            transportProcessor.configureClassifierTableMatchAny(sffNodeName);
             this.sfcOfFlowProgrammer.configureTransportIngressTableMatchAny(sffNodeName);
             this.sfcOfFlowProgrammer.configurePathMapperTableMatchAny(sffNodeName);
             this.sfcOfFlowProgrammer.configurePathMapperAclTableMatchAny(sffNodeName);

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorLogicalSff.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorLogicalSff.java
index fa3c1c4422d028c2576bab73a1bfdcbd561e7a9a..3cf0c21e4cc22f56e66db1a1688c02ba419d1093 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorLogicalSff.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspProcessorLogicalSff.java
@@ -120,14 +120,16 @@ public class SfcRspProcessorLogicalSff extends SfcRspTransportProcessorBase {
                                      SffDataPlaneLocator srcSffDpl,
                                      SfDataPlaneLocator dstSfDpl) {
 
-        Optional<MacAddress> theMacAddr = getMacAddress(dstSfDpl);
-        if (!theMacAddr.isPresent()) {
+        Optional<MacAddress> srcSfMac = getMacAddress(dstSfDpl, true);
+        Optional<MacAddress> dstSfMac = getMacAddress(dstSfDpl, false);
+        if (!srcSfMac.isPresent() || !dstSfMac.isPresent()) {
             throw new RuntimeException("Failed on mac address retrieval for dst SF dpl [" + dstSfDpl + "]");
         }
         this.sfcFlowProgrammer.configureNshEthNextHopFlow(
                 sfcProviderUtils.getSffOpenFlowNodeName(entry.getDstSff(),
                         entry.getPathId(), entry.getDstDpnId()),
-                theMacAddr.get().getValue(), entry.getPathId(),
+                srcSfMac.get().getValue(), dstSfMac.get().getValue(),
+                entry.getPathId(),
                 entry.getServiceIndex());
     }
 
@@ -151,12 +153,14 @@ public class SfcRspProcessorLogicalSff extends SfcRspTransportProcessorBase {
     @Override
     public void configureNextHopFlow(SffGraphEntry entry, SfDataPlaneLocator srcSfDpl, SfDataPlaneLocator dstSfDpl) {
 
-        Optional<MacAddress> dstSfMac = getMacAddress(dstSfDpl);
-        if (!dstSfMac.isPresent()) {
+        Optional<MacAddress> srcSfMac = getMacAddress(dstSfDpl, true);
+        Optional<MacAddress> dstSfMac = getMacAddress(dstSfDpl, false);
+        if (!srcSfMac.isPresent() || !dstSfMac.isPresent()) {
             throw new RuntimeException("Failed on mac address retrieval for dst SF dpl [" + dstSfDpl + "]");
         }
         this.sfcFlowProgrammer.configureNshEthNextHopFlow(
                 sfcProviderUtils.getSffOpenFlowNodeName(entry.getSrcSff(), entry.getPathId(), entry.getDstDpnId()),
+                srcSfMac.get().getValue(),
                 dstSfMac.get().getValue(),
                 entry.getPathId(),
                 entry.getServiceIndex());
@@ -228,7 +232,18 @@ public class SfcRspProcessorLogicalSff extends SfcRspTransportProcessorBase {
 
         if (entry.getDstSff().equals(SffGraph.EGRESS)) {
             LOG.debug("configureSffTransportEgressFlow: called for chain egress");
-            this.sfcFlowProgrammer.configureNshEthLastHopTransportEgressFlow(sffNodeName,nsp,nsi);
+            SfDataPlaneLocator srcSfDpl = sfcProviderUtils
+                    .getSfDataPlaneLocator(
+                            sfcProviderUtils.getServiceFunction(
+                                    entry.getPrevSf(), entry.getPathId()),
+                            entry.getSrcSff());
+            Optional<MacAddress> macAddress = getMacAddress(srcSfDpl, false);
+            if (!macAddress.isPresent()) {
+                throw new RuntimeException("Failed on mac address retrieval for dst SF dpl [" + srcSfDpl + "]");
+            }
+
+            this.sfcFlowProgrammer.configureNshEthLastHopTransportEgressFlow(
+                    sffNodeName, nsp, nsi, macAddress.get());
         } else {
             LOG.debug("configureSffTransportEgressFlow: called for non-final graph entry");
             if (entry.isIntraLogicalSFFEntry()) {
@@ -268,17 +283,27 @@ public class SfcRspProcessorLogicalSff extends SfcRspTransportProcessorBase {
     }
 
     /** Given a {@link}SfDataPlaneLocator for a SF which uses a logical
-     * interface locator, the method returns the SF mac address
+     * interface locator, the method returns the SF mac address (local end)
+     * or the mac address for the OVS port to which the SF is connected
+     * (remote end)
      * @param dstSfDpl the data plane locator
+     * @param returnRemoteEnd true when the MAC for the OVS side is requested,
+     *          false when the MAC for the SF side is requested
+     *
      * @return  the optional {@link}MacAddress
      */
-    private Optional<MacAddress> getMacAddress(SfDataPlaneLocator dstSfDpl) {
-        LOG.debug("getMacAddress:starting. dstSfDpl:{}", dstSfDpl);
+    private Optional<MacAddress> getMacAddress(SfDataPlaneLocator dstSfDpl,
+            boolean returnRemoteEnd) {
+        LOG.debug("getMacAddress:starting. dstSfDpl:{}, requested side is SFF? {}", dstSfDpl, returnRemoteEnd);
         String ifName = ((LogicalInterfaceLocator) dstSfDpl.getLocatorType())
                 .getInterfaceName();
-        Optional<MacAddress> theMacAddr = SfcGeniusDataUtils.getServiceFunctionMacAddress(ifName);
-        LOG.debug("Read interface's [{}] MAC address [{}]", ifName,
-                theMacAddr.isPresent() ? theMacAddr.get().getValue() : "(empty)");
+        Optional<MacAddress> theMacAddr = returnRemoteEnd
+                ? SfcGeniusDataUtils.getServiceFunctionForwarderPortMacAddress(ifName)
+                : SfcGeniusDataUtils.getServiceFunctionMacAddress(ifName);
+        LOG.debug(
+                "Read interface's [{}] (remoteEndMAC requested {}) MAC address [{}]",
+                ifName, returnRemoteEnd, theMacAddr.isPresent()
+                        ? theMacAddr.get().getValue() : "(empty)");
         return theMacAddr;
     }
 
@@ -287,4 +312,10 @@ public class SfcRspProcessorLogicalSff extends SfcRspTransportProcessorBase {
         return Optional.of(tableIndexMapper);
     }
 
+
+    @Override
+    public void configureClassifierTableMatchAny(final String sffNodeName) {
+        // classifier table is not used in chains rendered by the LogicalSFF processor
+    }
+
 }
\ No newline at end of file

diff --git a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
index 568a2badfcf68fbc5afa3f0b41110dc096c3b603..5300d452cb406f96fed66efc06c63c1690f2ffc6 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/main/java/org/opendaylight/sfc/ofrenderer/processors/SfcRspTransportProcessorBase.java
@@ -348,4 +348,12 @@ public abstract class SfcRspTransportProcessorBase {
 
         return false;
     }
+
+    /**
+     * Create the appropriate rules in the classifier table (when necessary)
+     * @param sffNodeName  the openflow node identifier
+     */
+    public void configureClassifierTableMatchAny(final String sffNodeName) {
+        this.sfcFlowProgrammer.configureClassifierTableMatchAny(sffNodeName);
+    }
 }

diff --git a/sfc-renderers/sfc-openflow-renderer/src/test/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfLogicalSffRspProcessorTest.java b/sfc-renderers/sfc-openflow-renderer/src/test/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfLogicalSffRspProcessorTest.java
index 66de56b5f4056be145005a5a3990ccc6705d2055..71694b3464336da0569e18084e977625d74e47f2 100644 (file)
--- a/sfc-renderers/sfc-openflow-renderer/src/test/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfLogicalSffRspProcessorTest.java
+++ b/sfc-renderers/sfc-openflow-renderer/src/test/java/org/opendaylight/sfc/ofrenderer/processors/SfcOfLogicalSffRspProcessorTest.java
@@ -25,13 +25,12 @@ import org.opendaylight.sfc.ofrenderer.openflow.SfcOfFlowWriterImpl;
 import org.opendaylight.sfc.ofrenderer.utils.SfcOfProviderUtilsTestMock;
 import org.opendaylight.sfc.ofrenderer.utils.SfcSynchronizer;
 import org.opendaylight.sfc.provider.OpendaylightSfc;
+import org.opendaylight.sfc.util.openflow.SfcOpenflowUtils;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.common.rev151017.SftTypeName;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.rsp.rev140701.rendered.service.paths.RenderedServicePath;
 import org.opendaylight.yang.gen.v1.urn.cisco.params.xml.ns.yang.sfc.sf.rev140701.service.functions.ServiceFunction;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.MacAddress;
 import org.opendaylight.sfc.genius.util.SfcGeniusRpcClient;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.SetFieldCase;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.set.field._case.SetField;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.Action;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.list.ActionBuilder;
 
@@ -49,13 +48,13 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpc
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetTunnelInterfaceNameInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetTunnelInterfaceNameOutputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.ItmRpcService;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.ethernet.match.fields.EthernetDestination;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.EthernetMatch;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.GeneralAugMatchNodesNodeTableFlow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.general.rev140714.general.extension.list.grouping.ExtensionList;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nodes.node.table.flow.instructions.instruction.instruction.apply.actions._case.apply.actions.action.action.NxActionPopNshNodesNodeTableFlowApplyActionsCase;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nodes.node.table.flow.instructions.instruction.instruction.apply.actions._case.apply.actions.action.action.NxActionRegLoadNodesNodeTableFlowApplyActionsCase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nodes.node.table.flow.instructions.instruction.instruction.write.actions._case.write.actions.action.action.NxActionResubmitNodesNodeTableFlowWriteActionsCase;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.pop.nsh.grouping.NxPopNsh;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.reg.load.grouping.NxRegLoad;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.action.rev140714.nx.action.resubmit.grouping.NxResubmit;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.NxAugMatchNodesNodeTableFlow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.nxm.nx.nsi.grouping.NxmNxNsi;
@@ -118,7 +117,8 @@ public class SfcOfLogicalSffRspProcessorTest {
     private List<SftTypeName> sfTypes;
 
     private static final String theLogicalIfName = "tap40c552e0-36";
-    private static final MacAddress theMacAddress = new MacAddress("11:22:33:44:55:66");
+    private static final MacAddress theMacAddressSfSide = new MacAddress("11:22:33:44:55:66");
+    private static final MacAddress theMacAddressOvsSide = new MacAddress("aa:bb:cc:dd:ee:ff");
 
     public SfcOfLogicalSffRspProcessorTest() {
         initMocks(this);
@@ -147,7 +147,9 @@ public class SfcOfLogicalSffRspProcessorTest {
 
         PowerMockito.mockStatic(SfcGeniusDataUtils.class);
         PowerMockito.when(SfcGeniusDataUtils.getServiceFunctionMacAddress(anyString()))
-                .thenReturn(Optional.of(theMacAddress));
+                .thenReturn(Optional.of(theMacAddressSfSide));
+        PowerMockito.when(SfcGeniusDataUtils.getServiceFunctionForwarderPortMacAddress(anyString()))
+                .thenReturn(Optional.of(theMacAddressOvsSide));
 
         PowerMockito.when(SfcGeniusDataUtils.getSfLogicalInterface(any(ServiceFunction.class)))
                 .thenReturn(theLogicalIfName);
@@ -195,6 +197,7 @@ public class SfcOfLogicalSffRspProcessorTest {
 
         int nHops = sfTypes.size() + 1;
 
+
         verify(interfaceManagerRpcService, times(nHops))
                 .getEgressActionsForInterface(any(GetEgressActionsForInterfaceInput.class));
 
@@ -210,19 +213,40 @@ public class SfcOfLogicalSffRspProcessorTest {
                 Whitebox.getInternalState(ofFlowWriter, "setOfFlowsToAdd");
 
         // Make sure we have the right amount of flows in each relevant table
-        Assert.assertEquals(1, addedFlows.stream().filter(
+
+        // Please note that there is only one switch being programmed in this
+        // test. Even though the chain includes 2 SFs, the mocking returns the
+        // same dpnid for both logical interfaces, i.e. simulating that
+        // both SFs are hosted in the same compute node. It is important to
+        // keep that in mind when accounting flows in the following tests
+
+        // Logical SFF processor never uses table 0 as classifier (it uses genius,
+        // which uses that table for service binding)
+        Assert.assertEquals(0, addedFlows.stream().filter(
                 flow -> flow.tableKey.getId().equals(TABLE_INDEX_CLASSIFIER)).count());
-        Assert.assertEquals(nHops, addedFlows.stream().filter(
+
+        // transport ingress: one (initialization in the only switch) + one per (hops -1, this is the
+        // number of "SF ingresses" in the chain)
+        Assert.assertEquals(1 + (nHops -1), addedFlows.stream().filter(
                 flow -> flow.tableKey.getId().equals(NwConstants.SFC_TRANSPORT_INGRESS_TABLE)).count());
+
+        // path mapper: only the initialization flow in the only switch that it is used in this test
         Assert.assertEquals(1, addedFlows.stream().filter(
                 flow -> flow.tableKey.getId().equals(NwConstants.SFC_TRANSPORT_PATH_MAPPER_TABLE)).count());
+
+        // path mapper acl: again, initialization only
         Assert.assertEquals(1, addedFlows.stream().filter(
                 flow -> flow.tableKey.getId().equals(NwConstants.SFC_TRANSPORT_PATH_MAPPER_ACL_TABLE)).count());
-        Assert.assertEquals(4, addedFlows.stream().filter(
+
+        // next hop: 1 (initialization in the only switch) + 2 * (nhops -1) (i.e. ingress + egress to each SF) -1 (both
+        // sfs are sharing the switch, so one less flow (the one for going from one SFF to the next one) is written
+        Assert.assertEquals(1 + (2 * (nHops -1) -1) , addedFlows.stream().filter(
                 flow -> flow.tableKey.getId().equals(NwConstants.SFC_TRANSPORT_NEXT_HOP_TABLE)).count());
 
-        Assert.assertEquals(2 * nHops, addedFlows.stream().map(flowDetail -> flowDetail.flow).filter(
+        // match any: these are the 5 initialization flows for the 5 SFC tables in the switch
+        Assert.assertEquals(5, addedFlows.stream().map(flowDetail -> flowDetail.flow).filter(
                 flow -> flow.getFlowName().equals("MatchAny")).count());
+
         Assert.assertEquals(sfTypes.size(), addedFlows.stream().map(flowDetail -> flowDetail.flow).filter(
                 flow -> flow.getFlowName().equals("ingress_Transport_Flow")).count());
         Assert.assertEquals(nHops, addedFlows.stream().map(flowDetail -> flowDetail.flow).filter(
@@ -340,6 +364,7 @@ public class SfcOfLogicalSffRspProcessorTest {
     }
 
     private boolean matchNextHop(Flow nextHopFlow, long rspId) {
+
         // handle the Match
         List<NxAugMatchNodesNodeTableFlow> theNciraExtensions = getNciraExtensions(nextHopFlow);
 
@@ -358,23 +383,34 @@ public class SfcOfLogicalSffRspProcessorTest {
         // assure the destination mac address is the expected
         List<Action> actionList = getInstructionsFromFlow(nextHopFlow).stream()
                 .map(inst -> filterInstructionType(inst, ApplyActionsCase.class))
-                .filter(Optional::isPresent)
                 .map(applyActionsInst -> applyActionsInst.get().getApplyActions())
                 .map(ApplyActions::getAction)
                 .findFirst()
                 .orElse(Collections.emptyList());
 
-        Optional<MacAddress> macAddress = actionList
+        List<MacAddress> macAddresses = actionList
                 .stream()
-                .map(action -> filterActionType(action, SetFieldCase.class))
+                .map(action -> filterActionType(action, NxActionRegLoadNodesNodeTableFlowApplyActionsCase.class))
                 .filter(Optional::isPresent)
-                .map(SetFieldCaseAction -> SetFieldCaseAction.get().getSetField())
-                .map(SetField::getEthernetMatch)
-                .map(EthernetMatch::getEthernetDestination)
-                .map(EthernetDestination::getAddress)
-                .findFirst();
+                .map(NxActionRegLoadNodesNodeTableFlowApplyActionsCase -> NxActionRegLoadNodesNodeTableFlowApplyActionsCase.get().getNxRegLoad())
+                .map(NxRegLoad::getValue)
+                .map(SfcOpenflowUtils::macStringFromBigInteger)
+                .map(value -> new MacAddress(value))
+                .collect(Collectors.toList());
+
+        // there must be exactly two actions: one for replacing the destination MAC address (the target SF) and other for setting
+        // the source MAC address (so the packet can be returned after SF processing)
+        if (macAddresses.size() != 2) {
+            return false;
+        }
+        List<MacAddress> expectedMacs = new ArrayList<MacAddress>();
+        expectedMacs.add(theMacAddressSfSide);
+        expectedMacs.add(theMacAddressOvsSide);
+        if (!macAddresses.containsAll(expectedMacs)) {
+            return false;
+        }
 
-        return macAddress.filter(theMac -> theMac.equals(theMacAddress)).isPresent();
+        return true;
     }
 
     private List<Instruction> getInstructionsFromFlow(Flow theFlow) {

diff --git a/sfc-util/sfc-openflow-utils/src/main/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtils.java b/sfc-util/sfc-openflow-utils/src/main/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtils.java
index 381867c50de945d0b56f95426eeda490d897f874..4b7dd62040ace0fb4ab1bee50dec1f8d6213d346 100644 (file)
--- a/sfc-util/sfc-openflow-utils/src/main/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtils.java
+++ b/sfc-util/sfc-openflow-utils/src/main/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtils.java
@@ -9,6 +9,10 @@ package org.opendaylight.sfc.util.openflow;
 
 import com.google.common.collect.Lists;
 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 org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.openflowplugin.extension.api.path.ActionPath;
 import org.opendaylight.openflowplugin.extension.vendor.nicira.convertor.action.ActionUtil;
@@ -187,16 +191,6 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.ni
 import org.opendaylight.yang.gen.v1.urn.opendaylight.openflowplugin.extension.nicira.match.rev140714.nxm.nx.tun.gpe.np.grouping.NxmNxTunGpeNpBuilder;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
-import java.math.BigInteger;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.atomic.AtomicLong;
-
-// Import Nicira extension
-
-
-// Import Nicira extension
-
 public class SfcOpenflowUtils {
     public static final int ETHERTYPE_IPV4 = 0x0800;
     public static final int ETHERTYPE_VLAN = 0x8100;
@@ -1828,4 +1822,30 @@ public class SfcOpenflowUtils {
                 .setOrder(order)
                 .build();
     }
+
+    /*
+     * Returns the string representation for a given {@link BigInteger) representing
+     * a mac address. (e.g. for the value 257, it returns "00:00:00:00:01:01")
+     * @param value    The BigInteger representation for the mac address
+     * @return         The string (i.e. 6 hex-formatted bytes separated with ":") representation
+     */
+    public static String macStringFromBigInteger(BigInteger value) {
+        // a mac (48 bytes max) always fits into a 64 bit integer
+        long macAddressLong = value.longValue();
+
+        byte[] macAddressComplete = new byte[6];
+        for (int i = 5; i >=0; i--) {
+            macAddressComplete[i] = (byte) (macAddressLong % 256);
+            macAddressLong = macAddressLong >> 8L;
+        }
+
+        StringBuilder sb = new StringBuilder(18);
+        for (int i = 0; i <= 5; i++) {
+            byte b = macAddressComplete[i];
+            if (sb.length() > 0)
+                sb.append(':');
+            sb.append(String.format("%02x", b));
+        }
+        return sb.toString();
+    }
 }

diff --git a/sfc-util/sfc-openflow-utils/src/test/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtilsTest.java b/sfc-util/sfc-openflow-utils/src/test/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtilsTest.java
index d8479ce20665cd765582550738920db02f662222..51eee17be6fcc665d9a9afc946a21fe114f11f02 100755 (executable)
--- a/sfc-util/sfc-openflow-utils/src/test/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtilsTest.java
+++ b/sfc-util/sfc-openflow-utils/src/test/java/org/opendaylight/sfc/util/openflow/SfcOpenflowUtilsTest.java
@@ -8,6 +8,7 @@
 \r
 package org.opendaylight.sfc.util.openflow;\r
 \r
+import java.math.BigInteger;\r
 import java.util.Random;\r
 \r
 import junitparams.JUnitParamsRunner;\r
@@ -20,6 +21,8 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.acti
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.flow.MatchBuilder;\r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.model.match.types.rev131026.match.VlanMatch;\r
 \r
+import org.junit.Assert;\r
+\r
 import static com.fasterxml.uuid.EthernetAddress.constructMulticastAddress;\r
 import static junitparams.JUnitParamsRunner.$;\r
 import static org.junit.Assert.assertEquals;\r
@@ -272,4 +275,34 @@ public class SfcOpenflowUtilsTest{
                 testAct.getImplementedInterface().getName());\r
     }\r
 \r
+    @Test\r
+    @Parameters(method = "bigIntegerToMacConversionsParams")\r
+    public void testBigIntegerToMacStringConversions(BigInteger bi, String expectedValue) {\r
+        Assert.assertEquals("bad bigint to mac format conversion!", SfcOpenflowUtils.macStringFromBigInteger(bi), expectedValue);\r
+    }\r
+\r
+    public Object[][] bigIntegerToMacConversionsParams() {\r
+\r
+        final BigInteger NUMBER_256 = new BigInteger("256");\r
+        final BigInteger MAX_MAC = NUMBER_256\r
+                .multiply(NUMBER_256)\r
+                .multiply(NUMBER_256)\r
+                .multiply(NUMBER_256)\r
+                .multiply(NUMBER_256)\r
+                .multiply(NUMBER_256)\r
+                .subtract(new BigInteger("1"));\r
+\r
+        return new Object[][]{\r
+            {new BigInteger("0"), "00:00:00:00:00:00"},\r
+            {new BigInteger("1"), "00:00:00:00:00:01"},\r
+            {new BigInteger("15"), "00:00:00:00:00:0f"},\r
+            {new BigInteger("16"), "00:00:00:00:00:10"},\r
+            {new BigInteger("17"), "00:00:00:00:00:11"},\r
+            {new BigInteger("255"), "00:00:00:00:00:ff"},\r
+            {new BigInteger("256"), "00:00:00:00:01:00"},\r
+            {new BigInteger("257"), "00:00:00:00:01:01"},\r
+            {new BigInteger(new Integer(256*256).toString()), "00:00:00:01:00:00"},\r
+            {MAX_MAC, "ff:ff:ff:ff:ff:ff"}\r
+        };\r
+    }\r
 }\r