diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index dc868c9b8e9ac..0430e33e40df5 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -123,8 +123,7 @@ int parse_ifindex(const char *s) { } int parse_mtu(int family, const char *s, uint32_t *ret) { - uint64_t u; - size_t m; + uint64_t u, m; int r; r = parse_size(s, 1024, &u); @@ -134,10 +133,16 @@ int parse_mtu(int family, const char *s, uint32_t *ret) { if (u > UINT32_MAX) return -ERANGE; - if (family == AF_INET6) + switch (family) { + case AF_INET: + m = IPV4_MIN_MTU; /* This is 68 */ + break; + case AF_INET6: m = IPV6_MIN_MTU; /* This is 1280 */ - else - m = IPV4_MIN_MTU; /* For all other protocols, including 'unspecified' we assume the IPv4 minimal MTU */ + break; + default: + m = 0; + } if (u < m) return -ERANGE; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index aad86eed99112..a063696c82b12 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1049,6 +1049,10 @@ static int link_configure(Link *link) { if (r < 0) return r; + r = link_configure_mtu(link); + if (r < 0) + return r; + if (link->iftype == ARPHRD_CAN) { /* let's shortcut things for CAN which doesn't need most of what's done below. */ r = link_request_to_set_can(link); @@ -1082,10 +1086,6 @@ static int link_configure(Link *link) { if (r < 0) return r; - r = link_configure_mtu(link); - if (r < 0) - return r; - r = link_request_to_set_addrgen_mode(link); if (r < 0) return r; diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 2b37c86d2351a..38f90969ec498 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -563,12 +563,20 @@ static int link_is_ready_to_set_link(Link *link, Request *req) { break; } case REQUEST_TYPE_SET_LINK_MTU: { - Request req_ipoib = { - .link = link, - .type = REQUEST_TYPE_SET_LINK_IPOIB, - }; + if (ordered_set_contains(link->manager->request_queue, + &(const Request) { + .link = link, + .type = REQUEST_TYPE_SET_LINK_IPOIB, + })) + return false; - return !ordered_set_contains(link->manager->request_queue, &req_ipoib); + /* Changing FD mode may affect MTU. */ + if (ordered_set_contains(link->manager->request_queue, + &(const Request) { + .link = link, + .type = REQUEST_TYPE_SET_LINK_CAN, + })) + return false; } default: break; @@ -858,7 +866,7 @@ int link_request_to_set_master(Link *link) { int link_request_to_set_mtu(Link *link, uint32_t mtu) { const char *origin; - uint32_t min_mtu; + uint32_t min_mtu, max_mtu; Request *req; int r; @@ -886,10 +894,19 @@ int link_request_to_set_mtu(Link *link, uint32_t mtu) { mtu = min_mtu; } - if (mtu > link->max_mtu) { + max_mtu = link->max_mtu; + if (link->iftype == ARPHRD_CAN) + /* The maximum MTU may be changed when FD mode is changed. + * See https://docs.kernel.org/networking/can.html#can-fd-flexible-data-rate-driver-support + * MTU = 16 (CAN_MTU) => Classical CAN device + * MTU = 72 (CANFD_MTU) => CAN FD capable device + * So, even if the current maximum is 16, we should not reduce the requested value now. */ + max_mtu = MAX(max_mtu, 72u); + + if (mtu > max_mtu) { log_link_warning(link, "Reducing the requested MTU %"PRIu32" to the interface's maximum MTU %"PRIu32".", - mtu, link->max_mtu); - mtu = link->max_mtu; + mtu, max_mtu); + mtu = max_mtu; } if (link->mtu == mtu) diff --git a/src/network/networkd-sysctl.c b/src/network/networkd-sysctl.c index 2ac6c3527bc28..2b226b2e2a183 100644 --- a/src/network/networkd-sysctl.c +++ b/src/network/networkd-sysctl.c @@ -2,6 +2,7 @@ #include #include +#include #include "missing_network.h" #include "networkd-link.h" @@ -12,10 +13,31 @@ #include "string-table.h" #include "sysctl-util.h" -static int link_update_ipv6_sysctl(Link *link) { +static bool link_is_configured_for_family(Link *link, int family) { assert(link); + if (!link->network) + return false; + if (link->flags & IFF_LOOPBACK) + return false; + + /* CAN devices do not support IP layer. Most of the functions below are never called for CAN devices, + * but link_set_ipv6_mtu() may be called after setting interface MTU, and warn about the failure. For + * safety, let's unconditionally check if the interface is not a CAN device. */ + if (IN_SET(family, AF_INET, AF_INET6) && link->iftype == ARPHRD_CAN) + return false; + + if (family == AF_INET6 && !socket_ipv6_is_supported()) + return false; + + return true; +} + +static int link_update_ipv6_sysctl(Link *link) { + assert(link); + + if (!link_is_configured_for_family(link, AF_INET6)) return 0; if (!link_ipv6_enabled(link)) @@ -27,10 +49,7 @@ static int link_update_ipv6_sysctl(Link *link) { static int link_set_proxy_arp(Link *link) { assert(link); - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET)) return 0; if (link->network->proxy_arp < 0) @@ -43,13 +62,7 @@ static bool link_ip_forward_enabled(Link *link, int family) { assert(link); assert(IN_SET(family, AF_INET, AF_INET6)); - if (family == AF_INET6 && !socket_ipv6_is_supported()) - return false; - - if (link->flags & IFF_LOOPBACK) - return false; - - if (!link->network) + if (!link_is_configured_for_family(link, family)) return false; return link->network->ip_forward & (family == AF_INET ? ADDRESS_FAMILY_IPV4 : ADDRESS_FAMILY_IPV6); @@ -92,10 +105,7 @@ static int link_set_ipv6_forward(Link *link) { static int link_set_ipv4_rp_filter(Link *link) { assert(link); - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET)) return 0; if (link->network->ipv4_rp_filter < 0) @@ -110,13 +120,7 @@ static int link_set_ipv6_privacy_extensions(Link *link) { assert(link); assert(link->manager); - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; val = link->network->ipv6_privacy_extensions; @@ -133,14 +137,7 @@ static int link_set_ipv6_privacy_extensions(Link *link) { static int link_set_ipv6_accept_ra(Link *link) { assert(link); - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; return sysctl_write_ip_property(AF_INET6, link->ifname, "accept_ra", "0"); @@ -149,14 +146,7 @@ static int link_set_ipv6_accept_ra(Link *link) { static int link_set_ipv6_dad_transmits(Link *link) { assert(link); - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; if (link->network->ipv6_dad_transmits < 0) @@ -168,14 +158,7 @@ static int link_set_ipv6_dad_transmits(Link *link) { static int link_set_ipv6_hop_limit(Link *link) { assert(link); - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; if (link->network->ipv6_hop_limit <= 0) @@ -189,13 +172,7 @@ static int link_set_ipv6_proxy_ndp(Link *link) { assert(link); - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; if (link->network->ipv6_proxy_ndp >= 0) @@ -211,14 +188,7 @@ int link_set_ipv6_mtu(Link *link) { assert(link); - /* Make this a NOP if IPv6 is not available */ - if (!socket_ipv6_is_supported()) - return 0; - - if (link->flags & IFF_LOOPBACK) - return 0; - - if (!link->network) + if (!link_is_configured_for_family(link, AF_INET6)) return 0; if (link->network->ipv6_mtu == 0) @@ -237,7 +207,7 @@ int link_set_ipv6_mtu(Link *link) { static int link_set_ipv4_accept_local(Link *link) { assert(link); - if (link->flags & IFF_LOOPBACK) + if (!link_is_configured_for_family(link, AF_INET)) return 0; if (link->network->ipv4_accept_local < 0) @@ -249,7 +219,7 @@ static int link_set_ipv4_accept_local(Link *link) { static int link_set_ipv4_route_localnet(Link *link) { assert(link); - if (link->flags & IFF_LOOPBACK) + if (!link_is_configured_for_family(link, AF_INET)) return 0; if (link->network->ipv4_route_localnet < 0) @@ -258,6 +228,20 @@ static int link_set_ipv4_route_localnet(Link *link) { return sysctl_write_ip_property_boolean(AF_INET, link->ifname, "route_localnet", link->network->ipv4_route_localnet > 0); } +static int link_set_ipv4_promote_secondaries(Link *link) { + assert(link); + + if (!link_is_configured_for_family(link, AF_INET)) + return 0; + + /* If promote_secondaries is not set, DHCP will work only as long as the IP address does not + * changes between leases. The kernel will remove all secondary IP addresses of an interface + * otherwise. The way systemd-networkd works is that the new IP of a lease is added as a + * secondary IP and when the primary one expires it relies on the kernel to promote the + * secondary IP. See also https://github.com/systemd/systemd/issues/7163 */ + return sysctl_write_ip_property_boolean(AF_INET, link->ifname, "promote_secondaries", true); +} + int link_set_sysctl(Link *link) { int r; @@ -321,12 +305,7 @@ int link_set_sysctl(Link *link) { if (r < 0) log_link_warning_errno(link, r, "Cannot set IPv4 reverse path filtering for interface, ignoring: %m"); - /* If promote_secondaries is not set, DHCP will work only as long as the IP address does not - * changes between leases. The kernel will remove all secondary IP addresses of an interface - * otherwise. The way systemd-networkd works is that the new IP of a lease is added as a - * secondary IP and when the primary one expires it relies on the kernel to promote the - * secondary IP. See also https://github.com/systemd/systemd/issues/7163 */ - r = sysctl_write_ip_property_boolean(AF_INET, link->ifname, "promote_secondaries", true); + r = link_set_ipv4_promote_secondaries(link); if (r < 0) log_link_warning_errno(link, r, "Cannot enable promote_secondaries for interface, ignoring: %m"); diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index 7a485f390fb51..58d22b6cfeed0 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -914,15 +914,30 @@ TEST(parse_mtu) { assert_se(parse_mtu(AF_UNSPEC, "4294967295", &mtu) >= 0 && mtu == 4294967295); assert_se(parse_mtu(AF_UNSPEC, "500", &mtu) >= 0 && mtu == 500); assert_se(parse_mtu(AF_UNSPEC, "1280", &mtu) >= 0 && mtu == 1280); + assert_se(parse_mtu(AF_UNSPEC, "4294967296", &mtu) == -ERANGE); + assert_se(parse_mtu(AF_UNSPEC, "68", &mtu) >= 0 && mtu == 68); + assert_se(parse_mtu(AF_UNSPEC, "67", &mtu) >= 0 && mtu == 67); + assert_se(parse_mtu(AF_UNSPEC, "0", &mtu) >= 0 && mtu == 0); + assert_se(parse_mtu(AF_UNSPEC, "", &mtu) == -EINVAL); + + assert_se(parse_mtu(AF_INET, "1500", &mtu) >= 0 && mtu == 1500); + assert_se(parse_mtu(AF_INET, "1400", &mtu) >= 0 && mtu == 1400); + assert_se(parse_mtu(AF_INET, "65535", &mtu) >= 0 && mtu == 65535); + assert_se(parse_mtu(AF_INET, "65536", &mtu) >= 0 && mtu == 65536); + assert_se(parse_mtu(AF_INET, "4294967295", &mtu) >= 0 && mtu == 4294967295); + assert_se(parse_mtu(AF_INET, "500", &mtu) >= 0 && mtu == 500); + assert_se(parse_mtu(AF_INET, "1280", &mtu) >= 0 && mtu == 1280); + assert_se(parse_mtu(AF_INET, "4294967296", &mtu) == -ERANGE); + assert_se(parse_mtu(AF_INET, "68", &mtu) >= 0 && mtu == 68); + assert_se(parse_mtu(AF_INET, "67", &mtu) == -ERANGE); + assert_se(parse_mtu(AF_INET, "0", &mtu) == -ERANGE); + assert_se(parse_mtu(AF_INET, "", &mtu) == -EINVAL); + assert_se(parse_mtu(AF_INET6, "1280", &mtu) >= 0 && mtu == 1280); assert_se(parse_mtu(AF_INET6, "1279", &mtu) == -ERANGE); - assert_se(parse_mtu(AF_UNSPEC, "4294967296", &mtu) == -ERANGE); assert_se(parse_mtu(AF_INET6, "4294967296", &mtu) == -ERANGE); assert_se(parse_mtu(AF_INET6, "68", &mtu) == -ERANGE); - assert_se(parse_mtu(AF_UNSPEC, "68", &mtu) >= 0 && mtu == 68); - assert_se(parse_mtu(AF_UNSPEC, "67", &mtu) == -ERANGE); - assert_se(parse_mtu(AF_UNSPEC, "0", &mtu) == -ERANGE); - assert_se(parse_mtu(AF_UNSPEC, "", &mtu) == -EINVAL); + assert_se(parse_mtu(AF_INET6, "", &mtu) == -EINVAL); } TEST(parse_loadavg_fixed_point) { diff --git a/test/test-network-generator-conversion.sh b/test/test-network-generator-conversion.sh index 6224a4d04f96a..9a4732c981dc9 100755 --- a/test/test-network-generator-conversion.sh +++ b/test/test-network-generator-conversion.sh @@ -296,7 +296,7 @@ INVALID_COMMAND_LINES=( "ip=10.0.0.1:::255.255.255::foo99:off" "ip=10.0.0.1:::255.255.255.0:invalid_hostname:foo99:off" "ip=10.0.0.1:::255.255.255.0::verylonginterfacename:off" - "ip=:::::dhcp99:dhcp6:0" + "ip=:::::dhcp99:dhcp6:4294967296" "ip=:::::dhcp99:dhcp6:-1" "ip=:::::dhcp99:dhcp6:666:52:54:00" "ip=fdef:c400:bd01:1096::2::[fdef:c400:bd01:1096::1]:64::ipv6:off:[fdef:c400:bd01:1096::aaaa]" diff --git a/test/test-network/conf/25-vcan.netdev b/test/test-network/conf/25-vcan.netdev index 29bd98e5c9d08..2762dd2374735 100644 --- a/test/test-network/conf/25-vcan.netdev +++ b/test/test-network/conf/25-vcan.netdev @@ -2,3 +2,4 @@ [NetDev] Name=vcan99 Kind=vcan +MTUBytes=16 diff --git a/test/test-network/conf/25-vcan98.netdev b/test/test-network/conf/25-vcan98.netdev new file mode 100644 index 0000000000000..5333c82da4598 --- /dev/null +++ b/test/test-network/conf/25-vcan98.netdev @@ -0,0 +1,4 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[NetDev] +Name=vcan98 +Kind=vcan diff --git a/test/test-network/conf/25-vcan98.network b/test/test-network/conf/25-vcan98.network new file mode 100644 index 0000000000000..97f824d2443d6 --- /dev/null +++ b/test/test-network/conf/25-vcan98.network @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=vcan98 + +[Link] +MTUBytes=16 diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index ea4f8d4b1571c..fc6606edb52e4 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1727,10 +1727,20 @@ def test_vrf(self): @expectedFailureIfModuleIsNotAvailable('vcan') def test_vcan(self): - copy_network_unit('25-vcan.netdev', '26-netdev-link-local-addressing-yes.network') + copy_network_unit('25-vcan.netdev', '26-netdev-link-local-addressing-yes.network', + '25-vcan98.netdev', '25-vcan98.network') start_networkd() - self.wait_online(['vcan99:carrier']) + self.wait_online(['vcan99:carrier', 'vcan98:carrier']) + + # https://github.com/systemd/systemd/issues/30140 + output = check_output('ip -d link show vcan99') + print(output) + self.assertIn('mtu 16 ', output) + + output = check_output('ip -d link show vcan98') + print(output) + self.assertIn('mtu 16 ', output) @expectedFailureIfModuleIsNotAvailable('vxcan') def test_vxcan(self):