ELAN Reboot fixes and Flow ID change

1) Table 51 MAC issue after CSC reboot
Description: This is to avoid stale entries being created.
The solution is to construct a flow key with ELAN tag and
MAC address, which would update the same flow after MAC movement.

2) VM Migration Table 51 stale entry
Description: Issue is observed when ElanLearnVpnVip MIP MAC
and Interface MAC are same. During migration, ElanLearntVpnVipToPortListener
remove deletes MAC entry from ElanInterfaceForwardingEntries,
which is referred by ElanInterfaceStateChangeListener remove workflow,
which is started after the ElanInterfaceForwardingEntries thread completes
its course of action. Both threads have an action to delete DMAC flow entries
but the first thread refers InterfaceManager for InterfaceInfo
which returns null, hence skips the delete DMAC workflow, but deletes the
MAC entry. Other thread refers MAC entry, which has been deleted
by first thread, hence this too skips delete workflow.
The solution is to delete MAC entry in first thread only if InterfaceInfo
is not null so that second thread can delete DMAC entry as well as delete
MAC entry during its course of action.

Signed-off-by: Amitesh Soni <amitesh.soni@ericsson.com>
Change-Id: I9a2b72e0d70a2edf9072c77fc35314e1b7a2c46d

diff --git a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanInterfaceManager.java b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanInterfaceManager.java
index cb1c5e9fb0c09883ac2075109d0f3daf8abf3868..80c981833d96cbaa2f91ca021e76ea60bfade092 100644 (file)
--- a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanInterfaceManager.java
+++ b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanInterfaceManager.java
@@ -520,8 +520,8 @@ public class ElanInterfaceManager extends AsyncDataTreeChangeListenerBase<ElanIn
                             continue;
                         }
                         for (MacEntry mac : macs.getMacEntry()) {
-                            removeTheMacFlowInTheDPN(dpId, elanTag, currentDpId, mac, confTx);
-                            removeEtreeMacFlowInTheDPN(dpId, elanTag, currentDpId, mac, confTx);
+                            removeTheMacFlowInTheDPN(dpId, elanTag, mac, confTx);
+                            removeEtreeMacFlowInTheDPN(dpId, elanTag, mac, confTx);
                         }
                     }
                 }
@@ -529,22 +529,20 @@ public class ElanInterfaceManager extends AsyncDataTreeChangeListenerBase<ElanIn
         }), LOG, "Error deleting remote MACs in DPN {}", dpId);
     }
 
