fix(agent): some fixes to the ebtables firewall and ovs agent driver

- don't try to create or delete standard ebtables chains
- reconfigure isoflat bridges and veth pairs every time on start up

neutron_isoflat/services/isoflat/agents/drivers/linux/linuxbridge.py

@@ -6,7 +6,7 @@ class IsoflatLinuxBridgeDriver(isoflat.IsoflatAgentDriverBase):
     def save_bridge_mappings(self):
         pass
 
-    def setup_mirror_bridges(self):
+    def setup_isoflat_bridges(self):
         pass
 
     def initialize(self):

neutron_isoflat/services/isoflat/agents/drivers/linux/openvswitch.py

@@ -55,41 +55,50 @@ def _allocate_bridge_name(self):
                 name = None
         return name
 
-    def setup_mirror_bridges(self):
+    def _setup_isoflat_bridge(self, phy_br_name, iso_br_name):
         ovs = ovs_lib.BaseOVS()
         ip_wrapper = ip_lib.IPWrapper()
+        phy_br = bridge_lib.BridgeDevice(phy_br_name)
+
+        iso_br = ovs.add_bridge(iso_br_name)
+        iso_br_link = ip_lib.IPDevice(iso_br_name)
+        iso_br_link.link.set_up()
+
+        phy_if_name = self._get_phy_if_name(iso_br_name)
+        iso_if_name = self._get_iso_if_name(iso_br_name)
+        device = ip_lib.IPDevice(iso_if_name)
+        if device.exists():
+            device.link.delete()
+            # Give udev a chance to process its rules here, to avoid
+            # race conditions between commands launched by udev rules
+            # and the subsequent call to ip_wrapper.add_veth
+            utils.execute(['udevadm', 'settle', '--timeout=10'])
+        phy_veth, iso_veth = ip_wrapper.add_veth(phy_if_name, iso_if_name)
+        iso_br.add_port(iso_if_name)
+        phy_br.addif(phy_veth)
+        LOG.info("Added OVS Isoflat bridge %s and veth port pair "
+                 "(%s, %s)" % (iso_br_name, phy_if_name, iso_if_name))
+        # enable veth to pass traffic
+        phy_veth.link.set_up()
+        iso_veth.link.set_up()
+
+    def setup_isoflat_bridges(self):
         for physical_network in self.iso_bridge_mappings:
+            phy_br_name = self.iso_bridge_mappings[physical_network]
             if physical_network not in self.ovs_bridge_mappings:
                 self._bridge_mappings_changed = True
-                br_name = self.iso_bridge_mappings[physical_network]
-                if not bridge_lib.BridgeDevice(br_name).exists():
+                if not bridge_lib.BridgeDevice(phy_br_name).exists():
                     LOG.error("Linux bridge %(bridge)s for physical network "
                               "%(physical_network)s does not exist. Isoflat agent "
                               "terminated!",
                               {'physical_network': physical_network,
-                               'bridge': br_name})
+                               'bridge': phy_br_name})
                     sys.exit(1)
-                mir_br_name = self._allocate_bridge_name()
-                self.ovs_bridge_mappings[physical_network] = mir_br_name
-                iso_br = ovs.add_bridge(mir_br_name)
-                phy_br = bridge_lib.BridgeDevice(br_name)
-                phy_if_name = self._get_phy_if_name(mir_br_name)
-                iso_if_name = self._get_iso_if_name(mir_br_name)
-                device = ip_lib.IPDevice(iso_if_name)
-                if device.exists():
-                    device.link.delete()
-                    # Give udev a chance to process its rules here, to avoid
-                    # race conditions between commands launched by udev rules
-                    # and the subsequent call to ip_wrapper.add_veth
-                    utils.execute(['udevadm', 'settle', '--timeout=10'])
-                phy_veth, iso_veth = ip_wrapper.add_veth(phy_if_name, iso_if_name)
-                iso_br.add_port(iso_if_name)
-                phy_br.addif(phy_veth)
-                LOG.info("Added OVS Isoflat bridge %s and veth port pair "
-                         "(%s, %s)" % (mir_br_name, phy_if_name, iso_if_name))
-                # enable veth to pass traffic
-                phy_veth.link.set_up()
-                iso_veth.link.set_up()
+                iso_br_name = self._allocate_bridge_name()
+                self.ovs_bridge_mappings[physical_network] = iso_br_name
+            else:
+                iso_br_name = self.ovs_bridge_mappings[physical_network]
+            self._setup_isoflat_bridge(phy_br_name, iso_br_name)
 
     def save_bridge_mappings(self):
         if not self._bridge_mappings_changed:

