CLOUDSTACK-6507: ensure sequence numbers are honoured while processing

OvsVpcPhysicalTopologyConfigCommand and OvsVpcRoutingPolicyConfigCommand

fix ensures only latest updates are applied (new openflow rules) to the
bidge enabled for distributed routing.

plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java

@@ -912,7 +912,7 @@ public class OvsTunnelManagerImpl extends ManagerBase implements OvsTunnelManage
                         seqVo = new VpcDistributedRouterSeqNoVO(vpcId);
                         _vpcDrSeqNoDao.persist(seqVo);
                     }
-                    _vpcDrSeqNoDao.lockRow(seqVo.getId(), true);
+                    seqVo = _vpcDrSeqNoDao.lockRow(seqVo.getId(), true);
                     seqVo.incrSequenceNo();
                     _vpcDrSeqNoDao.update(seqVo.getId(), seqVo);
                     return seqVo.getSequenceNo();

scripts/vm/hypervisor/xenserver/cloudstack_pluginlib.py

@@ -22,6 +22,7 @@ import logging
 import os
 import subprocess
 import simplejson as json
+import copy
 
 from time import localtime, asctime
 
@@ -349,7 +350,6 @@ def configure_bridge_for_network_topology(bridge, this_host_id, json_config, seq
                 action_str = "table=0, in_port=%s," % of_port + " ip, dl_dst=%s," %network.gatewaymac +\
                              "nw_dst=%s," %vpconfig.cidr + "actions=resubmit(,3)"
                 addflow = [OFCTL_PATH, "add-flow", bridge, action_str]
-
                 do_cmd(addflow)
 
         # get the list of hosts on which VPC spans from the JSON config
@@ -510,4 +510,83 @@ def configure_ovs_bridge_for_routing_policies(bridge, json_config, sequence_no):
 
     except:
         logging.debug("An unexpected error occurred while configuring bridge as per VPC's routing policies.")
+        raise
+
+def update_flooding_rules_on_port_plug_unplug(bridge, interface, command, if_network_id):
+
+    vnet_vif_ofports = []
+    vnet_tunnelif_ofports = []
+    vnet_all_ofports = []
+
+    logging.debug("Updating the flooding rules as interface  %s" %interface + " is %s"%command + " now.")
+    try:
+        vsctl_output = do_cmd([VSCTL_PATH, 'list-ports', bridge])
+        ports = vsctl_output.split('\n')
+
+        for port in ports:
+            if_ofport = do_cmd([VSCTL_PATH, 'get', 'Interface', port, 'ofport'])
+            if port.startswith('vif'):
+                # check VIF is in same network as that of plugged vif
+                if if_network_id != get_network_id_for_vif(port):
+                    continue
+                vnet_vif_ofports.append(if_ofport)
+                vnet_all_ofports.append(if_ofport)
+
+            if port.startswith('t'):
+                # check tunnel port is in same network as that of plugged vif
+                if if_network_id != get_network_id_for_tunnel_port(port)[1:-1]:
+                    continue
+                vnet_tunnelif_ofports.append(if_ofport)
+                vnet_all_ofports.append(if_ofport)
+
+        if command == 'online':
+
+            if len(vnet_all_ofports) == 1 :
+                return
+
+            for port in vnet_all_ofports:
+                clear_flooding_rules_for_port(bridge, port)
+
+            # for a packet arrived from tunnel port, flood only on VIF ports
+            for port in vnet_tunnelif_ofports:
+                add_flooding_rules_for_port(bridge, port, vnet_vif_ofports)
+
+            # for a packet arrived from VIF port send on all VIF and tunnel port excluding the port
+            # on which packet arrived
+            for port in vnet_vif_ofports:
+                vnet_all_ofports_copy = copy.copy(vnet_all_ofports)
+                vnet_all_ofports_copy.remove(port)
+                add_flooding_rules_for_port(bridge, port, vnet_all_ofports_copy)
+
+            this_if_ofport = do_cmd([VSCTL_PATH, 'get', 'Interface', interface, 'ofport'])
+
+            #learn that MAC is reachable through the VIF port
+            if interface.startswith('vif'):
+                mac = get_macaddress_of_vif(interface)
+                add_mac_lookup_table_entry(bridge, mac, this_if_ofport)
+
+        if command == 'offline':
+            for port in vnet_all_ofports:
+                clear_flooding_rules_for_port(bridge, port)
+
+            vnet_all_ofports.remove(this_if_ofport)
+            vnet_vif_ofports.remove(this_if_ofport)
+
+            # for a packet arrived from tunnel port, flood only on VIF ports
+            for port in vnet_tunnelif_ofports:
+                add_flooding_rules_for_port(bridge, port, vnet_vif_ofports)
+
+            # for a packet from VIF port send on all VIF's and tunnel ports excluding the port on which packet arrived
+            for port in vnet_vif_ofports:
+                vnet_all_ofports_copy = copy.copy(vnet_all_ofports)
+                vnet_all_ofports_copy.remove(port)
+                add_flooding_rules_for_port(bridge, port, vnet_all_ofports_copy)
+
+            #un-learn that MAC is reachable through the VIF port
+            if interface.startswith('vif'):
+                mac = get_macaddress_of_vif(interface)
+                delete_mac_lookup_table_entry(bridge, mac)
+    except:
+        logging.debug("An unexpected error occurred while updating the flooding rules when interface "
+                    + " %s" %interface + " is %s"%command)
         raise

scripts/vm/hypervisor/xenserver/ovs-vif-flows.py

@@ -15,8 +15,8 @@
 # specific language governing permissions and limitations
 # under the License.
 
-# A simple script for enabling and disabling per-vif rules for explicitly
-# allowing broadcast/multicast traffic on the port where the VIF is attached
+# A simple script for enabling and disabling per-vif and tunnel interface rules for explicitly
+# allowing broadcast/multicast traffic from the tunnel ports and on the port where the VIF is attached
 
 import copy
 import os
@@ -28,9 +28,11 @@ import cloudstack_pluginlib as pluginlib
 pluginlib.setup_logging("/var/log/cloud/ovstunnel.log")
 
 def clear_flows(bridge, this_vif_ofport, vif_ofports):
+    action = "".join("output:%s," %ofport
+                for ofport in vif_ofports)[:-1]
     # Remove flow entries originating from given ofport
     pluginlib.del_flows(bridge, in_port=this_vif_ofport)
-	# The following will remove the port being delete from actions
+    # The following will remove the port being delete from actions
     pluginlib.add_flow(bridge, priority=1100,
                        dl_dst='ff:ff:ff:ff:ff:ff', actions=action)
     pluginlib.add_flow(bridge, priority=1100,
@@ -40,7 +42,7 @@ def clear_flows(bridge, this_vif_ofport, vif_ofports):
 def apply_flows(bridge, this_vif_ofport, vif_ofports):
     action = "".join("output:%s," %ofport
                 for ofport in vif_ofports)[:-1]
-	# Ensure {b|m}casts sent from VIF ports are always allowed
+    # Ensure {b|m}casts sent from VIF ports are always allowed
     pluginlib.add_flow(bridge, priority=1200,
 					   in_port=this_vif_ofport,
 					   dl_dst='ff:ff:ff:ff:ff:ff',
@@ -49,7 +51,7 @@ def apply_flows(bridge, this_vif_ofport, vif_ofports):
 					   in_port=this_vif_ofport,
 					   nw_dst='224.0.0.0/24',
 					   actions='NORMAL')
-	# Ensure {b|m}casts are always propagated to VIF ports
+    # Ensure {b|m}casts are always propagated to VIF ports
     pluginlib.add_flow(bridge, priority=1100,
                        dl_dst='ff:ff:ff:ff:ff:ff', actions=action)
     pluginlib.add_flow(bridge, priority=1100,
@@ -116,6 +118,7 @@ def main(command, vif_raw):
                                          'list-ports', bridge])
         vifs = vsctl_output.split('\n')
         vif_ofports = []
+        vif_other_ofports = []
         for vif in vifs:
             vif_ofport = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'get',
                                            'Interface', vif, 'ofport'])
