Skip to content
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

vyos.ifconfig: T5103: force dhclient restart on VRF change #4335

Merged
merged 3 commits into from
Feb 6, 2025
Merged

vyos.ifconfig: T5103: force dhclient restart on VRF change #4335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68002a3
vyos.ifconfig: T5103: force dhclient restart on VRF change
c-po Feb 5, 2025
bc4adcf
vyos.ifconfig: T7135: only restart DHCPv6 client if needed
c-po Feb 5, 2025
a04bd49
vyos.ifconfig: T5103: revert change to del_addr() signature
c-po Feb 5, 2025
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
12 changes: 8 additions & 4 deletions python/vyos/configdict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,10 +491,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
# Check if any DHCP options changed which require a client restat
dhcp = is_node_changed(config, base + [ifname, 'dhcp-options'])
if dhcp: dict.update({'dhcp_options_changed' : {}})

# Changine interface VRF assignemnts require a DHCP restart, too
dhcp = is_node_changed(config, base + [ifname, 'vrf'])
if dhcp: dict.update({'dhcp_options_changed' : {}})
dhcpv6 = is_node_changed(config, base + [ifname, 'dhcpv6-options'])
if dhcpv6: dict.update({'dhcpv6_options_changed' : {}})

# Some interfaces come with a source_interface which must also not be part
# of any other bond or bridge interface as it is exclusivly assigned as the
Expand Down Expand Up @@ -543,6 +541,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
# Check if any DHCP options changed which require a client restat
dhcp = is_node_changed(config, base + [ifname, 'vif', vif, 'dhcp-options'])
if dhcp: dict['vif'][vif].update({'dhcp_options_changed' : {}})
dhcpv6 = is_node_changed(config, base + [ifname, 'vif', vif, 'dhcpv6-options'])
if dhcpv6: dict['vif'][vif].update({'dhcpv6_options_changed' : {}})

for vif_s, vif_s_config in dict.get('vif_s', {}).items():
# Add subinterface name to dictionary
Expand All @@ -569,6 +569,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
# Check if any DHCP options changed which require a client restat
dhcp = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'dhcp-options'])
if dhcp: dict['vif_s'][vif_s].update({'dhcp_options_changed' : {}})
dhcpv6 = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'dhcpv6-options'])
if dhcpv6: dict['vif_s'][vif_s].update({'dhcpv6_options_changed' : {}})

for vif_c, vif_c_config in vif_s_config.get('vif_c', {}).items():
# Add subinterface name to dictionary
Expand Down Expand Up @@ -597,6 +599,8 @@ def get_interface_dict(config, base, ifname='', recursive_defaults=True, with_pk
# Check if any DHCP options changed which require a client restat
dhcp = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'vif-c', vif_c, 'dhcp-options'])
if dhcp: dict['vif_s'][vif_s]['vif_c'][vif_c].update({'dhcp_options_changed' : {}})
dhcpv6 = is_node_changed(config, base + [ifname, 'vif-s', vif_s, 'vif-c', vif_c, 'dhcpv6-options'])
if dhcpv6: dict['vif_s'][vif_s]['vif_c'][vif_c].update({'dhcpv6_options_changed' : {}})

# Check vif, vif-s/vif-c VLAN interfaces for removal
dict = get_removed_vlans(config, base + [ifname], dict)
Expand Down
57 changes: 34 additions & 23 deletions python/vyos/ifconfig/interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,12 +595,16 @@ def set_vrf(self, vrf: str) -> bool:
"""
Add/Remove interface from given VRF instance.

Keyword arguments:
vrf: VRF instance name or empty string (default VRF)

Return True if VRF was changed, False otherwise

Example:
>>> from vyos.ifconfig import Interface
>>> Interface('eth0').set_vrf('foo')
>>> Interface('eth0').set_vrf()
"""

# Don't allow for netns yet
if 'netns' in self.config:
return False
Expand All @@ -617,15 +621,17 @@ def set_vrf(self, vrf: str) -> bool:
# Get routing table ID number for VRF
vrf_table_id = get_vrf_tableid(vrf)
# Add map element with interface and zone ID
if vrf_table_id:
if vrf_table_id and old_vrf_tableid != vrf_table_id:
# delete old table ID from nftables if it has changed, e.g. interface moved to a different VRF
if old_vrf_tableid and old_vrf_tableid != int(vrf_table_id):
self._del_interface_from_ct_iface_map()
self._del_interface_from_ct_iface_map()
self._add_interface_to_ct_iface_map(vrf_table_id)
return True
else:
self._del_interface_from_ct_iface_map()
if old_vrf_tableid != get_vrf_tableid(self.ifname):
self._del_interface_from_ct_iface_map()
return True

