Migrate ElanUtils to use NamedLocks

This use was missed by previous translation due to indirection
created by utility methods.

getElanMacDPNKey() is serving two purposes, really, which is as
a job key for JobCoordinator and also as the locking object.

This patch moves the first use to ElanPacketInHandler, which is
the sole user and removes the intern() call, as JobCoordinator
works on equality.

Second use case is locking of elans, which is migrated to NamedLocks,
as there do not seem to be any other callers who would be using
magic strings involved there. Rather than using strings, we expose
a lockElanMacDPN() method, which looks up and acquires the specific
lock, returning the handle back to caller. Callers are converted
to use normal try-with-resources to ensure the lock is released when
no longer needed.

JIRA: NETVIRT-1510
Change-Id: I0292e0e9a4e51ab98e62be5f01d59cb337ff1421
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/evpn/utils/ElanEvpnFlowUtils.java b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/evpn/utils/ElanEvpnFlowUtils.java
index 5e5b93367ca35dbb9d95d7f4c2698024a1961a5e..19324f133891a9823dc7458bdbd519f1af3d1d13 100644 (file)
--- a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/evpn/utils/ElanEvpnFlowUtils.java
+++ b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/evpn/utils/ElanEvpnFlowUtils.java
@@ -23,6 +23,7 @@ import org.opendaylight.genius.mdsalutil.MDSALUtil;
 import org.opendaylight.genius.mdsalutil.MatchInfo;
 import org.opendaylight.genius.mdsalutil.NwConstants;
 import org.opendaylight.genius.mdsalutil.interfaces.IMdsalApiManager;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.netvirt.elan.utils.ElanConstants;
 import org.opendaylight.netvirt.elan.utils.ElanEtreeUtils;
 import org.opendaylight.netvirt.elan.utils.ElanItmUtils;
