From d236718c167af46bbf4460fc6b82db8396bd43a5 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Tue, 29 Oct 2019 10:31:58 -0400 Subject: [PATCH 1/6] network: set ipv6 mtu after link-up or device mtu change The kernel resets the ipv6 mtu after NETDEV_UP or NETDEV_CHANGEMTU event, so we must reset the ipv6 mtu to our configured value after we detect IFF_UP flag set or after we set the device mtu. Fixes: #13914. --- src/network/networkd-link.c | 61 ++++++++++++++++++++++++++++++++----- src/network/networkd-link.h | 1 + 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index e650ef7e0a7..62444dd87ca 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1341,7 +1341,7 @@ int link_set_mtu(Link *link, uint32_t mtu) { if (link_ipv6_enabled(link) && mtu < IPV6_MIN_MTU) { log_link_warning(link, "Bumping MTU to " STRINGIFY(IPV6_MIN_MTU) ", as " - "IPv6 is requested and requires a minimum MTU of " STRINGIFY(IPV6_MIN_MTU) " bytes: %m"); + "IPv6 is requested and requires a minimum MTU of " STRINGIFY(IPV6_MIN_MTU) " bytes"); mtu = IPV6_MIN_MTU; } @@ -2378,9 +2378,23 @@ static int link_set_ipv6_mtu(Link *link) { if (link->network->ipv6_mtu == 0) return 0; + /* IPv6 protocol requires a minimum MTU of IPV6_MTU_MIN(1280) bytes + * on the interface. Bump up IPv6 MTU bytes to IPV6_MTU_MIN. */ + if (link->network->ipv6_mtu < IPV6_MIN_MTU) { + log_link_notice(link, "Bumping IPv6 MTU to "STRINGIFY(IPV6_MIN_MTU)" byte minimum required"); + link->network->ipv6_mtu = IPV6_MIN_MTU; + } + r = sysctl_write_ip_property_uint32(AF_INET6, link->ifname, "mtu", link->network->ipv6_mtu); - if (r < 0) - log_link_warning_errno(link, r, "Cannot set IPv6 MTU for interface: %m"); + if (r < 0) { + if (link->mtu < link->network->ipv6_mtu) + log_link_warning(link, "Cannot set IPv6 MTU %"PRIu32" higher than device MTU %"PRIu32, + link->network->ipv6_mtu, link->mtu); + else + log_link_warning_errno(link, r, "Cannot set IPv6 MTU for interface: %m"); + } + + link->ipv6_mtu_set = true; return 0; } @@ -2669,10 +2683,6 @@ static int link_configure(Link *link) { if (r < 0) return r; - r = link_set_ipv6_mtu(link); - if (r < 0) - return r; - if (link_ipv4ll_enabled(link, ADDRESS_FAMILY_IPV4 | ADDRESS_FAMILY_FALLBACK_IPV4)) { r = ipv4ll_configure(link); if (r < 0) @@ -2745,6 +2755,12 @@ static int link_configure_after_setting_mtu(Link *link) { if (link->setting_mtu) return 0; + /* The kernel resets ipv6 mtu after changing device mtu; + * we must set this here, after we've set device mtu */ + r = link_set_ipv6_mtu(link); + if (r < 0) + return r; + if (link_has_carrier(link) || link->network->configure_without_carrier) { r = link_acquire_conf(link); if (r < 0) @@ -3453,11 +3469,30 @@ int link_carrier_reset(Link *link) { return 0; } +/* This is called every time an interface admin state changes to up; + * specifically, when IFF_UP flag changes from unset to set */ +static int link_admin_state_up(Link *link) { + int r; + + /* We set the ipv6 mtu after the device mtu, but the kernel resets + * ipv6 mtu on NETDEV_UP, so we need to reset it. The check for + * ipv6_mtu_set prevents this from trying to set it too early before + * the link->network has been setup; we only need to reset it + * here if we've already set it during normal initialization. */ + if (link->ipv6_mtu_set) { + r = link_set_ipv6_mtu(link); + if (r < 0) + return r; + } + + return 0; +} + int link_update(Link *link, sd_netlink_message *m) { struct ether_addr mac; const char *ifname; uint32_t mtu; - bool had_carrier, carrier_gained, carrier_lost; + bool had_carrier, carrier_gained, carrier_lost, link_was_admin_up; int old_master, r; assert(link); @@ -3587,12 +3622,22 @@ int link_update(Link *link, sd_netlink_message *m) { old_master = link->master_ifindex; (void) sd_netlink_message_read_u32(m, IFLA_MASTER, (uint32_t *) &link->master_ifindex); + link_was_admin_up = link->flags & IFF_UP; had_carrier = link_has_carrier(link); r = link_update_flags(link, m, old_master != link->master_ifindex); if (r < 0) return r; + if (!link_was_admin_up && (link->flags & IFF_UP)) { + log_link_info(link, "Link UP"); + + r = link_admin_state_up(link); + if (r < 0) + return r; + } else if (link_was_admin_up && !(link->flags & IFF_UP)) + log_link_info(link, "Link DOWN"); + r = link_update_lldp(link); if (r < 0) return r; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index e944f2b7155..39ef475daf5 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -116,6 +116,7 @@ typedef struct Link { bool routing_policy_rules_configured:1; bool qdiscs_configured:1; bool setting_mtu:1; + bool ipv6_mtu_set:1; LIST_HEAD(Address, pool_addresses); From 9dfc1a9339ee4f458a017f46da0df5ec0d3bb574 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Sat, 2 Nov 2019 04:43:18 -0400 Subject: [PATCH 2/6] test-network: allow specifying only individual drop-in files --- test/test-network/systemd-networkd-tests.py | 25 ++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index f47463956e7..bcfcc3fbeb3 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -267,14 +267,33 @@ def read_ipv4_sysctl_attr(link, attribute): with open(os.path.join(os.path.join(network_sysctl_ipv4_path, link), attribute)) as f: return f.readline().strip() -def copy_unit_to_networkd_unit_path(*units): +def copy_unit_to_networkd_unit_path(*units, dropins=True): + """Copy networkd unit files into the testbed. + + Any networkd unit file type can be specified, as well as drop-in files. + + By default, all drop-ins for a specified unit file are copied in; + to avoid that specify dropins=False. + + When a drop-in file is specified, its unit file is also copied in automatically. + """ print() for unit in units: - shutil.copy(os.path.join(networkd_ci_path, unit), network_unit_file_path) - if (os.path.exists(os.path.join(networkd_ci_path, unit + '.d'))): + if dropins and os.path.exists(os.path.join(networkd_ci_path, unit + '.d')): copytree(os.path.join(networkd_ci_path, unit + '.d'), os.path.join(network_unit_file_path, unit + '.d')) + if unit.endswith('.conf'): + dropin = unit + dropindir = os.path.join(network_unit_file_path, os.path.dirname(dropin)) + os.makedirs(dropindir, exist_ok=True) + shutil.copy(os.path.join(networkd_ci_path, dropin), dropindir) + unit = os.path.dirname(dropin).rstrip('.d') + shutil.copy(os.path.join(networkd_ci_path, unit), network_unit_file_path) def remove_unit_from_networkd_path(units): + """Remove previously copied unit files from the testbed. + + Drop-ins will be removed automatically. + """ for unit in units: if (os.path.exists(os.path.join(network_unit_file_path, unit))): os.remove(os.path.join(network_unit_file_path, unit)) From befd4b8b60dd01a3bea9bb0730fd6e83d3714256 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Thu, 31 Oct 2019 10:15:53 -0400 Subject: [PATCH 3/6] test-network: read link attribute at any depth --- test/test-network/systemd-networkd-tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index bcfcc3fbeb3..415b3ef82b0 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -216,8 +216,8 @@ def tearDownModule(): for u in running_units: check_output(f'systemctl start {u}') -def read_link_attr(link, dev, attribute): - with open(os.path.join(os.path.join(os.path.join('/sys/class/net/', link), dev), attribute)) as f: +def read_link_attr(*args): + with open(os.path.join('/sys/class/net/', *args)) as f: return f.readline().strip() def read_bridge_port_attr(bridge, link, attribute): From fd372b1a68a6ffe03bf715fc5538c74d4f2b2729 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Sat, 2 Nov 2019 05:09:11 -0400 Subject: [PATCH 4/6] test-network: in wait_online() allow a few seconds to reach setup_state --- test/test-network/systemd-networkd-tests.py | 26 ++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 415b3ef82b0..4556cb36ce0 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -371,7 +371,7 @@ class Utilities(): def check_operstate(self, link, expected, show_status=True, setup_state='configured'): self.assertRegex(get_operstate(link, show_status, setup_state), expected) - def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured'): + def wait_online(self, links_with_operstate, timeout='20s', bool_any=False, setup_state='configured', setup_timeout=5): args = wait_online_cmd + [f'--timeout={timeout}'] + [f'--interface={link}' for link in links_with_operstate] if bool_any: args += ['--any'] @@ -382,13 +382,23 @@ class Utilities(): output = check_output(*networkctl_cmd, 'status', link.split(':')[0], env=env) print(output) raise - if not bool_any: - for link in links_with_operstate: - output = check_output(*networkctl_cmd, 'status', link.split(':')[0]) - print(output) - for line in output.splitlines(): - if 'State:' in line: - self.assertRegex(line, setup_state) + if not bool_any and setup_state: + # check at least once now, then once per sec for setup_timeout secs + for secs in range(setup_timeout + 1): + for link in links_with_operstate: + output = check_output(*networkctl_cmd, 'status', link.split(':')[0]) + print(output) + if not re.search(rf'(?m)^\s*State:.*({setup_state}).*$', output): + # this link isn't in the right state; break into the sleep below + break + else: + # all the links were in the right state; break to exit the timer loop + break + # don't bother sleeping if time is up + if secs < setup_timeout: + time.sleep(1) + else: + self.fail(f'link {link} state does not match {setup_state}') def wait_address(self, link, address_regex, scope='global', ipv='', timeout_sec=100): for i in range(timeout_sec): From 3e82152543597a6e7d5f2243bf57e64bdb1fc88a Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Tue, 26 Nov 2019 15:36:53 -0500 Subject: [PATCH 5/6] test-network: disable restart limiting for networkd Some of the tests restart networkd rapidly, so restart limiting must be disabled to avoid start ratelimiting from causing test failures. --- test/test-network/systemd-networkd-tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 4556cb36ce0..e63cca50f29 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -142,6 +142,8 @@ def setUpModule(): running_units.append(u) drop_in = [ + '[Unit]', + 'StartLimitIntervalSec=0', '[Service]', 'Restart=no', 'ExecStart=', From 7db05447058906966f4a1d5ad90f94744be1c903 Mon Sep 17 00:00:00 2001 From: Dan Streetman Date: Wed, 30 Oct 2019 08:35:37 -0400 Subject: [PATCH 6/6] test-network: add tests to verify IPv6MTUBytes --- test/test-network/conf/12-dummy-mtu.link | 5 + test/test-network/conf/12-dummy-mtu.netdev | 4 + test/test-network/conf/12-dummy.network | 7 ++ .../12-dummy.network.d/ipv6-mtu-1400.conf | 2 + .../12-dummy.network.d/ipv6-mtu-1550.conf | 2 + .../conf/12-dummy.network.d/mtu.conf | 2 + test/test-network/systemd-networkd-tests.py | 95 +++++++++++++++++++ 7 files changed, 117 insertions(+) create mode 100644 test/test-network/conf/12-dummy-mtu.link create mode 100644 test/test-network/conf/12-dummy-mtu.netdev create mode 100644 test/test-network/conf/12-dummy.network create mode 100644 test/test-network/conf/12-dummy.network.d/ipv6-mtu-1400.conf create mode 100644 test/test-network/conf/12-dummy.network.d/ipv6-mtu-1550.conf create mode 100644 test/test-network/conf/12-dummy.network.d/mtu.conf diff --git a/test/test-network/conf/12-dummy-mtu.link b/test/test-network/conf/12-dummy-mtu.link new file mode 100644 index 00000000000..cb7965c37a8 --- /dev/null +++ b/test/test-network/conf/12-dummy-mtu.link @@ -0,0 +1,5 @@ +[Match] +OriginalName=dummy98 + +[Link] +MTUBytes=1600 diff --git a/test/test-network/conf/12-dummy-mtu.netdev b/test/test-network/conf/12-dummy-mtu.netdev new file mode 100644 index 00000000000..c51d2a47a51 --- /dev/null +++ b/test/test-network/conf/12-dummy-mtu.netdev @@ -0,0 +1,4 @@ +[NetDev] +Name=dummy98 +Kind=dummy +MTUBytes=1600 diff --git a/test/test-network/conf/12-dummy.network b/test/test-network/conf/12-dummy.network new file mode 100644 index 00000000000..29ced8b90ba --- /dev/null +++ b/test/test-network/conf/12-dummy.network @@ -0,0 +1,7 @@ +[Match] +Name=dummy98 + +[Network] +IPv6AcceptRA=no +Address=10.1.2.3/16 +Address=2001:db8:0:f101::15/64 diff --git a/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1400.conf b/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1400.conf new file mode 100644 index 00000000000..7a2eb5389fa --- /dev/null +++ b/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1400.conf @@ -0,0 +1,2 @@ +[Network] +IPv6MTUBytes=1400 diff --git a/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1550.conf b/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1550.conf new file mode 100644 index 00000000000..6a88073b41f --- /dev/null +++ b/test/test-network/conf/12-dummy.network.d/ipv6-mtu-1550.conf @@ -0,0 +1,2 @@ +[Network] +IPv6MTUBytes=1550 diff --git a/test/test-network/conf/12-dummy.network.d/mtu.conf b/test/test-network/conf/12-dummy.network.d/mtu.conf new file mode 100644 index 00000000000..6e7da63b4fb --- /dev/null +++ b/test/test-network/conf/12-dummy.network.d/mtu.conf @@ -0,0 +1,2 @@ +[Link] +MTUBytes=1600 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index e63cca50f29..3b041f88cff 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3297,6 +3297,101 @@ class NetworkdIPv6PrefixTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, '2001:db8:0:1::/64 proto ra') +class NetworkdMTUTests(unittest.TestCase, Utilities): + links = ['dummy98'] + + units = [ + '12-dummy.netdev', + '12-dummy-mtu.netdev', + '12-dummy-mtu.link', + '12-dummy.network', + ] + + def setUp(self): + remove_links(self.links) + stop_networkd(show_logs=False) + + def tearDown(self): + remove_log_file() + remove_links(self.links) + remove_unit_from_networkd_path(self.units) + stop_networkd(show_logs=True) + + def check_mtu(self, mtu, ipv6_mtu=None, reset=True): + if not ipv6_mtu: + ipv6_mtu = mtu + + # test normal start + start_networkd() + self.wait_online(['dummy98:routable']) + self.assertEqual(read_ipv6_sysctl_attr('dummy98', 'mtu'), ipv6_mtu) + self.assertEqual(read_link_attr('dummy98', 'mtu'), mtu) + + # test normal restart + restart_networkd() + self.wait_online(['dummy98:routable']) + self.assertEqual(read_ipv6_sysctl_attr('dummy98', 'mtu'), ipv6_mtu) + self.assertEqual(read_link_attr('dummy98', 'mtu'), mtu) + + if reset: + self.reset_check_mtu(mtu, ipv6_mtu) + + def reset_check_mtu(self, mtu, ipv6_mtu=None): + ''' test setting mtu/ipv6_mtu with interface already up ''' + stop_networkd() + + # note - changing the device mtu resets the ipv6 mtu + run('ip link set up mtu 1501 dev dummy98') + run('ip link set up mtu 1500 dev dummy98') + self.assertEqual(read_link_attr('dummy98', 'mtu'), '1500') + self.assertEqual(read_ipv6_sysctl_attr('dummy98', 'mtu'), '1500') + + self.check_mtu(mtu, ipv6_mtu, reset=False) + + def test_mtu_network(self): + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy.network.d/mtu.conf') + self.check_mtu('1600') + + def test_mtu_netdev(self): + copy_unit_to_networkd_unit_path('12-dummy-mtu.netdev', '12-dummy.network', dropins=False) + # note - MTU set by .netdev happens ONLY at device creation! + self.check_mtu('1600', reset=False) + + def test_mtu_link(self): + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy-mtu.link', '12-dummy.network', dropins=False) + # must reload udev because it only picks up new files after 3 second delay + call('udevadm control --reload') + # note - MTU set by .link happens ONLY at udev processing of device 'add' uevent! + self.check_mtu('1600', reset=False) + + def test_ipv6_mtu(self): + ''' set ipv6 mtu without setting device mtu ''' + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy.network.d/ipv6-mtu-1400.conf') + self.check_mtu('1500', '1400') + + def test_ipv6_mtu_toolarge(self): + ''' try set ipv6 mtu over device mtu (it shouldn't work) ''' + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy.network.d/ipv6-mtu-1550.conf') + self.check_mtu('1500', '1500') + + def test_mtu_network_ipv6_mtu(self): + ''' set ipv6 mtu and set device mtu via network file ''' + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy.network.d/mtu.conf', '12-dummy.network.d/ipv6-mtu-1550.conf') + self.check_mtu('1600', '1550') + + def test_mtu_netdev_ipv6_mtu(self): + ''' set ipv6 mtu and set device mtu via netdev file ''' + copy_unit_to_networkd_unit_path('12-dummy-mtu.netdev', '12-dummy.network.d/ipv6-mtu-1550.conf') + self.check_mtu('1600', '1550', reset=False) + + def test_mtu_link_ipv6_mtu(self): + ''' set ipv6 mtu and set device mtu via link file ''' + copy_unit_to_networkd_unit_path('12-dummy.netdev', '12-dummy-mtu.link', '12-dummy.network.d/ipv6-mtu-1550.conf') + # must reload udev because it only picks up new files after 3 second delay + call('udevadm control --reload') + self.check_mtu('1600', '1550', reset=False) + + if __name__ == '__main__': parser = argparse.ArgumentParser() parser.add_argument('--build-dir', help='Path to build dir', dest='build_dir')