Cache faucet_config_table_names to avoid long delay

clib/clib_mininet_test_main.py

@@ -152,8 +152,8 @@ def import_hw_config():
                     valid_types, config_file_name))
                 sys.exit(-1)
         dp_ports = config['dp_ports']
-        if len(dp_ports) != REQUIRED_TEST_PORTS:
-            print('Exactly %u dataplane ports are required, '
+        if len(dp_ports) < REQUIRED_TEST_PORTS:
+            print('At least %u dataplane ports are required, '
                   '%d are provided in %s.' %
                   (REQUIRED_TEST_PORTS, len(dp_ports), config_file_name))
             sys.exit(-1)
@@ -680,6 +680,10 @@ def parse_args():
         '-d', '--dumpfail', action='store_true', help='dump logs for failed tests')
     parser.add_argument(
         '-k', '--keep_logs', action='store_true', help='keep logs even for OK tests')
+    loglevels = ('debug', 'error', 'warning', 'info', 'output')
+    parser.add_argument(
+        '-l', '--loglevel', choices=loglevels, default='warning',
+        help='set mininet logging level')
     parser.add_argument(
         '-n', '--nocheck', action='store_true', help='skip dependency check')
     parser.add_argument(
@@ -722,16 +726,19 @@ def parse_args():
     return (
         requested_test_classes, args.clean, args.dumpfail,
         args.keep_logs, args.nocheck, args.serial, args.repeat,
-        excluded_test_classes, report_json_filename, port_order)
+        excluded_test_classes, report_json_filename, port_order,
+        args.loglevel)
 
 
 def test_main(module):
     """Test main."""
-    setLogLevel('error')
     print('testing module %s' % module)
 
     (requested_test_classes, clean, dumpfail, keep_logs, nocheck,
-     serial, repeat, excluded_test_classes, report_json_filename, port_order) = parse_args()
+     serial, repeat, excluded_test_classes, report_json_filename, port_order,
+     loglevel) = parse_args()
+
+    setLogLevel(loglevel)
 
     if clean:
         print('Cleaning up test interfaces, processes and openvswitch '

clib/mininet_test_base.py

@@ -26,9 +26,10 @@
 
 from ryu.ofproto import ofproto_v1_3 as ofp
 
-from mininet.log import error, output # pylint: disable=import-error
-from mininet.net import Mininet # pylint: disable=import-error
-from mininet.util import dumpNodeConnections, pmonitor # pylint: disable=import-error
+from mininet.link import Intf as HWIntf  # pylint: disable=import-error
+from mininet.log import error, output  # pylint: disable=import-error
+from mininet.net import Mininet  # pylint: disable=import-error
+from mininet.util import dumpNodeConnections, pmonitor  # pylint: disable=import-error
 
 from clib import mininet_test_util
 from clib import mininet_test_topo
@@ -374,7 +375,9 @@ def tearDown(self, ignore_oferrors=False):
                     'dump-flows', 'dump-groups', 'dump-meters',
                     'dump-group-stats', 'dump-ports', 'dump-ports-desc'):
                 switch_dump_name = os.path.join(self.tmpdir, '%s-%s.log' % (switch.name, dump_cmd))
-                switch.cmd('%s %s %s > %s' % (self.OFCTL, dump_cmd, switch.name, switch_dump_name))
+                # This seems to fail occasionally, so we warn on error rather than quitting
+                switch.cmd('%s %s %s > %s' % (self.OFCTL, dump_cmd, switch.name, switch_dump_name),
+                           success=None)
             for other_cmd in ('show', 'list controller', 'list manager'):
                 other_dump_name = os.path.join(self.tmpdir, '%s.log' % other_cmd.replace(' ', ''))
                 switch.cmd('%s %s > %s' % (self.VSCTL, other_cmd, other_dump_name))
@@ -429,31 +432,49 @@ def _cmd(cmd):
                 _cmd(cmd)
 
     def _attach_physical_switch(self):
-        """Bridge a physical switch into test topology."""
+        """Bridge a physical switch into test topology.
+
+           We do this for now to enable us to reconnect
+           virtual ethernet interfaces which may already
+           exist on emulated hosts and other OVS instances.
+
+           (One alternative would be to create a Link() class
+           that uses the hardware interfaces directly.)
+
+           We repurpose the first OvS switch in the topology
+           as a patch panel that transparently connects the
+           hardware interfaces to the host/switch veth links."""
         switch = self.first_switch()
