BUG 7264 Fix missing flows for Remote SG rule

  Problem: Missing SG remote flows to the first VM associated
  with the SG. The sample reproduction scenario as below:
    1. create empty SG1
    2. create VM1 with the SG1
    3. add custom TCP rule to SG1 with remote group id as SG1
    4. create VM2 with the SG1
  There is flow for TCP rule added for VM1 to VM2 but the flow
  from VM2 to VM1 is missing

  Solution: In this problem when in step 3), VM1 is not added to
  the remote SG cache as a first member of the remote SG.
  As a result, in step 4 VM1 is not seen as not associating with
  SG1 and therefore flow to it is not created.
  The fix is to add the VMport-SG association to the remote SG
  cache when the first rule is created for a VM with empty SG

Change-Id: I9f4c500a8ec87659d44801474a92227420a789e2
Signed-off-by: Vinh Nguyen <vinh.nguyen@hcl.com>

diff --git a/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/PortSecurityHandler.java b/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/PortSecurityHandler.java
index 1e1bd5a1c07c1b455446ee1749313cda944b0d5b..0152b182ebd0a206ac67eed9d1affdb6a0aa40c5 100644 (file)
--- a/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/PortSecurityHandler.java
+++ b/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/PortSecurityHandler.java
@@ -12,6 +12,10 @@ import java.net.HttpURLConnection;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.opendaylight.netvirt.openstack.netvirt.api.Action;
+import org.opendaylight.netvirt.openstack.netvirt.api.EventDispatcher;
+import org.opendaylight.netvirt.openstack.netvirt.api.SecurityGroupCacheManger;
+import org.opendaylight.netvirt.openstack.netvirt.api.SecurityServicesManager;
 import org.opendaylight.netvirt.openstack.netvirt.translator.iaware.INeutronSecurityRuleAware;
 import org.opendaylight.netvirt.openstack.netvirt.translator.NeutronPort;
 import org.opendaylight.netvirt.openstack.netvirt.translator.NeutronSecurityGroup;
@@ -19,9 +23,6 @@ import org.opendaylight.netvirt.openstack.netvirt.translator.NeutronSecurityRule
 import org.opendaylight.netvirt.openstack.netvirt.translator.Neutron_IPs;
 import org.opendaylight.netvirt.openstack.netvirt.translator.crud.INeutronPortCRUD;
 import org.opendaylight.netvirt.openstack.netvirt.translator.iaware.INeutronSecurityGroupAware;
-import org.opendaylight.netvirt.openstack.netvirt.api.Action;
-import org.opendaylight.netvirt.openstack.netvirt.api.EventDispatcher;
-import org.opendaylight.netvirt.openstack.netvirt.api.SecurityServicesManager;
 import org.opendaylight.netvirt.utils.servicehelper.ServiceHelper;
 import org.osgi.framework.ServiceReference;
 import org.slf4j.Logger;
@@ -36,6 +37,7 @@ public class PortSecurityHandler extends AbstractHandler
     private static final Logger LOG = LoggerFactory.getLogger(PortSecurityHandler.class);
     private volatile INeutronPortCRUD neutronPortCache;
     private volatile SecurityServicesManager securityServicesManager;
+    private volatile SecurityGroupCacheManger securityGroupCacheManger;
 
     @Override
     public int canCreateNeutronSecurityGroup(NeutronSecurityGroup neutronSecurityGroup) {
@@ -141,28 +143,42 @@ public class PortSecurityHandler extends AbstractHandler
     private void processNeutronSecurityRuleAdded(NeutronSecurityRule neutronSecurityRule) {
         List<NeutronPort> portList = getPortWithSecurityGroup(neutronSecurityRule.getSecurityRuleGroupID());
         for (NeutronPort port:portList) {
-            syncSecurityGroup(neutronSecurityRule,port,true);
+            syncSecurityGroup(neutronSecurityRule, port, true);
         }
     }
 
     private void processNeutronSecurityRuleDeleted(NeutronSecurityRule neutronSecurityRule) {
         List<NeutronPort> portList = getPortWithSecurityGroup(neutronSecurityRule.getSecurityRuleGroupID());
         for (NeutronPort port:portList) {
-            syncSecurityGroup(neutronSecurityRule,port,false);
+            syncSecurityGroup(neutronSecurityRule, port, false);
         }
     }
 
-    private void syncSecurityGroup(NeutronSecurityRule  securityRule,NeutronPort port,
+    private void syncSecurityGroup(NeutronSecurityRule securityRule, NeutronPort port,
                                    boolean write) {
+        LOG.debug("syncSecurityGroup {} port {} ", securityRule, port);
         if (!port.getPortSecurityEnabled()) {
-            LOG.info("Port security not enabled port", port);
+            LOG.info("Port security not enabled port {}", port);
             return;
         }
         if (null != securityRule.getSecurityRemoteGroupID()) {
             List<Neutron_IPs> vmIpList  = securityServicesManager
                     .getVmListForSecurityGroup(port.getID(), securityRule.getSecurityRemoteGroupID());
-            for (Neutron_IPs vmIp :vmIpList ) {
-                securityServicesManager.syncSecurityRule(port, securityRule, vmIp, write);
+
+            // the returned vmIpList contains the list of VMs belong to the remote security group
+            // excluding ones on this port.
+            // If the list is empty, this port is the first member of the remote security group
+            // we need to add/remove from the remote security group cache accordingly
+            if (vmIpList.isEmpty()) {
+                if (write) {
+                    securityGroupCacheManger.addToCache(securityRule.getSecurityRemoteGroupID(), port.getPortUUID());
+                } else {
+                    securityGroupCacheManger.removeFromCache(securityRule.getSecurityRemoteGroupID(), port.getPortUUID());
+                }
+            } else {
+                for (Neutron_IPs vmIp : vmIpList) {
+                    securityServicesManager.syncSecurityRule(port, securityRule, vmIp, write);
+                }
             }
         } else {
             securityServicesManager.syncSecurityRule(port, securityRule, null, write);
@@ -194,6 +210,8 @@ public class PortSecurityHandler extends AbstractHandler
                 (INeutronPortCRUD) ServiceHelper.getGlobalInstance(INeutronPortCRUD.class, this);
         securityServicesManager =
                 (SecurityServicesManager) ServiceHelper.getGlobalInstance(SecurityServicesManager.class, this);
+        securityGroupCacheManger =
+                (SecurityGroupCacheManger) ServiceHelper.getGlobalInstance(SecurityGroupCacheManger.class, this);
     }
 
     @Override

diff --git a/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/impl/SecurityGroupCacheManagerImpl.java b/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/impl/SecurityGroupCacheManagerImpl.java
index 06b610f3987edd5819402373323112171270646b..d253c01274a88f04eb74fa45acc0199502b7ac0a 100644 (file)
--- a/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/impl/SecurityGroupCacheManagerImpl.java
+++ b/openstack/net-virt/src/main/java/org/opendaylight/netvirt/openstack/netvirt/impl/SecurityGroupCacheManagerImpl.java
@@ -75,7 +75,7 @@ public class SecurityGroupCacheManagerImpl implements ConfigInterface, SecurityG
 
     @Override
     public void addToCache(String remoteSgUuid, String portUuid) {
-        LOG.debug("In addToCache remoteSgUuid:" + remoteSgUuid + "portUuid:" + portUuid);
+        LOG.debug("In addToCache remoteSgUuid:" + remoteSgUuid + " portUuid:" + portUuid);
         Set<String> portList = securityGroupCache.get(remoteSgUuid);
         if (null == portList) {
             portList = new HashSet<>();