From ccb20da469942960826b942af6696c1f5c3300a2 Mon Sep 17 00:00:00 2001 From: bviswa Date: Thu, 22 Oct 2015 08:38:01 +0530 Subject: [PATCH] BUG-4205 : VM delete doesnot remove all related flows 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 Signed-off-by: Flavio Fernandes --- .../openstack/netvirt/SouthboundHandler.java | 16 +++ .../netvirt/api/SecurityServicesManager.java | 7 ++ .../netvirt/impl/NeutronL3Adapter.java | 46 +++++++- .../netvirt/impl/SecurityServicesImpl.java | 101 +++++++++++++++--- .../netvirt/impl/NeutronL3AdapterTest.java | 3 + 5 files changed, 154 insertions(+), 19 deletions(-) diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java index 108a7680fe..0e3832a65a 100644 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java @@ -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 phyIfName) { diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/SecurityServicesManager.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/SecurityServicesManager.java index 843116c917..4291bcfeb7 100644 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/SecurityServicesManager.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/api/SecurityServicesManager.java @@ -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. diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3Adapter.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3Adapter.java index 84ad361b5b..39efd4bc46 100644 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3Adapter.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3Adapter.java @@ -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 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) * diff --git a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/SecurityServicesImpl.java b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/SecurityServicesImpl.java index dc482a130b..c5f7f58691 100644 --- a/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/SecurityServicesImpl.java +++ b/openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/impl/SecurityServicesImpl.java @@ -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 neutronPortFixedIp = neutronPort.getFixedIPs(); if (null == neutronPortFixedIp || neutronPortFixedIp.isEmpty()) { @@ -297,16 +357,21 @@ public class SecurityServicesImpl implements ConfigInterface, SecurityServicesMa @Override public List 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 = diff --git a/openstack/net-virt/src/test/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3AdapterTest.java b/openstack/net-virt/src/test/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3AdapterTest.java index 54c18b1648..da6b042b55 100644 --- a/openstack/net-virt/src/test/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3AdapterTest.java +++ b/openstack/net-virt/src/test/java/org/opendaylight/ovsdb/openstack/netvirt/impl/NeutronL3AdapterTest.java @@ -138,6 +138,7 @@ public class NeutronL3AdapterTest { @Test public void testHandleNeutronPortEvent() throws Exception { Map subnetIdToRouterInterfaceCache = new HashMap<>(); + Set 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)); -- 2.36.6