return True
return False

def set_arp_cache_tmo(self, tmo):
"""
Expand Down Expand Up @@ -1181,7 +1187,7 @@ def get_addr(self):
"""
return self.get_addr_v4() + self.get_addr_v6()

def add_addr(self, addr):
def add_addr(self, addr: str, vrf_changed: bool=False) -> bool:
"""
Add IP(v6) address to interface. Address is only added if it is not
already assigned to that interface. Address format must be validated
Expand Down Expand Up @@ -1214,15 +1220,14 @@ def add_addr(self, addr):

# add to interface
if addr == 'dhcp':
self.set_dhcp(True)
self.set_dhcp(True, vrf_changed=vrf_changed)
elif addr == 'dhcpv6':
self.set_dhcpv6(True)
self.set_dhcpv6(True, vrf_changed=vrf_changed)
elif not is_intf_addr_assigned(self.ifname, addr, netns=netns):
netns_cmd = f'ip netns exec {netns}' if netns else ''
tmp = f'{netns_cmd} ip addr add {addr} dev {self.ifname}'
# Add broadcast address for IPv4
if is_ipv4(addr): tmp += ' brd +'

self._cmd(tmp)
else:
return False
Expand All @@ -1232,7 +1237,7 @@ def add_addr(self, addr):

return True

def del_addr(self, addr):
def del_addr(self, addr: str) -> bool:
"""
Delete IP(v6) address from interface. Address is only deleted if it is
assigned to that interface. Address format must be exactly the same as
Expand Down Expand Up @@ -1356,7 +1361,7 @@ def add_to_bridge(self, bridge_dict):
cmd = f'bridge vlan add dev {ifname} vid {native_vlan_id} pvid untagged master'
self._cmd(cmd)

def set_dhcp(self, enable):
def set_dhcp(self, enable: bool, vrf_changed: bool=False):
"""
Enable/Disable DHCP client on a given interface.
"""
Expand Down Expand Up @@ -1396,7 +1401,9 @@ def set_dhcp(self, enable):
# the old lease is released a new one is acquired (T4203). We will
# only restart DHCP client if it's option changed, or if it's not
# running, but it should be running (e.g. on system startup)
if 'dhcp_options_changed' in self.config or not is_systemd_service_active(systemd_service):
if (vrf_changed or
('dhcp_options_changed' in self.config) or
(not is_systemd_service_active(systemd_service))):
return self._cmd(f'systemctl restart {systemd_service}')
else:
if is_systemd_service_active(systemd_service):
Expand All @@ -1423,7 +1430,7 @@ def set_dhcp(self, enable):

return None

def set_dhcpv6(self, enable):
def set_dhcpv6(self, enable: bool, vrf_changed: bool=False):
"""
Enable/Disable DHCPv6 client on a given interface.
"""
Expand Down Expand Up @@ -1452,7 +1459,10 @@ def set_dhcpv6(self, enable):

# We must ignore any return codes. This is required to enable
# DHCPv6-PD for interfaces which are yet not up and running.
return self._popen(f'systemctl restart {systemd_service}')
if (vrf_changed or
('dhcpv6_options_changed' in self.config) or
(not is_systemd_service_active(systemd_service))):
return self._popen(f'systemctl restart {systemd_service}')
else:
if is_systemd_service_active(systemd_service):
self._cmd(f'systemctl stop {systemd_service}')
Expand Down Expand Up @@ -1669,30 +1679,31 @@ def update(self, config):
else:
self.del_addr(addr)

# start DHCPv6 client when only PD was configured
if dhcpv6pd:
self.set_dhcpv6(True)

# XXX: Bind interface to given VRF or unbind it if vrf is not set. Unbinding
# will call 'ip link set dev eth0 nomaster' which will also drop the
# interface out of any bridge or bond - thus this is checked before.
vrf_changed = False
if 'is_bond_member' in config:
bond_if = next(iter(config['is_bond_member']))
tmp = get_interface_config(config['ifname'])
if 'master' in tmp and tmp['master'] != bond_if:
self.set_vrf('')
vrf_changed = self.set_vrf('')

