BUG-4205 : VM delete doesnot remove all related flows
authorbviswa <badrinath_viswanatha@dell.com>
Thu, 22 Oct 2015 03:08:01 +0000 (08:38 +0530)
committerFlavio Fernandes <ffernand@redhat.com>
Fri, 15 Jan 2016 14:29:48 +0000 (09:29 -0500)
Patch set 3: Rebase and add ref to bug 4971
Patch set 5: Rebase again and move processSecurityGroupUpdate()

Change-Id: Ia3cbba3087232cb56eee6e5424cdd011a01ce266
Signed-off-by: bviswa <badrinath_viswanatha@dell.com>
Signed-off-by: Flavio Fernandes <ffernand@redhat.com>
openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java
openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/SecurityServicesManager.java
openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3Adapter.java
openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/SecurityServicesImpl.java
openstack/net-virt/src/test/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3AdapterTest.java

index 108a7680fef52f98e4006e4e4362b0c920853125..0e3832a65af431441b85baef7d749657aea5f778 100644 (file)
@@ -11,6 +11,7 @@ package org.opendaylight.ovsdb.openstack.netvirt;
 import java.util.List;
 
 import org.opendaylight.ovsdb.openstack.netvirt.translator.NeutronNetwork;
+import org.opendaylight.ovsdb.openstack.netvirt.translator.NeutronPort;
 import org.opendaylight.ovsdb.openstack.netvirt.api.*;
 import org.opendaylight.ovsdb.openstack.netvirt.impl.NeutronL3Adapter;
 import org.opendaylight.ovsdb.utils.servicehelper.ServiceHelper;
@@ -170,6 +171,21 @@ public class SouthboundHandler extends AbstractHandler
                 LOG.error("Error fetching Interface Rows for node {}", node, e);
             }
         }
