diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c index 0ae5fbdc43..38f06d43b7 100644 --- a/src/network/netdev/bond.c +++ b/src/network/netdev/bond.c @@ -73,7 +73,7 @@ static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlin return r; } - bool up = link && FLAGS_SET(link->flags, IFF_UP); + bool up = link && link_is_up(link); bool has_slaves = link && !set_isempty(link->slaves); if (b->mode != _NETDEV_BOND_MODE_INVALID && !up && !has_slaves) { diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index e49d5946aa..c77b02b8e0 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -815,7 +815,6 @@ int link_ipv6ll_gained(Link *link) { int link_handle_bound_to_list(Link *link) { bool required_up = false; - bool link_is_up = false; Link *l; assert(link); @@ -826,18 +825,15 @@ int link_handle_bound_to_list(Link *link) { if (hashmap_isempty(link->bound_to_links)) return 0; - if (link->flags & IFF_UP) - link_is_up = true; - HASHMAP_FOREACH(l, link->bound_to_links) if (link_has_carrier(l)) { required_up = true; break; } - if (!required_up && link_is_up) + if (!required_up && link_is_up(link)) return link_request_to_bring_up_or_down(link, /* up= */ false); - if (required_up && !link_is_up) + if (required_up && !link_is_up(link)) return link_request_to_bring_up_or_down(link, /* up= */ true); return 0; @@ -2012,7 +2008,7 @@ void link_update_operstate(Link *link, bool also_update_master) { carrier_state = LINK_CARRIER_STATE_ENSLAVED; else carrier_state = LINK_CARRIER_STATE_CARRIER; - } else if (link->flags & IFF_UP) + } else if (link_is_up(link)) carrier_state = LINK_CARRIER_STATE_NO_CARRIER; else carrier_state = LINK_CARRIER_STATE_OFF; @@ -2147,6 +2143,11 @@ bool link_has_carrier(Link *link) { return netif_has_carrier(link->kernel_operstate, link->flags); } +bool link_is_up(Link *link) { + assert(link); + return FLAGS_SET(link->flags, IFF_UP); +} + bool link_multicast_enabled(Link *link) { assert(link); @@ -2226,7 +2227,7 @@ static int link_update_flags(Link *link, sd_netlink_message *message) { log_link_debug(link, "Unknown link flags lost, ignoring: %#.5x", unknown_flags_removed); } - link_was_admin_up = link->flags & IFF_UP; + link_was_admin_up = link_is_up(link); had_carrier = link_has_carrier(link); link->flags = flags; @@ -2236,9 +2237,9 @@ static int link_update_flags(Link *link, sd_netlink_message *message) { r = 0; - if (!link_was_admin_up && (link->flags & IFF_UP)) + if (!link_was_admin_up && link_is_up(link)) r = link_admin_state_up(link); - else if (link_was_admin_up && !(link->flags & IFF_UP)) + else if (link_was_admin_up && !link_is_up(link)) r = link_admin_state_down(link); if (r < 0) return r; diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index c5b9421bc0..9c641fad39 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -229,6 +229,7 @@ void link_check_ready(Link *link); void link_update_operstate(Link *link, bool also_update_master); bool link_has_carrier(Link *link); +bool link_is_up(Link *link); bool link_multicast_enabled(Link *link); bool link_ipv6_enabled(Link *link); diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 5e5f0d0e68..0e453e1597 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -772,26 +772,33 @@ static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) { if (!link_is_ready_to_configure(link, false)) return false; + /* Currently, we support the following three types of nexthops: + * 1. Simple nexthop - bound to the link, requires the underlying link is up. + * 2. Blackhole nexthop - not bound to the link. + * 3. Group nexthop - not bound to the link, but all group members must be configured first. + * + * Note, the kernel also supports fdb nexthop, but currently we do not support it. Note, fdb nexthop + * does not require IFF_UP. See rtm_to_nh_config() in net/ipv4/nexthop.c of kernel. */ + + /* Simple nexthop */ if (nexthop_bound_to_link(nexthop)) { assert(nexthop->ifindex == link->ifindex); - /* TODO: fdb nexthop does not require IFF_UP. The conditions below needs to be updated - * when fdb nexthop support is added. See rtm_to_nh_config() in net/ipv4/nexthop.c of - * kernel. */ - if (link->set_flags_messages > 0) - return false; - if (!FLAGS_SET(link->flags, IFF_UP)) - return false; + return gateway_is_ready(link, FLAGS_SET(nexthop->flags, RTNH_F_ONLINK), nexthop->family, &nexthop->gw.address); } - /* All group members must be configured first. */ + /* Blackhole nexthop */ + if (nexthop->blackhole) + return true; + + /* Group nexthop */ HASHMAP_FOREACH(nhg, nexthop->group) { r = nexthop_is_ready(link->manager, nhg->id, NULL); if (r <= 0) return r; } - return gateway_is_ready(link, FLAGS_SET(nexthop->flags, RTNH_F_ONLINK), nexthop->family, &nexthop->gw.address); + return true; } static int nexthop_process_request(Request *req, Link *link, NextHop *nexthop) { @@ -995,7 +1002,7 @@ void link_forget_nexthops(Link *link) { assert(link); assert(link->manager); assert(link->ifindex > 0); - assert(!FLAGS_SET(link->flags, IFF_UP)); + assert(!link_is_up(link)); /* See comments in link_forget_routes(). */ diff --git a/src/network/networkd-route-util.c b/src/network/networkd-route-util.c index 37ace2d335..3a7156fc41 100644 --- a/src/network/networkd-route-util.c +++ b/src/network/networkd-route-util.c @@ -139,6 +139,12 @@ bool gateway_is_ready(Link *link, bool onlink, int family, const union in_addr_u assert(link); assert(link->manager); + if (link->set_flags_messages > 0) + return false; + + if (!link_is_up(link)) + return false; + if (onlink) return true; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 2d84ba1db0..593832df51 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1641,7 +1641,7 @@ int link_drop_routes(Link *link, bool only_static) { void link_forget_routes(Link *link) { assert(link); assert(link->ifindex > 0); - assert(!FLAGS_SET(link->flags, IFF_UP)); + assert(!link_is_up(link)); /* When an interface went down, IPv4 non-local routes bound to the interface are silently removed by * the kernel, without any notifications. Let's forget them in that case. Otherwise, when the link diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 7069b101f9..f5a43788ef 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -562,7 +562,7 @@ static int link_is_ready_to_set_link(Link *link, Request *req) { case REQUEST_TYPE_SET_LINK_CAN: /* Do not check link->set_flags_messages here, as it is ok even if link->flags * is outdated, and checking the counter causes a deadlock. */ - if (FLAGS_SET(link->flags, IFF_UP)) { + if (link_is_up(link)) { /* The CAN interface must be down to configure bitrate, etc... */ r = link_down_now(link); if (r < 0) @@ -626,14 +626,14 @@ static int link_is_ready_to_set_link(Link *link, Request *req) { /* Do not check link->set_flags_messages here, as it is ok even if link->flags is outdated, * and checking the counter causes a deadlock. */ - if (link->network->bond && FLAGS_SET(link->flags, IFF_UP)) { + if (link->network->bond && link_is_up(link)) { /* link must be down when joining to bond master. */ r = link_down_now(link); if (r < 0) return r; } - if (link->network->bridge && !FLAGS_SET(link->flags, IFF_UP) && link->dev) { + if (link->network->bridge && !link_is_up(link) && link->dev) { /* Some devices require the port to be up before joining the bridge. * * E.g. Texas Instruments SoC Ethernet running in switch mode: @@ -755,8 +755,7 @@ int link_request_to_set_addrgen_mode(Link *link) { * link goes down. Hence, we need to reset the interface. However, setting the mode by sysctl * does not need that. Let's use the sysctl interface when the link is already up. * See also issue #22424. */ - if (mode != IPV6_LINK_LOCAL_ADDRESSS_GEN_MODE_NONE && - FLAGS_SET(link->flags, IFF_UP)) { + if (mode != IPV6_LINK_LOCAL_ADDRESSS_GEN_MODE_NONE && link_is_up(link)) { r = link_set_ipv6ll_addrgen_mode(link, mode); if (r < 0) log_link_warning_errno(link, r, "Cannot set IPv6 address generation mode, ignoring: %m"); @@ -1223,7 +1222,7 @@ static bool link_is_ready_to_bring_up_or_down(Link *link, bool up) { if (link_get_by_index(link->manager, link->dsa_master_ifindex, &master) < 0) return false; - if (!FLAGS_SET(master->flags, IFF_UP)) + if (!link_is_up(master)) return false; } diff --git a/test/test-network/conf/25-route-static-issue-40106-dummy.network b/test/test-network/conf/25-route-static-issue-40106-dummy.network new file mode 100644 index 0000000000..08553b97f5 --- /dev/null +++ b/test/test-network/conf/25-route-static-issue-40106-dummy.network @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=dummy99 + +[Network] +ConfigureWithoutCarrier=true +VLAN=vlan99 diff --git a/test/test-network/conf/25-route-static-issue-40106-vlan.netdev b/test/test-network/conf/25-route-static-issue-40106-vlan.netdev new file mode 100644 index 0000000000..c40e7b755a --- /dev/null +++ b/test/test-network/conf/25-route-static-issue-40106-vlan.netdev @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[NetDev] +Kind=vlan +Name=vlan99 + +[VLAN] +Id=99 diff --git a/test/test-network/conf/25-route-static-issue-40106-vlan.network b/test/test-network/conf/25-route-static-issue-40106-vlan.network new file mode 100644 index 0000000000..442cf480fb --- /dev/null +++ b/test/test-network/conf/25-route-static-issue-40106-vlan.network @@ -0,0 +1,15 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +Name=vlan99 + +[Network] +ConfigureWithoutCarrier=true +BindCarrier=dummy99 + +[Address] +Address=192.0.2.1/24 + +[Route] +Destination=198.51.100.1 +Type=multicast +Scope=link diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index bbbab8e3fe..6f2fe1b9eb 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -4603,6 +4603,47 @@ class NetworkdNetworkTests(unittest.TestCase, Utilities): self.assertRegex(output, '149.10.124.48/28 proto kernel scope link src 149.10.124.58') self.assertRegex(output, '149.10.124.66 via inet6 2001:1234:5:8fff:ff:ff:ff:ff proto static') + def test_route_static_issue_40106(self): + check_output('ip link add dummy99 type dummy') + check_output('ip link set dummy99 up carrier off') + copy_network_unit( + '25-route-static-issue-40106-dummy.network', + '25-route-static-issue-40106-vlan.netdev', + '25-route-static-issue-40106-vlan.network', + ) + start_networkd() + self.wait_online('dummy99:no-carrier') + self.wait_operstate('vlan99', operstate='off', setup_state='configuring') + + # address can be configured even when the interface is down. + self.wait_address('vlan99', '192.0.2.1/24', ipv='-4', timeout_sec=10) + print('### ip -4 address show dev vlan99') + output = check_output('ip -4 address show dev vlan99') + print(output) + self.assertIn('inet 192.0.2.1/24 brd 192.0.2.255 scope global vlan99', output) + + # route cannot be configured when the interface is down. + print('### ip -4 route show dev vlan99') + output = check_output('ip -4 route show dev vlan99') + print(output) + self.assertEqual(output, '') + + # When cable is connected, the vlan becomes up by BindCarrier=, then + # the pending route is also configured. + check_output('ip link set dummy99 carrier on') + self.wait_online('dummy99:degraded', 'vlan99:routable') + + print('### ip -4 address show dev vlan99') + output = check_output('ip -4 address show dev vlan99') + print(output) + self.assertIn('inet 192.0.2.1/24 brd 192.0.2.255 scope global vlan99', output) + + print('### ip -4 route show dev vlan99') + output = check_output('ip -4 route show dev vlan99') + print(output) + self.assertIn('192.0.2.0/24 proto kernel scope link src 192.0.2.1', output) + self.assertIn('multicast 198.51.100.1 proto static scope link', output) + @expectedFailureIfModuleIsNotAvailable('tcp_dctcp') def test_route_congctl(self): copy_network_unit('25-route-congctl.network', '12-dummy.netdev')