@@ -70,7 +71,7 @@ public class ElanEvpnFlowUtils {
 
     public List<ListenableFuture<Void>> evpnDeleteDmacFlowsToExternalMac(EvpnDmacFlow evpnDmacFlow) {
         List<ListenableFuture<Void>> futures = new ArrayList<>();
-        synchronized (ElanUtils.getElanMacDPNKey(evpnDmacFlow.getElanTag(), evpnDmacFlow.getDstMacAddress(),
+        try (Acquired lock = ElanUtils.lockElanMacDPN(evpnDmacFlow.getElanTag(), evpnDmacFlow.getDstMacAddress(),
                 evpnDmacFlow.getDpId())) {
             futures.addAll(
                     evpnRemoveFlowThatSendsThePacketOnAnExternalTunnel(evpnDmacFlow.getElanTag(), evpnDmacFlow.dpId,
@@ -165,12 +166,12 @@ public class ElanEvpnFlowUtils {
     }
 
     static class EvpnDmacFlow {
-        private BigInteger dpId;
-        private String nexthopIP;
-        private long elanTag;
-        private Long vni;
-        private String dstMacAddress;
-        private String elanName;
+        private final BigInteger dpId;
+        private final String nexthopIP;
+        private final long elanTag;
+        private final Long vni;
+        private final String dstMacAddress;
+        private final String elanName;
 
         EvpnDmacFlow(BigInteger dpId, String nexthopIP, long elanTag, Long vni, String dstMacAddress,
                             String elanName) {

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 e368255f6b96606918af4d7d2cdf622b9febf40d..d0ec7557a63934b3b1b8e348c7dc4292c03d9fd6 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
@@ -72,6 +72,7 @@ import org.opendaylight.genius.utils.JvmGlobalLocks;
 import org.opendaylight.genius.utils.ServiceIndex;
 import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
 import org.opendaylight.infrautils.utils.concurrent.LoggingFutures;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.netvirt.elan.cache.ElanInstanceCache;
 import org.opendaylight.netvirt.elan.cache.ElanInterfaceCache;
 import org.opendaylight.netvirt.elan.l2gw.utils.ElanL2GatewayMulticastUtils;
@@ -1138,7 +1139,7 @@ public class ElanInterfaceManager extends AsyncDataTreeChangeListenerBase<ElanIn
                         String macAddress = macEntry.getMacAddress().getValue();
                         LOG.info("Installing remote dmac for mac address {} and interface {}", macAddress,
                             interfaceName);
-                        synchronized (ElanUtils.getElanMacDPNKey(elanInfo.getElanTag(), macAddress,
+                        try (Acquired lock = ElanUtils.lockElanMacDPN(elanInfo.getElanTag(), macAddress,
                             interfaceInfo.getDpId())) {
                             LOG.info("Acquired lock for mac : {}, proceeding with remote dmac install operation",
                                 macAddress);

diff --git a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
index 556a039b7cf7367f1d16e1af4419d3467d8a1afd..2f2934a8fb7cebd36cbd80f98495efc389a6b74f 100755 (executable)
--- a/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
+++ b/elanmanager/impl/src/main/java/org/opendaylight/netvirt/elan/internal/ElanPacketInHandler.java
@@ -201,11 +201,15 @@ public class ElanPacketInHandler implements PacketProcessingListener {
         return "MAC-" + macAddress + " ELAN_TAG-" + elanTag;
     }
 
+    private static String getElanMacDPNKey(long elanTag, String macAddress, BigInteger dpnId) {
+        return "MAC-" + macAddress + " ELAN_TAG-" + elanTag + "DPN_ID-" + dpnId;
+    }
+
     private void enqueueJobForDPNSpecificTasks(final String macAddress, final long elanTag, String interfaceName,
                                                PhysAddress physAddress, ElanInstance elanInstance,
                                                InterfaceInfo interfaceInfo, MacEntry oldMacEntry,
                                                MacEntry newMacEntry, boolean isVlanOrFlatProviderIface) {
-        jobCoordinator.enqueueJob(ElanUtils.getElanMacDPNKey(elanTag, macAddress, interfaceInfo.getDpId()), () -> {
+        jobCoordinator.enqueueJob(getElanMacDPNKey(elanTag, macAddress, interfaceInfo.getDpId()), () -> {
             macMigrationFlowsCleanup(interfaceName, elanInstance, oldMacEntry, isVlanOrFlatProviderIface);
             BigInteger dpId = interfaceManager.getDpnForInterface(interfaceName);
             elanL2GatewayUtils.scheduleAddDpnMacInExtDevices(elanInstance.getElanInstanceName(), dpId,

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 3568592c6396706be65f655e54b438cbe90ef600..a25c8a25eed3c7b10a2475eca48ccbe96fbee4e6 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
@@ -74,6 +74,8 @@ import org.opendaylight.genius.mdsalutil.packet.ARP;
 import org.opendaylight.genius.mdsalutil.packet.Ethernet;
 import org.opendaylight.genius.mdsalutil.packet.IPv4;
 import org.opendaylight.infrautils.utils.concurrent.LoggingFutures;
+import org.opendaylight.infrautils.utils.concurrent.NamedLocks;
+import org.opendaylight.infrautils.utils.concurrent.NamedSimpleReentrantLock.Acquired;
 import org.opendaylight.netvirt.elan.arp.responder.ArpResponderUtil;
 import org.opendaylight.netvirt.elan.cache.ElanInterfaceCache;
 import org.opendaylight.netvirt.elanmanager.api.ElanHelper;
@@ -188,8 +190,38 @@ import org.slf4j.LoggerFactory;
 
 @Singleton
 public class ElanUtils {
+    private static final class ElanLockName {
+        private final String macAddress;
+        private final BigInteger dpnId;
+        private final long elanTag;
+
+        ElanLockName(long elanTag, String macAddress, BigInteger dpnId) {
+            this.elanTag = elanTag;
+            this.macAddress = macAddress;
+            this.dpnId = dpnId;
+        }
+
+        @Override
+        public int hashCode() {
+            return 31 * Long.hashCode(elanTag) + Objects.hash(macAddress, dpnId);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (!(obj instanceof ElanLockName)) {
+                return false;
+            }
+            final ElanLockName other = (ElanLockName) obj;
+            return elanTag == other.elanTag && Objects.equals(macAddress, other.macAddress)
+                    && Objects.equals(dpnId, other.dpnId);
+        }
+    }
 
     private static final Logger LOG = LoggerFactory.getLogger(ElanUtils.class);
+    private static final NamedLocks<ElanLockName> ELAN_LOCKS = new NamedLocks<>();
 
     private final DataBroker broker;
     private final ManagedNewTransactionRunner txRunner;
@@ -565,7 +597,7 @@ public class ElanUtils {
     public void setupMacFlows(ElanInstance elanInfo, InterfaceInfo interfaceInfo,
                               long macTimeout, String macAddress, boolean configureRemoteFlows,
                               TypedWriteTransaction<Configuration> writeFlowGroupTx) {
-        synchronized (getElanMacDPNKey(elanInfo.getElanTag(), macAddress, interfaceInfo.getDpId())) {
+        try (Acquired lock = lockElanMacDPN(elanInfo.getElanTag(), macAddress, interfaceInfo.getDpId())) {
             setupKnownSmacFlow(elanInfo, interfaceInfo, macTimeout, macAddress, mdsalManager,
                 writeFlowGroupTx);
             setupOrigDmacFlows(elanInfo, interfaceInfo, macAddress, configureRemoteFlows, mdsalManager,
@@ -999,7 +1031,7 @@ public class ElanUtils {
             return;
         }
         String macAddress = macEntry.getMacAddress().getValue();
-        synchronized (getElanMacDPNKey(elanInfo.getElanTag(), macAddress, interfaceInfo.getDpId())) {
+        try (Acquired lock = lockElanMacDPN(elanInfo.getElanTag(), macAddress, interfaceInfo.getDpId())) {
             deleteMacFlows(elanInfo, interfaceInfo, macAddress, /* alsoDeleteSMAC */ true, flowTx);
         }
     }
@@ -1452,9 +1484,8 @@ public class ElanUtils {
         return null;
     }
 
-    public static String getElanMacDPNKey(long elanTag, String macAddress, BigInteger dpnId) {
-        String elanMacDmacDpnKey = "MAC-" + macAddress + " ELAN_TAG-" + elanTag + "DPN_ID-" + dpnId;
-        return elanMacDmacDpnKey.intern();
+    public static Acquired lockElanMacDPN(long elanTag, String macAddress, BigInteger dpnId) {
+        return ELAN_LOCKS.acquire(new ElanLockName(elanTag, macAddress, dpnId));
     }
 
     public static List<ListenableFuture<Void>>