bug-8046 fix for mac movement issue

* updating only elan-interface-forwarding-entries when mac is learned
from vlan provider port which is already learned on VM port.
* updating MACs in elan ds and dpn when MAC is actually moved from VM port
to another VM port.
* handling MAC learning from VLAN provider port if it is a newly learned
mac

Change-Id: I4ff9d79463f0f238c5a8b345438514b93ed4bb17
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@ericsson.com>

diff --git a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
index be876e6a7a992b2943537e5100353ff979099412..cf4fcd330e661be400812a860fff0d862e591ec8 100755 (executable)
--- a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
+++ b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
@@ -12,7 +12,7 @@ import com.google.common.util.concurrent.ListenableFuture;
 
 import java.math.BigInteger;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import org.opendaylight.controller.liblldp.NetUtils;
 import org.opendaylight.controller.liblldp.PacketException;
@@ -96,10 +96,8 @@ public class ElanPacketInHandler implements PacketProcessingListener {
                 PhysAddress physAddress = new PhysAddress(macAddress);
                 ElanInstance elanInstance = ElanUtils.getElanInstanceByName(broker, elanName);
                 InterfaceInfo interfaceInfo = interfaceManager.getInterfaceInfo(interfaceName);
-                MacEntry macEntry = elanUtils.getInterfaceMacEntriesOperationalDataPath(interfaceName, physAddress);
-                if (didMacMigrated(interfaceName, macEntry)) {
-                    return;
-                }
+                MacEntry oldMacEntry = elanUtils.getMacEntryForElanInstance(elanName, physAddress).orNull();
+                boolean isVlanOrFlatProviderIface = interfaceManager.isExternalInterface(interfaceName);
 
                 BigInteger timeStamp = new BigInteger(String.valueOf(System.currentTimeMillis()));
                 MacEntry newMacEntry = new MacEntryBuilder().setInterface(interfaceName).setMacAddress(physAddress)
@@ -107,11 +105,11 @@ public class ElanPacketInHandler implements PacketProcessingListener {
                         .setIsStaticAddress(false).build();
 
                 final DataStoreJobCoordinator portDataStoreCoordinator = DataStoreJobCoordinator.getInstance();
-                enqueueJobForMacSpecificTasks(macAddress, elanTag, interfaceName, elanName, physAddress, newMacEntry,
-                        portDataStoreCoordinator);
+                enqueueJobForMacSpecificTasks(macAddress, elanTag, interfaceName, elanName, physAddress, oldMacEntry,
+                        newMacEntry, isVlanOrFlatProviderIface, portDataStoreCoordinator);
 
                 enqueueJobForDPNSpecificTasks(macAddress, elanTag, interfaceName, physAddress, elanInstance,
-                        interfaceInfo, macEntry, newMacEntry, portDataStoreCoordinator);
+                        interfaceInfo, oldMacEntry, newMacEntry, isVlanOrFlatProviderIface, portDataStoreCoordinator);
 
 
             } catch (PacketException e) {
@@ -120,51 +118,39 @@ public class ElanPacketInHandler implements PacketProcessingListener {
         }
     }
 
