Exception on snatint-ip-port-map DS 66/74766/7
authorshaik <shaik.b@altencalsoftlabs.com>
Thu, 2 Aug 2018 12:45:37 +0000 (18:15 +0530)
committerSam Hague <shague@redhat.com>
Wed, 3 Oct 2018 19:51:23 +0000 (19:51 +0000)
Description: Lot of conflict modification exception are observed on
the snatint-ip-port-map DS.

When multiple SNAT session gets establish from a given VM
simultanously, the snatint-ip-port-map DS gets updated simultanously
to update with allocated external ports for an given VM resulting in
exception.

Java synchronization is used to prevent simultanous modification
on this DS.

JIRA : NETVIRT-1391

Change-Id: Ib455b2cbab171440c77067f9d347346cdcda81f7
Signed-off-by: shaik <shaik.b@altencalsoftlabs.com>
natservice/impl/src/main/java/org/opendaylight/netvirt/natservice/internal/NaptManager.java

index 0eced3cc024b9c41db1003a38cef9cce9040b235..f49b2fc258d09378987ea71924e80a5b2e27df49 100644 (file)
@@ -332,23 +332,29 @@ public class NaptManager {
                         String internalIpAddress = sourceAddress.getIpAddress();
                         int ipPort = sourceAddress.getPortNumber();
                         ProtocolTypes protocolType = NatUtil.getProtocolType(protocol);
-                        List<Integer> portList = new ArrayList<>(
-                                NatUtil.getInternalIpPortListInfo(dataBroker, segmentId, internalIpAddress,
-                                        protocolType));
-                        portList.add(ipPort);
-
-                        IntIpProtoTypeBuilder builder = new IntIpProtoTypeBuilder();
-                        IntIpProtoType intIpProtocolType =
-                            builder.withKey(new IntIpProtoTypeKey(protocolType)).setPorts(portList).build();
-                        try {
-                            MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION,
-                                NatUtil.buildSnatIntIpPortIdentifier(segmentId, internalIpAddress, protocolType),
-                                intIpProtocolType);
-                        } catch (Exception ex) {
-                            LOG.error("getExternalAddressMapping : Failed to write into snat-internal-ip-port-info "
-                                    + "with exception", ex);
+                        String lock = new StringBuilder(Long.toString(segmentId))
+                                .append(NatConstants.COLON_SEPARATOR)
+                                .append(internalIpAddress)
+                                .append(NatConstants.COLON_SEPARATOR)
+                                .append(protocolType.getName()).toString();
+                        synchronized (lock.intern()) {
+                            List<Integer> portList = new ArrayList<>(
+                                    NatUtil.getInternalIpPortListInfo(dataBroker, segmentId, internalIpAddress,
+                                            protocolType));
+                            portList.add(ipPort);
+
+                            IntIpProtoTypeBuilder builder = new IntIpProtoTypeBuilder();
+                            IntIpProtoType intIpProtocolType =
+                                    builder.withKey(new IntIpProtoTypeKey(protocolType)).setPorts(portList).build();
+                            try {
+                                MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.CONFIGURATION,
+                                    NatUtil.buildSnatIntIpPortIdentifier(segmentId, internalIpAddress, protocolType),
+                                    intIpProtocolType);
+                            } catch (Exception ex) {
+                                LOG.error("getExternalAddressMapping : Failed to write into snat-internal-ip-port-info "
+                                        + "with exception", ex);
+                            }
                         }
-
                         SessionAddress externalIpPort = new SessionAddress(extIp, extPort);
                         LOG.debug("getExternalAddressMapping : successfully returning externalIP {} "
                             + "and port {}", externalIpPort.getIpAddress(), externalIpPort.getPortNumber());
@@ -449,11 +455,19 @@ public class NaptManager {
         }
 
         //delete the entry of port for InternalIp from snatIntIpportMappingDS
-        try {
-            removeSnatIntIpPortDS(segmentId, address, protocol);
-        } catch (Exception e) {
-            LOG.error("releaseSnatIpPortMapping : failed, Removal of snatipportmap {} for "
-                + "router {} failed",address.getIpAddress(), segmentId, e);
+        ProtocolTypes protocolType = NatUtil.getProtocolType(protocol);
+        String lock = new StringBuilder(Long.toString(segmentId))
+                .append(NatConstants.COLON_SEPARATOR)
+                .append(address.getIpAddress())
+                .append(NatConstants.COLON_SEPARATOR)
+                .append(protocolType.getName()).toString();
+        synchronized (lock.intern()) {
+            try {
+                removeSnatIntIpPortDS(segmentId, address, protocolType);
+            } catch (Exception e) {
+                LOG.error("releaseSnatIpPortMapping : failed, Removal of snatipportmap {} for "
+                        + "router {} failed", address.getIpAddress(), segmentId, e);
+            }
         }
     }
 
@@ -578,10 +592,9 @@ public class NaptManager {
 
     // TODO Clean up the exception handling
     @SuppressWarnings("checkstyle:IllegalCatch")
-    protected void removeSnatIntIpPortDS(long segmentId, SessionAddress address, NAPTEntryEvent.Protocol protocol) {
+    protected void removeSnatIntIpPortDS(long segmentId, SessionAddress address, ProtocolTypes protocolType) {
         LOG.trace("removeSnatIntIpPortDS : method called for IntIpport {} of router {} ",
             address, segmentId);
-        ProtocolTypes protocolType = NatUtil.getProtocolType(protocol);
         List<Integer> portList =
             NatUtil.getInternalIpPortListInfo(dataBroker, segmentId, address.getIpAddress(), protocolType);
         if (portList.isEmpty() || !portList.contains(address.getPortNumber())) {