-        phys_macs = set()
-        mapped_base = len(self.switch_map)
-        for port_i, test_host_port in enumerate(sorted(self.switch_map), start=1):
-            mapped_port_i = mapped_base + port_i
-            phys_port = FaucetIntf(self.switch_map[test_host_port], node=switch)
-            phys_mac = self.get_mac_of_intf(phys_port.name)
-            self.assertFalse(phys_mac in phys_macs, 'duplicate physical MAC %s' % phys_mac)
-            phys_macs.add(phys_mac)
-            switch.cmd(
-                ('ovs-vsctl add-port %s %s -- '
-                 'set Interface %s ofport_request=%u') % (
-                     switch.name,
-                     phys_port.name,
-                     phys_port.name,
-                     mapped_port_i))
-            switch.cmd('%s add-flow %s in_port=%u,eth_src=%s,priority=2,actions=drop' % (
-                self.OFCTL, switch.name, mapped_port_i, phys_mac))
-            switch.cmd('%s add-flow %s in_port=%u,eth_dst=%s,priority=2,actions=drop' % (
-                self.OFCTL, switch.name, port_i, phys_mac))
-            for port_pair in ((port_i, mapped_port_i), (mapped_port_i, port_i)):
-                in_port, out_port = port_pair
-                switch.cmd('%s add-flow %s in_port=%u,priority=1,actions=output:%u' % (
-                    self.OFCTL, switch.name, in_port, out_port))
+        # hw_names are the names of the server hardware interfaces
+        # that are cabled to the device under test, sorted by OF port number
+        hw_names = [self.switch_map[port] for port in sorted(self.switch_map)]
+        hw_macs = set()
+        # ovs_ports are the (sorted) OF port numbers of the OvS interfaces
+        # that are already attached to the emulated network.
+        # The actual tests reorder them according to port_map
+        ovs_ports = sorted(self.topo.switch_ports[switch.name])
+        # Patch hardware interfaces through to to OvS interfaces
+        for hw_name, ovs_port in zip(hw_names, ovs_ports):
+            # Note we've already removed any Linux IP addresses from hw_name
+            # and blocked traffic to/from its meaningless MAC
+            hw_mac = self.get_mac_of_intf(hw_name)
+            self.assertFalse(hw_mac in hw_macs,
+                             'duplicate hardware MAC %s' % hw_mac)
+            hw_macs.add(hw_mac)
+            # Create mininet Intf and attach it to the switch
+            hw_intf = HWIntf(hw_name, node=switch)
+            switch.attach(hw_intf)
+            hw_port = switch.ports[hw_intf]
+            # Connect hw_port <-> ovs_port
+            src, dst = hw_port, ovs_port
+            for flow in (
+                    # Drop anything to or from the meaningless hw_mac
+                    'eth_src=%s,priority=2,actions=drop' % hw_mac,
+                    'eth_dst=%s,priority=2,actions=drop' % hw_mac,
+                    # Forward traffic bidirectionally src <-> dst
+                    'in_port=%u,priority=1,actions=output:%u' % (src, dst),
+                    'in_port=%u,priority=1,actions=output:%u' % (dst, src)):
+                switch.cmd(self.OFCTL, 'add-flow', switch, flow)
 
     def create_port_map(self, dpid):
         """Return a port map {'port_1': port...} for a dpid in self.topo"""
@@ -1194,7 +1215,15 @@ def _prometheus_url(self, controller):
                 self.get_prom_addr(), self.config_ports['gauge_prom_port'])
         raise NotImplementedError
 
+    _last_scrape_time = 0.0  # last scrape time
+
     def scrape_prometheus(self, controller='faucet', timeout=15, var=None):
+        # Pause a bit if it has been less than a second since last request
+        min_interval = 1
+        scrape_interval = time.time() - self._last_scrape_time
+        if scrape_interval < min_interval:
+            time.sleep(min_interval - scrape_interval)
+        self._last_scrape_time = time.time()
         url = self._prometheus_url(controller)
         try:
             prom_raw = requests.get(url, {}, timeout=timeout).text
@@ -1229,16 +1258,24 @@ def wait_for_prometheus_var(self, var, result_wanted, labels=None, any_labels=Fa
                 dpid=dpid, multiple=multiple, controller=controller, retries=retries)
             if result == result_wanted:
                 return True
-            time.sleep(1)
         return False
 
+    _prometheus_cache = ''
+
     def scrape_prometheus_var(self, var, labels=None, any_labels=False, default=None,
-                              dpid=True, multiple=False, controller='faucet', retries=3):
+                              dpid=True, multiple=False, controller='faucet', retries=3,
+                              cache=False):
+        """Scrape a variable from prometheus
+           var: prometheus variable name
+           label: label if any
+           any_labels: accept any/all labels? (False)
+           default: default value to return if var is missing (None)
+           dpid: dpid | True (use self.dpid) | None (True)
+           controller: controller to scrape ('faucet')
+           retries: number of times to retry before giving up
+           cache: check cached results instead? (False)"""
         if dpid:
