Skip to content

Enable warm start for vlans with routing enabled #4850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gizmoguy merged 5 commits into from
Sep 30, 2026
Merged

Enable warm start for vlans with routing enabled #4850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9c9adb9
Enable warm start for vlans with routing enabled
gizmoguy Sep 4, 2026
6378a85
Add integration test for warm starting vlan with routing enabled
gizmoguy Sep 10, 2026
80ef65c
Add unit tests for warm starting vlan with routing enabled
gizmoguy Sep 16, 2026
8524925
Skip sleeping for the last iteration of the retry loop
gizmoguy Sep 9, 2026
67e888b
Use same number of retries for one_ipv4_ping and one_ipv6_ping
gizmoguy Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
219 changes: 133 additions & 86 deletions clib/mininet_test_base.py
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 12 additions & 8 deletions clib/mininet_test_base_topo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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):
Expand Down Expand Up @@ -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,
Expand All @@ -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):
Expand Down Expand Up @@ -793,18 +795,19 @@ 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):
"""Checks to see if the host has properly formed into a bonded state"""
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(
Expand All @@ -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"
Expand Down
68 changes: 68 additions & 0 deletions clib/valve_test_lib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
59 changes: 32 additions & 27 deletions faucet/dp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
(
Expand All @@ -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]
Expand All @@ -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
Expand Down Expand Up @@ -1545,6 +1545,7 @@ def _get_port_config_changes(
self,
logger,
new_dp,
added_vlans,
changed_vlans,
deleted_vlans,
changed_acl_vlans,
Expand All @@ -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.
Expand Down Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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)
)
(
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -1809,6 +1813,7 @@ def get_config_changes(self, logger, new_dp):
set(),
set(),
set(),
set(),
True,
True,
set(),
Expand Down
Loading
Loading