-    private void removeEtreeMacFlowInTheDPN(Uint64 dpId, Uint32 elanTag, Uint64 currentDpId, MacEntry mac,
+    private void removeEtreeMacFlowInTheDPN(Uint64 dpId, Uint32 elanTag, MacEntry mac,
             TypedReadWriteTransaction<Configuration> confTx) throws ExecutionException, InterruptedException {
         EtreeLeafTagName etreeLeafTag = elanEtreeUtils.getEtreeLeafTagByElanTag(elanTag.longValue());
         if (etreeLeafTag != null) {
-            removeTheMacFlowInTheDPN(dpId, etreeLeafTag.getEtreeLeafTag().getValue(),
-                currentDpId, mac, confTx);
+            removeTheMacFlowInTheDPN(dpId, etreeLeafTag.getEtreeLeafTag().getValue(), mac, confTx);
         }
     }
 
-    private void removeTheMacFlowInTheDPN(Uint64 dpId, Uint32 elanTag, Uint64 currentDpId, MacEntry mac,
+    private void removeTheMacFlowInTheDPN(Uint64 dpId, Uint32 elanTag, MacEntry mac,
             TypedReadWriteTransaction<Configuration> confTx) throws ExecutionException, InterruptedException {
         mdsalManager
                 .removeFlow(confTx, dpId,
                         MDSALUtil.buildFlow(NwConstants.ELAN_DMAC_TABLE,
-                                ElanUtils.getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, dpId, currentDpId,
-                                        mac.getMacAddress().getValue(), elanTag)));
+                                ElanUtils.getKnownDynamicmacFlowRef(elanTag, mac.getMacAddress().getValue())));
     }
 
     /*

diff --git a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
index a1544cff8a88e76ca05eeb5833da093cae90d073..9576757d2c2cd8976443dbaa28da250b57c403cb 100644 (file)
--- a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
+++ b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
@@ -18,17 +18,21 @@ import com.google.common.util.concurrent.MoreExecutors;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.ExecutionException;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.genius.infra.Datastore;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunner;
 import org.opendaylight.genius.infra.ManagedNewTransactionRunnerImpl;
+import org.opendaylight.genius.infra.TypedReadWriteTransaction;
 import org.opendaylight.genius.interfacemanager.globals.InterfaceInfo;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
 import org.opendaylight.genius.mdsalutil.MetaDataUtil;
 import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock;
 import org.opendaylight.netvirt.elan.cache.ElanInstanceCache;
 import org.opendaylight.netvirt.elan.utils.ElanConstants;
 import org.opendaylight.netvirt.elan.utils.ElanUtils;
@@ -41,6 +45,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.Node
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.SalFlowListener;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.service.rev130819.SwitchFlowRemoved;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.meta.rev160406._if.indexes._interface.map.IfIndexInterface;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.instances.ElanInstance;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.elan.tag.name.map.ElanTagName;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.elan.rev150602.forwarding.entries.MacEntry;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
@@ -113,13 +118,25 @@ public class ElanSmacFlowEventListener implements SalFlowListener {
                 InterfaceInfo interfaceInfo = interfaceManager.getInterfaceInfo(interfaceName);
                 String elanInstanceName = elanTagInfo.getName();
                 LOG.info("Deleting the Mac-Entry:{} present on ElanInstance:{}", macEntry, elanInstanceName);
-                if (macEntry != null && interfaceInfo != null) {
-                    ListenableFuture<Void> result = txRunner.callWithNewReadWriteTransactionAndSubmit(CONFIGURATION,
-                        tx -> elanUtils.deleteMacFlows(elanInstanceCache.get(elanInstanceName).orNull(),
-                                interfaceInfo, macEntry, tx));
-                    elanFutures.add(result);
-                    addCallBack(result, srcMacAddress);
-                }
+                ListenableFuture<Void> result = txRunner.callWithNewReadWriteTransactionAndSubmit(CONFIGURATION, tx -> {
+                    if (macEntry != null && interfaceInfo != null) {
+                        deleteSmacAndDmacFlows(elanInstanceCache.get(elanInstanceName).orNull(),
+                                interfaceInfo, srcMacAddress, tx);
+                    } else if (macEntry == null) { //Remove flow of src flow entry only for MAC movement
+                        MacEntry macEntryOfElanForwarding = elanUtils.getMacEntryForElanInstance(elanTagInfo.getName(),
+                                physAddress).orNull();
+                        if (macEntryOfElanForwarding != null) {
+                            String macAddress = macEntryOfElanForwarding.getMacAddress().getValue();
+                            elanUtils.deleteSmacFlowOnly(elanInstanceCache.get(elanInstanceName).orNull(),
+                                    interfaceInfo, macAddress, tx);
+                        } else {
+                            deleteSmacAndDmacFlows(elanInstanceCache.get(elanInstanceName).orNull(), interfaceInfo,
+                                    srcMacAddress, tx);
+                        }
+                    }
+                });
+                elanFutures.add(result);
+                addCallBack(result, srcMacAddress);
                 InstanceIdentifier<MacEntry> macEntryIdForElanInterface = ElanUtils
                         .getInterfaceMacEntriesIdentifierOperationalDataPath(interfaceName, physAddress);
                 Optional<MacEntry> existingInterfaceMacEntry = ElanUtils.read(broker,
@@ -160,6 +177,18 @@ public class ElanSmacFlowEventListener implements SalFlowListener {
         }, MoreExecutors.directExecutor());
     }
 
+    private void deleteSmacAndDmacFlows(ElanInstance elanInfo, InterfaceInfo interfaceInfo, String macAddress,
+                                        TypedReadWriteTransaction<Datastore.Configuration> deleteFlowTx)
+            throws ExecutionException, InterruptedException {
+        if (elanInfo == null || interfaceInfo == null) {
+            return;
+        }
+        try (NamedSimpleReentrantLock.Acquired lock = elanUtils.lockElanMacDPN(elanInfo.getElanTag().toJava(),
+                macAddress, interfaceInfo.getDpId())) {
+            elanUtils.deleteMacFlows(elanInfo, interfaceInfo, macAddress, /* alsoDeleteSMAC */ true, deleteFlowTx);
+        }
+    }
+
     @Override
     public void onFlowUpdated(FlowUpdated arg0) {
         // TODO Auto-generated method stub

diff --git a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/utils/ElanUtils.java b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/utils/ElanUtils.java
index 60563c855c3f21d7b4603d758b043adb99412d2a..850a2e08c5cbc3e5459d486baba8de3d6f1bb99f 100755 (executable)
--- a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/utils/ElanUtils.java
+++ b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/utils/ElanUtils.java
@@ -28,6 +28,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Locale;
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
@@ -670,7 +671,7 @@ public class ElanUtils {
         return new FlowEntityBuilder()
             .setDpnId(dpId)
             .setTableId(NwConstants.ELAN_SMAC_TABLE)
-            .setFlowId(getKnownDynamicmacFlowRef(NwConstants.ELAN_SMAC_TABLE, dpId, lportTag, macAddress, elanTag))
+            .setFlowId(getKnownDynamicmacFlowRef(elanTag, macAddress))
             .setPriority(20)
             .setFlowName(elanInfo.getDescription())
             .setIdleTimeOut((int) macTimeout)
@@ -892,18 +893,8 @@ public class ElanUtils {
         }
     }
 
-    public static String getKnownDynamicmacFlowRef(short tableId, Uint64 dpId, long lporTag, String macAddress,
-            Uint32 elanTag) {
-        return String.valueOf(tableId) + elanTag + dpId + lporTag + macAddress;
-    }
-
-    public static String getKnownDynamicmacFlowRef(short tableId, Uint64 dpId, Uint64 remoteDpId,
-            String macAddress, Uint32 elanTag) {
-        return String.valueOf(tableId) + elanTag + dpId + remoteDpId + macAddress;
-    }
-
-    public static String getKnownDynamicmacFlowRef(short tableId, Uint64 dpId, String macAddress, Uint32 elanTag) {
-        return String.valueOf(tableId) + elanTag + dpId + macAddress;
+    public static String getKnownDynamicmacFlowRef(Uint32 elanTag, String macAddress) {
+        return new StringBuffer().append(elanTag).append(macAddress.toLowerCase(Locale.getDefault())).toString();
     }
 
     public static String getKnownDynamicmacFlowRef(short elanDmacTable, Uint64 dpId, String extDeviceNodeId,
@@ -945,7 +936,7 @@ public class ElanUtils {
         List<Action> actions = getEgressActionsForInterface(ifName, /* tunnelKey */ null);
         mkInstructions.add(MDSALUtil.buildApplyActionsInstruction(actions));
         Flow flow = MDSALUtil.buildFlowNew(NwConstants.ELAN_DMAC_TABLE,
-                getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, dpId, ifTag, macAddress, elanTag), 20,
+                getKnownDynamicmacFlowRef(elanTag, macAddress), 20,
                 elanInfo.getElanInstanceName(), 0, 0,
                 Uint64.valueOf(ElanConstants.COOKIE_ELAN_KNOWN_DMAC.longValue() + elanTag.longValue()),
                 mkMatches, mkInstructions);
@@ -1044,7 +1035,7 @@ public class ElanUtils {
         }
 
         Flow flow = MDSALUtil.buildFlowNew(NwConstants.ELAN_DMAC_TABLE,
-                getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, srcDpId, destDpId, macAddress, elanTag),
+                getKnownDynamicmacFlowRef(elanTag, macAddress),
                 20, /* prio */
                 displayName, 0, /* idleTimeout */
                 0, /* hardTimeout */
@@ -1110,30 +1101,39 @@ public class ElanUtils {
         } else if (isDpnPresent(dstDpId)) {
             mdsalManager
                 .removeFlow(flowTx, dstDpId,
-                    MDSALUtil.buildFlow(NwConstants.ELAN_DMAC_TABLE, getKnownDynamicmacFlowRef(
-                        NwConstants.ELAN_DMAC_TABLE, dstDpId, srcdpId, macAddress, elanTag)));
+                    MDSALUtil.buildFlow(NwConstants.ELAN_DMAC_TABLE, getKnownDynamicmacFlowRef(elanTag, macAddress)));
             LOG.debug("Dmac flow entry deleted for elan:{}, logical interface port:{} and mac address:{} on dpn:{}",
                     elanInstanceName, interfaceInfo.getPortName(), macAddress, dstDpId);
         }
         return isFlowsRemovedInSrcDpn;
     }
 
+    public void deleteSmacFlowOnly(ElanInstance elanInfo, InterfaceInfo interfaceInfo, String macAddress,
+                                   TypedReadWriteTransaction<Configuration> flowTx) throws ExecutionException,
+            InterruptedException {
+        Uint64 srcdpId = interfaceInfo.getDpId();
+        Uint32 elanTag = elanInfo.getElanTag();
+        LOG.debug("Deleting SMAC flow with id {}", getKnownDynamicmacFlowRef(elanTag, macAddress));
+        mdsalManager.removeFlow(flowTx, srcdpId,
+                MDSALUtil.buildFlow(NwConstants.ELAN_SMAC_TABLE, getKnownDynamicmacFlowRef(elanTag, macAddress)));
+    }
+
     private void deleteSmacAndDmacFlows(ElanInstance elanInfo, InterfaceInfo interfaceInfo, String macAddress,
             boolean deleteSmac, TypedReadWriteTransaction<Configuration> flowTx)
             throws ExecutionException, InterruptedException {
         String elanInstanceName = elanInfo.getElanInstanceName();
-        long ifTag = interfaceInfo.getInterfaceTag();
         Uint64 srcdpId = interfaceInfo.getDpId();
         Uint32 elanTag = elanInfo.getElanTag();
         if (deleteSmac) {
+            LOG.debug("Deleting SMAC flow with id {}", getKnownDynamicmacFlowRef(elanTag, macAddress));
             mdsalManager
                     .removeFlow(flowTx, srcdpId,
-                            MDSALUtil.buildFlow(NwConstants.ELAN_SMAC_TABLE, getKnownDynamicmacFlowRef(
-                                    NwConstants.ELAN_SMAC_TABLE, srcdpId, ifTag, macAddress, elanTag)));
+                            MDSALUtil.buildFlow(NwConstants.ELAN_SMAC_TABLE,
+                                    getKnownDynamicmacFlowRef(elanTag, macAddress)));
         }
         mdsalManager.removeFlow(flowTx, srcdpId,
             MDSALUtil.buildFlow(NwConstants.ELAN_DMAC_TABLE,
-                getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, srcdpId, ifTag, macAddress, elanTag)));
+                getKnownDynamicmacFlowRef(elanTag, macAddress)));
         LOG.debug("All the required flows deleted for elan:{}, logical Interface port:{} and MAC address:{} on dpn:{}",
                 elanInstanceName, interfaceInfo.getPortName(), macAddress, srcdpId);
     }
