Traffic not flowing after delete and add TEP

Two issues are addressed in this patch:

Issue-1:
-------
During controller restart, while processing onSubnetAddedToVpn,
VpnInstanceOpData was not populated due to race conditions. Due to
this, further processing breaks.

Solution-1:
-----------
VpnOpDataSyncer is used to wait for VpnInstanceOpData to be available
during processing of onSubnetAddedToVpn.

Issue-2:
-------
After controller restart, LearntIp adjacencies were lost for
VpnInterfaces because of out of order events. When VpnInterface-add
event comes first followed by Neutron port event, it was not
populating learnt IPs in the adjacency list. Hence the remote groups
refered by table 21 flows related to LearntIps were not updated during
TEP delete/add. It would remain as stale. It was resulting in traffic
loss.

Solution-2:
-----------
After controller restart, LearntIp adjacencies are populated back for
VpnInterfaces.

JIRA: NETVIRT-1356

Change-Id: If21ebf1d330ae3f2970c8fedafb0422cc4d38bdc
Signed-off-by: Somashekar Byrappa <somashekar.b@altencalsoftlabs.com>

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
index 42f07a2cc43f3b373fee8db7bbf44377316cbbd8..668782db79682bd74b7f308be88522ef405c6070 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronPortChangeListener.java
@@ -134,6 +134,7 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
 
     @Override
     protected void add(InstanceIdentifier<Port> identifier, Port input) {
+        LOG.trace("Received port add event: port={}", input);
         String portName = input.getUuid().getValue();
         LOG.trace("Adding Port : key: {}, value={}", identifier, input);
         Network network = neutronvpnUtils.getNeutronNetwork(input.getNetworkId());
@@ -204,6 +205,7 @@ public class NeutronPortChangeListener extends AsyncDataTreeChangeListenerBase<P
 
     @Override
     protected void update(InstanceIdentifier<Port> identifier, Port original, Port update) {
+        LOG.trace("Received port update event: original={}, update={}", original, update);
         // Switchdev ports need to be bounded to a host before creation
         // in order to validate the supported vnic types from the hostconfig
         if (isPortTypeSwitchdev(original)

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
index 0c52cdf6be298f813cc6fb3f7d9ab18b9ac23e60..d393085460878d717c984ab1b616829178b42d1e 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnManager.java
@@ -847,8 +847,10 @@ public class NeutronvpnManager implements NeutronvpnService, AutoCloseable, Even
         if (port.getDeviceOwner() != null) {
             isRouterInterface = NeutronConstants.DEVICE_OWNER_ROUTER_INF.equals(port.getDeviceOwner());
         }
-        Adjacencies adjs = createPortIpAdjacencies(port, isRouterInterface, wrtConfigTxn, null, null);
         String infName = port.getUuid().getValue();
+        // Handling cluster reboot scenario where VpnInterface already exists in datastore.
+        VpnInterface vpnIface = VpnHelper.getVpnInterface(dataBroker, infName);
+        Adjacencies adjs = createPortIpAdjacencies(port, isRouterInterface, wrtConfigTxn, null, vpnIface);
         LOG.trace("createVpnInterface for Port: {}, isRouterInterface: {}", infName, isRouterInterface);
         writeVpnInterfaceToDs(vpnIds, infName, adjs, port.getNetworkId(), isRouterInterface, wrtConfigTxn);
     }

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/SubnetRoutePacketInHandler.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/SubnetRoutePacketInHandler.java
index 2a7759a797c289366e1904c0da4fce1d58f0a7c5..75caa8523ed3bac5f9b182776ef8aa6aaee105f0 100644 (file)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/SubnetRoutePacketInHandler.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/SubnetRoutePacketInHandler.java
@@ -270,8 +270,8 @@ public class SubnetRoutePacketInHandler implements PacketProcessingListener {
             String dstIpAddress, long elanTag) throws UnknownHostException {
         long groupid = VpnUtil.getRemoteBCGroup(elanTag);
         if (NWUtil.isIpv4Address(dstIpAddress)) {
-            LOG.debug("Sending ARP: srcIp={}, srcMac={}, dstIp={}, dpId={}, elan-tag={}", sourceIpAddress, sourceMac,
-                    dstIpAddress, dpnId, elanTag);
+            LOG.debug("Sending ARP: srcIp={}, srcMac={}, dstIp={}, dpId={}, elan-tag={}, groupid={}", sourceIpAddress,
+                    sourceMac, dstIpAddress, dpnId, elanTag, groupid);
             vpnManagerCounters.subnetRoutePacketArpSent();
 
             TransmitPacketInput packetInput =
@@ -280,8 +280,8 @@ public class SubnetRoutePacketInHandler implements PacketProcessingListener {
             JdkFutures.addErrorLogging(packetService.transmitPacket(packetInput), LOG, "Transmit packet");
         } else {
             // IPv6 case
-            LOG.debug("Sending NS: srcIp={}, srcMac={}, dstIp={}, dpId={}, elan-tag={}", sourceIpAddress, sourceMac,
-                    dstIpAddress, dpnId, elanTag);
+            LOG.debug("Sending NS: srcIp={}, srcMac={}, dstIp={}, dpId={}, elan-tag={}, groupid={}", sourceIpAddress,
+                    sourceMac, dstIpAddress, dpnId, elanTag, groupid);
             vpnManagerCounters.subnetRoutePacketNsSent();
 
             VpnUtil.sendNeighborSolicationToOfGroup(this.ipv6NdUtilService, new Ipv6Address(sourceIpAddress),

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
index 4695f30c582a182a71cf4b67e6c11393827ecdc8..b46aa0981f543067e96ef09aea00bf74718d316c 100755 (executable)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
@@ -212,9 +212,9 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
 
     @Override
     public void add(final InstanceIdentifier<VpnInterface> identifier, final VpnInterface vpnInterface) {
-        LOG.info("add: intfName {} onto vpnName {}",
-                 vpnInterface.getName(),
-                 VpnHelper.getVpnInterfaceVpnInstanceNamesString(vpnInterface.getVpnInstanceNames()));
+        LOG.trace("Received VpnInterface add event: vpnInterface={}", vpnInterface);
+        LOG.info("add: intfName {} onto vpnName {}", vpnInterface.getName(),
+                VpnHelper.getVpnInterfaceVpnInstanceNamesString(vpnInterface.getVpnInstanceNames()));
         addVpnInterface(identifier, vpnInterface, null, null);
     }
 
@@ -1219,6 +1219,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
 
     @Override
     public void remove(InstanceIdentifier<VpnInterface> identifier, VpnInterface vpnInterface) {
+        LOG.trace("Received VpnInterface remove event: vpnInterface={}", vpnInterface);
         final VpnInterfaceKey key = identifier.firstKeyOf(VpnInterface.class, VpnInterfaceKey.class);
         final String interfaceName = key.getName();
         for (VpnInstanceNames vpnInterfaceVpnInstance : requireNonNullElse(vpnInterface.getVpnInstanceNames(),
@@ -1542,9 +1543,9 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
     @Override
     protected void update(final InstanceIdentifier<VpnInterface> identifier, final VpnInterface original,
         final VpnInterface update) {
-        LOG.info("update: VPN Interface update event - intfName {} on dpn {} oldVpn {} newVpn {}" ,update.getName(),
-                update.getDpnId(), original.getVpnInstanceNames(),
-                update.getVpnInstanceNames());
+        LOG.trace("Received VpnInterface update event: original={}, update={}", original, update);
+        LOG.info("update: VPN Interface update event - intfName {} on dpn {} oldVpn {} newVpn {}", update.getName(),
+                update.getDpnId(), original.getVpnInstanceNames(), update.getVpnInstanceNames());
         final String vpnInterfaceName = update.getName();
         final BigInteger dpnId = InterfaceUtils.getDpnForInterface(ifaceMgrRpcService, vpnInterfaceName);
         LOG.info("VPN Interface update event - intfName {}", vpnInterfaceName);

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnSubnetRouteHandler.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnSubnetRouteHandler.java
index 10230883fb0da19e00ff6cd714cb29a116303498..59806511365a660b40840453193d0f88437df6aa 100644 (file)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnSubnetRouteHandler.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnSubnetRouteHandler.java
@@ -36,8 +36,6 @@ import org.opendaylight.netvirt.vpnmanager.populator.registry.L3vpnRegistry;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.state.Interface.OperStatus;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.yang.types.rev130715.Uuid;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.idmanager.rev160406.IdManagerService;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.LockManagerService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.fibmanager.rev150330.vrfentries.VrfEntry;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.PortOpData;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.SubnetOpData;
@@ -67,8 +65,6 @@ public class VpnSubnetRouteHandler {
     private final DataBroker dataBroker;
     private final SubnetOpDpnManager subOpDpnManager;
     private final IBgpManager bgpManager;
-    private final IdManagerService idManager;
-    private final LockManagerService lockManager;
     private final VpnOpDataSyncer vpnOpDataSyncer;
     private final VpnNodeListener vpnNodeListener;
     private final IFibManager fibManager;
@@ -76,14 +72,11 @@ public class VpnSubnetRouteHandler {
 
     @Inject
     public VpnSubnetRouteHandler(final DataBroker dataBroker, final SubnetOpDpnManager subnetOpDpnManager,
-            final IBgpManager bgpManager, final IdManagerService idManager,
-            LockManagerService lockManagerService, final VpnOpDataSyncer vpnOpDataSyncer,
-        final VpnNodeListener vpnNodeListener, final IFibManager fibManager, VpnUtil vpnUtil) {
+            final IBgpManager bgpManager, final VpnOpDataSyncer vpnOpDataSyncer, final VpnNodeListener vpnNodeListener,
+            final IFibManager fibManager, VpnUtil vpnUtil) {
         this.dataBroker = dataBroker;
         this.subOpDpnManager = subnetOpDpnManager;
         this.bgpManager = bgpManager;
-        this.idManager = idManager;
-        this.lockManager = lockManagerService;
         this.vpnOpDataSyncer = vpnOpDataSyncer;
         this.vpnNodeListener = vpnNodeListener;
         this.fibManager = fibManager;
@@ -106,24 +99,28 @@ public class VpnSubnetRouteHandler {
                 LOGGING_PREFIX + " onSubnetAddedToVpn: SubnetPrefix cannot be null or empty!");
         Preconditions.checkNotNull(elanTag, LOGGING_PREFIX + " onSubnetAddedToVpn: ElanTag cannot be null or empty!");
 
-        String vpnName;
-        if (subnetmap.getVpnId() != null) {
-            vpnName = subnetmap.getVpnId().getValue();
-            long vpnId = vpnUtil.getVpnId(vpnName);
-            if (vpnId == VpnConstants.INVALID_ID) {
-                vpnOpDataSyncer.waitForVpnDataReady(VpnOpDataType.vpnInstanceToId, vpnName,
-                        VpnConstants.PER_VPN_INSTANCE_MAX_WAIT_TIME_IN_MILLISECONDS);
-                vpnId = vpnUtil.getVpnId(vpnName);
-                if (vpnId == VpnConstants.INVALID_ID) {
-                    LOG.error("{} onSubnetAddedToVpn: VpnInstance to VPNId mapping not yet available for VpnName {} "
-                              + "processing subnet {} with IP {}, bailing out now.", LOGGING_PREFIX, vpnName, subnetId,
-                            subnetIp);
-                    return;
-                }
-            }
-        } else {
+        if (subnetmap.getVpnId() == null) {
             LOG.error("onSubnetAddedToVpn: VpnId {} for subnet {} not found, bailing out", subnetmap.getVpnId(),
-                      subnetId);
+                    subnetId);
+            return;
+        }
+        String vpnName = subnetmap.getVpnId().getValue();
+        long vpnId = waitAndGetVpnIdIfInvalid(vpnName);
+        if (vpnId == VpnConstants.INVALID_ID) {
+            LOG.error(
+                    "{} onSubnetAddedToVpn: VpnInstance to VPNId mapping not yet available for VpnName {} "
+                            + "processing subnet {} with IP {}, bailing out now.",
+                    LOGGING_PREFIX, vpnName, subnetId, subnetIp);
+            return;
+        }
+
+        String primaryRd = vpnUtil.getPrimaryRd(vpnName);
+        VpnInstanceOpDataEntry vpnInstanceOpData = waitAndGetVpnInstanceOpDataIfNull(vpnName, primaryRd);
+        if (vpnInstanceOpData == null) {
+            LOG.error(
+                    "{} onSubnetAddedToVpn: VpnInstanceOpData not yet available for VpnName {} "
+                            + "processing subnet {} with IP {}, bailing out now.",
+                    LOGGING_PREFIX, vpnName, subnetId, subnetIp);
             return;
         }
         LOG.info("{} onSubnetAddedToVpn: Subnet {} with IP {} being added to vpn {}", LOGGING_PREFIX,
@@ -171,7 +168,6 @@ public class VpnSubnetRouteHandler {
             subOpBuilder = new SubnetOpDataEntryBuilder().withKey(new SubnetOpDataEntryKey(subnetId));
             subOpBuilder.setSubnetId(subnetId);
             subOpBuilder.setSubnetCidr(subnetIp);
-            String primaryRd = vpnUtil.getPrimaryRd(vpnName);
 
             if (isBgpVpn && !VpnUtil.isBgpVpn(vpnName, primaryRd)) {
                 LOG.error("{} onSubnetAddedToVpn: The VPN Instance name {} does not have RD. Bailing out for"
@@ -183,7 +179,7 @@ public class VpnSubnetRouteHandler {
             subOpBuilder.setSubnetToDpn(new ArrayList<>());
             subOpBuilder.setRouteAdvState(TaskState.Idle);
             subOpBuilder.setElanTag(elanTag);
-            Long l3Vni = vpnUtil.getVpnInstanceOpData(primaryRd).getL3vni();
+            Long l3Vni = vpnInstanceOpData.getL3vni();
             subOpBuilder.setL3vni(l3Vni);
             subOpEntry = subOpBuilder.build();
             SingleTransactionDataBroker.syncWrite(dataBroker, LogicalDatastoreType.OPERATIONAL, subOpIdentifier,
@@ -280,6 +276,28 @@ public class VpnSubnetRouteHandler {
         }
     }
 
+    private long waitAndGetVpnIdIfInvalid(String vpnName) {
+        long vpnId = vpnUtil.getVpnId(vpnName);
+        if (vpnId == VpnConstants.INVALID_ID) {
+            LOG.debug("VpnId is invalid, waiting to fetch again: vpnName={}, vpnId={}", vpnName, vpnId);
+            vpnOpDataSyncer.waitForVpnDataReady(VpnOpDataType.vpnInstanceToId, vpnName,
+                    VpnConstants.PER_VPN_INSTANCE_MAX_WAIT_TIME_IN_MILLISECONDS);
+            vpnId = vpnUtil.getVpnId(vpnName);
+        }
+        return vpnId;
+    }
+
+    private VpnInstanceOpDataEntry waitAndGetVpnInstanceOpDataIfNull(String vpnName, String primaryRd) {
+        VpnInstanceOpDataEntry vpnInstanceOpData = vpnUtil.getVpnInstanceOpData(primaryRd);
+        if (vpnInstanceOpData == null) {
+            LOG.debug("vpnInstanceOpData is null, waiting to fetch again: vpnName={}", vpnName);
+            vpnOpDataSyncer.waitForVpnDataReady(VpnOpDataType.vpnOpData, vpnName,
+                    VpnConstants.PER_VPN_INSTANCE_OPDATA_MAX_WAIT_TIME_IN_MILLISECONDS);
+            vpnInstanceOpData = vpnUtil.getVpnInstanceOpData(primaryRd);
+        }
+        return vpnInstanceOpData;
+    }
+
     // TODO Clean up the exception handling
     @SuppressWarnings("checkstyle:IllegalCatch")
     public void onSubnetDeletedFromVpn(Subnetmap subnetmap, boolean isBgpVpn) {

diff --git a/vpnmanager/impl/src/test/java/org/opendaylight/netvirt/vpnmanager/test/VpnSubnetRouteHandlerTest.java b/vpnmanager/impl/src/test/java/org/opendaylight/netvirt/vpnmanager/test/VpnSubnetRouteHandlerTest.java
index de954ea88db3de401f0a5470028e75745d3806b6..b04e31549c894a291c2e9f5499c86c951e646316 100644 (file)
--- a/vpnmanager/impl/src/test/java/org/opendaylight/netvirt/vpnmanager/test/VpnSubnetRouteHandlerTest.java
+++ b/vpnmanager/impl/src/test/java/org/opendaylight/netvirt/vpnmanager/test/VpnSubnetRouteHandlerTest.java
@@ -208,8 +208,8 @@ public class VpnSubnetRouteHandlerTest {
     public void setUp() throws Exception {
         setupMocks();
 
-        vpnSubnetRouteHandler = new VpnSubnetRouteHandler(dataBroker, subnetOpDpnManager, bgpManager,
-            idManager, lockManagerService, vpnOpDataSyncer, vpnNodeListener, fibManager, vpnUtil);
+        vpnSubnetRouteHandler = new VpnSubnetRouteHandler(dataBroker, subnetOpDpnManager, bgpManager, vpnOpDataSyncer,
+                vpnNodeListener, fibManager, vpnUtil);
         final Future<RpcResult<AllocateIdOutput>> idOutputOptional =
             RpcResultBuilder.success(allocateIdOutput).buildFuture();