Remote group flow fails in conntrack SNAT

The flow to take the packet to the packet to the NAPT switch from
non-napt switch fails occasionally, since the group is not programmed in
time.

The flow is now programmed only when the group is available in
Config datastore.

NETVIRT-1352 CSIT sporadic failures Random ping failure in snat
conntrack mode

Change-Id: I0d27372470cd3219a8ede5c5c35bc4679f772fb8
Signed-off-by: Aswin Suryanarayanan <asuryana@redhat.com>

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/AbstractSnatService.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/AbstractSnatService.java
index 5899ef73bd4331432d855f7051dd4abf6b012a8f..5b602b057a5e8e10f01527990574e37194e1f4c5 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/AbstractSnatService.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/AbstractSnatService.java
@@ -7,8 +7,11 @@
  */
 package org.opendaylight.netvirt.natservice.internal;
 
+import static org.opendaylight.genius.infra.Datastore.CONFIGURATION;
+
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.math.BigInteger;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -20,6 +23,7 @@ import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.genius.datastoreutils.SingleTransactionDataBroker;
+import org.opendaylight.genius.datastoreutils.listeners.DataTreeEventCallbackRegistrar;
 import org.opendaylight.genius.infra.Datastore.Configuration;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl;
@@ -49,6 +53,7 @@ import org.opendaylight.genius.mdsalutil.matches.MatchEthernetType;
 import org.opendaylight.genius.mdsalutil.matches.MatchIpv4Destination;
 import org.opendaylight.genius.mdsalutil.matches.MatchMetadata;
 import org.opendaylight.genius.mdsalutil.matches.MatchTunnelId;
+import org.opendaylight.infrautils.utils.concurrent.ListenableFutures;
 import org.opendaylight.netvirt.fibmanager.api.IFibManager;
 import org.opendaylight.netvirt.fibmanager.api.RouteOrigin;
 import org.opendaylight.netvirt.natservice.api.SnatServiceListener;