-            if dpid is True:
-                dpid = int(self.dpid)
-            else:
-                dpid = int(dpid)
+            dpid = int(self.dpid) if dpid is True else int(dpid)
         label_values_re = r''
         if any_labels:
             label_values_re = r'\{[^\}]+\}'
@@ -1254,10 +1291,17 @@ def scrape_prometheus_var(self, var, labels=None, any_labels=False, default=None
                 label_values_re = r'\{%s\}' % r'\S+'.join(label_values)
         var_re = re.compile(r'^%s%s$' % (var, label_values_re))
         for _ in range(retries):
-            results = []
-            prom_lines = self.scrape_prometheus(controller, var=var)
-            for prom_line in prom_lines:
-                prom_var, prom_val = self.parse_prom_var(prom_line)
+            results, prom_lines = [], []
+            if cache:
+                prom_lines = [line for line in self._prometheus_cache
+                              if line.startswith(var)]
+            if not prom_lines:
+                prom_lines = self.scrape_prometheus(controller)
+                self._prometheus_cache = prom_lines
+                prom_lines = [line for line in prom_lines
+                              if line.startswith(var)]
+            for line in prom_lines:
+                prom_var, prom_val = self.parse_prom_var(line)
                 if var_re.match(prom_var):
                     results.append((var, prom_val))
                     if not multiple:
@@ -1266,7 +1310,8 @@ def scrape_prometheus_var(self, var, labels=None, any_labels=False, default=None
                 if multiple:
                     return results
                 return results[0][1]
-            time.sleep(1)
+            if cache and prom_lines:
+                break
         return default
 
     def gauge_smoke_test(self):
@@ -1312,7 +1357,6 @@ def get_configure_count(self, retries=5):
                 'faucet_config_reload_requests_total', default=None, dpid=False)
             if count:
                 break
-            time.sleep(1)
         self.assertTrue(count, msg='configure count stayed zero')
         return count
 
@@ -1434,7 +1478,6 @@ def verify_traveling_dhcp_mac(self, retries=10):
                         new_locations.add(location)
                 if locations != new_locations:
                     break
-                time.sleep(1)
             # TODO: verify port/host association, not just that host moved.
             self.assertNotEqual(locations, new_locations)
             locations = new_locations
@@ -1905,7 +1948,6 @@ def verify_faucet_reconf(self, timeout=10,
             configure_count = self.get_configure_count()
             if configure_count > start_configure_count:
                 break
-            time.sleep(1)
         self.assertNotEqual(
             start_configure_count, configure_count, 'FAUCET did not reconfigure')
         if change_expected:
@@ -1914,7 +1956,6 @@ def verify_faucet_reconf(self, timeout=10,
                     self.scrape_prometheus_var(var, dpid=True, default=0))
                 if new_count > old_count:
                     break
-                time.sleep(1)
             self.assertTrue(
                 new_count > old_count,
                 msg='%s did not increment: %u' % (var, new_count))
@@ -2002,7 +2043,6 @@ def wait_port_status(self, dpid, port_no, status, expected_status, timeout=10):
             if port_status is not None and port_status == expected_status:
                 return
             self._portmod(dpid, port_no, status, ofp.OFPPC_PORT_DOWN)
-            time.sleep(1)
         self.fail('dpid %x port %s status %s != expected %u' % (
             dpid, port_no, port_status, expected_status))
 
@@ -2026,26 +2066,30 @@ def wait_dp_status(self, expected_status, controller='faucet', timeout=30):
         return self.wait_for_prometheus_var(
             'dp_status', expected_status, any_labels=True, controller=controller, default=None, timeout=timeout)
 
-    def _get_tableid(self, name):
-        return self.scrape_prometheus_var(
-            'faucet_config_table_names', {'table_name': name})
-
     def quiet_commands(self, host, commands):
         for command in commands:
             result = host.cmd(command)
             self.assertEqual('', result, msg='%s: %s' % (command, result))
 
     def _config_tableids(self):
-        self._PORT_ACL_TABLE = self._get_tableid('port_acl')
-        self._VLAN_TABLE = self._get_tableid('vlan')
-        self._VLAN_ACL_TABLE = self._get_tableid('vlan_acl')
-        self._ETH_SRC_TABLE = self._get_tableid('eth_src')
-        self._IPV4_FIB_TABLE = self._get_tableid('ipv4_fib')
-        self._IPV6_FIB_TABLE = self._get_tableid('ipv6_fib')
-        self._VIP_TABLE = self._get_tableid('vip')
-        self._ETH_DST_HAIRPIN_TABLE = self._get_tableid('eth_dst_hairpin')
-        self._ETH_DST_TABLE = self._get_tableid('eth_dst')
-        self._FLOOD_TABLE = self._get_tableid('flood')
+        def get_tableid(name, retries=1, cache=True):
+            return self.scrape_prometheus_var(
+                'faucet_config_table_names',
+                {'table_name': name}, retries=retries, cache=cache)
+        # Always-there tables
+        self._VLAN_TABLE = get_tableid('vlan', retries=3, cache=False)
+        self._ETH_SRC_TABLE = get_tableid('eth_src')
+        self._ETH_DST_TABLE = get_tableid('eth_dst')
+        self._FLOOD_TABLE = get_tableid('flood')
+        self.assertTrue(None not in (self._VLAN_TABLE, self._ETH_SRC_TABLE,
+                                     self._ETH_DST_TABLE, self._FLOOD_TABLE))
+        # Sometimes-there tables
+        self._PORT_ACL_TABLE = get_tableid('port_acl')
+        self._VLAN_ACL_TABLE = get_tableid('vlan_acl')
+        self._IPV4_FIB_TABLE = get_tableid('ipv4_fib')
+        self._IPV6_FIB_TABLE = get_tableid('ipv6_fib')
+        self._VIP_TABLE = get_tableid('vip')
+        self._ETH_DST_HAIRPIN_TABLE = get_tableid('eth_dst_hairpin')
 
     def _dp_ports(self):
         return list(sorted(self.port_map.values()))

clib/mininet_test_topo.py

@@ -12,7 +12,7 @@
 
 # pylint: disable=too-many-arguments
 
-from mininet.log import output
+from mininet.log import output, warn
 from mininet.topo import Topo
 from mininet.node import Controller
 from mininet.node import CPULimitedHost
@@ -24,7 +24,6 @@
 
 # TODO: this should be configurable (e.g for randomization)
 SWITCH_START_PORT = 5
-HW_OFFSET = 16  # Must be > max(dp_ports)
 
 
 class FaucetIntf(TCIntf):
@@ -69,18 +68,40 @@ def __init__(self, name, **params):
         super().__init__(
             name=name, reconnectms=8000, **params)
 
+    @staticmethod
+    def _workaround(args):
+        """Workarounds/hacks for errors resulting from
+           cmd() calls within Mininet"""
+        # Workaround: ignore ethtool errors on tap interfaces
+        # This allows us to use tap tunnels as cables to switch ports,
+        # for example to test against OvS in a VM.
+        if (len(args) > 1 and args[0] == 'ethtool -K' and
+                getattr(args[1], 'name', '').startswith('tap')):
+            return True
+        return False
+
     def cmd(self, *args, success=0, **kwargs):
-        """Switch commands should always succeed,
+        """Commands typically must succeed for proper switch operation,
            so we check the exit code of the last command in *args.
            success: desired exit code (or None to skip check)"""
         # pylint: disable=arguments-differ
         cmd_output = super().cmd(*args, **kwargs)
         exit_code = int(super().cmd('echo $?'))
         if success is not None and exit_code != success:
-            raise RuntimeError(
-                "%s exited with (%d):'%s'" % (args, exit_code, cmd_output))
+            msg = "%s exited with (%d):'%s'" % (args, exit_code, cmd_output)
+            if self._workaround(args):
+                warn('Ignoring:', msg, '\n')
+            else:
+                raise RuntimeError(msg)
         return cmd_output
 
+    def attach(self, intf):
+        "Attach an interface and set its port"
+        super().attach(intf)
+        # This should be done in Mininet, but we do it for now
+        port = self.ports[intf]
+        self.cmd('ovs-vsctl set Interface', intf, 'ofport_request=%s' % port)
+
     def start(self, controllers):
         # Transcluded from Mininet source, since need to insert
         # controller parameters at switch creation time.

docker/runtests.sh

@@ -9,7 +9,7 @@ set -e  # quit on error
 
 # allow user to skip parts of docker test
 # this wrapper script only cares about -n, -u, -i, others passed to test suite.
-while getopts "cdijknrsuxoz" o $FAUCET_TESTS; do
+while getopts "cdijknrsuxozl" o $FAUCET_TESTS; do
   case "${o}" in
         i)
             # run only integration tests

faucet/dp.py

@@ -303,6 +303,7 @@ def __init__(self, _id, dp_id, conf):
         self.stack_root_name = None
         self.stack_roots_names = None
         self.stack_route_learning = None
+        self.stack_root_flood_reflection = None
 
         self.acls = {}
         self.vlans = {}
@@ -800,6 +801,9 @@ def resolve_stack_topology(self, dps, meta_dp_state):
                 test_config_condition(
                     path_to_root_len == 0, '%s not connected to stack' % dp)
 
+            if self.stack_longest_path_to_root_len() > 2:
+                self.stack_root_flood_reflection = True
+
         if self.tunnel_acls:
             self.finalize_tunnel_acls(dps)