@@ -125,7 +128,9 @@ def main(command, vif_raw):
                 vif_ofports.append(vif_ofport)
 
         if command == 'offline':
-            clear_flows(bridge,  this_vif_ofport, vif_ofports)
+            vif_other_ofports = copy.copy(vif_ofports)
+            vif_other_ofports.remove(this_vif_ofport)
+            clear_flows(bridge,  this_vif_ofport, vif_other_ofports)
 
         if command == 'online':
             apply_flows(bridge,  this_vif_ofport, vif_ofports)
@@ -138,69 +143,8 @@ def main(command, vif_raw):
                 # We need the REAL bridge name
                 bridge = pluginlib.do_cmd([pluginlib.VSCTL_PATH,
                                            'br-to-parent', bridge])
-        vsctl_output = pluginlib.do_cmd([pluginlib.VSCTL_PATH,
-                                         'list-ports', bridge])
         vif_network_id = pluginlib.get_network_id_for_vif(this_vif)
-        vnet_vif_ofports = []
-        vnet_tunnelif_ofports = []
-        vnet_all_ofports = []
-
-        ports = vsctl_output.split('\n')
-        for port in ports:
-            if_ofport = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'get', 'Interface', port, 'ofport'])
-            if port.startswith('vif'):
-                # check VIF is in same network as that of plugged vif
-                if vif_network_id != pluginlib.get_network_id_for_vif(port):
-                    continue
-                vnet_vif_ofports.append(if_ofport)
-                vnet_all_ofports.append(if_ofport)
-
-            if port.startswith('t'):
-                # check tunnel port is in same network as that of plugged vif
-                if vif_network_id != pluginlib.get_network_id_for_tunnel_port(port)[1:-1]:
-                    continue
-                vnet_tunnelif_ofports.append(if_ofport)
-                vnet_all_ofports.append(if_ofport)
-
-        if command == 'online':
-            for port in vnet_all_ofports:
-                pluginlib.clear_flooding_rules_for_port(bridge, port)
-
-            # for a packet arrived from tunnel port, flood only on VIF ports
-            for port in vnet_tunnelif_ofports:
-                pluginlib.add_flooding_rules_for_port(bridge, port, vnet_vif_ofports)
-
-            # for a packet arrived from VIF port send on all VIF and tunnel port excluding the port
-            # on which packet arrived
-            for port in vnet_vif_ofports:
-                vnet_all_ofports_copy = copy.copy(vnet_all_ofports)
-                vnet_all_ofports_copy.remove(port)
-                pluginlib.add_flooding_rules_for_port(bridge, port, vnet_all_ofports_copy)
-
-            #learn that MAC is reachable through the VIF port
-            mac = pluginlib.get_macaddress_of_vif(this_vif)
-            this_vif_ofport = pluginlib.do_cmd([pluginlib.VSCTL_PATH, 'get', 'Interface', this_vif, 'ofport'])
-            pluginlib.add_mac_lookup_table_entry(bridge, mac, this_vif_ofport)
-
-        if command == 'offline':
-            for port in vnet_all_ofports:
-                pluginlib.clear_flooding_rules_for_port(bridge, port)
-            vnet_all_ofports.remove(this_vif_ofport)
-            vnet_vif_ofports.remove(this_vif_ofport)
-
-            # for a packet arrived from tunnel port, flood only on VIF ports
-            for port in vnet_tunnelif_ofports:
-                pluginlib.add_flooding_rules_for_port(bridge, port, vnet_vif_ofports)
-
-            # for a packet from VIF port send on all VIF's and tunnel ports excluding the port on which packet arrived
-            for port in vnet_vif_ofports:
-                vnet_all_ofports_copy = copy.copy(vnet_all_ofports)
-                vnet_all_ofports_copy.remove(port)
-                pluginlib.add_flooding_rules_for_port(bridge, port, vnet_all_ofports_copy)
-
-            #un-learn that MAC is reachable through the VIF port
-            mac = pluginlib.get_macaddress_of_vif(this_vif)
-            pluginlib.delete_mac_lookup_table_entry(bridge, mac)
+        pluginlib.update_flooding_rules_on_port_plug_unplug(bridge, this_vif, command, vif_network_id)
 
     return
 