@@ -71,6 +76,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.G
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetTunnelInterfaceNameOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.ItmRpcService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.GroupTypes;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.Group;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.Adjacencies;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.AdjacenciesBuilder;
@@ -104,13 +110,15 @@ public abstract class AbstractSnatService implements SnatServiceListener {
     protected final IVpnFootprintService vpnFootprintService;
     protected final IFibManager fibManager;
     protected final NatDataUtil natDataUtil;
+    protected final DataTreeEventCallbackRegistrar eventCallbacks;
 
     protected AbstractSnatService(final DataBroker dataBroker, final IMdsalApiManager mdsalManager,
                                   final ItmRpcService itmManager, final OdlInterfaceRpcService odlInterfaceRpcService,
                                   final IdManagerService idManager, final NAPTSwitchSelector naptSwitchSelector,
                                   final IInterfaceManager interfaceManager,
                                   final IVpnFootprintService vpnFootprintService,
-                                  final IFibManager fibManager, final NatDataUtil natDataUtil) {
+                                  final IFibManager fibManager, final NatDataUtil natDataUtil,
+                                  final DataTreeEventCallbackRegistrar eventCallbacks) {
         this.dataBroker = dataBroker;
         this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker);
         this.mdsalManager = mdsalManager;
@@ -122,6 +130,7 @@ public abstract class AbstractSnatService implements SnatServiceListener {
         this.vpnFootprintService = vpnFootprintService;
         this.fibManager = fibManager;
         this.natDataUtil = natDataUtil;
+        this.eventCallbacks = eventCallbacks;
     }
 
     protected DataBroker getDataBroker() {
@@ -329,7 +338,7 @@ public abstract class AbstractSnatService implements SnatServiceListener {
     }
 
     protected void addSnatMissEntry(TypedReadWriteTransaction<Configuration> confTx, BigInteger dpnId,
-        Long routerId, String routerName, BigInteger primarySwitchId) {
+        Long routerId, String routerName, BigInteger primarySwitchId)  {
         LOG.debug("installSnatMissEntry : Installing SNAT miss entry in switch {}", dpnId);
         List<ActionInfo> listActionInfoPrimary = new ArrayList<>();
         String ifNamePrimary = getTunnelInterfaceName(dpnId, primarySwitchId);
@@ -351,9 +360,34 @@ public abstract class AbstractSnatService implements SnatServiceListener {
         LOG.debug("installing the PSNAT to NAPTSwitch GroupEntity:{} with GroupId: {}", groupEntity, groupId);
         mdsalManager.addGroup(confTx, groupEntity);
 
+        //Add the flow to send the packet to the group only after group is available in Config datastore
+        InstanceIdentifier<Group> groupIid = NatUtil.getGroupInstanceId(dpnId, groupId);
+        try {
+            if (confTx.read(groupIid).get().isPresent()) {
+                LOG.info("group {} is present in the config hence adding the flow", groupId);
+                addSnatMissFlowForGroup(confTx, dpnId, routerId, groupId);
+                return;
+            }
+            eventCallbacks.onAddOrUpdate(LogicalDatastoreType.CONFIGURATION,
+                    NatUtil.getGroupInstanceId(dpnId, groupId), (unused, newGroupId) -> {
+                    LOG.info("group {} is created in the config", groupId);
+                    ListenableFutures.addErrorLogging(
+                            txRunner.callWithNewReadWriteTransactionAndSubmit(CONFIGURATION,
+                                innerConfTx -> addSnatMissFlowForGroup(innerConfTx, dpnId, routerId, groupId)),
+                            LOG, "Error adding flow for the group {}",groupId);
+                    return DataTreeEventCallbackRegistrar.NextAction.UNREGISTER;
+                }, Duration.ofSeconds(5), iid -> LOG.error("groupId {} not found in config datastore",
+                        groupId));
+        } catch (InterruptedException | ExecutionException e) {
+            LOG.error("Error programming SNAT miss entryfor dpn {}", dpnId);
+        }
+    }
+
+    private void addSnatMissFlowForGroup(TypedReadWriteTransaction<Configuration> confTx,
+            BigInteger dpnId, Long routerId, long groupId) {
         // Install miss entry pointing to group
-        LOG.debug("installSnatMissEntry : buildSnatFlowEntity is called for dpId {}, routerName {} and groupId {}",
-            dpnId, routerName, groupId);
+        LOG.debug("installSnatMissEntry : buildSnatFlowEntity is called for dpId {}, routerId {} and groupId {}",
+            dpnId, routerId, groupId);
         List<MatchInfo> matches = new ArrayList<>();
         matches.add(new MatchEthernetType(0x0800L));
         matches.add(new MatchMetadata(MetaDataUtil.getVpnIdMetadata(routerId), MetaDataUtil.METADATA_MASK_VRFID));

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ConntrackBasedSnatService.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ConntrackBasedSnatService.java
index ded171b366ac342efed6b25fcae58dbc46971515..65608d17e9063954fdec4f7ecd5dae6d5d2227ce 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ConntrackBasedSnatService.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/ConntrackBasedSnatService.java
@@ -13,6 +13,7 @@ import java.util.ArrayList;
 import java.util.List;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.genius.datastoreutils.listeners.DataTreeEventCallbackRegistrar;
 import org.opendaylight.genius.infra.Datastore.Configuration;
 import org.opendaylight.genius.infra.TypedReadWriteTransaction;
 import org.opendaylight.genius.infra.TypedWriteTransaction;
@@ -68,9 +69,10 @@ public abstract class ConntrackBasedSnatService extends AbstractSnatService {
                                      IdManagerService idManager, NAPTSwitchSelector naptSwitchSelector,
                                      OdlInterfaceRpcService odlInterfaceRpcService,
                                      IInterfaceManager interfaceManager, IVpnFootprintService vpnFootprintService,
-                                     IFibManager fibManager, NatDataUtil natDataUtil) {
+                                     IFibManager fibManager, NatDataUtil natDataUtil,
+                                     DataTreeEventCallbackRegistrar eventCallbacks) {
         super(dataBroker, mdsalManager, itmManager, odlInterfaceRpcService, idManager, naptSwitchSelector,
-                interfaceManager, vpnFootprintService, fibManager, natDataUtil);
+                interfaceManager, vpnFootprintService, fibManager, natDataUtil, eventCallbacks);
     }
 
     @Override

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/FlatVlanConntrackBasedSnatService.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/FlatVlanConntrackBasedSnatService.java
index b141801929792bd6a365369c8e9ec97a546e4750..887788f47519e44d8ff583921a97550c85827561 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/FlatVlanConntrackBasedSnatService.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/FlatVlanConntrackBasedSnatService.java
@@ -10,6 +10,7 @@ package org.opendaylight.netvirt.natservice.internal;
 import java.math.BigInteger;
 
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.genius.datastoreutils.listeners.DataTreeEventCallbackRegistrar;
 import org.opendaylight.genius.infra.Datastore.Configuration;
 import org.opendaylight.genius.infra.TypedReadWriteTransaction;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
@@ -34,10 +35,11 @@ public class FlatVlanConntrackBasedSnatService extends ConntrackBasedSnatService
                                              IdManagerService idManager, NAPTSwitchSelector naptSwitchSelector,
                                              IInterfaceManager interfaceManager,
                                              IVpnFootprintService vpnFootprintService,
-                                             IFibManager fibManager, NatDataUtil natDataUtil) {
+                                             IFibManager fibManager, NatDataUtil natDataUtil,
+                                             DataTreeEventCallbackRegistrar eventCallbacks) {
         super(dataBroker, mdsalManager, itmManager, idManager, naptSwitchSelector,
                 odlInterfaceRpcService, interfaceManager, vpnFootprintService, fibManager ,
-                natDataUtil);
+                natDataUtil, eventCallbacks);
     }
 
     @Override

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
index 8566a9ea0f2b4d2c79edce631f87b998c0d10970..3e624902c9228b7f9d532768200c42680ecacdf4 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NatUtil.java
@@ -87,6 +87,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.acti
 import org.opendaylight.yang.gen.v1.urn.opendaylight.action.types.rev131112.action.action.PushVlanActionCase;
 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.list.Action;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.tables.table.Flow;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.types.rev131026.instruction.list.Instruction;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.AllocateIdInput;
@@ -109,7 +110,14 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.dpn
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetEgressActionsForTunnelInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetEgressActionsForTunnelOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.ItmRpcService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.GroupId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.Group;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.groups.GroupKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorId;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeId;
+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.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.ElanDpnInterfaces;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.ElanInstances;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.dpn.interfaces.ElanDpnInterfacesList;
@@ -2282,6 +2290,11 @@ public final class NatUtil {
                 .child(DpnInterfaces.class, new DpnInterfacesKey(dpId)).build();
     }
 
+    public static InstanceIdentifier<Group> getGroupInstanceId(BigInteger dpnId, long groupId) {
+        return InstanceIdentifier.builder(Nodes.class).child(Node.class, new NodeKey(new NodeId("openflow:" + dpnId)))
+                .augmentation(FlowCapableNode.class).child(Group.class, new GroupKey(new GroupId(groupId))).build();
+    }
+
     public static void createGroupIdPool(IdManagerService idManager) {
         CreateIdPoolInput createPool = new CreateIdPoolInputBuilder()
                 .setPoolName(NatConstants.SNAT_IDPOOL_NAME)

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/SnatServiceImplFactory.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/SnatServiceImplFactory.java
index c9f8d680a4e4d8694d0b17f6c7e0a9ecfe7921c2..a389cc8be7f721b6aa54849ddbdf7e69f52e58fa 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/SnatServiceImplFactory.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/SnatServiceImplFactory.java
@@ -10,6 +10,7 @@ package org.opendaylight.netvirt.natservice.internal;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.genius.datastoreutils.listeners.DataTreeEventCallbackRegistrar;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
 import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager;
 import org.opendaylight.infrautils.inject.AbstractLifecycle;
@@ -45,6 +46,7 @@ public class SnatServiceImplFactory extends AbstractLifecycle {
     private final IVpnFootprintService vpnFootprintService;
     private final IFibManager fibManager;
     private final NatDataUtil natDataUtil;
+    private final DataTreeEventCallbackRegistrar eventCallbacks;
 
     @Inject
     public SnatServiceImplFactory(final DataBroker dataBroker, final IMdsalApiManager mdsalManager,
@@ -59,7 +61,8 @@ public class SnatServiceImplFactory extends AbstractLifecycle {
                                   final IInterfaceManager interfaceManager,
                                   final IVpnFootprintService vpnFootprintService,
                                   final IFibManager fibManager,
-                                  final NatDataUtil natDataUtil) {
+                                  final NatDataUtil natDataUtil,
+                                  final DataTreeEventCallbackRegistrar eventCallbacks) {
         this.dataBroker = dataBroker;
         this.mdsalManager = mdsalManager;
         this.itmManager = itmManager;
@@ -78,6 +81,7 @@ public class SnatServiceImplFactory extends AbstractLifecycle {
         this.vpnFootprintService = vpnFootprintService;
         this.fibManager = fibManager;
         this.natDataUtil = natDataUtil;
+        this.eventCallbacks = eventCallbacks;
     }
 
     @Override
@@ -94,7 +98,8 @@ public class SnatServiceImplFactory extends AbstractLifecycle {
 
         if (natMode == NatMode.Conntrack) {
             return new FlatVlanConntrackBasedSnatService(dataBroker, mdsalManager, itmManager, odlInterfaceRpcService,
-                    idManager, naptSwitchSelector, interfaceManager, vpnFootprintService, fibManager,  natDataUtil);
+                    idManager, naptSwitchSelector, interfaceManager, vpnFootprintService, fibManager,  natDataUtil,
+                    eventCallbacks);
         }
         return null;
     }
@@ -106,7 +111,7 @@ public class SnatServiceImplFactory extends AbstractLifecycle {
                     NatConstants.ODL_VNI_POOL_NAME);
             return new VxlanGreConntrackBasedSnatService(dataBroker, mdsalManager, itmManager, odlInterfaceRpcService,
                     idManager, naptSwitchSelector, externalRouterListener, elanManager, interfaceManager,
-                    vpnFootprintService, fibManager, natDataUtil);
+                    vpnFootprintService, fibManager, natDataUtil, eventCallbacks);
         }
         return null;
     }

diff --git a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VxlanGreConntrackBasedSnatService.java b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VxlanGreConntrackBasedSnatService.java
index 672ad4407f479295db00067069587368a7d77082..fa75cbeebf6a3411cec57e70d257ac77c6b98655 100644 (file)
--- a/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VxlanGreConntrackBasedSnatService.java
+++ b/natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/VxlanGreConntrackBasedSnatService.java
@@ -13,6 +13,7 @@ import java.util.Collections;
 import java.util.List;
 
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.genius.datastoreutils.listeners.DataTreeEventCallbackRegistrar;
 import org.opendaylight.genius.infra.Datastore.Configuration;
 import org.opendaylight.genius.infra.TypedReadWriteTransaction;
 import org.opendaylight.genius.infra.TypedWriteTransaction;
@@ -70,9 +71,10 @@ public class VxlanGreConntrackBasedSnatService extends ConntrackBasedSnatService
                                              ExternalRoutersListener externalRouterListener, IElanService elanManager,
                                              IInterfaceManager interfaceManager,
                                              IVpnFootprintService vpnFootprintService,
-                                             IFibManager fibManager, NatDataUtil natDataUtil) {
+                                             IFibManager fibManager, NatDataUtil natDataUtil,
+                                             DataTreeEventCallbackRegistrar eventCallbacks) {
         super(dataBroker, mdsalManager, itmManager, idManager, naptSwitchSelector, odlInterfaceRpcService,
-                interfaceManager, vpnFootprintService, fibManager, natDataUtil);
+                interfaceManager, vpnFootprintService, fibManager, natDataUtil, eventCallbacks);
         this.externalRouterListener = externalRouterListener;
         this.elanManager = elanManager;
     }