Set Proper EtherType for MplsPopAction

Problem Description:
======================
When IPv6 L3VPN traffic is coming from DC-GW to OVS switch
via MPLSoGRE tunnel at table L3_LFIB_TABLE(20) is doing
removal of MPLS label and it is setting with ethertype
is 0x800(IPv4) always which is wrong for IPv6 traffic.

Example issue flow for
IPv6 Prefix(2001:db8:1111:0:f816:3eff:fed3:155a) and
MPLS Label (103021)
----------------------------------------------------
cookie=0x8000002, duration=87904.264s, table=20,
n_packets=5,n_bytes=340, priority=10,mpls,
mpls_label=103021 actions=pop_mpls:0x0800,group:152511

From the above example flow which is currently programmed on OVS
switch for IPv6 prefix and it is doing Pop MPLS label action
followed by setting with IPv4 etherType which is wrong for
IPv6 L3VPN traffic.

Solution:
=========
As part of this code changes, passing proper ethertype either
ETHTYPE_IPV4(0X0800) or ETHTYPE_IPV6(0x86dd) based on the
IpPrefix address family type.

Issue: NETVIRT-1307

Dependent Review:
==================
This issue is dependent on below GENIUS review

https://git.opendaylight.org/gerrit/#/c/72990/

Change-Id: Ia4d5449877b686b0b28bf384ed5908647d373419
Signed-off-by: Karthikeyan Krishnan <karthikeyangceb007@gmail.com>

diff --git a/cloud-servicechain/impl/src/main/java/org/opendaylight/netvirt/cloudservicechain/utils/VpnServiceChainUtils.java b/cloud-servicechain/impl/src/main/java/org/opendaylight/netvirt/cloudservicechain/utils/VpnServiceChainUtils.java
index f986ff8aa1b88be8ea192feac381b21bcdf7272b..51082bc517c12832a2d5b9f01648e6e0e6fb27d7 100755 (executable)
--- a/cloud-servicechain/impl/src/main/java/org/opendaylight/netvirt/cloudservicechain/utils/VpnServiceChainUtils.java
+++ b/cloud-servicechain/impl/src/main/java/org/opendaylight/netvirt/cloudservicechain/utils/VpnServiceChainUtils.java
@@ -23,6 +23,7 @@ import org.opendaylight.genius.mdsalutil.InstructionInfo;
 import org.opendaylight.genius.mdsalutil.MDSALUtil;
 import org.opendaylight.genius.mdsalutil.MatchInfo;
 import org.opendaylight.genius.mdsalutil.MetaDataUtil;
+import org.opendaylight.genius.mdsalutil.NWUtil;
 import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.genius.mdsalutil.actions.ActionPopMpls;
 import org.opendaylight.genius.mdsalutil.actions.ActionRegLoad;