@@ -1479,7 +1479,7 @@ public class ElanUtils {
         LoggingFutures.addErrorLogging(
             txRunner.callWithNewReadWriteTransactionAndSubmit(Datastore.CONFIGURATION, tx -> {
                 for (Uint64 dpId : dpnIds) {
-                    String flowId = getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, dpId, macAddress, elanTag);
+                    String flowId = getKnownDynamicmacFlowRef(elanTag, macAddress);
                     mdsalManager.removeFlow(tx, dpId, MDSALUtil.buildFlow(NwConstants.ELAN_DMAC_TABLE, flowId));
                 }
             }), LOG, "Error removing DMAC redirect to dispatcher flows");
@@ -1496,7 +1496,7 @@ public class ElanUtils {
         actions.add(new ActionNxResubmit(NwConstants.LPORT_DISPATCHER_TABLE));
 
         instructions.add(new InstructionApplyActions(actions));
-        String flowId = getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE, dpId, dstMacAddress, elanTag);
+        String flowId = getKnownDynamicmacFlowRef(elanTag, dstMacAddress);
         return MDSALUtil.buildFlowEntity(dpId, NwConstants.ELAN_DMAC_TABLE, flowId, 20, displayName, 0, 0,
                 Uint64.valueOf(ElanConstants.COOKIE_ELAN_KNOWN_DMAC.longValue() + elanTag.longValue()),
                 matches, instructions);

