From: Robert Varga Date: Thu, 20 Jun 2019 17:59:40 +0000 (+0200) Subject: Migrate ElanUtils to use NamedLocks X-Git-Tag: release/sodium~26 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=be1b0517d62d3aa1a21062363b21f6d9f4da4fba;p=netvirt.git 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 --- 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 5e5b93367c..19324f1338 100644 --- 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> evpnDeleteDmacFlowsToExternalMac(EvpnDmacFlow evpnDmacFlow) { List> 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 e368255f6b..d0ec7557a6 100644 --- 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 { + 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 3568592c63..a25c8a25ee 100755 --- 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 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 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>