scripts/vm/hypervisor/xenserver/ovstunnel

@@ -165,9 +165,12 @@ def setup_ovs_bridge_for_distributed_routing(session, args):
         lib.do_cmd([lib.XE_PATH, "network-param-set", "uuid=%s" % xs_nw_uuid,
                    "other-config:ovs-host-setup=%s" % conf_hosts])
 
+        # first clear the default rule (i.e rule for NORMAL processing)
+        lib.del_flows(bridge, table=0)
+
         # add a default flow rule to send broadcast and multi-cast packets to L2 flooding table
-        lib.add_flow(bridge, priority=1000, dl_dst='ff:ff:ff:ff:ff:ff', table=0, actions='resubmit(,2)')
-        lib.add_flow(bridge, priority=1000, nw_dst='224.0.0.0/24', table=0, actions='resubmit(,2)')
+        lib.add_flow(bridge, priority=1200, dl_dst='ff:ff:ff:ff:ff:ff', table=0, actions='resubmit(,2)')
+        lib.add_flow(bridge, priority=1200, nw_dst='224.0.0.0/24', table=0, actions='resubmit(,2)')
 
         # add a default flow rule to send uni-cast traffic to L2 lookup table
         lib.add_flow(bridge, priority=0, table=0, actions='resubmit(,1)')