neutron_isoflat/services/isoflat/agents/extensions/isoflat.py

@@ -97,7 +97,7 @@ def consume_api(self, agent_api):
         """
 
     @abc.abstractmethod
-    def setup_mirror_bridges(self):
+    def setup_isoflat_bridges(self):
         """
         Check if the [ovs] or [linux_bridges] section bridge_mappings has all the provider networks.
         If not, set up the bridges and restart the agent.
@@ -138,7 +138,7 @@ def initialize(self, connection, driver_type):
         self.driver = manager.NeutronManager.load_class_for_provider(
             'neutron_isoflat.isoflat.agent_drivers', driver_type)(self)
         self.driver.consume_api(self.agent_api)
-        self.driver.setup_mirror_bridges()
+        self.driver.setup_isoflat_bridges()
         self.driver.save_bridge_mappings()
         self.driver.initialize()
 

neutron_isoflat/services/isoflat/agents/firewall/linux/ebtables_firewall.py

@@ -110,7 +110,7 @@ def _port_arg(direction, protocol, port_range_min, port_range_max):
             return args
 
         protocol = n_const.IPTABLES_PROTOCOL_NAME_MAP.get(protocol, protocol)
-        # TODO: can't filter icmp right now
+        # TODO: can't filter icmp by type and code right now
         if protocol in ['ipv6-icmp']:
             protocol_type = 'icmpv6' if protocol == 'ipv6-icmp' else 'icmp'
             # Note(xuhanp): port_range_min/port_range_max represent
@@ -119,7 +119,7 @@ def _port_arg(direction, protocol, port_range_min, port_range_max):
             # icmp code can be 0 so we cannot use "if port_range_max" here
             if port_range_max is not None:
                 args[-1] += '/%s' % port_range_max
-        elif port_range_min == port_range_max:
+        elif port_range_min == port_range_max or port_range_max is None:
             args += ['--%s' % direction, '%s' % (port_range_min,)]
         else:
             args += ['--%s' % direction, '%s:%s' % (port_range_min, port_range_max)]

neutron_isoflat/services/isoflat/agents/firewall/linux/ebtables_manager.py

@@ -29,6 +29,12 @@
 # a failure during ebtables-restore
 EBTABLES_ERROR_LINES_OF_CONTEXT = 5
 
+STANDARD_CHAINS = {
+    'filter': ['FORWARD', 'INPUT', 'OUTPUT'],
+    'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING'],
+    'broute': ['BROUTING']
+}
+
 
 class EbTablesApplyException(qexceptions.NeutronException):
     def __init__(self, message=None):
@@ -362,7 +368,8 @@ def _run_restore(self, args, commands):
                 elif command.startswith(':'):
                     # recreate the chain
                     chain = command[1:].strip()
-                    _args += ['-t', table, '-N', chain]
+                    if chain not in STANDARD_CHAINS[table]:
+                        _args += ['-t', table, '-N', chain]
                 elif command.strip() != '':
                     _args += ['-t', table] + command.split(' ')
                 else:
@@ -431,7 +438,7 @@ def _apply_synchronized(self):
                 new_rules = self._modify_rules(old_rules, table)
                 # generate the ebtables commands to get between the old state
                 # and the new state
-                changes = _generate_path_between_rules(old_rules, new_rules)
+                changes = _generate_path_between_rules(table_name, old_rules, new_rules)
                 if changes:
                     # if there are changes to the table, we put on the header
                     # and footer that ebtables-save needs
@@ -586,7 +593,7 @@ def _weed_out_duplicates(line):
         return new_filter
 
 
-def _generate_path_between_rules(old_rules, new_rules):
+def _generate_path_between_rules(table_name, old_rules, new_rules):
     """Generates ebtables commands to get from old_rules to new_rules.
 
     This function diffs the two rule sets and then calculates the ebtables
@@ -614,7 +621,8 @@ def _generate_path_between_rules(old_rules, new_rules):
             chain, old_by_chain[chain], new_by_chain[chain])
     # unreferenced chains get the axe
     for chain in sorted(old_chains - new_chains):
-        statements += ['-X %s' % chain]
+        if chain not in STANDARD_CHAINS[table_name]:
+            statements += ['-X %s' % chain]
     return statements