From 9c9adb966322bf805bf3a50aab76171c46118b3f Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Fri, 4 Sep 2026 14:17:22 +1200 Subject: [PATCH 1/5] Enable warm start for vlans with routing enabled --- faucet/dp.py | 59 +++-- faucet/valve.py | 52 +++- faucet/valve_acl.py | 2 +- faucet/valve_manager_base.py | 2 +- faucet/valve_route.py | 284 ++++++++++++++++++++- faucet/valve_switch_standalone.py | 15 +- tests/integration/mininet_multidp_tests.py | 2 +- tests/integration/mininet_tests.py | 6 +- 8 files changed, 365 insertions(+), 57 deletions(-) diff --git a/faucet/dp.py b/faucet/dp.py index 45dfaea576..58e8378aff 100644 --- a/faucet/dp.py +++ b/faucet/dp.py @@ -1483,8 +1483,9 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): changed_acls (set): changed/added ACL IDs. Returns: changes (tuple) of: + added_vlans (set): added VLAN IDs. deleted_vlans (set): deleted VLAN IDs. - changed_vlans (set): changed/added VLAN IDs. + changed_vlans (set): changed VLAN IDs. changed_acl_vlans (set): changed ACL only VLAN IDs. """ ( @@ -1502,7 +1503,6 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): diff=True, ignore_keys=frozenset(["acls_in"]), ) - changed_vlans = added_vlans.union(changed_vlans) changed_acl_vlans = set() for vlan_id in same_vlans: old_vlan = self.vlans[vlan_id] @@ -1516,7 +1516,7 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): same_vlans -= changed_acl_vlans logger.info("vlans where ACL only changed: %s" % changed_acl_vlans) - return (deleted_vlans, changed_vlans, changed_acl_vlans) + return (added_vlans, deleted_vlans, changed_vlans, changed_acl_vlans) def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): changed = False @@ -1545,6 +1545,7 @@ def _get_port_config_changes( self, logger, new_dp, + added_vlans, changed_vlans, deleted_vlans, changed_acl_vlans, @@ -1555,7 +1556,8 @@ def _get_port_config_changes( Args: logger (ValveLogger): logger instance. new_dp (DP): new dataplane configuration. - changed_vlans (set): changed/added VLAN IDs. + added_vlans (set): added VLAN IDs. + changed_vlans (set): changed VLAN IDs. deleted_vlans (set): deleted VLAN IDs. changed_acl_vlans (set): changed ACL only VLAN IDs. changed_acls (set): changed/added ACL IDs. @@ -1584,6 +1586,7 @@ def _get_port_config_changes( ignore_keys=frozenset(["acls_in"]), ) + changed_vlan_ports = set() changed_acl_ports = set() all_ports_changed = False @@ -1601,18 +1604,25 @@ def _get_port_config_changes( logger.info("Stack topology change detected, restarting stack ports") same_ports -= changed_ports - if not same_ports: + all_ports = frozenset(new_dp.ports.keys()) + if not same_ports or changed_ports == all_ports: all_ports_changed = True else: - if changed_vlans: - all_ports = frozenset(new_dp.ports.keys()) - new_changed_vlans = { - vlan for vlan in new_dp.vlans.values() if vlan.vid in changed_vlans - } - for vlan in new_changed_vlans: - changed_port_nums = {port.number for port in vlan.get_ports()} - changed_ports.update(changed_port_nums) - all_ports_changed = changed_ports == all_ports + if added_vlans or changed_vlans: + added_changed_vlans = [] + if added_vlans: + added_changed_vlans.extend(added_vlans) + if changed_vlans: + added_changed_vlans.extend(changed_vlans) + if len(added_changed_vlans) >= 1: + new_changed_vlans = { + vlan + for vlan in new_dp.vlans.values() + if vlan.vid in added_changed_vlans + } + for vlan in new_changed_vlans: + changed_port_nums = {port.number for port in vlan.get_ports()} + changed_vlan_ports.update(changed_port_nums) if changed_acl_vlans: # Adding first VLAN ACL or deleting final VLAN ACL affects # whether packets for port need to use the VLAN ACL table or not, @@ -1680,19 +1690,10 @@ def _add_changed_vlans(old_port, new_port): same_ports -= changed_acl_ports logger.info("ports where ACL only changed: %s" % changed_acl_ports) + changed_ports.update(changed_vlan_ports) same_ports -= changed_ports + changed_vlans -= added_vlans changed_vlans -= deleted_vlans - # TODO: limit scope to only routers that have affected VLANs. - changed_vlans_with_vips = [] - for vid in changed_vlans: - vlan = new_dp.vlans[vid] - if vlan.faucet_vips: - changed_vlans_with_vips.append(vlan) - if changed_vlans_with_vips: - logger.info( - "forcing cold start because %s has routing" % changed_vlans_with_vips - ) - all_ports_changed = True return ( all_ports_changed, @@ -1737,8 +1738,9 @@ def get_config_changes(self, logger, new_dp): changed_ports (set): changed port numbers. added_ports (set): added port numbers. changed_acl_ports (set): changed ACL only port numbers. + added_vlans (set): added VLAN IDs. deleted_vlans (set): deleted VLAN IDs. - changed_vlans (set): changed/added VLAN IDs. + changed_vlans (set): changed VLAN IDs. changed_acl_vlans (set): changed ACL only VLAN IDs. all_ports_changed (bool): True if all ports changed. all_meters_changed (bool): True if all meters changed @@ -1762,7 +1764,7 @@ def get_config_changes(self, logger, new_dp): ) else: changed_acls = self._get_acl_config_changes(logger, new_dp) - deleted_vlans, changed_vlans, changed_acl_vlans = ( + added_vlans, deleted_vlans, changed_vlans, changed_acl_vlans = ( self._get_vlan_config_changes(logger, new_dp, changed_acls) ) ( @@ -1781,6 +1783,7 @@ def get_config_changes(self, logger, new_dp): ) = self._get_port_config_changes( logger, new_dp, + added_vlans, changed_vlans, deleted_vlans, changed_acl_vlans, @@ -1791,6 +1794,7 @@ def get_config_changes(self, logger, new_dp): changed_ports, added_ports, changed_acl_ports, + added_vlans, deleted_vlans, changed_vlans, changed_acl_vlans, @@ -1809,6 +1813,7 @@ def get_config_changes(self, logger, new_dp): set(), set(), set(), + set(), True, True, set(), diff --git a/faucet/valve.py b/faucet/valve.py index a98e69b19e..b00cce3870 100644 --- a/faucet/valve.py +++ b/faucet/valve.py @@ -449,22 +449,22 @@ def add_vlans(self, vlans, cold_start=False): ofmsgs.extend(self.add_vlan(vlan, cold_start=cold_start)) return ofmsgs - def del_vlan(self, vlan): + def del_vlan(self, vlan, dp_vlans): """Delete a configured VLAN.""" self.logger.info("Delete VLAN %s" % vlan) ofmsgs = [] for manager in self._managers: - ofmsgs.extend(manager.del_vlan(vlan)) + ofmsgs.extend(manager.del_vlan(vlan, dp_vlans)) expired_hosts = list(vlan.dyn_host_cache.values()) for entry in expired_hosts: self._update_expired_host(entry, vlan) vlan.reset_caches() return ofmsgs - def del_vlans(self, vlans): + def del_vlans(self, vlans, dp_vlans): ofmsgs = [] for vlan in vlans: - ofmsgs.extend(self.del_vlan(vlan)) + ofmsgs.extend(self.del_vlan(vlan, dp_vlans)) return ofmsgs def _get_all_configured_port_nos(self): @@ -1579,8 +1579,9 @@ def _apply_config_changes(self, new_dp, changes, valves=None): changed_ports (set): changed port numbers. added_ports (set): added port numbers. changed_acl_ports (set): changed ACL only port numbers. + added_vids (set): added VLAN IDs. deleted_vids (set): deleted VLAN IDs. - changed_vids (set): changed/added VLAN IDs. + changed_vids (set): changed VLAN IDs. all_ports_changed (bool): True if all ports changed. all_meters_changed (bool): True if all meters changed. deleted_meters: (set): deleted meter numbers. @@ -1597,6 +1598,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None): changed_ports, added_ports, changed_acl_ports, + added_vids, deleted_vids, changed_vids, changed_acl_vlans, @@ -1636,7 +1638,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None): ofmsgs.extend(self.ports_delete(changed_ports)) if deleted_vids: deleted_vlans = [self.dp.vlans[vid] for vid in deleted_vids] - ofmsgs.extend(self.del_vlans(deleted_vlans)) + ofmsgs.extend(self.del_vlans(deleted_vlans, new_dp.vlans.values())) # TODO: optimize for all meters being erased if changed_meters: # If a meter changed meter IDs, delete the old ID first and consider @@ -1651,6 +1653,10 @@ def _apply_config_changes(self, new_dp, changes, valves=None): if deleted_meters: ofmsgs.extend(self.acl_manager.del_meters(deleted_meters)) + if changed_vids: + changed_vlans = [self.dp.vlans[vid] for vid in changed_vids] + ofmsgs.extend(self.del_vlans(changed_vlans, new_dp.vlans.values())) + self.dp_init(new_dp, valves) if self.acl_manager: @@ -1674,14 +1680,36 @@ def _apply_config_changes(self, new_dp, changes, valves=None): if changed_acl_vlans: for vid in changed_acl_vlans: vlan = self.dp.vlans[vid] - ofmsgs.extend(self.acl_manager.del_vlan(vlan)) + ofmsgs.extend( + self.acl_manager.del_vlan(vlan, self.dp.vlans.values()) + ) ofmsgs.extend(self.acl_manager.add_vlan(vlan, False)) + if added_vids: + added_vlans = [self.dp.vlans[vid] for vid in added_vids] + ofmsgs.extend(self.add_vlans(added_vlans, cold_start=True)) if changed_vids: - changed_vlans = [self.dp.vlans[vid] for vid in changed_vids] - # TODO: handle change versus add separately so can avoid delete first. - ofmsgs.extend(self.del_vlans(changed_vlans)) - # The proceeding delete operation means we don't have to generate more deletes. - ofmsgs.extend(self.add_vlans(changed_vlans, cold_start=True)) + changed_vlans = {self.dp.vlans[vid] for vid in changed_vids} + ofmsgs.extend(self.add_vlans(changed_vlans, cold_start=False)) + + routed_vlans = set() + for changed_vlan in changed_vlans: + if len(changed_vlan.faucet_vips) == 0: + # VLAN not configured for routing + continue + if self.dp.routers: + for router in self.dp.routers.values(): + if changed_vlan in router.vlans: + routed_vlans.update(router.vlans) + routed_vlans -= changed_vlans + for vlan in routed_vlans: + self.logger.info( + "Expiring next hops for %s which is in the same router as a VLAN that changed configuration" + % vlan + ) + for route_manager in self._route_manager_by_ipv.values(): + # Expire next hop cache for vlans that are in the same router as a changed vlan + # to force next hops to be relearned and repopulated in FIB of changed vlan + ofmsgs.extend(route_manager.expire_vlan_nexthops(vlan)) if self.stack_manager: ofmsgs.extend(self.stack_manager.add_tunnel_acls()) return restart_type, ofmsgs diff --git a/faucet/valve_acl.py b/faucet/valve_acl.py index b85f6ea2f6..baf3ef9db3 100644 --- a/faucet/valve_acl.py +++ b/faucet/valve_acl.py @@ -522,7 +522,7 @@ def cold_start_port(self, port): ofmsgs.extend(self.add_port(port)) return ofmsgs - def del_vlan(self, vlan): + def del_vlan(self, vlan, dp_vlans): """Remove VLAN ACLs if configured.""" ofmsgs = [] for table in (self.vlan_acl_table, self.egress_acl_table): diff --git a/faucet/valve_manager_base.py b/faucet/valve_manager_base.py index 14c1b7c5cb..ded929bfdf 100644 --- a/faucet/valve_manager_base.py +++ b/faucet/valve_manager_base.py @@ -33,7 +33,7 @@ def add_port(self, port): """install flows in response to a new port""" return [] - def del_vlan(self, vlan): + def del_vlan(self, vlan, dp_vlans): """delete flows in response to a VLAN removal""" return [] diff --git a/faucet/valve_route.py b/faucet/valve_route.py index 393c24138d..0d4f36700e 100644 --- a/faucet/valve_route.py +++ b/faucet/valve_route.py @@ -322,6 +322,29 @@ def _vlan_nexthop_cache(self, vlan): """Return vlan neighbour cache""" return vlan.neigh_cache_by_ipv(self.IPV) + def _expire_nexthops(self, now, vlan, dead_nexthops): + """Expire list of next hops for a vlan""" + ofmsgs = [] + for ip_gw, nexthop_cache_entry in dead_nexthops: + self.logger.info( + "marking %s as a dead nexthop" % nexthop_cache_entry.eth_src + ) + ofmsgs.extend( + self._expire_gateway_flows(ip_gw, nexthop_cache_entry, vlan, now) + ) + return ofmsgs + + def expire_vlan_nexthops(self, vlan): + """Expire all hosts on a vlan""" + now = time.time() + nexthop_cache = self._vlan_nexthop_cache(vlan) + dead_nexthops = [ + (ip_gw, nexthop_cache_entry) + for ip_gw, nexthop_cache_entry in nexthop_cache.items() + if nexthop_cache_entry + ] + return self._expire_nexthops(now, vlan, dead_nexthops) + def expire_port_nexthops(self, port): """Expire all hosts on a port""" ofmsgs = [] @@ -335,13 +358,7 @@ def expire_port_nexthops(self, port): and nexthop_cache_entry.port and port.number == nexthop_cache_entry.port.number ] - for ip_gw, nexthop_cache_entry in dead_nexthops: - self.logger.info( - "marking %s as a dead nexthop" % nexthop_cache_entry.eth_src - ) - ofmsgs.extend( - self._expire_gateway_flows(ip_gw, nexthop_cache_entry, vlan, now) - ) + ofmsgs.extend(self._expire_nexthops(now, vlan, dead_nexthops)) return ofmsgs def _vlan_nexthop_cache_entry(self, vlan, ip_gw): @@ -503,13 +520,166 @@ def add_vlan(self, vlan, cold_start): ofmsgs.extend( self._add_faucet_fib_to_vip(vlan, priority, faucet_vip, faucet_vip_host) ) + routed_vlans = self._routed_vlans(vlan) + if not cold_start and not self.global_routing: + for routed_vlan in routed_vlans: + if routed_vlan == vlan or isinstance(routed_vlan, AnonVLAN): + continue + for faucet_vip in routed_vlan.faucet_vips_by_ipv(self.IPV): + if self.proactive_learn and not faucet_vip.ip.is_link_local: + # When warm starting, reinstall FIB routes for FAUCET VIP subnets + # of other VLANs in the same router as this VLAN + learn_connected_priority = ( + self.route_priority + faucet_vip.network.prefixlen + ) + ofmsgs.append( + self.fib_table.flowmod( + self._route_match(vlan, faucet_vip), + priority=learn_connected_priority, + inst=(self.fib_table.goto(self.vip_table),), + ) + ) + return ofmsgs + + def _del_faucet_mac(self, faucet_mac, dp_vlans): + """Delete flows associated with a given faucet mac""" + ofmsgs = [] + max_prefixlen = 32 if self.IPV == 4 else 128 + + dp_macs = set() + dp_mac_global_vip_present = {} + for dp_vlan in dp_vlans: + if dp_vlan.faucet_vips_by_ipv(self.IPV): + dp_macs.add(dp_vlan.faucet_mac) + if dp_vlan.faucet_mac not in dp_mac_global_vip_present: + dp_mac_global_vip_present[dp_vlan.faucet_mac] = False + for faucet_vip in dp_vlan.faucet_vips_by_ipv(self.IPV): + if not faucet_vip.ip.is_link_local: + dp_mac_global_vip_present[dp_vlan.faucet_mac] = True + break + + if faucet_mac not in dp_macs: + # FAUCET MAC is no longer used by any VLAN on DP + for eth_type in self.CONTROL_ETH_TYPES: + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_dst=faucet_mac, eth_type=eth_type + ) + ) + ) + elif not dp_mac_global_vip_present[faucet_mac]: + # FAUCET MAC remains active on DP, but only used by link-local VIP + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_dst=faucet_mac, + eth_type=self.ETH_TYPE, + nw_proto=self.ICMP_TYPE, + ), + priority=self.route_priority + max_prefixlen - 1, + strict=True, + ) + ) + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_dst=faucet_mac, + eth_type=self.ETH_TYPE, + ), + priority=self.route_priority + max_prefixlen - 3, + strict=True, + ) + ) + + if True not in dp_mac_global_vip_present.values(): + # No global scope VIPs left on DP + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_type=self.ETH_TYPE, nw_proto=self.ICMP_TYPE + ), + priority=self.route_priority + max_prefixlen - 2, + strict=True, + ) + ) + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match(eth_type=self.ETH_TYPE), + priority=self.route_priority + max_prefixlen - 4, + strict=True, + ) + ) + return ofmsgs + + def _del_faucet_vip(self, vlan, faucet_vip, dp_faucet_vip_hosts, dp_faucet_vips): + """Delete flows associated with a given faucet vip""" + ofmsgs = [] + if self.global_routing: + faucet_vip_host = self._host_from_faucet_vip(faucet_vip) + if faucet_vip_host not in dp_faucet_vip_hosts: + # FAUCET VIP host route no longer in use on DP + ofmsgs.append( + self.fib_table.flowdel( + match=self._route_match(self.global_vlan, faucet_vip_host) + ) + ) + if ( + self.proactive_learn + and not faucet_vip.ip.is_link_local + and faucet_vip not in dp_faucet_vips + ): + # FAUCET VIP no longer in use on DP + ofmsgs.append( + self.fib_table.flowdel( + match=self._route_match(self.global_vlan, faucet_vip) + ) + ) + return ofmsgs + if self.proactive_learn and not faucet_vip.ip.is_link_local: + routed_vlans = self._routed_vlans(vlan) + router_faucet_vips = set() + for routed_vlan in routed_vlans: + if routed_vlan == vlan or isinstance(routed_vlan, AnonVLAN): + continue + router_faucet_vips.update(routed_vlan.faucet_vips) + if faucet_vip not in router_faucet_vips: + for routed_vlan in routed_vlans: + # FAUCET VIP no longer in use by any VLANs in router + ofmsgs.append( + self.fib_table.flowdel( + match=self._route_match(routed_vlan, faucet_vip) + ) + ) return ofmsgs - def del_vlan(self, vlan): + def del_vlan(self, vlan, dp_vlans): """Delete a VLAN.""" ofmsgs = [] - if vlan.faucet_vips_by_ipv: - ofmsgs.append(self.fib_table.flowdel(match=self.fib_table.match(vlan=vlan))) + if not vlan.faucet_vips_by_ipv(self.IPV): + return ofmsgs + ofmsgs.append(self.fib_table.flowdel(match=self.fib_table.match(vlan=vlan))) + ofmsgs.extend(self._del_faucet_mac(vlan.faucet_mac, dp_vlans)) + + # Expire next hops for this VLAN to remove static routes + # from FIB of VLANs in same router as this one + self.expire_vlan_nexthops(vlan) + + dp_faucet_vips = set() + dp_faucet_vip_hosts = set() + if len(vlan.faucet_vips_by_ipv(self.IPV)) >= 1 and self.global_routing: + for dp_vlan in dp_vlans: + for faucet_vip in dp_vlan.faucet_vips_by_ipv(self.IPV): + dp_faucet_vips.add(faucet_vip) + faucet_vip_host = self._host_from_faucet_vip(faucet_vip) + dp_faucet_vip_hosts.add(faucet_vip_host) + + for faucet_vip in vlan.faucet_vips_by_ipv(self.IPV): + ofmsgs.extend( + self._del_faucet_vip( + vlan, faucet_vip, dp_faucet_vip_hosts, dp_faucet_vips + ) + ) return ofmsgs def _add_resolved_route(self, vlan, ip_gw, ip_dst, eth_dst, is_updated): @@ -1046,6 +1216,33 @@ def _add_faucet_vip_nd(self, vlan, priority, faucet_vip, faucet_vip_host): ) return ofmsgs + def del_vlan(self, vlan, dp_vlans): + """Delete a VLAN.""" + ofmsgs = super().del_vlan(vlan, dp_vlans) + if not vlan.faucet_vips_by_ipv(self.IPV): + return ofmsgs + + dp_faucet_vip_hosts = set() + for dp_vlan in dp_vlans: + for faucet_vip in dp_vlan.faucet_vips_by_ipv(self.IPV): + faucet_vip_host = self._host_from_faucet_vip(faucet_vip) + dp_faucet_vip_hosts.add(faucet_vip_host) + + for faucet_vip in vlan.faucet_vips_by_ipv(self.IPV): + faucet_vip_host = self._host_from_faucet_vip(faucet_vip) + if faucet_vip_host not in dp_faucet_vip_hosts: + # Remove ARP for FAUCET VIP flow + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_type=valve_of.ether.ETH_TYPE_ARP, + eth_dst=valve_of.mac.BROADCAST_STR, + nw_dst=faucet_vip_host, + ), + ) + ) + return ofmsgs + def _control_plane_arp_handler(self, now, pkt_meta): """Handle ARP packets destined for the router""" ofmsgs = [] @@ -1260,6 +1457,73 @@ def _add_faucet_fib_to_vip(self, vlan, priority, faucet_vip, faucet_vip_host): ) return ofmsgs + def del_vlan(self, vlan, dp_vlans): + """Delete a VLAN.""" + ofmsgs = super().del_vlan(vlan, dp_vlans) + if not vlan.faucet_vips_by_ipv(self.IPV): + return ofmsgs + + dp_mcast_macs = set() + dp_faucet_vip_broadcasts = set() + dp_link_local_present = False + for dp_vlan in dp_vlans: + for faucet_vip in dp_vlan.faucet_vips_by_ipv(self.IPV): + if faucet_vip.is_link_local: + dp_link_local_present = True + faucet_vip_host_nd_mcast = valve_packet.ipv6_link_eth_mcast( + valve_packet.ipv6_solicited_node_from_ucast(faucet_vip.ip) + ) + dp_mcast_macs.add(faucet_vip_host_nd_mcast) + if self.global_routing: + faucet_vip_broadcast = ipaddress.IPv6Interface( + faucet_vip.network.broadcast_address + ) + dp_faucet_vip_broadcasts.add(faucet_vip_broadcast) + + if not dp_link_local_present: + # No link local FAUCET VIPs present on any VLAN on DP + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_type=self.ETH_TYPE, + eth_dst=valve_packet.IPV6_ALL_ROUTERS_MCAST, + nw_proto=valve_of.inet.IPPROTO_ICMPV6, + icmpv6_type=icmpv6.ND_ROUTER_SOLICIT, + ), + ) + ) + + for faucet_vip in vlan.faucet_vips_by_ipv(self.IPV): + faucet_vip_host_nd_mcast = valve_packet.ipv6_link_eth_mcast( + valve_packet.ipv6_solicited_node_from_ucast(faucet_vip.ip) + ) + if faucet_vip_host_nd_mcast not in dp_mcast_macs: + # Remove IPv6 NS for FAUCET VIP flow + ofmsgs.append( + self.vip_table.flowdel( + match=self.vip_table.match( + eth_type=self.ETH_TYPE, + eth_dst=faucet_vip_host_nd_mcast, + nw_proto=valve_of.inet.IPPROTO_ICMPV6, + icmpv6_type=icmpv6.ND_NEIGHBOR_SOLICIT, + ), + ) + ) + if self.global_routing: + faucet_vip_broadcast = ipaddress.IPv6Interface( + faucet_vip.network.broadcast_address + ) + if faucet_vip_broadcast not in dp_faucet_vip_broadcasts: + # FAUCET VIP broadcast route no longer in use on DP + ofmsgs.append( + self.fib_table.flowdel( + match=self._route_match( + self.global_vlan, faucet_vip_broadcast + ), + ) + ) + return ofmsgs + def _nd_solicit_handler(self, now, pkt_meta, _ipv6_pkt, icmpv6_pkt): ofmsgs = [] solicited_ip = ipaddress.ip_address(icmpv6_pkt.data.dst) diff --git a/faucet/valve_switch_standalone.py b/faucet/valve_switch_standalone.py index 0d18f3af75..c0aad01978 100644 --- a/faucet/valve_switch_standalone.py +++ b/faucet/valve_switch_standalone.py @@ -526,6 +526,15 @@ def add_drop_spoofed_faucet_mac_rules(self, vlan): ofmsgs.extend(self.pipeline.filter_packets({"eth_src": vlan.faucet_mac})) return ofmsgs + def del_drop_spoofed_faucet_mac_rules(self, vlan, dp_vlans): + """Remove rules to drop spoofed faucet mac""" + ofmsgs = [] + if self.drop_spoofed_faucet_mac: + dp_macs = [vlan.faucet_mac for vlan in dp_vlans] + if vlan.faucet_mac not in dp_macs: + ofmsgs.extend(self.pipeline.remove_filter({"eth_src": vlan.faucet_mac})) + return ofmsgs + def add_vlan(self, vlan, cold_start): ofmsgs = [] ofmsgs.extend(self.add_drop_spoofed_faucet_mac_rules(vlan)) @@ -539,11 +548,13 @@ def add_vlan(self, vlan, cold_start): ofmsgs.extend(self._build_flood_rules(vlan, cold_start)) return ofmsgs - def del_vlan(self, vlan): - return [ + def del_vlan(self, vlan, dp_vlans): + ofmsgs = [ self.flood_table.flowdel(match=self.flood_table.match(vlan=vlan)), self.eth_src_table.flowdel(match=self.eth_src_table.match(vlan=vlan)), ] + ofmsgs.extend(self.del_drop_spoofed_faucet_mac_rules(vlan, dp_vlans)) + return ofmsgs def update_vlan(self, vlan): return self._build_flood_rules(vlan, cold_start=False, modify=True) diff --git a/tests/integration/mininet_multidp_tests.py b/tests/integration/mininet_multidp_tests.py index 0cb57e9efd..91e8c48a8e 100644 --- a/tests/integration/mininet_multidp_tests.py +++ b/tests/integration/mininet_multidp_tests.py @@ -2158,7 +2158,7 @@ def test_mclag_warmstart(self): self.faucet_config_path, restart=True, cold_start=False, - change_expected=False, + change_expected=True, ) self.host_information.pop(1) diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index 941edd080c..21269b82bf 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -1685,7 +1685,7 @@ def test_untagged(self): vid = random.randint(2, mininet_test_base.MAX_TEST_VID) if vid == last_vid: continue - self.change_vlan_config("randvlan", "vid", vid, cold_start=True, hup=True) + self.change_vlan_config("randvlan", "vid", vid, cold_start=False, hup=True) self.ping_all_when_learned() last_vid = vid @@ -7727,7 +7727,7 @@ def test_untagged(self): self._ip_neigh(second_host, second_faucet_vip.ip, 4), self.FAUCET_MAC2 ) self.change_vlan_config( - "vlanb", "vid", vlanb_vid, restart=True, cold_start=True + "vlanb", "vid", vlanb_vid, restart=True, cold_start=False ) @@ -7793,7 +7793,7 @@ def test_connectivity(host_a, host_b): self.port_map["port_3"], {"native_vlan": "vlana"}, restart=True, - cold_start=True, + cold_start=False, ) test_connectivity(third_host, second_host) From 6378a85b480a359a67f37c0786b6c0a7a11a7329 Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Thu, 10 Sep 2026 13:42:28 +1200 Subject: [PATCH 2/5] Add integration test for warm starting vlan with routing enabled --- clib/mininet_test_base.py | 7 +- tests/integration/mininet_tests.py | 138 +++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) diff --git a/clib/mininet_test_base.py b/clib/mininet_test_base.py index b8ad911748..41c5e9e409 100644 --- a/clib/mininet_test_base.py +++ b/clib/mininet_test_base.py @@ -3790,8 +3790,11 @@ def verify_ipv4_routing_mesh(self): ) @staticmethod - def host_drop_all_ips(host): - host.cmd("ip addr flush dev %s" % host.defaultIntf()) + def host_drop_all_ips(host, scope=None): + if scope: + host.cmd("ip addr flush dev %s scope %s" % (host.defaultIntf(), scope)) + else: + host.cmd("ip addr flush dev %s" % host.defaultIntf()) def setup_ipv6_hosts_addresses( self, diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index 21269b82bf..6488fbc2e8 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -7902,6 +7902,144 @@ def test_untagged(self): self.one_ipv6_ping(second_host, first_host_net.ip) +class FaucetUntaggedChangeVIPInterVLANRouteTest(FaucetUntaggedTest): + CONFIG_GLOBAL = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: ["10.10.0.254/24", "fa00::254/64", "fe80::c00:ff:fe00:1/64"] + vlan2: + vid: 0x200 + faucet_vips: ["10.20.0.254/24", "fb00::254/64", "fe80::c00:ff:fe00:2/64"] + faucet_mac: "0e:00:00:00:00:02" +routers: + router-1: + vlans: [vlan1, vlan2] +""" + + CONFIG = """ + arp_neighbor_timeout: 2 + max_resolve_backoff_time: 1 + interfaces: + %(port_1)d: + native_vlan: vlan1 + %(port_2)d: + native_vlan: vlan2 + %(port_3)d: + native_vlan: vlan2 + %(port_4)d: + native_vlan: vlan2 +""" + + def test_untagged(self): + def first_host_add_ips(): + self.host_drop_all_ips(first_host, scope="global") + first_host.setIP( + str(first_host_ip.ip), prefixLen=first_host_ip.network.prefixlen + ) + self.add_host_ipv6_address(first_host, first_host_ipv6) + + def second_host_add_ips(): + self.host_drop_all_ips(second_host, scope="global") + second_host.setIP( + str(second_host_ip.ip), prefixLen=second_host_ip.network.prefixlen + ) + self.add_host_ipv6_address(second_host, second_host_ipv6) + + def add_host_routes(): + self.add_host_route(first_host, second_host_ip, first_faucet_vip.ip) + self.add_host_route(first_host, second_host_ipv6, first_faucet_vip_ipv6.ip) + self.add_host_route(second_host, first_host_ip, second_faucet_vip.ip) + self.add_host_route(second_host, first_host_ipv6, second_faucet_vip_ipv6.ip) + + def first_host_connectivity_check(link_local_reachable=True): + self.one_ipv4_ping(first_host, second_host_ip.ip) + self.one_ipv6_ping(first_host, second_host_ipv6.ip) + self.one_ipv6_ping( + first_host, + first_faucet_vip_link_local_ipv6.ip, + expected_result=link_local_reachable, + ) + + def second_host_connectivity_check(link_local_reachable=True): + self.one_ipv4_ping(second_host, first_host_ip.ip) + self.one_ipv6_ping(second_host, first_host_ipv6.ip) + self.one_ipv6_ping( + second_host, + second_faucet_vip_link_local_ipv6.ip, + expected_result=link_local_reachable, + ) + + first_host, second_host = self.hosts_name_ordered()[:2] + first_host_ip = ipaddress.ip_interface("10.10.0.1/24") + first_faucet_vip = ipaddress.ip_interface("10.10.0.254/24") + first_host_ipv6 = ipaddress.ip_interface("fa00::1/64") + first_faucet_vip_ipv6 = ipaddress.ip_interface("fa00::254/64") + first_faucet_vip_link_local_ipv6 = ipaddress.ip_interface( + "fe80::c00:ff:fe00:1/64" + ) + second_host_ip = ipaddress.ip_interface("10.20.0.1/24") + second_faucet_vip = ipaddress.ip_interface("10.20.0.254/24") + second_host_ipv6 = ipaddress.ip_interface("fb00::1/64") + second_faucet_vip_ipv6 = ipaddress.ip_interface("fb00::254/64") + second_faucet_vip_link_local_ipv6 = ipaddress.ip_interface( + "fe80::c00:ff:fe00:2/64" + ) + first_host_add_ips() + second_host_add_ips() + add_host_routes() + + first_host_connectivity_check() + second_host_connectivity_check() + + self.change_vlan_config( + "vlan2", + "faucet_mac", + "0e:00:00:00:00:03", + hup=True, + cold_start=False, + ) + + self.change_vlan_config( + "vlan2", + "faucet_vips", + ["10.22.0.254/25", "fc00::254/112", "fe80::c00:ff:fe00:3/64"], + hup=True, + cold_start=False, + ) + + second_host_ip = ipaddress.ip_interface("10.22.0.129/25") + second_faucet_vip = ipaddress.ip_interface("10.22.0.254/25") + second_host_ipv6 = ipaddress.ip_interface("fc00::1/112") + second_faucet_vip_ipv6 = ipaddress.ip_interface("fc00::254/112") + second_faucet_vip_link_local_ipv6 = ipaddress.ip_interface( + "fe80::c00:ff:fe00:3/64" + ) + second_host_add_ips() + add_host_routes() + + second_host_connectivity_check() + first_host_connectivity_check() + + self.change_vlan_config( + "vlan1", + "faucet_vips", + ["10.11.0.254/24", "fd00::254/64"], + hup=True, + cold_start=False, + ) + + first_host_ip = ipaddress.ip_interface("10.11.0.1/24") + first_faucet_vip = ipaddress.ip_interface("10.11.0.254/24") + first_host_ipv6 = ipaddress.ip_interface("fd00::1/64") + first_faucet_vip_ipv6 = ipaddress.ip_interface("fd00::254/64") + first_host_add_ips() + add_host_routes() + + second_host_connectivity_check() + first_host_connectivity_check(link_local_reachable=False) + + class FaucetUntaggedIPv4PolicyRouteTest(FaucetUntaggedTest): CONFIG_GLOBAL = """ vlans: From 80ef65c499bb552bda05518aba60ca4cf53a415d Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Thu, 17 Sep 2026 11:17:35 +1200 Subject: [PATCH 3/5] Add unit tests for warm starting vlan with routing enabled --- clib/valve_test_lib.py | 68 ++ tests/unit/faucet/test_valve_config.py | 1049 ++++++++++++++++++++++++ 2 files changed, 1117 insertions(+) diff --git a/clib/valve_test_lib.py b/clib/valve_test_lib.py index 749467039e..8d0e23d5ee 100644 --- a/clib/valve_test_lib.py +++ b/clib/valve_test_lib.py @@ -1546,6 +1546,74 @@ def learn_hosts(self): }, ) + def l2_learn_host(self, port, vid, host_mac): + """L2 learn a host by sending broadcast dhcp request packet from host.""" + self.rcv_packet( + port, + vid, + { + "eth_src": host_mac, + "eth_dst": self.BROADCAST_MAC, + "eth_type": 0x800, + "ipv4_src": "0.0.0.0", + "ipv4_dst": "255.255.255.255", + }, + ) + + def l3_learn_host(self, port, vid, host_mac, host_ips, faucet_vips): + """L3 learn a host by sending v4/v6 ND packet from host to faucet.""" + host_ip = next((ip for ip in host_ips if ip.version == 4), None) + faucet_vip = next((ip for ip in faucet_vips if ip.version == 4), None) + + if host_ip and faucet_vip: + # ARP for faucet vip + self.rcv_packet( + port, + vid, + { + "eth_src": host_mac, + "eth_dst": self.BROADCAST_MAC, + "eth_type": 0x806, + "arp_code": arp.ARP_REQUEST, + "arp_source_ip": str(host_ip), + "arp_target_ip": str(faucet_vip), + }, + ) + + for is_link_local in (False, True): + host_ip = next( + ( + ip + for ip in host_ips + if ip.version == 6 and ip.is_link_local is is_link_local + ), + None, + ) + faucet_vip = next( + ( + ip + for ip in faucet_vips + if ip.version == 6 and ip.is_link_local is is_link_local + ), + None, + ) + + if host_ip and faucet_vip: + # ND NS for faucet vip + self.rcv_packet( + port, + vid, + { + "eth_src": host_mac, + "eth_dst": valve_packet.ipv6_link_eth_mcast(faucet_vip), + "ipv6_src": str(host_ip), + "ipv6_dst": str( + valve_packet.ipv6_solicited_node_from_ucast(faucet_vip) + ), + "neighbor_solicit_ip": str(faucet_vip), + }, + ) + def verify_expiry(self): """Verify FIB resolution attempts expire.""" valve = self.valves_manager.valves[self.DP_ID] diff --git a/tests/unit/faucet/test_valve_config.py b/tests/unit/faucet/test_valve_config.py index 7ec57862df..8865c3565b 100755 --- a/tests/unit/faucet/test_valve_config.py +++ b/tests/unit/faucet/test_valve_config.py @@ -2018,5 +2018,1054 @@ def test_dead_nexthop(self): self.assertEqual(next_table, eth_dst_table.table_id) +class ValveAddVIPTestCase(ValveTestBases.ValveTestNetwork): + """Test adding VIPs to a VLAN.""" + + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_add_vip(self): + """Test adding VIPs to a VLAN is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.assertTrue( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P2_V200_MAC, + "eth_dst": self.FAUCET_MAC2, + "ipv4_src": "10.20.0.1", + "ipv4_dst": "10.20.0.253", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not sent to controller", + ) + self.assertTrue( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P2_V200_MAC, + "eth_dst": self.FAUCET_MAC2, + "ipv6_src": "fb00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not sent to controller", + ) + self.assertTrue( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P2_V200_MAC, + "eth_dst": valve_packet.ipv6_link_eth_mcast( + ip_address("fe80::c00:ff:fe00:2") + ), + "ipv6_src": "fe80::200:ff:fe02:2", + "ipv6_dst": "fe80::c00:ff:fe00:2", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not sent to controller", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveDeleteVIPTestCase(ValveTestBases.ValveTestNetwork): + """Test deleting VIPs from a VLAN.""" + + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_delete_vip(self): + """Test deleting VIPs from a VLAN is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.l2_learn_host(2, 0x200, self.P2_V200_MAC) + self.assertFalse( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P2_V200_MAC, + "eth_dst": self.FAUCET_MAC2, + "ipv4_src": "10.20.0.1", + "ipv4_dst": "10.20.0.254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet sent to controller", + ) + self.assertFalse( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P2_V200_MAC, + "eth_dst": self.FAUCET_MAC2, + "ipv6_src": "fb00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet sent to controller", + ) + self.assertFalse( + table.is_output( + { + "in_port": 2, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P2_V200_MAC, + "eth_dst": valve_packet.ipv6_link_eth_mcast( + ip_address("fe80::c00:ff:fe00:2") + ), + "ipv6_src": "fe80::200:ff:fe02:2", + "ipv6_dst": "fe80::c00:ff:fe00:2", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet sent to controller", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveAddVIPInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_add_vip(self): + """Test adding VIPs to a VLAN with InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.20.0.254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not routed", + ) + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveDeleteVIPInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_delete_vip(self): + """Test deleting VIPs from a VLAN with InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.20.0.254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet routed", + ) + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveAddVIPGlobalInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + global_vlan: 4094 + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + global_vlan: 4094 + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_add_vip(self): + """Test adding VIPs to a VLAN with global InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.20.0.254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not routed", + ) + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet not routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveDeleteVIPGlobalInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + global_vlan: 4094 + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + global_vlan: 4094 + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_delete_vip(self): + """Test deleting VIPs from a VLAN with global InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + + def verify_func(): + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.20.0.254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet routed", + ) + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "fb00::254", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=CONTROLLER_PORT, + ), + msg="Packet routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + ) + + +class ValveAddStaticRouteInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + routes: + - route: + ip_dst: 10.99.99.0/24 + ip_gw: 10.20.0.1 + - route: + ip_dst: 2000::/64 + ip_gw: fb00::1 + - route: + ip_dst: 2001::/64 + ip_gw: fe80::200:ff:fe02:2 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_add_static_route(self): + """Test adding static routes to a VLAN with InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + self.l2_learn_host(1, 0x100, self.P1_V100_MAC) + before_table_state = table.table_state() + + def verify_func(): + self.l3_learn_host( + 1, + 0x100, + self.P1_V100_MAC, + [ + ip_address("10.10.0.1"), + ip_address("fa00::1"), + ip_address("fe80::200:ff:fe01:1"), + ], + [ + ip_address("10.10.0.254"), + ip_address("fa00::254"), + ip_address("fe80::c00:ff:fe00:1"), + ], + ) + self.l3_learn_host( + 2, + 0x200, + self.P2_V200_MAC, + [ + ip_address("10.20.0.1"), + ip_address("fb00::1"), + ip_address("fe80::200:ff:fe02:2"), + ], + [ + ip_address("10.20.0.254"), + ip_address("fb00::254"), + ip_address("fe80::c00:ff:fe00:2"), + ], + ) + + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.99.99.1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet not routed", + ) + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "2000::1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet not routed", + ) + self.assertTrue( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "2001::1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet not routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + before_table_states={self.DP_ID: before_table_state}, + ) + + +class ValveDeleteStaticRouteInterVLANRouteTestCase(ValveTestBases.ValveTestNetwork): + FAUCET_MAC2 = "0e:00:00:00:00:02" + + CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + routes: + - route: + ip_dst: 10.99.99.0/24 + ip_gw: 10.20.0.1 + - route: + ip_dst: 2000::/64 + ip_gw: fb00::1 + - route: + ip_dst: 2001::/64 + ip_gw: fe80::200:ff:fe02:2 + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + MORE_CONFIG = """ +vlans: + vlan1: + vid: 0x100 + faucet_vips: + - "10.10.0.254/24" + - "fa00::254/64" + - "fe80::c00:ff:fe00:1/64" + vlan2: + vid: 0x200 + faucet_vips: + - "10.20.0.254/24" + - "fb00::254/64" + - "fe80::c00:ff:fe00:2/64" + faucet_mac: "%s" +routers: + router-1: + vlans: [vlan1, vlan2] +dps: + s1: +%s + interfaces: + 1: + native_vlan: vlan1 + 2: + native_vlan: vlan2 +""" % ( + FAUCET_MAC2, + DP1_CONFIG, + ) + + def setUp(self): + """Setup basic port and vlan config""" + self.setup_valves(self.CONFIG) + + def test_delete_static_route(self): + """Test deleting static routes from a VLAN with InterVLAN routing is a warm start.""" + table = self.network.tables[self.DP_ID] + self.l2_learn_host(1, 0x100, self.P1_V100_MAC) + before_table_state = table.table_state() + + def verify_func(): + self.l3_learn_host( + 1, + 0x100, + self.P1_V100_MAC, + [ + ip_address("10.10.0.1"), + ip_address("fa00::1"), + ip_address("fe80::200:ff:fe01:1"), + ], + [ + ip_address("10.10.0.254"), + ip_address("fa00::254"), + ip_address("fe80::c00:ff:fe00:1"), + ], + ) + self.l3_learn_host( + 2, + 0x200, + self.P2_V200_MAC, + [ + ip_address("10.20.0.1"), + ip_address("fb00::1"), + ip_address("fe80::200:ff:fe02:2"), + ], + [ + ip_address("10.20.0.254"), + ip_address("fb00::254"), + ip_address("fe80::c00:ff:fe00:2"), + ], + ) + + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x800, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv4_src": "10.10.0.1", + "ipv4_dst": "10.99.99.1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet routed", + ) + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "2000::1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet routed", + ) + self.assertFalse( + table.is_output( + { + "in_port": 1, + "vlan_vid": 0, + "eth_type": 0x86DD, + "eth_src": self.P1_V100_MAC, + "eth_dst": self.FAUCET_MAC, + "ipv6_src": "fa00::1", + "ipv6_dst": "2001::1", + "echo_request_data": self.ICMP_PAYLOAD, + }, + port=2, + ), + msg="Packet routed", + ) + + self.update_and_revert_config( + self.CONFIG, + self.MORE_CONFIG, + reload_type="warm", + verify_func=verify_func, + before_table_states={self.DP_ID: before_table_state}, + ) + + if __name__ == "__main__": unittest.main() # pytype: disable=module-attr From 8524925a5af536eb27e238ba886f7a0146bd33bf Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Wed, 9 Sep 2026 16:33:49 +1200 Subject: [PATCH 4/5] Skip sleeping for the last iteration of the retry loop --- clib/mininet_test_base.py | 210 ++++++++++++++++++++------------- clib/mininet_test_base_topo.py | 20 ++-- 2 files changed, 139 insertions(+), 91 deletions(-) diff --git a/clib/mininet_test_base.py b/clib/mininet_test_base.py index 41c5e9e409..89f0b5c975 100644 --- a/clib/mininet_test_base.py +++ b/clib/mininet_test_base.py @@ -325,7 +325,7 @@ def _wait_until_matching_event(self, match_func, timeout=30): """Return the next matching event from the event sock, else fail""" assert timeout >= 1 assert self.event_log and os.path.exists(self.event_log) - for _ in range(timeout): + for i in range(timeout): with open(self.event_log, encoding="utf-8") as events: for event_str in events: event = json.loads(event_str) @@ -338,7 +338,8 @@ def _wait_until_matching_event(self, match_func, timeout=30): return event except KeyError: pass # Allow for easy dict traversal. - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("matching event not found in event stream") @staticmethod @@ -434,12 +435,13 @@ def _tmpdir_name(self): def _wait_load(self, load_retries=10): load = os.getloadavg()[0] - for _ in range(load_retries): + for i in range(load_retries): if load < self.max_test_load: return output("load average too high %f, waiting" % load) - time.sleep(random.uniform(0.1, 1.0)) - load = os.getloadavg()[0] + if i < (load_retries - 1): + time.sleep(random.uniform(0.1, 1.0)) + load = os.getloadavg()[0] self.fail("load average %f consistently too high" % load) def _allocate_config_ports(self): @@ -802,7 +804,8 @@ def _start_faucet(self, controller_intf, controller_ipv6): self._wait_load() last_error_txt = None - for _ in range(3): + retries = 3 + for i in range(retries): # Start Mininet, connected to the first controller self.net = Mininet( self.topo, link=FaucetLink, controller=self.faucet_controllers[0] @@ -831,7 +834,8 @@ def _start_faucet(self, controller_intf, controller_ipv6): error("%s: %s" % (self._test_name(), last_error_txt)) # The port server (mininet_test_util.serve_ports) already enforces # MIN_PORT_AGE on dispense; sleeping it here wastes 30+s per retry. - time.sleep(1) + if i < (retries - 1): + time.sleep(1) if last_error_txt is not None: self.fail(last_error_txt) @@ -876,14 +880,15 @@ def _ofctl_up(self): return isinstance(switches, list) and switches def _wait_ofctl_up(self, timeout=10): - for _ in range(timeout * 4): + for i in range(timeout * 4): if self._ofctl_up(): return True - time.sleep(0.25) + if i < (timeout * 4 - 1): + time.sleep(0.25) return False def _ofctl_post(self, int_dpid, req, timeout, params=None): - for _ in range(timeout): + for i in range(timeout): try: ofctl_result = requests.post( self._ofctl_rest_url(req), json=params @@ -891,18 +896,20 @@ def _ofctl_post(self, int_dpid, req, timeout, params=None): return ofctl_result[int_dpid] except (ValueError, TypeError, requests.exceptions.ConnectionError): # Didn't get valid JSON, try again - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) continue return [] def _ofctl_get(self, int_dpid, req, timeout, params=None): - for _ in range(timeout): + for i in range(timeout): ofctl_result = self._ofctl(self._ofctl_rest_url(req), params=params) try: return ofctl_result[int_dpid] except (ValueError, TypeError): # Didn't get valid JSON, try again - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) continue return [] @@ -970,30 +977,34 @@ def _controllers_connected(self): return True def _wait_controllers_healthy(self, timeout=90): - for _ in range(timeout * 4): + for i in range(timeout * 4): if self._controllers_healthy(): return True - time.sleep(0.25) + if i < (timeout * 4 - 1): + time.sleep(0.25) return False def _wait_controllers_connected(self, timeout=90): # Slow CI runners can spend ~30s between Faucet startup and OVS # establishing the OpenFlow channel; the previous 30s window # turned that into a hard flake. - for _ in range(timeout * 4): + for i in range(timeout * 4): if self._controllers_connected(): return True - time.sleep(0.25) + if i < (timeout * 4 - 1): + time.sleep(0.25) return False def _wait_debug_log(self): """Require all switches to have exchanged flows with controller.""" ofchannel_logs = self._get_ofchannel_logs() + timeout = 60 for _, debug_log in ofchannel_logs: - for _ in range(60 * 4): + for i in range(timeout * 4): if os.path.exists(debug_log) and os.path.getsize(debug_log) > 0: break - time.sleep(0.25) + if i < (timeout * 4 - 1): + time.sleep(0.25) else: return False return True @@ -1225,7 +1236,7 @@ def get_all_meters_from_dpid(self, dpid): def wait_matching_in_group_table(self, action, group_id, timeout=10): groupdump = os.path.join(self.tmpdir, "groupdump-%s.txt" % self.dpid) - for _ in range(timeout): + for i in range(timeout): group_dump = self.get_all_groups_desc_from_dpid(self.dpid, 1) with open(groupdump, "w", encoding="utf-8") as groupdump_file: for group_dict in group_dump: @@ -1234,7 +1245,8 @@ def wait_matching_in_group_table(self, action, group_id, timeout=10): actions = set(group_dict["buckets"][0]["actions"]) if set([action]).issubset(actions): return True - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) return False # TODO: Should this have meter_confs as well or can we just match meter_ids @@ -1304,7 +1316,7 @@ def to_old_match(match): if actions: actions_set = frozenset(actions) - for _ in range(timeout): + for i in range(timeout): flow_dicts = [] if ofa_match: flow_dump = self.get_all_flows_from_dpid(dpid, table_id, match=match) @@ -1342,7 +1354,8 @@ def to_old_match(match): flow_dicts.append(flow_dict) if flow_dicts: return flow_dicts - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) return flow_dicts def get_matching_flow_on_dpid( @@ -1392,14 +1405,15 @@ def get_matching_flow( ) def get_group_id_for_matching_flow(self, match, table_id, timeout=10): - for _ in range(timeout): + for i in range(timeout): flow_dict = self.get_matching_flow(match, table_id, timeout=timeout) if flow_dict: for action in flow_dict["actions"]: if action.startswith("GROUP"): _, group_id = action.split(":") return int(group_id) - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) return None def matching_flow_present_on_dpid( @@ -1589,19 +1603,20 @@ def wait_ports_updating(self, ports, port_vars, stimulate_counters_func=None): first_counters = self.scrape_port_counters( ports_not_updated, port_vars, assert_present=False ) - for _ in range(self.DB_TIMEOUT * 3): + for i in range(self.DB_TIMEOUT * 3): if all( first_counters[port].get(var) is not None for port in ports_not_updated for var in port_vars ): break - time.sleep(1) - first_counters = self.scrape_port_counters( - ports_not_updated, port_vars, assert_present=False - ) + if i < (self.DB_TIMEOUT * 3 - 1): + time.sleep(1) + first_counters = self.scrape_port_counters( + ports_not_updated, port_vars, assert_present=False + ) - for _ in range(self.DB_TIMEOUT * 3): + for i in range(self.DB_TIMEOUT * 3): stimulate_counters_func() now_counters = self.scrape_port_counters(ports_not_updated, port_vars) updated_ports = set() @@ -1623,7 +1638,8 @@ def wait_ports_updating(self, ports, port_vars, stimulate_counters_func=None): updated_ports.add(port) ports_not_updated -= updated_ports if ports_not_updated: - time.sleep(1) + if i < (self.DB_TIMEOUT * 3 - 1): + time.sleep(1) else: break @@ -1849,7 +1865,7 @@ def wait_for_prometheus_var( ): if controller is None: controller = self.faucet_controllers[0].name - for _ in range(timeout * 4): + for i in range(timeout * 4): result = self.scrape_prometheus_var( var, labels=labels, @@ -1864,7 +1880,8 @@ def wait_for_prometheus_var( return True if orgreater and result is not None and result > result_wanted: return True - time.sleep(0.25) + if i < (timeout * 4 - 1): + time.sleep(0.25) return False def scrape_prometheus_var( @@ -1988,13 +2005,14 @@ def get_configure_count(self, retries=5, controller=None): """Return the number of times FAUCET has processed a reload request.""" if controller is None: controller = self.faucet_controllers[0].name - for _ in range(retries): + for i in range(retries): count = self.scrape_prometheus_var( "faucet_config_reload_requests_total", dpid=False, controller=controller ) if count: break - time.sleep(1) + if i < (retries - 1): + time.sleep(1) self.assertTrue(count, msg="configure count stayed zero") return count @@ -2154,7 +2172,7 @@ def verify_traveling_dhcp_mac(self, retries=10): mac = "0e:00:00:00:00:ff" locations = set() for host in self.hosts_name_ordered(): - for _ in range(retries): + for i in range(retries): host.cmd(self.scapy_dhcp(mac, host.defaultIntf())) new_locations = set() for line in self.scrape_prometheus(var="learned_macs"): @@ -2163,7 +2181,8 @@ def verify_traveling_dhcp_mac(self, retries=10): new_locations.add(location) if locations != new_locations: break - time.sleep(1) + if i < (retries - 1): + time.sleep(1) # TODO: verify port/host association, not just that host moved. self.assertNotEqual(locations, new_locations) locations = new_locations @@ -2332,7 +2351,8 @@ def verify_controller_fping( def verify_learn_counters(self, vlan, ports, verify_neighbors=False): # Need to synchronize with stats update thread. - for _ in range(7): + retries = 7 + for i in range(retries): vlan_hosts_learned = self.scrape_prometheus_var( "vlan_hosts_learned", {"vlan": str(vlan)} ) @@ -2353,7 +2373,8 @@ def verify_learn_counters(self, vlan, ports, verify_neighbors=False): and vlan_hosts_learned == prom_macs_learned ): break - time.sleep(1) + if i < (retries - 1): + time.sleep(1) self.assertEqual(vlan_hosts_learned, port_vlan_hosts_learned) self.assertEqual(vlan_hosts_learned, prom_macs_learned) if verify_neighbors: @@ -2388,7 +2409,7 @@ def generate_test_ipas(): def generate_mac_intfs(test_ipas, other_hosts): mac_intf_ipv4s = [] - for i in range(0, max_hosts): + for i in range(max_hosts): host = other_hosts[i % len(other_hosts)] mac_intf = "mac%u" % i mac_ipv4 = str(test_ipas[i]) @@ -2796,11 +2817,12 @@ def verify_faucet_reconf( for i, controller in enumerate(self.faucet_controllers): cont_name = controller.name start_configure_count = start_configure_counts[i] - for _ in range(timeout * 4): + for j in range(timeout * 4): configure_count = self.get_configure_count(controller=cont_name) if configure_count > start_configure_count: break - time.sleep(0.25) + if j < (timeout * 4 - 1): + time.sleep(0.25) self.assertNotEqual( start_configure_count, configure_count, @@ -2810,7 +2832,7 @@ def verify_faucet_reconf( old_count = old_counts[i] if change_expected: old_count = old_counts[i] - for _ in range(timeout * 4): + for j in range(timeout * 4): new_count = int( self.scrape_prometheus_var( var, controller=cont_name, dpid=dpid, default=0 @@ -2818,7 +2840,8 @@ def verify_faucet_reconf( ) if new_count > old_count: break - time.sleep(0.25) + if j < (timeout * 4 - 1): + time.sleep(0.25) self.assertTrue( new_count > old_count, msg="FAUCET %s %s did not increment: %u" @@ -2861,12 +2884,13 @@ def wait_host_stats_updated( self, hosts_switch_ports, timeout, sync_counters_func=None ): first = self.get_host_port_stats(hosts_switch_ports) - for _ in range(timeout): + for i in range(timeout): if sync_counters_func: sync_counters_func() if self.get_host_port_stats(hosts_switch_ports) != first: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("port stats for %s never updated" % hosts_switch_ports) def of_bytes_mbps(self, start_port_stats, end_port_stats, var, seconds): @@ -2897,7 +2921,8 @@ def verify_iperf_min( iperf_mbps = self.iperf(client_host, client_ip, server_host, server_ip, seconds) self.assertGreater(iperf_mbps, min_mbps) # TODO: account for drops. - for _ in range(3): + retries = 3 + for i in range(retries): end_port_stats = self.get_host_port_stats(hosts_switch_ports) approx_match = True for host in hosts: @@ -2922,7 +2947,8 @@ def verify_iperf_min( approx_match = False if approx_match: return - time.sleep(1) + if i < (retries - 1): + time.sleep(1) self.fail(msg=msg) @staticmethod @@ -2934,14 +2960,15 @@ def set_dpid_names(self, dpid_names): self.dpid_names = copy.deepcopy(dpid_names) def wait_port_status(self, dpid, port_no, status, expected_status, timeout=10): - for _ in range(timeout): + for i in range(timeout): port_status = self.scrape_prometheus_var( "port_status", self.port_labels(port_no), default=None, dpid=dpid ) 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) + if i < (timeout - 1): + time.sleep(1) self.fail( "dpid %x port %s status %s != expected %u" % (dpid, port_no, port_status, expected_status) @@ -3098,13 +3125,14 @@ def _ip_ping( if require_host_learned: self.require_host_learned(host) pause = timeout / 1e3 - for _ in range(retries): + for i in range(retries): ping_out = host.cmd(ping_cmd) ping_result = bool(re.search(good_ping, ping_out)) if ping_result: break - time.sleep(pause) - pause *= 2 + if i < (retries - 1): + time.sleep(pause) + pause *= 2 self.assertEqual( ping_result, expected_result, @@ -3185,14 +3213,15 @@ def ping(self, hosts, timeout=3): def retry_net_ping(self, hosts=None, required_loss=0, retries=3, timeout=2): loss = None - for _ in range(retries): + for i in range(retries): if hosts is None: loss = self.ping_all(timeout=timeout) else: loss = self.net.ping(hosts, timeout=timeout) if loss <= required_loss: return - time.sleep(1) + if i < (retries - 1): + time.sleep(1) self.fail("ping %f loss > required loss %f" % (loss, required_loss)) def _ip_traceroute( @@ -3219,13 +3248,14 @@ def _ip_traceroute( dst, ) pause = timeout / 1e3 - for _ in range(retries): + for i in range(retries): traceroute_out = host.cmd(traceroute_cmd) traceroute_result = bool(re.search(good_traceroute, traceroute_out)) if traceroute_result: break - time.sleep(pause) - pause *= 2 + if i < (retries - 1): + time.sleep(pause) + pause *= 2 self.assertEqual( traceroute_result, expected_result, @@ -3288,20 +3318,22 @@ def tcp_port_free(host, port, ipv=4): def wait_for_tcp_free(self, host, port, timeout=10, ipv=4): """Wait for a host to start listening on a port.""" - for _ in range(timeout): + for i in range(timeout): listen_out = self.tcp_port_free(host, port, ipv) if listen_out is None: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("%s busy on port %u (%s)" % (host, port, listen_out)) def wait_for_tcp_listen(self, host, port, timeout=10, ipv=4): """Wait for a host to start listening on a port.""" - for _ in range(timeout): + for i in range(timeout): listen_out = self.tcp_port_free(host, port, ipv) if listen_out is not None: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("%s never listened on port %u" % (host, port)) def serve_str_on_tcp_port(self, host, port, serve_str="hello", timeout=20): @@ -3319,13 +3351,14 @@ def wait_nonzero_packet_count_flow( """Wait for a flow to be present and have a non-zero packet_count.""" if dpid is None: dpid = self.dpid - for _ in range(timeout): + for i in range(timeout): flow = self.get_matching_flow_on_dpid( dpid, match, table_id, timeout=1, actions=actions, ofa_match=ofa_match ) if flow and flow["packet_count"] > 0: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) if flow: self.fail( "DPID %s flow %s matching %s table ID %s had zero packet count" @@ -3374,7 +3407,7 @@ def bcast_dst_blocked_helper( "ff:ff:ff:ff:ff:ff", ) target_addr = str(self.FAUCET_VIPV4.network.broadcast_address) - for _ in range(retries): + for i in range(retries): tcpdump_txt = self.tcpdump_helper( second_host, tcpdump_filter, @@ -3391,7 +3424,8 @@ def bcast_dst_blocked_helper( ) if re.search(success_re, tcpdump_txt): return True - time.sleep(1) + if i < (retries - 1): + time.sleep(1) return False def verify_bcast_dst_blocked(self, port, first_host, second_host): @@ -3442,10 +3476,11 @@ def start_exabgp(self, exabgp_conf, timeout=30, log_prefix=""): ) exabgp_cli = "env %s %s" % (exabgp_env, exabgp_cmd) controller.cmd(exabgp_cli) - for _ in range(timeout): + for i in range(timeout): if os.path.exists(exabgp_log): break - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.assertTrue( os.path.exists(exabgp_log), msg="exabgp (%s) did not start" % exabgp_cli ) @@ -3457,13 +3492,15 @@ def wait_bgp_up(self, neighbor, vlan, exabgp_log, exabgp_err): "neighbor": neighbor, "vlan": vlan, } - for _ in range(60): + timeout = 60 + for i in range(timeout): uptime = self.scrape_prometheus_var( "bgp_neighbor_uptime", label_values, default=0 ) if uptime > 0: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) exabgp_log_content = [] for log_name in (exabgp_log, exabgp_err): if os.path.exists(log_name): @@ -3482,12 +3519,13 @@ def wait_until_matching_lines_from_file(self, exp, log_name, timeout=30, count=1 """Require (count) matching lines to be present in file.""" assert timeout >= 1 lines = [] - for _ in range(timeout): + for i in range(timeout): if os.path.exists(log_name): lines = self.matching_lines_from_file(exp, log_name) if len(lines) >= count: return lines - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("%s not found in %s (%d/%d)" % (exp, log_name, len(lines), count)) def wait_until_no_matching_lines_from_file( @@ -3496,14 +3534,15 @@ def wait_until_no_matching_lines_from_file( """Require (count) matching lines to be non-existent in file.""" assert timeout >= 1 lines = [] - for _ in range(timeout): + for i in range(timeout): if os.path.exists(log_name): lines = self.matching_lines_from_file(exp, log_name) if len(lines) >= count: return self.fail( "%s found in %s (%d/%d)" % (exp, log_name, len(lines), count) ) - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) return lines def wait_until_matching_lines_from_faucet_log_files(self, exp, timeout=30, count=1): @@ -3525,13 +3564,14 @@ def exabgp_updates(self, exabgp_log, timeout=60): controller = self._get_controller() updates = [] # exabgp should have received our BGP updates - for _ in range(timeout): + for i in range(timeout): updates = controller.cmd( r'grep UPDATE %s |grep -Eo "\S+ next-hop \S+"' % exabgp_log ) if updates: break - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.assertTrue(updates, "exabgp did not receive BGP updates") return updates @@ -3574,10 +3614,11 @@ def start_wpasupplicant( 300, ) host.cmd(wpasupplicant_cmd) - for _ in range(timeout): + for i in range(timeout): if os.path.exists(wpasupplicant_log): break - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.assertTrue( os.path.exists(wpasupplicant_log), msg="wpasupplicant (%s) did not start" % wpasupplicant_cmd, @@ -3653,10 +3694,11 @@ def _ip_neigh(host, ipa, ip_ver): return None def _verify_host_learned_mac(self, host, ipa, ip_ver, mac, retries): - for _ in range(retries): + for i in range(retries): if self._ip_neigh(host, ipa, ip_ver) == mac: return - time.sleep(1) + if i < (retries - 1): + time.sleep(1) self.fail("could not verify %s resolved to %s" % (ipa, mac)) def verify_ipv4_host_learned_mac(self, host, ipa, mac, retries=3): @@ -3701,7 +3743,8 @@ def run_iperf(iperf_server_cmd, server_host, server_start_exp, port): return None timeout = (seconds * 3) + 5 - for _ in range(3): + retries = 3 + for i in range(retries): port = mininet_test_util.find_free_port(self.ports_sock, self._test_name()) iperf_base_cmd = "iperf -f M -p %u" % port if server_ip.version == 6: @@ -3719,7 +3762,8 @@ def run_iperf(iperf_server_cmd, server_host, server_start_exp, port): ) if iperf_mbps is not None and iperf_mbps > 0: return iperf_mbps - time.sleep(1) + if i < (retries - 1): + time.sleep(1) if iperf_mbps == -1: self.fail( "iperf client %s did not connect to server %s" diff --git a/clib/mininet_test_base_topo.py b/clib/mininet_test_base_topo.py index 04f38cb9d2..121489cb02 100644 --- a/clib/mininet_test_base_topo.py +++ b/clib/mininet_test_base_topo.py @@ -435,7 +435,7 @@ def one_stack_port_up(self, dpid, dp_name, port): def verify_stack_up(self, prop=1.0, timeout=25): """Verify all stack ports are up""" - for _ in range(timeout): + for i in range(timeout): links = 0 links_up = 0 for link, ports in self.link_port_maps.items(): @@ -451,7 +451,8 @@ def verify_stack_up(self, prop=1.0, timeout=25): prop_up = links_up / links if prop_up >= prop: return - time.sleep(1) + if i < (timeout - 1): + time.sleep(1) self.fail("not enough links up: %f / %f" % (links_up, links)) def verify_one_stack_down(self, stack_offset_port, coldstart=False): @@ -706,7 +707,7 @@ def bcast_dst_blocked_helper( "ff:ff:ff:ff:ff:ff", ) target_addr = str(self.faucet_vips[0].network.broadcast_address) - for _ in range(retries): + for i in range(retries): tcpdump_txt = self.tcpdump_helper( second_host, tcpdump_filter, @@ -723,7 +724,8 @@ def bcast_dst_blocked_helper( ) if re.search(success_re, tcpdump_txt): return True - time.sleep(1) + if i < (retries - 1): + time.sleep(1) return False def get_expected_synced_states(self, host_id): @@ -793,10 +795,11 @@ def prom_lacp_up_ports(self, dpid): def verify_num_lag_up_ports(self, expected_up_ports, dpid): """Checks to see if Prometheus has the expected number of up LAG ports on the specified DP""" - for _ in range(self.LACP_TIMEOUT * 10): + for i in range(self.LACP_TIMEOUT * 10): if self.prom_lacp_up_ports(dpid) == expected_up_ports: return - time.sleep(1) + if i < (self.LACP_TIMEOUT * 10 - 1): + time.sleep(1) self.assertEqual(self.prom_lacp_up_ports(dpid), expected_up_ports) def require_linux_bond_up(self, host_id): @@ -804,7 +807,7 @@ def require_linux_bond_up(self, host_id): synced_state_list = self.get_expected_synced_states(host_id) host = self.host_information[host_id]["host"] bond_name = self.host_information[host_id]["bond"] - for _ in range(self.LACP_TIMEOUT * 2): + for i in range(self.LACP_TIMEOUT * 2): result = host.cmd('cat /proc/net/bonding/%s|sed "s/[ \t]*$//g"' % bond_name) result = "\n".join([line.rstrip() for line in result.splitlines()]) with open( @@ -818,7 +821,8 @@ def require_linux_bond_up(self, host_id): break if matched_all: return - time.sleep(1) + if i < (self.LACP_TIMEOUT * 2 - 1): + time.sleep(1) synced_state_txt = r"""""" for state_txt in synced_state_list: synced_state_txt += state_txt + "\n\n" From 67e888b60d9bc53e5a040b6e8e60447f48e61ed3 Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Wed, 9 Sep 2026 16:37:52 +1200 Subject: [PATCH 5/5] Use same number of retries for one_ipv4_ping and one_ipv6_ping --- clib/mininet_test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clib/mininet_test_base.py b/clib/mininet_test_base.py index 89f0b5c975..91eaa2efc0 100644 --- a/clib/mininet_test_base.py +++ b/clib/mininet_test_base.py @@ -3143,7 +3143,7 @@ def one_ipv4_ping( self, host, dst, - retries=3, + retries=5, timeout=1000, intf=None, require_host_learned=True,