diff --git a/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/ElanServiceTest.java b/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/ElanServiceTest.java
index b89107107d86a36366f461eb40c268851ed4c099..17e67e5067e4efd00fedc7fa080c790c47afe7e0 100644 (file)
--- a/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/ElanServiceTest.java
+++ b/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/ElanServiceTest.java
@@ -239,10 +239,7 @@ public class ElanServiceTest extends  ElanServiceTestBase {
 
         // Read and Compare SMAC flow
         String flowId = new StringBuilder()
-                .append(NwConstants.ELAN_SMAC_TABLE)
                 .append(actualElanInstances.getElanTag())
-                .append(DPN1_ID)
-                .append(interfaceInfo.getInterfaceTag())
                 .append(interfaceInfo.getMacAddress())
                 .toString();
         InstanceIdentifier<Flow> flowInstanceIidSrc = getFlowIid(NwConstants.ELAN_SMAC_TABLE,
@@ -272,10 +269,7 @@ public class ElanServiceTest extends  ElanServiceTestBase {
 
         // Read DMAC Flow in DPN1
         String flowId = new StringBuilder()
-                .append(NwConstants.ELAN_DMAC_TABLE)
                 .append(actualElanInstances.getElanTag())
-                .append(DPN1_ID)
-                .append(interfaceInfo.getInterfaceTag())
                 .append(interfaceInfo.getMacAddress())
                 .toString();
         InstanceIdentifier<Flow> flowInstanceIidDst = getFlowIid(NwConstants.ELAN_DMAC_TABLE,
@@ -306,11 +300,8 @@ public class ElanServiceTest extends  ElanServiceTestBase {
         addElanInterface(ExpectedObjects.ELAN1, interfaceInfo, DPN2IP1);
 
         // Read and Compare DMAC flow in DPN1 for MAC1 of DPN2
-        String flowId = ElanUtils.getKnownDynamicmacFlowRef((short)51,
-                DPN1_ID,
-                DPN2_ID,
-                interfaceInfo.getMacAddress().toString(),
-                actualElanInstances.getElanTag());
+        String flowId = ElanUtils.getKnownDynamicmacFlowRef(actualElanInstances.getElanTag(),
+                interfaceInfo.getMacAddress());
 
         InstanceIdentifier<Flow> flowInstanceIidDst = getFlowIid(NwConstants.ELAN_DMAC_TABLE,
                 new FlowId(flowId), DPN1_ID);

diff --git a/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/Verifications.java b/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/Verifications.java
index 10c032e270a2b34645dfc84d20ce9eb1c5cbdbc2..f7f79cf70b075cd407eb1804b0a783c7d1d97f13 100644 (file)
--- a/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/Verifications.java
+++ b/elanmanager/impl/src/test/java/org/opendaylight/netvirt/elanmanager/tests/Verifications.java
@@ -389,11 +389,7 @@ public class Verifications {
                 .child(ElanInstance.class, new ElanInstanceKey(ExpectedObjects.ELAN1)).build();
         ElanInstance actualElanInstances = singleTxdataBroker.syncRead(CONFIGURATION, elanInstanceIid);
         FlowId flowId = new FlowId(
-                ElanUtils.getKnownDynamicmacFlowRef(NwConstants.ELAN_DMAC_TABLE,
-                        srcDpnId,
-                        dpnId,
-                        dpnMac,
-                        actualElanInstances.getElanTag()));
+                ElanUtils.getKnownDynamicmacFlowRef(actualElanInstances.getElanTag(), dpnMac));
         InstanceIdentifier<Flow> flowInstanceIidDst = getFlowIid(NwConstants.ELAN_DMAC_TABLE, flowId, srcDpnId);
         if (createFlag) {
             awaitForData(LogicalDatastoreType.CONFIGURATION, flowInstanceIidDst);