elif 'is_bridge_member' in config:
bridge_if = next(iter(config['is_bridge_member']))
tmp = get_interface_config(config['ifname'])
if 'master' in tmp and tmp['master'] != bridge_if:
self.set_vrf('')
vrf_changed = self.set_vrf('')
else:
self.set_vrf(config.get('vrf', ''))
vrf_changed = self.set_vrf(config.get('vrf', ''))

# start DHCPv6 client when only PD was configured
if dhcpv6pd:
self.set_dhcpv6(True, vrf_changed=vrf_changed)

# Add this section after vrf T4331
for addr in new_addr:
self.add_addr(addr)
self.add_addr(addr, vrf_changed=vrf_changed)

# Configure MSS value for IPv4 TCP connections
tmp = dict_search('ip.adjust_mss', config)
Expand Down
47 changes: 46 additions & 1 deletion smoketest/scripts/cli/base_interfaces_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from vyos.utils.network import is_ipv6_link_local
from vyos.utils.network import get_nft_vrf_zone_mapping
from vyos.xml_ref import cli_defined
from vyos.xml_ref import default_value

dhclient_base_dir = directories['isc_dhclient_dir']
dhclient_process_name = 'dhclient'
Expand Down Expand Up @@ -282,6 +283,9 @@ def test_dhcp_vrf(self):
if not self._test_dhcp or not self._test_vrf:
self.skipTest('not supported')

cli_default_metric = default_value(self._base_path + [self._interfaces[0],
'dhcp-options', 'default-route-distance'])

vrf_name = 'purple4'
self.cli_set(['vrf', 'name', vrf_name, 'table', '65000'])

Expand All @@ -307,7 +311,28 @@ def test_dhcp_vrf(self):
self.assertIn(str(dhclient_pid), vrf_pids)
# and the commandline has the appropriate options
cmdline = read_file(f'/proc/{dhclient_pid}/cmdline')
self.assertIn('-e\x00IF_METRIC=210', cmdline) # 210 is the default value
self.assertIn(f'-e\x00IF_METRIC={cli_default_metric}', cmdline)

# T5103: remove interface from VRF instance and move DHCP client
# back to default VRF. This must restart the DHCP client process
for interface in self._interfaces:
self.cli_delete(self._base_path + [interface, 'vrf'])

self.cli_commit()

# Validate interface state
for interface in self._interfaces:
tmp = get_interface_vrf(interface)
self.assertEqual(tmp, 'default')
# Check if dhclient process runs
dhclient_pid = process_named_running(dhclient_process_name, cmdline=interface, timeout=10)
self.assertTrue(dhclient_pid)
# .. inside the appropriate VRF instance
vrf_pids = cmd(f'ip vrf pids {vrf_name}')
self.assertNotIn(str(dhclient_pid), vrf_pids)
# and the commandline has the appropriate options
cmdline = read_file(f'/proc/{dhclient_pid}/cmdline')
self.assertIn(f'-e\x00IF_METRIC={cli_default_metric}', cmdline)

self.cli_delete(['vrf', 'name', vrf_name])

Expand Down Expand Up @@ -341,6 +366,26 @@ def test_dhcpv6_vrf(self):
vrf_pids = cmd(f'ip vrf pids {vrf_name}')
self.assertIn(str(tmp), vrf_pids)

# T7135: remove interface from VRF instance and move DHCP client
# back to default VRF. This must restart the DHCP client process
for interface in self._interfaces:
self.cli_delete(self._base_path + [interface, 'vrf'])

self.cli_commit()

# Validate interface state
for interface in self._interfaces:
tmp = get_interface_vrf(interface)
self.assertEqual(tmp, 'default')

# Check if dhclient process runs
tmp = process_named_running(dhcp6c_process_name, cmdline=interface, timeout=10)
self.assertTrue(tmp)
# .. inside the appropriate VRF instance
vrf_pids = cmd(f'ip vrf pids {vrf_name}')
self.assertNotIn(str(tmp), vrf_pids)


self.cli_delete(['vrf', 'name', vrf_name])

def test_move_interface_between_vrf_instances(self):
Expand Down
Loading