+        //remove neutronPort from the CleanupCache, if it has the entry.
+        NeutronPort neutronPort = null;
+        String neutronPortId = southbound.getInterfaceExternalIdsValue(ovsdbTerminationPointAugmentation,
+                Constants.EXTERNAL_ID_INTERFACE_ID);
+        if (neutronPortId != null) {
+            LOG.trace("Clean up the NeutronPortCache for {} ", neutronPortId);
+            neutronPort = neutronL3Adapter.getPortFromCleanupCache(neutronPortId);
+        }
+        if (neutronPort != null) {
+            LOG.debug("Clean up the NeutronPortCache ");
+            neutronL3Adapter.removePortFromCleanupCache(neutronPort);
+        } else {
+            LOG.trace("Nothing to Clean up in the NeutronPortCache ");
+        }
+
     }
 
     private boolean isInterfaceOfInterest(OvsdbTerminationPointAugmentation terminationPoint, List<String> phyIfName) {
index 843116c9173f1455c739a76f5ae2bc3903eab7a3..4291bcfeb7143abfbcc525c6a78d37800418e0f9 100644 (file)
@@ -43,6 +43,13 @@ public interface SecurityServicesManager {
      * @return the dhcp server port
      */
     NeutronPort getDhcpServerPort(OvsdbTerminationPointAugmentation intf);
+    /**
+     * Gets the NeutronPort from the cleanup cache.
+     *
+     * @param intf the intf
+     * @return the NeutronPort stored in the cleanupCache of NeutronL3Adapter
+     */
+    NeutronPort getNeutronPortFromCache(OvsdbTerminationPointAugmentation intf);
 
     /**
       * Check if the given interface corresponds to a DHCP server port.
index 84ad361b5bec0ead8932699b66ccb95db7752e90..39efd4bc46e6788f64b3e0acdf9b9c7cb26591da 100644 (file)
@@ -67,9 +67,11 @@ import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Neutron L3 Adapter implements a hub-like adapter for the various Neutron events. Based on
@@ -133,6 +135,7 @@ public class NeutronL3Adapter extends AbstractHandler implements GatewayMacResol
     private Boolean enabled = false;
     private Boolean flgDistributedARPEnabled = true;
     private Boolean isCachePopulationDone = false;
+    private Set<NeutronPort> portCleanupCache;
 
     private Southbound southbound;
     private NeutronModelsDataStoreHelper neutronModelsDataStoreHelper;
@@ -174,6 +177,7 @@ public class NeutronL3Adapter extends AbstractHandler implements GatewayMacResol
         } else {
             LOG.debug("OVSDB L3 forwarding is disabled");
         }
+        this.portCleanupCache = new HashSet<>();
     }
 
     //
@@ -420,10 +424,15 @@ public class NeutronL3Adapter extends AbstractHandler implements GatewayMacResol
     public void handleNeutronPortEvent(final NeutronPort neutronPort, Action action) {
         LOG.debug("Neutron port {} event : {}", action, neutronPort.toString());
 
+        if (action == Action.UPDATE) {
+            // FIXME: Bug 4971 Move cleanup cache to SG Impl
+            this.updatePortInCleanupCache(neutronPort, neutronPort.getOriginalPort());
+            this.processSecurityGroupUpdate(neutronPort);
+        }
+
         if (!this.enabled) {
             return;
         }
-        this.processSecurityGroupUpdate(neutronPort);
 
         final boolean isDelete = action == Action.DELETE;
 
@@ -769,11 +778,17 @@ public class NeutronL3Adapter extends AbstractHandler implements GatewayMacResol
                                      final NeutronNetwork neutronNetwork, Action action) {
         LOG.debug("southbound interface {} node:{} interface:{}, neutronNetwork:{}",
                      action, bridgeNode.getNodeId().getValue(), intf.getName(), neutronNetwork);
+
+        final NeutronPort neutronPort = tenantNetworkManager.getTenantPort(intf);
+        if (action != Action.DELETE && neutronPort != null) {
+            // FIXME: Bug 4971 Move cleanup cache to SG Impl
+            storePortInCleanupCache(neutronPort);
+        }
+
         if (!this.enabled) {
             return;
         }
 
-        final NeutronPort neutronPort = tenantNetworkManager.getTenantPort(intf);
         final Long dpId = getDpidForIntegrationBridge(bridgeNode);
         final Uuid interfaceUuid = intf.getInterfaceUuid();
 
@@ -1523,6 +1538,33 @@ public class NeutronL3Adapter extends AbstractHandler implements GatewayMacResol
         }
     }
 
+
+    private void storePortInCleanupCache(NeutronPort port) {
+        this.portCleanupCache.add(port);
+    }
+
+
+    private void updatePortInCleanupCache(NeutronPort updatedPort,NeutronPort originalPort) {
+        removePortFromCleanupCache(originalPort);
+        storePortInCleanupCache(updatedPort);
+    }
+
+    public void removePortFromCleanupCache(NeutronPort port) {
+        this.portCleanupCache.remove(port);
+    }
+
+    public NeutronPort getPortFromCleanupCache(String portid) {
+        for (NeutronPort neutronPort : this.portCleanupCache) {
+            if (neutronPort.getPortUUID() != null ) {
+                if (neutronPort.getPortUUID().equals(portid)) {
+                    LOG.info("getPortFromCleanupCache: Matching NeutronPort found {}", portid);
+                    return neutronPort;
+                    }
+                }
+            }
+        return null;
+    }
+
     /**
      * Return String that represents OF port with marker explicitly provided (reverse of MatchUtils:parseExplicitOFPort)
      *
index dc482a130b4ce757acde47f6ded896a38281ccf1..c5f7f58691f429a4f5dc66ee956cf4a9db4487ed 100644 (file)
@@ -45,6 +45,7 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
     private volatile ConfigurationService configurationService;
     private volatile IngressAclProvider ingressAclProvider;
     private volatile EgressAclProvider egressAclProvider;
+    private volatile NeutronL3Adapter neutronL3Adapter;
     private boolean isConntrackEnabled = false;
 
     public SecurityServicesImpl() {
@@ -113,21 +114,34 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
     @Override
     public NeutronPort getDhcpServerPort(OvsdbTerminationPointAugmentation terminationPointAugmentation) {
         if (neutronPortCache == null) {
-            LOG.error("getDHCPServerPort: neutron port is null");
-            return null;
+            LOG.warn("getDHCPServerPort: neutron port cache is null");
         }
         LOG.trace("getDHCPServerPort for {}",
                   terminationPointAugmentation.getName());
+        NeutronPort neutronPort = null;
         try {
             String neutronPortId = southbound.getInterfaceExternalIdsValue(terminationPointAugmentation,
                                                                            Constants.EXTERNAL_ID_INTERFACE_ID);
             if (neutronPortId == null) {
                 return null;
             }
-            NeutronPort neutronPort = neutronPortCache.getPort(neutronPortId);
-            if (neutronPort == null) {
-                LOG.error("getDHCPServerPort: neutron port of {} is not found", neutronPortId);
-                return null;
+            if (null != neutronPortCache) {
+                neutronPort = neutronPortCache.getPort(neutronPortId);
+
+            }
+            if (neutronPort == null ){
+                neutronPort = neutronL3Adapter.getPortFromCleanupCache(neutronPortId);
+                if (neutronPort == null)
+                {
+                    LOG.info("getDHCPServerPort: neutron port of {} is not found", neutronPortId);
+                    return null;
+                }
+                LOG.info("getDHCPServerPort: neutron port of {} got from cleanupcache", neutronPortId);
+
+            }
+            /* if the current port is a DHCP port, return the same*/
+            if (neutronPort.getDeviceOwner().contains("dhcp")) {
+                return neutronPort;
             }
             /* if the current port is a DHCP port, return the same*/
             if (neutronPort.getDeviceOwner().contains("dhcp")) {
@@ -186,20 +200,61 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
         return null;
     }
 
+
+    @Override
+    public NeutronPort getNeutronPortFromCache(OvsdbTerminationPointAugmentation terminationPointAugmentation) {
+        NeutronPort neutronPort = null;
+        LOG.trace("getNeutronPortFromCache for {}",
+                terminationPointAugmentation.getName());
+        try {
+            String neutronPortId = southbound.getInterfaceExternalIdsValue(terminationPointAugmentation,
+                    Constants.EXTERNAL_ID_INTERFACE_ID);
+            if (neutronPortId == null) {
+                return null;
+            }
+            if (null != neutronPortCache) {
+                neutronPort = neutronPortCache.getPort(neutronPortId);
+
+            }
+            if (neutronPort == null ){
+                LOG.trace("getNeutronPortFromCache: neutron port of {} search in cleanupcache", neutronPortId);
+
+                neutronPort = neutronL3Adapter.getPortFromCleanupCache(neutronPortId);
+                if (neutronPort == null)
+                {
+                    LOG.info("getNeutronPortFromCache: neutron port of {} is not found", neutronPortId);
+                    return null;
+                }
+                LOG.trace("getNeutronPortFromCache: neutron port of {} got from cleanupcache", neutronPortId);
+
+            }
+        }catch (Exception e) {
+            LOG.warn("getNeutronPortFromCache:getNeutronPortFromCache failed due to ", e);
+            return null;
+        }
+        return neutronPort;
+    }
+
+
+
     @Override
     public boolean isComputePort(OvsdbTerminationPointAugmentation terminationPointAugmentation) {
         if (neutronPortCache == null) {
-            LOG.error("neutron port is null");
-            return false;
+           LOG.warn("isComputePort : neutronPortCache is null");
         }
+        NeutronPort neutronPort = null;
         LOG.trace("isComputePort for {}", terminationPointAugmentation.getName());
         String neutronPortId = southbound.getInterfaceExternalIdsValue(terminationPointAugmentation,
                                                                        Constants.EXTERNAL_ID_INTERFACE_ID);
         if (neutronPortId == null) {
             return false;
         }
-        NeutronPort neutronPort = neutronPortCache.getPort(neutronPortId);
+        if (neutronPortCache != null) {
+            neutronPort = neutronPortCache.getPort(neutronPortId);
+        }
         if (neutronPort == null) {
+            neutronPort = getNeutronPortFromCache(terminationPointAugmentation);
+            if (neutronPort == null)
             return false;
         }
         /*Check the device owner and if it contains compute to identify
@@ -216,9 +271,9 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
     @Override
     public boolean isLastPortinSubnet(Node node, OvsdbTerminationPointAugmentation terminationPointAugmentation) {
         if (neutronPortCache == null) {
-            LOG.error("isLastPortinSubnet: neutron port is null");
-            return false;
+            LOG.error("isLastPortinSubnet: neutronPortCache is null");
         }
+        NeutronPort neutronPort = null;
         try {
             LOG.trace("isLastPortinSubnet: for {}", terminationPointAugmentation.getName());
             String neutronPortId = southbound.getInterfaceExternalIdsValue(terminationPointAugmentation,
@@ -226,10 +281,15 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
             if (neutronPortId == null) {
                 return false;
             }
-            NeutronPort neutronPort = neutronPortCache.getPort(neutronPortId);
+            if (neutronPortCache != null) {
+                neutronPort = neutronPortCache.getPort(neutronPortId);
+            }
             if (neutronPort == null) {
-                LOG.error("isLastPortinSubnet: neutron port of {} is not found", neutronPortId);
-                return false;
+                neutronPort = getNeutronPortFromCache(terminationPointAugmentation);
+                if (neutronPort == null) {
+                    LOG.error("isLastPortinSubnet: neutron port of {} is not found", neutronPortId);
+                    return false;
+                }
             }
             List<Neutron_IPs> neutronPortFixedIp = neutronPort.getFixedIPs();
             if (null == neutronPortFixedIp || neutronPortFixedIp.isEmpty()) {
@@ -297,16 +357,21 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
     @Override
     public List<Neutron_IPs> getIpAddressList(OvsdbTerminationPointAugmentation terminationPointAugmentation) {
         if (neutronPortCache == null) {
-            LOG.error("getIpAddress: neutron port is null");
-            return null;
+            LOG.warn("getIpAddress: neutronPortCache is null");
         }
+        NeutronPort neutronPort = null;
         LOG.trace("getIpAddress: for {}", terminationPointAugmentation.getName());
         String neutronPortId = southbound.getInterfaceExternalIdsValue(terminationPointAugmentation,
                                                                        Constants.EXTERNAL_ID_INTERFACE_ID);
         if (neutronPortId == null) {
             return null;
         }
-        NeutronPort neutronPort = neutronPortCache.getPort(neutronPortId);
+        if (neutronPortCache != null) {
+            neutronPort = neutronPortCache.getPort(neutronPortId);
+        }
+        if (neutronPort == null) {
+            neutronPort = getNeutronPortFromCache(terminationPointAugmentation);
+        }
         if (neutronPort == null) {
             LOG.error("getIpAddress: neutron port of {} is not found", neutronPortId);
             return null;
@@ -498,6 +563,8 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa
 
     @Override
     public void setDependencies(ServiceReference serviceReference) {
+        neutronL3Adapter =
+                (NeutronL3Adapter) ServiceHelper.getGlobalInstance(NeutronL3Adapter.class, this);
         southbound =
                 (Southbound) ServiceHelper.getGlobalInstance(Southbound.class, this);
         neutronNetworkCache =
index 54c18b16482a06837c672381f7d92aa91d59a174..da6b042b55b91bea483f5a3d9a5c37cdb87667e2 100644 (file)
@@ -138,6 +138,7 @@ public class NeutronL3AdapterTest {
     @Test
     public void testHandleNeutronPortEvent() throws Exception {
         Map<String, NeutronRouter_Interface> subnetIdToRouterInterfaceCache = new HashMap<>();
+        Set<NeutronPort> portCleanupCache = new HashSet<>();
         // Mock variables
         Neutron_IPs neutronIP = mock(Neutron_IPs.class);
         when(neutronIP.getSubnetUUID()).thenReturn(UUID);
@@ -151,6 +152,7 @@ public class NeutronL3AdapterTest {
         MemberModifier.field(NeutronL3Adapter.class, "neutronPortCache").set(neutronL3Adapter , mock(INeutronPortCRUD.class));
         subnetIdToRouterInterfaceCache.put(UUID, mock(NeutronRouter_Interface.class));
         MemberModifier.field(NeutronL3Adapter.class, "subnetIdToRouterInterfaceCache").set(neutronL3Adapter , subnetIdToRouterInterfaceCache);
+        MemberModifier.field(NeutronL3Adapter.class, "portCleanupCache").set(neutronL3Adapter , portCleanupCache);
 
         // Suppress the called to these functions
         MemberModifier.suppress(MemberMatcher.method(NeutronL3Adapter.class, "updateL3ForNeutronPort", NeutronPort.class, boolean.class));
@@ -431,6 +433,7 @@ public class NeutronL3AdapterTest {
         MemberModifier.suppress(MemberMatcher.method(NeutronL3Adapter.class, "getDpidForIntegrationBridge", Node.class));
         MemberModifier.suppress(MemberMatcher.method(NeutronL3Adapter.class, "handleInterfaceEventAdd", String.class, Long.class, Uuid.class));
         MemberModifier.suppress(MemberMatcher.method(NeutronL3Adapter.class, "handleInterfaceEventDelete", OvsdbTerminationPointAugmentation.class, Long.class));
+        MemberModifier.suppress(MemberMatcher.method(NeutronL3Adapter.class, "storePortInCleanupCache", NeutronPort.class));
 
         PowerMockito.when(neutronL3Adapter, "getDpidForIntegrationBridge", any(Node.class)).thenReturn(45L);
         Mockito.doNothing().when(neutronL3Adapter).handleNeutronPortEvent(any(NeutronPort.class), any(Action.class));