@@ -313,13 +314,14 @@ public final class VpnServiceChainUtils {
      * @param lportTag Pseudo Logical Port tag
      * @return the FlowEntity
      */
-    public static FlowEntity buildLFibVpnPseudoPortFlow(BigInteger dpId, Long label, String nextHop, int lportTag) {
+    public static FlowEntity buildLFibVpnPseudoPortFlow(BigInteger dpId, Long label, int etherType, String nextHop,
+                                                        int lportTag) {
 
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(MatchEthernetType.MPLS_UNICAST);
         matches.add(new MatchMplsLabel(label));
 
-        List<ActionInfo> actionsInfos = Collections.singletonList(new ActionPopMpls());
+        List<ActionInfo> actionsInfos = Collections.singletonList(new ActionPopMpls(etherType));
         List<InstructionInfo> instructions = new ArrayList<>();
         instructions.add(new InstructionWriteMetadata(
                        MetaDataUtil.getMetaDataForLPortDispatcher(lportTag,
@@ -352,16 +354,22 @@ public final class VpnServiceChainUtils {
                     .forEach(routePath -> {
                         Long label = routePath.getLabel();
                         String nextHop = routePath.getNexthopAddress();
-                        FlowEntity flowEntity = buildLFibVpnPseudoPortFlow(dpId, label, nextHop, lportTag);
-                        if (addOrRemove == NwConstants.ADD_FLOW) {
-                            mdsalMgr.installFlow(flowEntity);
-                        } else {
-                            mdsalMgr.removeFlow(flowEntity);
+                        try {
+                            int etherType = NWUtil.getEtherTypeFromIpPrefix(vrfEntry.getDestPrefix());
+                            FlowEntity flowEntity = buildLFibVpnPseudoPortFlow(dpId, label, etherType, nextHop,
+                                    lportTag);
+                            if (addOrRemove == NwConstants.ADD_FLOW) {
+                                mdsalMgr.installFlow(flowEntity);
+                            } else {
+                                mdsalMgr.removeFlow(flowEntity);
+                            }
+                            LOG.debug(
+                                    "LFIBEntry for label={}, destination={}, nexthop={} {} successfully in dpn={}",
+                                    label, vrfEntry.getDestPrefix(), nextHop,
+                                    addOrRemove == NwConstants.DEL_FLOW ? "removed" : "installed", dpId);
+                        } catch (IllegalArgumentException ex) {
+                            LOG.warn("Unable to get etherType for IP Prefix {}", vrfEntry.getDestPrefix());
                         }
-                        LOG.debug(
-                                "LFIBEntry for label={}, destination={}, nexthop={} {} successfully in dpn={}",
-                                label, vrfEntry.getDestPrefix(), nextHop,
-                                addOrRemove == NwConstants.DEL_FLOW ? "removed" : "installed", dpId);
                     }));
         }
     }

diff --git a/cloud-servicechain/impl/src/test/java/org/opendaylight/netvirt/cloudservicechain/VPNServiceChainHandlerTest.java b/cloud-servicechain/impl/src/test/java/org/opendaylight/netvirt/cloudservicechain/VPNServiceChainHandlerTest.java
index c420779d70d642218c39f575dcb157cbdf9629c8..085affbc7474c274aa3460d56c7f222955bb357b 100755 (executable)
--- a/cloud-servicechain/impl/src/test/java/org/opendaylight/netvirt/cloudservicechain/VPNServiceChainHandlerTest.java
+++ b/cloud-servicechain/impl/src/test/java/org/opendaylight/netvirt/cloudservicechain/VPNServiceChainHandlerTest.java
@@ -15,6 +15,7 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+import static org.opendaylight.genius.mdsalutil.NWUtil.getEtherTypeFromIpPrefix;
 
 import com.google.common.base.Optional;
 import com.google.common.util.concurrent.CheckedFuture;
@@ -368,6 +369,7 @@ public class VPNServiceChainHandlerTest {
         RoutePaths routePath = vrfEntry.getRoutePaths().get(0);
         FlowEntity expectedLFibFlowEntity =
             VpnServiceChainUtils.buildLFibVpnPseudoPortFlow(DPN_ID, routePath.getLabel(),
+                    getEtherTypeFromIpPrefix(vrfEntry.getDestPrefix()),
                     routePath.getNexthopAddress(), LPORT_TAG);
         assert new FlowEntityMatcher(expectedLFibFlowEntity).matches(installedFlowsCaptured.get(0));
 
@@ -410,6 +412,7 @@ public class VPNServiceChainHandlerTest {
         RoutePaths routePath = vrfEntry.getRoutePaths().get(0);
         FlowEntity expectedLFibFlowEntity =
             VpnServiceChainUtils.buildLFibVpnPseudoPortFlow(DPN_ID, routePath.getLabel(),
+                    getEtherTypeFromIpPrefix(vrfEntry.getDestPrefix()),
                     routePath.getNexthopAddress(), LPORT_TAG);
         assert new FlowEntityMatcher(expectedLFibFlowEntity).matches(installedFlowsCaptured.get(0));
 

diff --git a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
index 4aec46ee74a82c6943383468b0a847f03a9e470e..d7e7a0a199dd8afe7b7cfd7608027bac8edd9eed 100755 (executable)
--- a/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
+++ b/fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
@@ -44,6 +44,7 @@ import org.opendaylight.genius.mdsalutil.InstructionInfo;
 import org.opendaylight.genius.mdsalutil.MDSALUtil;
 import org.opendaylight.genius.mdsalutil.MatchInfo;
 import org.opendaylight.genius.mdsalutil.MetaDataUtil;
+import org.opendaylight.genius.mdsalutil.NWUtil;
 import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.genius.mdsalutil.actions.ActionDrop;
 import org.opendaylight.genius.mdsalutil.actions.ActionGroup;
@@ -392,8 +393,16 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             }
             return;
         }
+        // Get etherType value based on the IpPrefix address family type
+        int etherType;
+        try {
+            etherType = NWUtil.getEtherTypeFromIpPrefix(vrfEntry.getDestPrefix());
+        } catch (IllegalArgumentException ex) {
+            LOG.error("Unable to get etherType for IP Prefix {}", vrfEntry.getDestPrefix());
+            return;
+        }
 
-        final List<BigInteger> localDpnIdList = createLocalFibEntry(vpnInstance.getVpnId(), rd, vrfEntry);
+        final List<BigInteger> localDpnIdList = createLocalFibEntry(vpnInstance.getVpnId(), rd, vrfEntry, etherType);
         if (!localDpnIdList.isEmpty() && vpnToDpnList != null) {
             jobCoordinator.enqueueJob(FibUtil.getJobKeyForRdPrefix(rd, vrfEntry.getDestPrefix()),
                 () -> Collections.singletonList(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> {
@@ -432,7 +441,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                         // This is an static route that points to the other endpoint of an InterVpnLink
                         // In that case, we should add another entry in FIB table pointing to LPortDispatcher table.
                         installIVpnLinkSwitchingFlows(interVpnLink, vpnUuid, vrfEntry, vpnId);
-                        installInterVpnRouteInLFib(interVpnLink, vpnUuid, vrfEntry);
+                        installInterVpnRouteInLFib(interVpnLink, vpnUuid, vrfEntry, etherType);
                     }
                 });
             }
@@ -479,6 +488,13 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                 "Error installing subnet route in FIB");
             return;
         }
+        int etherType;
+        try {
+            etherType = NWUtil.getEtherTypeFromIpPrefix(vrfEntry.getDestPrefix());
+        } catch (IllegalArgumentException ex) {
+            LOG.error("Unable to get etherType for IP Prefix {}", vrfEntry.getDestPrefix());
+            return;
+        }
         FibUtil.getLabelFromRoutePaths(vrfEntry).ifPresent(label -> {
             List<String> nextHopAddressList = FibHelper.getNextHopListFromRoutePaths(vrfEntry);
             synchronized (label.toString().intern()) {
@@ -507,15 +523,13 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
         instructions.add(new InstructionGotoTable(NwConstants.L3_SUBNET_ROUTE_TABLE));
         baseVrfEntryHandler.makeConnectedRoute(dpnId, vpnId, vrfEntry, rd, instructions,
                 NwConstants.ADD_FLOW, tx, null);
-
         if (vrfEntry.getRoutePaths() != null) {
             for (RoutePaths routePath : vrfEntry.getRoutePaths()) {
                 if (RouteOrigin.value(vrfEntry.getOrigin()) != RouteOrigin.SELF_IMPORTED) {
                     List<ActionInfo> actionsInfos = new ArrayList<>();
                     // reinitialize instructions list for LFIB Table
                     final List<InstructionInfo> LFIBinstructions = new ArrayList<>();
-
-                    actionsInfos.add(new ActionPopMpls());
+                    actionsInfos.add(new ActionPopMpls(etherType));
                     LFIBinstructions.add(new InstructionApplyActions(actionsInfos));
                     LFIBinstructions.add(new InstructionWriteMetadata(subnetRouteMeta,
                             MetaDataUtil.METADATA_MASK_SUBNET_ROUTE));
@@ -587,7 +601,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
      * LportDispatcher table (via table 80)
      */
     private void installInterVpnRouteInLFib(final InterVpnLinkDataComposite interVpnLink, final String vpnName,
-                                            final VrfEntry vrfEntry) {
+                                            final VrfEntry vrfEntry, int etherType) {
         // INTERVPN routes are routes in a Vpn1 that have been leaked to Vpn2. In DC-GW, this Vpn2 route is pointing
         // to a list of DPNs where Vpn2's VpnLink was instantiated. In these DPNs LFIB must be programmed so that the
         // packet is commuted from Vpn2 to Vpn1.
@@ -598,7 +612,6 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             return;
         }
 
-        List<BigInteger> targetDpns = interVpnLink.getEndpointDpnsByVpnName(vpnName);
         Optional<Long> optLportTag = interVpnLink.getEndpointLportTagByVpnName(vpnName);
         if (!optLportTag.isPresent()) {
             LOG.warn("Could not retrieve lportTag for VPN {} endpoint in InterVpnLink {}", vpnName, interVpnLinkName);
@@ -612,7 +625,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                       vrfEntry.getDestPrefix(), vrfEntry.getRoutePaths());
             return;
         }
-        List<ActionInfo> actionsInfos = Collections.singletonList(new ActionPopMpls());
+        List<ActionInfo> actionsInfos = Collections.singletonList(new ActionPopMpls(etherType));
         List<InstructionInfo> instructions = Arrays.asList(
             new InstructionApplyActions(actionsInfos),
             new InstructionWriteMetadata(MetaDataUtil.getMetaDataForLPortDispatcher(lportTag.intValue(),
@@ -621,6 +634,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                                          MetaDataUtil.getMetaDataMaskForLPortDispatcher()),
             new InstructionGotoTable(NwConstants.L3_INTERFACE_TABLE));
         List<String> interVpnNextHopList = FibHelper.getNextHopListFromRoutePaths(vrfEntry);
+        List<BigInteger> targetDpns = interVpnLink.getEndpointDpnsByVpnName(vpnName);
 
         for (BigInteger dpId : targetDpns) {
             LOG.debug("Installing flow: VrfEntry=[prefix={} label={} nexthop={}] dpn {} for InterVpnLink {} in LFIB",
@@ -705,7 +719,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
         }
     }
 
-    private List<BigInteger> createLocalFibEntry(Long vpnId, String rd, VrfEntry vrfEntry) {
+    private List<BigInteger> createLocalFibEntry(Long vpnId, String rd, VrfEntry vrfEntry, int etherType) {
         List<BigInteger> returnLocalDpnId = new ArrayList<>();
         String localNextHopIP = vrfEntry.getDestPrefix();
         Prefixes localNextHopInfo = fibUtil.getPrefixToInterface(vpnId, localNextHopIP);
@@ -729,7 +743,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                     localNextHopSeen = true;
                     BigInteger dpnId =
                             checkCreateLocalFibEntry(localNextHopInfoLocal, localNextHopInfoLocal.getIpAddress(),
-                                    vpnId, rd, vrfEntry, vpnExtraRoute, vpnExtraRoutes);
+                                    vpnId, rd, vrfEntry, vpnExtraRoute, vpnExtraRoutes, etherType);
                     returnLocalDpnId.add(dpnId);
                 }
             }
@@ -758,12 +772,12 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                                         label, localNextHopInfo.getVpnInterfaceName(), lri.getDpnId());
                                 if (vpnExtraRoutes.isEmpty()) {
                                     BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP,
-                                            vpnId, rd, vrfEntry, null, vpnExtraRoutes);
+                                            vpnId, rd, vrfEntry, null, vpnExtraRoutes, etherType);
                                     returnLocalDpnId.add(dpnId);
                                 } else {
                                     for (Routes extraRoutes : vpnExtraRoutes) {
                                         BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP,
-                                                vpnId, rd, vrfEntry, extraRoutes, vpnExtraRoutes);
+                                                vpnId, rd, vrfEntry, extraRoutes, vpnExtraRoutes, etherType);
                                         returnLocalDpnId.add(dpnId);
                                     }
                                 }
@@ -777,7 +791,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             }
         } else {
             BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP, vpnId,
-                    rd, vrfEntry, /*routes*/ null, /*vpnExtraRoutes*/ null);
+                    rd, vrfEntry, /*routes*/ null, /*vpnExtraRoutes*/ null, etherType);
             if (dpnId != null) {
                 returnLocalDpnId.add(dpnId);
             }