-    /**
-     * The code currently has a bug, and this condition will never be true.
-     * I am preserving the same logic.
-     */
-    private boolean didMacMigrated(String interfaceName, MacEntry macEntry) {
-        if (macEntry != null && !macEntry.getInterface().equals(interfaceName)) {
-            long macTimeStamp = macEntry.getControllerLearnedForwardingEntryTimestamp().longValue();
-            if (System.currentTimeMillis() < macTimeStamp + 10000) {
-                // New FEs flood their packets on all interfaces. This
-                // can lead
-                // to many contradicting packet_ins. Ignore all packets
-                // received
-                // within 1s after the first packet_in
-                ElanManagerCounters.unknown_smac_pktin_mac_migration_ignored_due_to_protection.inc();
-                return true;
-            }
-        }
-        return false;
-    }
-
     private void enqueueJobForMacSpecificTasks(final String macAddress, final long elanTag, String interfaceName,
-            String elanName, PhysAddress physAddress,
-            MacEntry newMacEntry, final DataStoreJobCoordinator portDataStoreCoordinator) {
+            String elanName, PhysAddress physAddress, MacEntry oldMacEntry, MacEntry newMacEntry,
+            final boolean isVlanOrFlatProviderIface, final DataStoreJobCoordinator portDataStoreCoordinator) {
         portDataStoreCoordinator.enqueueJob(ElanUtils.getElanMacKey(elanTag, macAddress), () -> {
-            MacEntry macEntry = elanUtils.getInterfaceMacEntriesOperationalDataPath(interfaceName, physAddress);
             WriteTransaction writeTx = broker.newWriteOnlyTransaction();
-            if (macEntry != null && macEntry.getInterface().equals(interfaceName)) {
+            if (oldMacEntry != null && oldMacEntry.getInterface().equals(interfaceName)) {
+                // This should never occur because of ovs temporary mac learning
                 ElanManagerCounters.unknown_smac_pktin_forwarding_entries_removed.inc();
-            } else if (macEntry != null) {
-                // MAC address has moved. Overwrite the mapping and replace
-                // MAC flows
+            } else if (oldMacEntry != null && !isVlanOrFlatProviderIface) {
+                long macTimeStamp = oldMacEntry.getControllerLearnedForwardingEntryTimestamp().longValue();
+                if (System.currentTimeMillis() > macTimeStamp + 1000) {
+                    InstanceIdentifier<MacEntry> macEntryId = ElanUtils
+                            .getInterfaceMacEntriesIdentifierOperationalDataPath(interfaceName,
+                                    physAddress);
+                    writeTx.delete(LogicalDatastoreType.OPERATIONAL, macEntryId);
+                } else {
+                    // New FEs flood their packets on all interfaces. This
+                    // can lead
+                    // to many contradicting packet_ins. Ignore all packets
+                    // received
+                    // within 1s after the first packet_in
+                    ElanManagerCounters.unknown_smac_pktin_mac_migration_ignored_due_to_protection.inc();
+                }
+            } else if (oldMacEntry != null) {
                 ElanManagerCounters.unknown_smac_pktin_removed_for_relearned.inc();
             }
-            /*
-             * Protection time expired. Even though the MAC has been learnt (it is in the cache) the packets are punted
-             * to controller. Which means, the the flows were not successfully created in the DPN, but the MAC entry has
-             * been added successfully in the cache.
-             *
-             * So, the cache has to be cleared and the flows and cache should be recreated (clearing of cache is
-             * required so that the timestamp is updated).
-             */
-
-            InstanceIdentifier<MacEntry> elanMacEntryId =
-                    ElanUtils.getMacEntryOperationalDataPath(elanName, physAddress);
-            writeTx.put(LogicalDatastoreType.OPERATIONAL, elanMacEntryId, newMacEntry, true);
+            // This check is required only to update elan-forwarding-tables when mac is learned
+            // in ports (example: VM interfaces) other than on vlan provider port.
+            if (!isVlanOrFlatProviderIface && oldMacEntry == null) {
+                InstanceIdentifier<MacEntry> elanMacEntryId =
+                        ElanUtils.getMacEntryOperationalDataPath(elanName, physAddress);
+                writeTx.put(LogicalDatastoreType.OPERATIONAL, elanMacEntryId, newMacEntry, true);
+            }
             List<ListenableFuture<Void>> futures = new ArrayList<>();
             futures.add(writeTx.submit());
             return futures;
@@ -172,38 +158,32 @@ public class ElanPacketInHandler implements PacketProcessingListener {
     }
 
     private void enqueueJobForDPNSpecificTasks(final String macAddress, final long elanTag, String interfaceName,
-            PhysAddress physAddress, ElanInstance elanInstance, InterfaceInfo interfaceInfo, MacEntry macEntry,
-            MacEntry newMacEntry, final DataStoreJobCoordinator portDataStoreCoordinator) {
+            PhysAddress physAddress, ElanInstance elanInstance, InterfaceInfo interfaceInfo,
+            MacEntry oldMacEntry, MacEntry newMacEntry, boolean isVlanOrFlatProviderIface,
+            final DataStoreJobCoordinator portDataStoreCoordinator) {
         portDataStoreCoordinator
-                .enqueueJob(ElanUtils.getElanMacDPNKey(elanTag, macAddress, interfaceInfo.getDpId()), () -> {
-
-                    macMigrationFlowsCleanup(interfaceName, elanInstance, macEntry);
-                    boolean isVlanOrFlatProviderIface = interfaceManager.isExternalInterface(interfaceName);
-
-                    BigInteger dpId = interfaceManager.getDpnForInterface(interfaceName);
-                    elanL2GatewayUtils.scheduleAddDpnMacInExtDevices(elanInstance.getElanInstanceName(), dpId,
-                            Arrays.asList(physAddress));
-
-                    ElanManagerCounters.unknown_smac_pktin_learned.inc();
-
-                    WriteTransaction flowWritetx = broker.newWriteOnlyTransaction();
-                    elanUtils.setupMacFlows(elanInstance, interfaceInfo, elanInstance.getMacTimeout(),
-                            macAddress, !isVlanOrFlatProviderIface, flowWritetx);
-                    InstanceIdentifier<MacEntry> macEntryId =
-                            ElanUtils.getInterfaceMacEntriesIdentifierOperationalDataPath(interfaceName, physAddress);
-                    flowWritetx.put(LogicalDatastoreType.OPERATIONAL, macEntryId, newMacEntry, true);
-
-                    List<ListenableFuture<Void>> futures = new ArrayList<>();
-                    futures.add(flowWritetx.submit());
-                    return futures;
-                });
+            .enqueueJob(ElanUtils.getElanMacDPNKey(elanTag, macAddress, interfaceInfo.getDpId()), () -> {
+                macMigrationFlowsCleanup(interfaceName, elanInstance, oldMacEntry, isVlanOrFlatProviderIface);
+                BigInteger dpId = interfaceManager.getDpnForInterface(interfaceName);
+                elanL2GatewayUtils.scheduleAddDpnMacInExtDevices(elanInstance.getElanInstanceName(), dpId,
+                        Collections.singletonList(physAddress));
+                ElanManagerCounters.unknown_smac_pktin_learned.inc();
+                WriteTransaction flowWritetx = broker.newWriteOnlyTransaction();
+                elanUtils.setupMacFlows(elanInstance, interfaceInfo, elanInstance.getMacTimeout(),
+                        macAddress, !isVlanOrFlatProviderIface, flowWritetx);
+                InstanceIdentifier<MacEntry> macEntryId =
+                        ElanUtils.getInterfaceMacEntriesIdentifierOperationalDataPath(interfaceName, physAddress);
+                flowWritetx.put(LogicalDatastoreType.OPERATIONAL, macEntryId, newMacEntry, true);
+                List<ListenableFuture<Void>> futures = new ArrayList<>();
+                futures.add(flowWritetx.submit());
+                return futures;
+            });
     }
 
-    /**
-     *  This condition is never true because of a bug. Preserving the logic.
-     */
-    private void macMigrationFlowsCleanup(String interfaceName, ElanInstance elanInstance, MacEntry macEntry) {
-        if (macEntry != null && !macEntry.getInterface().equals(interfaceName)) {
+    private void macMigrationFlowsCleanup(String interfaceName, ElanInstance elanInstance, MacEntry macEntry,
+            boolean isVlanOrFlatProviderIface) {
+        if (macEntry != null && !macEntry.getInterface().equals(interfaceName)
+                && !isVlanOrFlatProviderIface) {
             tryAndRemoveInvalidMacEntry(elanInstance.getElanInstanceName(), macEntry);
             ElanManagerCounters.unknown_smac_pktin_flows_removed_for_relearned.inc();
         }
@@ -225,14 +205,15 @@ public class ElanPacketInHandler implements PacketProcessingListener {
         InterfaceInfo oldInterfaceLport = interfaceManager.getInterfaceInfo(macEntry.getInterface());
         if (oldInterfaceLport == null) {
             LOG.warn("MAC {} is been added (either statically or dynamically) on an invalid Logical Port {}. "
-                     + "Manual cleanup may be necessary",
-                     macEntry.getMacAddress(), macEntry.getInterface());
+                    + "Manual cleanup may be necessary",
+                    macEntry.getMacAddress(), macEntry.getInterface());
             return;
         }
         WriteTransaction flowDeletetx = broker.newWriteOnlyTransaction();
         elanUtils.deleteMacFlows(elanInfo, oldInterfaceLport, macEntry, flowDeletetx);
         flowDeletetx.submit();
-        elanL2GatewayUtils.removeMacsFromElanExternalDevices(elanInfo, Arrays.asList(macEntry.getMacAddress()));
+        elanL2GatewayUtils.removeMacsFromElanExternalDevices(elanInfo,
+                Collections.singletonList(macEntry.getMacAddress()));
     }
 
 }

diff --git a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
index 1b212925e39efa09a6660e819e8d77b50abda5ba..bd5906c379197a35e5d15aa9baa03c865905bbd6 100644 (file)
--- a/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
+++ b/vpnservice/elanmanager/elanmanager-impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanSmacFlowEventListener.java
@@ -86,32 +86,33 @@ public class ElanSmacFlowEventListener implements SalFlowListener {
             }
             MacEntry macEntry = elanUtils.getInterfaceMacEntriesOperationalDataPath(interfaceName, physAddress);
             InterfaceInfo interfaceInfo = interfaceManager.getInterfaceInfo(interfaceName);
+            String elanInstanceName = elanTagInfo.getName();
             if (macEntry != null && interfaceInfo != null) {
                 WriteTransaction deleteFlowTx = broker.newWriteOnlyTransaction();
-                elanUtils.deleteMacFlows(ElanUtils.getElanInstanceByName(broker, elanTagInfo.getName()), interfaceInfo,
+                elanUtils.deleteMacFlows(ElanUtils.getElanInstanceByName(broker, elanInstanceName), interfaceInfo,
                         macEntry, deleteFlowTx);
                 ListenableFuture<Void> result = deleteFlowTx.submit();
                 addCallBack(result, srcMacAddress);
             }
             InstanceIdentifier<MacEntry> macEntryIdForElanInterface = ElanUtils
                     .getInterfaceMacEntriesIdentifierOperationalDataPath(interfaceName, physAddress);
-            InstanceIdentifier<MacEntry> macEntryIdForElanInstance = ElanUtils
-                    .getMacEntryOperationalDataPath(elanTagInfo.getName(), physAddress);
             WriteTransaction tx = broker.newWriteOnlyTransaction();
             Optional<MacEntry> existingInterfaceMacEntry = elanUtils.read(broker,
                 LogicalDatastoreType.OPERATIONAL, macEntryIdForElanInterface);
             if (existingInterfaceMacEntry.isPresent()) {
                 tx.delete(LogicalDatastoreType.OPERATIONAL, macEntryIdForElanInterface);
+                MacEntry macEntryInElanInstance = elanUtils.getMacEntryForElanInstance(elanInstanceName,
+                        physAddress).orNull();
+                if (macEntryInElanInstance != null
+                        && macEntryInElanInstance.getInterface().equals(interfaceName)) {
+                    InstanceIdentifier<MacEntry> macEntryIdForElanInstance = ElanUtils
+                            .getMacEntryOperationalDataPath(elanInstanceName, physAddress);
+                    tx.delete(LogicalDatastoreType.OPERATIONAL, macEntryIdForElanInstance);
+                }
+                ListenableFuture<Void> writeResult = tx.submit();
+                addCallBack(writeResult, srcMacAddress);
             }
-            Optional<MacEntry> existingMacEntryForElanInstance = elanUtils.read(broker,
-                LogicalDatastoreType.OPERATIONAL, macEntryIdForElanInstance);
-            if (existingMacEntryForElanInstance.isPresent()) {
-                tx.delete(LogicalDatastoreType.OPERATIONAL, macEntryIdForElanInstance);
-            }
-            ListenableFuture<Void> writeResult = tx.submit();
-            addCallBack(writeResult, srcMacAddress);
         }
-
     }
 
     private void addCallBack(ListenableFuture<Void> writeResult, String srcMacAddress) {