@@ -178,7 +181,7 @@ def setup_ovs_bridge_for_distributed_routing(session, args):
         # add a default rule in L2 flood table to drop packet
         lib.add_flow(bridge, priority=0, table=2, actions='drop')
 
-        # add a default rule in egress table to forward packet to L3 lookup table
+        # add a default rule in egress ACL table to forward packet to L3 lookup table
         lib.add_flow(bridge, priority=0, table=3, actions='resubmit(,4)')
 
         # add a default rule in L3 lookup table to forward packet to L2 lookup table
@@ -187,6 +190,9 @@ def setup_ovs_bridge_for_distributed_routing(session, args):
         # add a default rule in ingress table to drop in bound packets
         lib.add_flow(bridge, priority=0, table=5, actions='drop')
 
+        # initialize the sequence number for the bridge
+        lib.do_cmd([lib.VSCTL_PATH, "set", "bridge", bridge, "other-config:sequence-number=0"])
+
         result = "SUCCESS: successfully setup bridge with flow rules"
 
     logging.debug("Setup_ovs_bridge completed with result:%s" % result)
@@ -326,8 +332,11 @@ def create_tunnel(session, args):
                      nw_dst='224.0.0.0/24', actions='drop')
 
             # add flow rule to send the traffic from tunnel ports to L2 switching table only
-            lib.add_flow(bridge, priority=1000, in_port=tun_ofport, table=0, actions='resubmit(,1)')
+            lib.add_flow(bridge, priority=1100, in_port=tun_ofport, table=0, actions='resubmit(,1)')
+
+            # mark tunnel interface with network id for which this tunnel was created
             lib.do_cmd([lib.VSCTL_PATH, "set", "interface", name, "options:cloudstack-network-id=%s" % network_uuid])
+            lib.update_flooding_rules_on_port_plug_unplug(bridge, name, 'online', network_uuid)
 
         return "SUCCESS:%s" % name
     except:
@@ -402,15 +411,29 @@ def configure_ovs_bridge_for_network_topology(session, args):
     this_host_id = args.pop("host-id")
     sequence_no = args.pop("seq-no")
 
-    return lib.configure_bridge_for_network_topology(bridge, this_host_id, json_config, sequence_no)
+    # get the last update sequence number
+    last_seq_no = lib.do_cmd([lib.VSCTL_PATH, "get", "bridge", bridge, "other-config:sequence-number"])
+    last_seq_no = last_seq_no[1:-1]
+    if sequence_no > last_seq_no:
+        lib.do_cmd([lib.VSCTL_PATH, "set", "bridge", bridge, "other-config:sequence-number=%s"%sequence_no])
+        return lib.configure_bridge_for_network_topology(bridge, this_host_id, json_config, sequence_no)
+    else:
+        return "SUCCESS: Ignoring the update as there is already recent update received and applied"
+
 
 @echo
 def configure_ovs_bridge_for_routing_policies(session, args):
     bridge = args.pop("bridge")
     json_config = args.pop("config")
     sequence_no = args.pop("seq-no")
-
-    return lib.configure_ovs_bridge_for_routing_policies(bridge, json_config, sequence_no)
+    # get the last update sequence number
+    last_seq_no = lib.do_cmd([lib.VSCTL_PATH, "get", "bridge", bridge, "other-config:sequence-number"])
+    last_seq_no = last_seq_no[1:-1]
+    if sequence_no > last_seq_no:
+        lib.do_cmd([lib.VSCTL_PATH, "set", "bridge", bridge, "other-config:sequence-number=%s"%sequence_no])
+        return lib.configure_ovs_bridge_for_routing_policies(bridge, json_config, sequence_no)
+    else:
+        return "SUCCESS: Ignoring the update as there is already recent update received and applied"
 
 if __name__ == "__main__":
     XenAPIPlugin.dispatch({"create_tunnel": create_tunnel,