@@ -788,7 +802,8 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
     private BigInteger checkCreateLocalFibEntry(Prefixes localNextHopInfo, String localNextHopIP,
                                                 final Long vpnId, final String rd,
                                                 final VrfEntry vrfEntry,
-                                                Routes routes, List<Routes> vpnExtraRoutes) {
+                                                Routes routes, List<Routes> vpnExtraRoutes,
+                                                int etherType) {
         String vpnName = fibUtil.getVpnNameFromId(vpnId);
         if (localNextHopInfo != null) {
             long groupId;
@@ -849,7 +864,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                             Collections.singletonList(new ActionGroup(groupId))));
             final List<InstructionInfo> lfibinstructions = Collections.singletonList(
                     new InstructionApplyActions(
-                            Arrays.asList(new ActionPopMpls(), new ActionGroup(groupId))));
+                            Arrays.asList(new ActionPopMpls(etherType), new ActionGroup(groupId))));
             java.util.Optional<Long> optLabel = FibUtil.getLabelFromRoutePaths(vrfEntry);
             List<String> nextHopAddressList = FibHelper.getNextHopListFromRoutePaths(vrfEntry);
             jobCoordinator.enqueueJob(jobKey, () -> {
@@ -1629,7 +1644,14 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                                     LabelRouteInfo lri = getLabelRouteInfo(optionalLabel.get());
                                     if (isPrefixAndNextHopPresentInLri(vrfEntry.getDestPrefix(), nextHopList, lri)) {
                                         if (lri.getDpnId().equals(dpnId)) {
-                                            createLocalFibEntry(vpnId, rd, vrfEntry);
+                                            try {
+                                                int etherType = NWUtil.getEtherTypeFromIpPrefix(
+                                                        vrfEntry.getDestPrefix());
+                                                createLocalFibEntry(vpnId, rd, vrfEntry, etherType);
+                                            } catch (IllegalArgumentException ex) {
+                                                LOG.warn("Unable to get etherType for IP Prefix {}",
+                                                        vrfEntry.getDestPrefix());
+                                            }
                                             continue;
                                         }
                                     }

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
index dafbcced8dd578cee83d1613bd6cfbe8e78ce603..e65ace001b5a64d5198a482010ca760d90119eb9 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ExternalRoutersListener.java
@@ -1195,7 +1195,8 @@ public class ExternalRoutersListener extends AsyncDataTreeChangeListenerBase<Rou
 
         List<Instruction> instructions = new ArrayList<>();
         List<ActionInfo> actionsInfos = new ArrayList<>();
-        actionsInfos.add(new ActionPopMpls());
+        //NAT is required for IPv4 only. Hence always etherType will be IPv4
+        actionsInfos.add(new ActionPopMpls(NwConstants.ETHTYPE_IPV4));
         Instruction writeInstruction = new InstructionApplyActions(actionsInfos).buildInstruction(0);
         instructions.add(writeInstruction);
         instructions.add(new InstructionGotoTable(tableId).buildInstruction(1));

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VpnFloatingIpHandler.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VpnFloatingIpHandler.java
index e47f43c453e5a30e77f436a5dd19f4f71523fc42..0d9d3433ba30d1e90701636446b61ae99e9229d2 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VpnFloatingIpHandler.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VpnFloatingIpHandler.java
@@ -442,7 +442,8 @@ public class VpnFloatingIpHandler implements FloatingIPHandler {
 
         List<Instruction> instructions = new ArrayList<>();
         List<ActionInfo> actionsInfos = new ArrayList<>();
-        actionsInfos.add(new ActionPopMpls());
+        //NAT is required for IPv4 only. Hence always etherType will be IPv4
+        actionsInfos.add(new ActionPopMpls(NwConstants.ETHTYPE_IPV4));
         actionsInfos.add(new ActionSetFieldEthernetDestination(new MacAddress(floatingIpPortMacAddress)));
         Instruction writeInstruction = new InstructionApplyActions(actionsInfos).buildInstruction(0);
         instructions.add(writeInstruction);