From ab6d4275473a3ddd4440202197a81514d94df994 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 13 Nov 2024 14:13:01 +0900 Subject: [PATCH 01/10] network/netdev: set interface name only when creating a new netdev Otherwise, the kernel older than v6.2 will refuse the netlink message. --- src/network/netdev/netdev.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 85760c741d..6c8ad1b3e9 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -662,9 +662,14 @@ static bool netdev_can_set_mtu(NetDev *netdev, uint32_t mtu) { static int netdev_create_message(NetDev *netdev, Link *link, sd_netlink_message *m) { int r; - r = sd_netlink_message_append_string(m, IFLA_IFNAME, netdev->ifname); - if (r < 0) - return r; + if (netdev->ifindex <= 0) { + /* Set interface name when it is newly created. Otherwise, the kernel older than + * bd039b5ea2a91ea707ee8539df26456bd5be80af (v6.2) will refuse the netlink message even if + * the name is unchanged. */ + r = sd_netlink_message_append_string(m, IFLA_IFNAME, netdev->ifname); + if (r < 0) + return r; + } struct hw_addr_data hw_addr; r = netdev_generate_hw_addr(netdev, link, netdev->ifname, &netdev->hw_addr, &hw_addr); From 09db4106064dd600c64d12a4e06bd88143b2e4f7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 13 Nov 2024 11:44:46 +0900 Subject: [PATCH 02/10] network/netdev: do not update MAC address if netdev is already running Follow-up for 17c5337f7b2993619d84acc2088b2ba1789e6477. Older kernels (older than v6.5) refuse RTM_NEWLINK messages with IFLA_ADDRESS attribute when the netdev already exists and is running, even if the MAC address is unchanged. So, let's not set IFLA_ADDRESS or IFLA_MTU if they are unchanged, and set the attributes only when we can update them. --- src/network/netdev/bridge.c | 5 +++++ src/network/netdev/dummy.c | 5 +++++ src/network/netdev/geneve.c | 5 +++++ src/network/netdev/netdev.c | 41 +++++++++++++++++++++++++++++----- src/network/netdev/netdevsim.c | 5 +++++ src/network/netdev/tunnel.c | 8 +++++++ src/network/netdev/vrf.c | 5 +++++ src/network/netdev/vxlan.c | 5 +++++ 8 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/network/netdev/bridge.c b/src/network/netdev/bridge.c index da5b332277..d3ba4989d9 100644 --- a/src/network/netdev/bridge.c +++ b/src/network/netdev/bridge.c @@ -281,12 +281,17 @@ static void bridge_init(NetDev *netdev) { b->ageing_time = USEC_INFINITY; } +static bool bridge_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + const NetDevVTable bridge_vtable = { .object_size = sizeof(Bridge), .init = bridge_init, .sections = NETDEV_COMMON_SECTIONS "Bridge\0", .post_create = netdev_bridge_post_create, .create_type = NETDEV_CREATE_INDEPENDENT, + .can_set_mac = bridge_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/dummy.c b/src/network/netdev/dummy.c index 00df1d2787..8b2893d5b4 100644 --- a/src/network/netdev/dummy.c +++ b/src/network/netdev/dummy.c @@ -4,10 +4,15 @@ #include "dummy.h" +static bool dummy_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + const NetDevVTable dummy_vtable = { .object_size = sizeof(Dummy), .sections = NETDEV_COMMON_SECTIONS, .create_type = NETDEV_CREATE_INDEPENDENT, + .can_set_mac = dummy_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index 493edcc42a..1d68be9bc8 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -254,6 +254,10 @@ static int netdev_geneve_verify(NetDev *netdev, const char *filename) { return 0; } +static bool geneve_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + static void geneve_init(NetDev *netdev) { Geneve *v = GENEVE(netdev); @@ -272,6 +276,7 @@ const NetDevVTable geneve_vtable = { .fill_message_create = netdev_geneve_fill_message_create, .create_type = NETDEV_CREATE_INDEPENDENT, .config_verify = netdev_geneve_verify, + .can_set_mac = geneve_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 6c8ad1b3e9..805ec09571 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -636,15 +636,32 @@ finalize: static bool netdev_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { assert(netdev); + assert(netdev->manager); assert(hw_addr); if (hw_addr->length <= 0) return false; - if (!NETDEV_VTABLE(netdev)->can_set_mac) - return true; + Link *link; + if (link_get_by_index(netdev->manager, netdev->ifindex, &link) < 0) + return true; /* The netdev does not exist yet. We can set MAC address. */ - return NETDEV_VTABLE(netdev)->can_set_mac(netdev, hw_addr); + if (hw_addr_equal(&link->hw_addr, hw_addr)) + return false; /* Unchanged, not necessary to set. */ + + /* Soem netdevs refuse to update MAC address even if the interface is not running, e.g. ipvlan. + * Some other netdevs have the IFF_LIVE_ADDR_CHANGE flag and can update update MAC address even if + * the interface is running, e.g. dummy. For those cases, use custom checkers. */ + if (NETDEV_VTABLE(netdev)->can_set_mac) + return NETDEV_VTABLE(netdev)->can_set_mac(netdev, hw_addr); + + /* Before ad72c4a06acc6762e84994ac2f722da7a07df34e and 0ec92a8f56ff07237dbe8af7c7a72aba7f957baf + * (both in v6.5), the kernel refuse to set MAC address for existing netdevs even if it is unchanged. + * So, by default, do not update MAC address if the it is running. See eth_prepare_mac_addr_change(), + * which is called by eth_mac_addr(). Note, the result of netif_running() is mapped to operstate + * and flags. See rtnl_fill_ifinfo() and dev_get_flags(). */ + return link->kernel_operstate == IF_OPER_DOWN && + (link->flags & (IFF_RUNNING | IFF_LOWER_UP | IFF_DORMANT)) == 0; } static bool netdev_can_set_mtu(NetDev *netdev, uint32_t mtu) { @@ -653,10 +670,22 @@ static bool netdev_can_set_mtu(NetDev *netdev, uint32_t mtu) { if (mtu <= 0) return false; - if (!NETDEV_VTABLE(netdev)->can_set_mtu) - return true; + Link *link; + if (link_get_by_index(netdev->manager, netdev->ifindex, &link) < 0) + return true; /* The netdev does not exist yet. We can set MTU. */ - return NETDEV_VTABLE(netdev)->can_set_mtu(netdev, mtu); + if (mtu < link->min_mtu || link->max_mtu < mtu) + return false; /* The MTU is out of range. */ + + if (link->mtu == mtu) + return false; /* Unchanged, not necessary to set. */ + + /* Some netdevs cannot change MTU, e.g. vxlan. Let's use the custom checkers in such cases. */ + if (NETDEV_VTABLE(netdev)->can_set_mtu) + return NETDEV_VTABLE(netdev)->can_set_mtu(netdev, mtu); + + /* By default, allow to update the MTU. */ + return true; } static int netdev_create_message(NetDev *netdev, Link *link, sd_netlink_message *m) { diff --git a/src/network/netdev/netdevsim.c b/src/network/netdev/netdevsim.c index 15d5c132f9..59958c3bbe 100644 --- a/src/network/netdev/netdevsim.c +++ b/src/network/netdev/netdevsim.c @@ -4,10 +4,15 @@ #include "netdevsim.h" +static bool netdevsim_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + const NetDevVTable netdevsim_vtable = { .object_size = sizeof(NetDevSim), .sections = NETDEV_COMMON_SECTIONS, .create_type = NETDEV_CREATE_INDEPENDENT, + .can_set_mac = netdevsim_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/tunnel.c b/src/network/netdev/tunnel.c index 0339c49c8f..af05cfda81 100644 --- a/src/network/netdev/tunnel.c +++ b/src/network/netdev/tunnel.c @@ -1119,6 +1119,11 @@ static void netdev_tunnel_init(NetDev *netdev) { t->ttl = DEFAULT_IPV6_TTL; } +static bool tunnel_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + assert(IN_SET(netdev->kind, NETDEV_KIND_GRETAP, NETDEV_KIND_IP6GRETAP, NETDEV_KIND_ERSPAN)); + return true; +} + const NetDevVTable ipip_vtable = { .object_size = sizeof(Tunnel), .init = netdev_tunnel_init, @@ -1188,6 +1193,7 @@ const NetDevVTable gretap_vtable = { .is_ready_to_create = netdev_tunnel_is_ready_to_create, .config_verify = netdev_tunnel_verify, .needs_reconfigure = tunnel_needs_reconfigure, + .can_set_mac = tunnel_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; @@ -1213,6 +1219,7 @@ const NetDevVTable ip6gretap_vtable = { .is_ready_to_create = netdev_tunnel_is_ready_to_create, .config_verify = netdev_tunnel_verify, .needs_reconfigure = tunnel_needs_reconfigure, + .can_set_mac = tunnel_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; @@ -1238,6 +1245,7 @@ const NetDevVTable erspan_vtable = { .is_ready_to_create = netdev_tunnel_is_ready_to_create, .config_verify = netdev_tunnel_verify, .needs_reconfigure = tunnel_needs_reconfigure, + .can_set_mac = tunnel_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/vrf.c b/src/network/netdev/vrf.c index 24079a7203..9e20bd22b2 100644 --- a/src/network/netdev/vrf.c +++ b/src/network/netdev/vrf.c @@ -21,11 +21,16 @@ static int netdev_vrf_fill_message_create(NetDev *netdev, Link *link, sd_netlink return 0; } +static bool vrf_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + const NetDevVTable vrf_vtable = { .object_size = sizeof(Vrf), .sections = NETDEV_COMMON_SECTIONS "VRF\0", .fill_message_create = netdev_vrf_fill_message_create, .create_type = NETDEV_CREATE_INDEPENDENT, + .can_set_mac = vrf_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, }; diff --git a/src/network/netdev/vxlan.c b/src/network/netdev/vxlan.c index 9f22794d34..d8a066370d 100644 --- a/src/network/netdev/vxlan.c +++ b/src/network/netdev/vxlan.c @@ -197,6 +197,10 @@ static int netdev_vxlan_fill_message_create(NetDev *netdev, Link *link, sd_netli return 0; } +static bool vxlan_can_set_mac(NetDev *netdev, const struct hw_addr_data *hw_addr) { + return true; +} + static bool vxlan_can_set_mtu(NetDev *netdev, uint32_t mtu) { assert(netdev); @@ -452,6 +456,7 @@ const NetDevVTable vxlan_vtable = { .create_type = NETDEV_CREATE_STACKED, .is_ready_to_create = netdev_vxlan_is_ready_to_create, .config_verify = netdev_vxlan_verify, + .can_set_mac = vxlan_can_set_mac, .can_set_mtu = vxlan_can_set_mtu, .needs_reconfigure = vxlan_needs_reconfigure, .iftype = ARPHRD_ETHER, From b0d2ce83423ac69c45d39dab1adabcb1b03ea565 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 9 Nov 2024 00:58:29 +0900 Subject: [PATCH 03/10] network/netdev: enter ready state only when it is created by us Follow-up for PR #34909. This fixes an issue that network interfaces cannot join a master netdev, like bond or bridge, when the corresponding .netdev is reloaded. With PR #34909, networkd supports reloading .netdev files. However, When a .netdev file is modified and reloaded, ifindex is copied from the old NetDev object to the new one. Thus, even if the interface is successfully updated, netdev_set_ifindex_impl() will return 0 and netdev_enter_ready() will never called. If the netdev is a kind of master netdev, then port interfaces cannot join the master netdev, as REQUEST_TYPE_SET_LINK_MASTER requires that the master netdev is in the ready state. --- src/network/netdev/netdev.c | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 805ec09571..67a98576fd 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -432,18 +432,17 @@ static int netdev_create_handler(sd_netlink *rtnl, sd_netlink_message *m, NetDev assert(netdev->state != _NETDEV_STATE_INVALID); r = sd_netlink_message_get_errno(m); - if (r == -EEXIST) - log_netdev_info(netdev, "netdev exists, using existing without changing its parameters"); - else if (r < 0) { - log_netdev_warning_errno(netdev, r, "netdev could not be created: %m"); + if (r >= 0) + log_netdev_debug(netdev, "Created."); + else if (r == -EEXIST && netdev->ifindex > 0) + log_netdev_debug(netdev, "Already exists."); + else { + log_netdev_warning_errno(netdev, r, "Failed to create netdev: %m"); netdev_enter_failed(netdev); - - return 1; + return 0; } - log_netdev_debug(netdev, "Created"); - - return 1; + return netdev_enter_ready(netdev); } int netdev_set_ifindex_internal(NetDev *netdev, int ifindex) { @@ -464,8 +463,6 @@ int netdev_set_ifindex_internal(NetDev *netdev, int ifindex) { } static int netdev_set_ifindex_impl(NetDev *netdev, const char *name, int ifindex) { - int r; - assert(netdev); assert(name); assert(ifindex > 0); @@ -478,11 +475,7 @@ static int netdev_set_ifindex_impl(NetDev *netdev, const char *name, int ifindex "Received netlink message with unexpected interface name %s (ifindex=%i).", name, ifindex); - r = netdev_set_ifindex_internal(netdev, ifindex); - if (r <= 0) - return r; - - return netdev_enter_ready(netdev); + return netdev_set_ifindex_internal(netdev, ifindex); } int netdev_set_ifindex(NetDev *netdev, sd_netlink_message *message) { @@ -875,18 +868,26 @@ static int stacked_netdev_process_request(Request *req, Link *link, void *userda } static int create_stacked_netdev_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, void *userdata) { + NetDev *netdev = ASSERT_PTR(userdata); int r; assert(m); assert(link); r = sd_netlink_message_get_errno(m); - if (r < 0 && r != -EEXIST) { - log_link_message_warning_errno(link, m, r, "Could not create stacked netdev"); + if (r >= 0) + log_netdev_debug(netdev, "Created."); + else if (r == -EEXIST && netdev->ifindex > 0) + log_netdev_debug(netdev, "Already exists."); + else { + log_netdev_warning_errno(netdev, r, "Failed to create netdev: %m"); + netdev_enter_failed(netdev); link_enter_failed(link); return 0; } + (void) netdev_enter_ready(netdev); + if (link->create_stacked_netdev_messages == 0) { link->stacked_netdevs_created = true; log_link_debug(link, "Stacked netdevs created."); From 259125d53d98541623b69e83000b5543f2352f5e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 1 Nov 2024 04:56:12 +0900 Subject: [PATCH 04/10] network/netdev: always queue request of creating netdev then process it later After PR #34909, networkd tries to update an existing netdev interface if possible. But, when .netdev files are loaded on start, we have not enumerate interfaces, so we do not know if the corresponding interface exists or not. Let's delay processing request a bit. --- src/network/netdev/netdev.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 67a98576fd..32050ecf60 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -965,21 +965,9 @@ static int netdev_request_to_create(NetDev *netdev) { if (netdev->state != NETDEV_STATE_LOADING) return 0; /* Already configured (at least tried previously). Not necessary to reconfigure. */ - r = netdev_is_ready_to_create(netdev, NULL); + r = netdev_queue_request(netdev, independent_netdev_process_request, NULL); if (r < 0) - return r; - if (r > 0) { - /* If the netdev has no dependency, then create it now. */ - r = independent_netdev_create(netdev); - if (r < 0) - return log_netdev_warning_errno(netdev, r, "Failed to create netdev: %m"); - - } else { - /* Otherwise, wait for the dependencies being resolved. */ - r = netdev_queue_request(netdev, independent_netdev_process_request, NULL); - if (r < 0) - return log_netdev_warning_errno(netdev, r, "Failed to request to create netdev: %m"); - } + return log_netdev_warning_errno(netdev, r, "Failed to request to create netdev: %m"); return 0; } From f264cd20372d8a2c26398dd46cf2823d1a82cb8c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 11 Nov 2024 11:13:39 +0900 Subject: [PATCH 05/10] network/netdev: fix counter handling if request is cancelled Follow-up for 1003093604661bd984574889167f2ff4dfd6209c. If a netdev is detached for some reasons, then previously the request was simply cancelled, and the underlying interface never enter the configured state, as the 'stacked_netdevs_created' flag never set. This makes the counter decremented manually by the function, and set the flag. So, the underlying interface can eter the configured state. --- src/network/netdev/netdev.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 32050ecf60..6cce3838d1 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -854,7 +854,7 @@ static int stacked_netdev_process_request(Request *req, Link *link, void *userda assert(link); if (!netdev_is_managed(netdev)) - return 1; /* Already detached, due to e.g. reloading .netdev files, cancelling the request. */ + goto cancelled; /* Already detached, due to e.g. reloading .netdev files, cancelling the request. */ r = netdev_is_ready_to_create(netdev, link); if (r <= 0) @@ -865,6 +865,18 @@ static int stacked_netdev_process_request(Request *req, Link *link, void *userda return log_netdev_warning_errno(netdev, r, "Failed to create netdev: %m"); return 1; + +cancelled: + assert_se(TAKE_PTR(req->counter) == &link->create_stacked_netdev_messages); + link->create_stacked_netdev_messages--; + + if (link->create_stacked_netdev_messages == 0) { + link->stacked_netdevs_created = true; + log_link_debug(link, "Stacked netdevs created."); + link_check_ready(link); + } + + return 1; } static int create_stacked_netdev_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, void *userdata) { From 422b7c857c47aa008af07b74080280c2d8577d95 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sun, 10 Nov 2024 09:51:32 +0900 Subject: [PATCH 06/10] network/netdev: do not try to update if not supported Some netdevs cannot update there properties after created. Let's skip requests in that case. --- src/network/netdev/bareudp.c | 1 + src/network/netdev/ipvlan.c | 2 ++ src/network/netdev/macvlan.c | 2 ++ src/network/netdev/netdev.c | 20 +++++++++++++++++++- src/network/netdev/netdev.h | 4 ++++ src/network/netdev/veth.c | 1 + src/network/netdev/vrf.c | 1 + src/network/netdev/vxcan.c | 1 + src/network/netdev/wireguard.c | 1 + 9 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/network/netdev/bareudp.c b/src/network/netdev/bareudp.c index b2a161ee7a..e122abd97f 100644 --- a/src/network/netdev/bareudp.c +++ b/src/network/netdev/bareudp.c @@ -72,4 +72,5 @@ const NetDevVTable bare_udp_vtable = { .fill_message_create = netdev_bare_udp_fill_message_create, .create_type = NETDEV_CREATE_INDEPENDENT, .iftype = ARPHRD_NONE, + .keep_existing = true, }; diff --git a/src/network/netdev/ipvlan.c b/src/network/netdev/ipvlan.c index 5cfae1eb64..6e50f72aaa 100644 --- a/src/network/netdev/ipvlan.c +++ b/src/network/netdev/ipvlan.c @@ -60,6 +60,7 @@ const NetDevVTable ipvlan_vtable = { .can_set_mac = ipvlan_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; const NetDevVTable ipvtap_vtable = { @@ -71,6 +72,7 @@ const NetDevVTable ipvtap_vtable = { .can_set_mac = ipvlan_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; IPVlanMode link_get_ipvlan_mode(Link *link) { diff --git a/src/network/netdev/macvlan.c b/src/network/netdev/macvlan.c index 1537d868b5..fd112b58e1 100644 --- a/src/network/netdev/macvlan.c +++ b/src/network/netdev/macvlan.c @@ -178,6 +178,7 @@ const NetDevVTable macvtap_vtable = { .create_type = NETDEV_CREATE_STACKED, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; const NetDevVTable macvlan_vtable = { @@ -189,4 +190,5 @@ const NetDevVTable macvlan_vtable = { .create_type = NETDEV_CREATE_STACKED, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 6cce3838d1..d3c2a8e442 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -400,7 +400,7 @@ int netdev_enter_ready(NetDev *netdev) { assert(netdev); assert(netdev->ifname); - if (netdev->state != NETDEV_STATE_CREATING) + if (!IN_SET(netdev->state, NETDEV_STATE_LOADING, NETDEV_STATE_CREATING)) return 0; netdev->state = NETDEV_STATE_READY; @@ -856,6 +856,15 @@ static int stacked_netdev_process_request(Request *req, Link *link, void *userda if (!netdev_is_managed(netdev)) goto cancelled; /* Already detached, due to e.g. reloading .netdev files, cancelling the request. */ + if (NETDEV_VTABLE(netdev)->keep_existing && netdev->ifindex > 0) { + /* Already exists, and the netdev does not support updating, entering the ready state. */ + r = netdev_enter_ready(netdev); + if (r < 0) + return r; + + goto cancelled; + } + r = netdev_is_ready_to_create(netdev, link); if (r <= 0) return r; @@ -948,6 +957,15 @@ static int independent_netdev_process_request(Request *req, Link *link, void *us if (!netdev_is_managed(netdev)) return 1; /* Already detached, due to e.g. reloading .netdev files, cancelling the request. */ + if (NETDEV_VTABLE(netdev)->keep_existing && netdev->ifindex > 0) { + /* Already exists, and the netdev does not support updating, entering the ready state. */ + r = netdev_enter_ready(netdev); + if (r < 0) + return r; + + return 1; /* Skip this request. */ + } + r = netdev_is_ready_to_create(netdev, NULL); if (r <= 0) return r; diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 765496d044..b330aa394d 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -199,6 +199,10 @@ typedef struct NetDevVTable { /* When assigning ifindex to the netdev, skip to check if the netdev kind matches. */ bool skip_netdev_kind_check; + + /* Provides if the netdev can be updated, that is, whether RTM_NEWLINK with existing ifindex is supported or not. + * If this is true, the netdev does not support updating. */ + bool keep_existing; } NetDevVTable; extern const NetDevVTable * const netdev_vtable[_NETDEV_KIND_MAX]; diff --git a/src/network/netdev/veth.c b/src/network/netdev/veth.c index edebcf0a74..54d3b59734 100644 --- a/src/network/netdev/veth.c +++ b/src/network/netdev/veth.c @@ -150,4 +150,5 @@ const NetDevVTable veth_vtable = { .get_ifindex = netdev_veth_get_ifindex, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; diff --git a/src/network/netdev/vrf.c b/src/network/netdev/vrf.c index 9e20bd22b2..c35419f859 100644 --- a/src/network/netdev/vrf.c +++ b/src/network/netdev/vrf.c @@ -33,4 +33,5 @@ const NetDevVTable vrf_vtable = { .can_set_mac = vrf_can_set_mac, .iftype = ARPHRD_ETHER, .generate_mac = true, + .keep_existing = true, }; diff --git a/src/network/netdev/vxcan.c b/src/network/netdev/vxcan.c index 929d70f61a..2de89b8e24 100644 --- a/src/network/netdev/vxcan.c +++ b/src/network/netdev/vxcan.c @@ -124,4 +124,5 @@ const NetDevVTable vxcan_vtable = { .set_ifindex = netdev_vxcan_set_ifindex, .get_ifindex = netdev_vxcan_get_ifindex, .iftype = ARPHRD_CAN, + .keep_existing = true, }; diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c index 9e8dfb259a..8d1dddf828 100644 --- a/src/network/netdev/wireguard.c +++ b/src/network/netdev/wireguard.c @@ -1265,4 +1265,5 @@ const NetDevVTable wireguard_vtable = { .create_type = NETDEV_CREATE_INDEPENDENT, .config_verify = wireguard_verify, .iftype = ARPHRD_NONE, + .keep_existing = true, }; From 69bd661a2d7281e571fc1479a892f67f373d6379 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 9 Nov 2024 01:46:05 +0900 Subject: [PATCH 07/10] network/bond: do not update several parameters if already up or has slaves Some bonding parameters cannot be updated when the netdev is already up or already has at least one slave interface. --- src/network/netdev/bond.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/network/netdev/bond.c b/src/network/netdev/bond.c index de122f8eb3..b866940b7a 100644 --- a/src/network/netdev/bond.c +++ b/src/network/netdev/bond.c @@ -56,13 +56,23 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_arp_all_targets, bond_arp_all_targets DEFINE_CONFIG_PARSE_ENUM(config_parse_bond_primary_reselect, bond_primary_reselect, BondPrimaryReselect); static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlink_message *m) { - assert(!link); - assert(m); - Bond *b = BOND(netdev); int r; - if (b->mode != _NETDEV_BOND_MODE_INVALID) { + assert(netdev->manager); + assert(!link); + assert(m); + + if (netdev->ifindex > 0) { + r = link_get_by_index(netdev->manager, netdev->ifindex, &link); + if (r < 0) + return r; + } + + bool up = link && FLAGS_SET(link->flags, IFF_UP); + bool has_slaves = link && !set_isempty(link->slaves); + + if (b->mode != _NETDEV_BOND_MODE_INVALID && !up && !has_slaves) { r = sd_netlink_message_append_u8(m, IFLA_BOND_MODE, b->mode); if (r < 0) return r; @@ -75,7 +85,8 @@ static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlin } if (b->lacp_rate != _NETDEV_BOND_LACP_RATE_INVALID && - b->mode == NETDEV_BOND_MODE_802_3AD) { + b->mode == NETDEV_BOND_MODE_802_3AD && + !up) { r = sd_netlink_message_append_u8(m, IFLA_BOND_AD_LACP_RATE, b->lacp_rate); if (r < 0) return r; @@ -119,14 +130,16 @@ static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlin } if (b->ad_select != _NETDEV_BOND_AD_SELECT_INVALID && - b->mode == NETDEV_BOND_MODE_802_3AD) { + b->mode == NETDEV_BOND_MODE_802_3AD && + !up) { r = sd_netlink_message_append_u8(m, IFLA_BOND_AD_SELECT, b->ad_select); if (r < 0) return r; } if (b->fail_over_mac != _NETDEV_BOND_FAIL_OVER_MAC_INVALID && - b->mode == NETDEV_BOND_MODE_ACTIVE_BACKUP) { + b->mode == NETDEV_BOND_MODE_ACTIVE_BACKUP && + !has_slaves) { r = sd_netlink_message_append_u8(m, IFLA_BOND_FAIL_OVER_MAC, b->fail_over_mac); if (r < 0) return r; @@ -181,7 +194,7 @@ static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlin return r; } - if (b->ad_user_port_key != 0) { + if (b->ad_user_port_key != 0 && !up) { r = sd_netlink_message_append_u16(m, IFLA_BOND_AD_USER_PORT_KEY, b->ad_user_port_key); if (r < 0) return r; @@ -197,7 +210,7 @@ static int netdev_bond_fill_message_create(NetDev *netdev, Link *link, sd_netlin if (r < 0) return r; - if (b->tlb_dynamic_lb >= 0) { + if (b->tlb_dynamic_lb >= 0 && !up) { r = sd_netlink_message_append_u8(m, IFLA_BOND_TLB_DYNAMIC_LB, b->tlb_dynamic_lb); if (r < 0) return r; From 34e5440fb252dd9a26e7e35b4f4cbaf17b7d7749 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 9 Nov 2024 02:48:17 +0900 Subject: [PATCH 08/10] network/tuntap: manage tun/tap fds by manager Otherwise, when a .netdev file for tun or tap netdev is updated, reloading the file leaks the previous file descriptor. --- src/network/netdev/tuntap.c | 147 +++++++++++++++++++----------------- src/network/netdev/tuntap.h | 1 - 2 files changed, 78 insertions(+), 70 deletions(-) diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c index c012bc1591..a421e66733 100644 --- a/src/network/netdev/tuntap.c +++ b/src/network/netdev/tuntap.c @@ -36,15 +36,33 @@ static TunTap* TUNTAP(NetDev *netdev) { DEFINE_PRIVATE_HASH_OPS_FULL(named_fd_hash_ops, char, string_hash_func, string_compare_func, free, void, close_fd_ptr); -int manager_add_tuntap_fd(Manager *m, int fd, const char *name) { +static int manager_add_tuntap_fd_impl(Manager *m, int fd, const char *name) { _cleanup_free_ char *tuntap_name = NULL; - const char *p; int r; assert(m); assert(fd >= 0); assert(name); + tuntap_name = strdup(name); + if (!tuntap_name) + return log_oom_debug(); + + r = hashmap_ensure_put(&m->tuntap_fds_by_name, &named_fd_hash_ops, tuntap_name, FD_TO_PTR(fd)); + if (r < 0) + return log_debug_errno(r, "Failed to store tuntap fd: %m"); + + TAKE_PTR(tuntap_name); + return 0; +} + +int manager_add_tuntap_fd(Manager *m, int fd, const char *name) { + const char *p; + + assert(m); + assert(fd >= 0); + assert(name); + p = startswith(name, "tuntap-"); if (!p) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unknown fd (%s).", name); @@ -52,66 +70,76 @@ int manager_add_tuntap_fd(Manager *m, int fd, const char *name) { if (!ifname_valid(p)) return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received tuntap fd with invalid name (%s).", p); - tuntap_name = strdup(p); - if (!tuntap_name) - return log_oom_debug(); + return manager_add_tuntap_fd_impl(m, fd, p); +} - r = hashmap_ensure_put(&m->tuntap_fds_by_name, &named_fd_hash_ops, tuntap_name, FD_TO_PTR(fd)); +static int netdev_take_tuntap_fd(Manager *m, const char *ifname) { + _unused_ _cleanup_free_ char *name = NULL; + void *p; + + assert(m); + assert(ifname); + + p = hashmap_remove2(m->tuntap_fds_by_name, ifname, (void**) &name); + if (!p) + return -EBADF; + + return PTR_TO_FD(p); +} + +static int netdev_push_tuntap_fd(NetDev *netdev, int fd) { + _unused_ _cleanup_close_ int fd_old = -EBADF; + int r; + + assert(netdev->manager); + + fd_old = netdev_take_tuntap_fd(netdev->manager, netdev->ifname); + + if (!TUNTAP(netdev)->keep_fd) + return 0; + + r = manager_add_tuntap_fd_impl(netdev->manager, fd, netdev->ifname); if (r < 0) - return log_debug_errno(r, "Failed to store tuntap fd: %m"); + return r; - TAKE_PTR(tuntap_name); - return 0; + (void) notify_push_fdf(fd, "tuntap-%s", netdev->ifname); + return 1; /* saved */ +} + +static void manager_close_and_notify_tuntap_fd(Manager *m, const char *ifname) { + assert(m); + assert(ifname); + + /* netdev_take_tuntap_fd() may invalidate ifname. Hence, need to create fdname earlier. */ + const char *fdname = strjoina("tuntap-", ifname); + close_and_notify_warn(netdev_take_tuntap_fd(m, ifname), fdname); } void manager_clear_unmanaged_tuntap_fds(Manager *m) { - char *name; + const char *name; void *p; assert(m); - while ((p = hashmap_steal_first_key_and_value(m->tuntap_fds_by_name, (void**) &name))) { - close_and_notify_warn(PTR_TO_FD(p), name); - name = mfree(name); + HASHMAP_FOREACH_KEY(p, name, m->tuntap_fds_by_name) { + NetDev *netdev; + + if (netdev_get(m, name, &netdev) < 0 || + !IN_SET(netdev->kind, NETDEV_KIND_TAP, NETDEV_KIND_TUN) || + !TUNTAP(netdev)->keep_fd) + manager_close_and_notify_tuntap_fd(m, name); } } -static int tuntap_take_fd(NetDev *netdev) { - _cleanup_free_ char *name = NULL; - void *p; - int r; - - assert(netdev); - assert(netdev->manager); - - r = link_get_by_name(netdev->manager, netdev->ifname, NULL); - if (r < 0) - return r; - - p = hashmap_remove2(netdev->manager->tuntap_fds_by_name, netdev->ifname, (void**) &name); - if (!p) - return -ENOENT; - - log_netdev_debug(netdev, "Found file descriptor in fd store."); - return PTR_TO_FD(p); -} - static int netdev_create_tuntap(NetDev *netdev) { _cleanup_close_ int fd = -EBADF; struct ifreq ifr = {}; - TunTap *t; + TunTap *t = TUNTAP(netdev); int r; - assert(netdev); assert(netdev->manager); - t = TUNTAP(netdev); - assert(t); - fd = TAKE_FD(t->fd); - if (fd < 0) - fd = tuntap_take_fd(netdev); - if (fd < 0) - fd = open(TUN_DEV, O_RDWR|O_CLOEXEC); + fd = open(TUN_DEV, O_RDWR|O_CLOEXEC); if (fd < 0) return log_netdev_error_errno(netdev, errno, "Failed to open " TUN_DEV ": %m"); @@ -175,42 +203,25 @@ static int netdev_create_tuntap(NetDev *netdev) { if (ioctl(fd, TUNSETPERSIST, 1) < 0) return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed: %m"); - if (t->keep_fd) { - t->fd = TAKE_FD(fd); - (void) notify_push_fdf(t->fd, "tuntap-%s", netdev->ifname); - } + r = netdev_push_tuntap_fd(netdev, fd); + if (r < 0) + return log_netdev_warning_errno(netdev, r, "Failed to save TUN/TAP fd: %m"); + if (r > 0) + TAKE_FD(fd); + netdev_enter_ready(netdev); return 0; } -static void tuntap_init(NetDev *netdev) { - TunTap *t; - - assert(netdev); - t = TUNTAP(netdev); - assert(t); - - t->fd = -EBADF; -} - static void tuntap_drop(NetDev *netdev) { - TunTap *t; - assert(netdev); - t = TUNTAP(netdev); - assert(t); - t->fd = close_and_notify_warn(t->fd, netdev->ifname); + manager_close_and_notify_tuntap_fd(netdev->manager, netdev->ifname); } static void tuntap_done(NetDev *netdev) { - TunTap *t; + TunTap *t = TUNTAP(netdev); - assert(netdev); - t = TUNTAP(netdev); - assert(t); - - t->fd = safe_close(t->fd); t->user_name = mfree(t->user_name); t->group_name = mfree(t->group_name); } @@ -237,7 +248,6 @@ const NetDevVTable tun_vtable = { .object_size = sizeof(TunTap), .sections = NETDEV_COMMON_SECTIONS "Tun\0", .config_verify = tuntap_verify, - .init = tuntap_init, .drop = tuntap_drop, .done = tuntap_done, .create = netdev_create_tuntap, @@ -249,7 +259,6 @@ const NetDevVTable tap_vtable = { .object_size = sizeof(TunTap), .sections = NETDEV_COMMON_SECTIONS "Tap\0", .config_verify = tuntap_verify, - .init = tuntap_init, .drop = tuntap_drop, .done = tuntap_done, .create = netdev_create_tuntap, diff --git a/src/network/netdev/tuntap.h b/src/network/netdev/tuntap.h index 88e0ce5f97..e2de8eb6bb 100644 --- a/src/network/netdev/tuntap.h +++ b/src/network/netdev/tuntap.h @@ -8,7 +8,6 @@ typedef struct TunTap TunTap; struct TunTap { NetDev meta; - int fd; char *user_name; char *group_name; bool multi_queue; From 5b73edfa7f9920a5d042343bea8e54d001af1707 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 9 Nov 2024 01:26:29 +0900 Subject: [PATCH 09/10] test-network: add tests for reloading .netdev files for independent netdevs --- test/test-network/systemd-networkd-tests.py | 113 +++++++++++++++++++- 1 file changed, 108 insertions(+), 5 deletions(-) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 5ae57dd951..97b21fa788 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1628,6 +1628,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertRegex(output, 'ethertype ip ') self.assertRegex(output, 'srcportmin 1001 ') + touch_network_unit('25-bareudp.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('bareudp99:degraded') + @expectedFailureIfModuleIsNotAvailable('batman-adv') def test_batadv(self): copy_network_unit('25-batadv.netdev', '26-netdev-link-local-addressing-yes.network') @@ -1640,6 +1644,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'batadv') + touch_network_unit('25-batadv.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('batadv99:degraded') + def test_bridge(self): copy_network_unit('25-bridge.netdev', '25-bridge-configure-without-carrier.network') start_networkd() @@ -1964,6 +1972,17 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertRegex(output, 'link/ether 12:34:56:78:9a:bf') self.assertRegex(output, 'mtu 1800') + touch_network_unit( + '25-veth.netdev', + '26-netdev-link-local-addressing-yes.network', + '25-veth-mtu.netdev') + networkctl_reload() + self.wait_online( + 'veth99:degraded', + 'veth-peer:degraded', + 'veth-mtu:degraded', + 'veth-mtu-peer:degraded') + def check_tuntap(self, attached): pid = networkd_pid() name = psutil.Process(pid).name()[:15] @@ -2023,6 +2042,12 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.check_tuntap(True) + touch_network_unit('25-tun.netdev', '25-tap.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('testtun99:degraded', 'testtap99:degraded') + + self.check_tuntap(True) + remove_network_unit('26-netdev-link-local-addressing-yes.network') restart_networkd() self.wait_online('testtun99:degraded', 'testtap99:degraded', setup_state='unmanaged') @@ -2048,6 +2073,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.wait_online('vrf99:carrier') self.networkctl_check_unit('vrf99', '25-vrf', '26-netdev-link-local-addressing-yes') + touch_network_unit('25-vrf.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl() + self.wait_online('vrf99:carrier') + @expectedFailureIfModuleIsNotAvailable('vcan') def test_vcan(self): copy_network_unit('25-vcan.netdev', '26-netdev-link-local-addressing-yes.network', @@ -2069,6 +2098,14 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): print(output) self.assertIn('mtu 16 ', output) + touch_network_unit( + '25-vcan.netdev', + '26-netdev-link-local-addressing-yes.network', + '25-vcan98.netdev', + '25-vcan98.network') + networkctl_reload() + self.wait_online('vcan99:carrier', 'vcan98:carrier') + @expectedFailureIfModuleIsNotAvailable('vxcan') def test_vxcan(self): copy_network_unit('25-vxcan.netdev', '26-netdev-link-local-addressing-yes.network') @@ -2080,6 +2117,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.networkctl_check_unit('vxcan99', '25-vxcan', '26-netdev-link-local-addressing-yes') self.networkctl_check_unit('vxcan-peer', '25-vxcan', '26-netdev-link-local-addressing-yes') + touch_network_unit('25-vxcan.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl() + self.wait_online('vxcan99:carrier', 'vxcan-peer:carrier') + @expectedFailureIfModuleIsNotAvailable('wireguard') def test_wireguard(self): copy_credential('25-wireguard-endpoint-peer0-cred.txt', 'network.wireguard.peer0.endpoint') @@ -2211,6 +2252,14 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): output = check_output('wg show wg97 fwmark') self.assertEqual(output, '0x4d3') + touch_network_unit( + '25-wireguard.netdev', '25-wireguard.network', + '25-wireguard-23-peers.netdev', '25-wireguard-23-peers.network', + '25-wireguard-public-key.txt', '25-wireguard-preshared-key.txt', '25-wireguard-private-key.txt', + '25-wireguard-no-peer.netdev', '25-wireguard-no-peer.network') + networkctl_reload() + self.wait_online('wg99:routable', 'wg98:routable', 'wg97:carrier') + def test_geneve(self): copy_network_unit('25-geneve.netdev', '26-netdev-link-local-addressing-yes.network') start_networkd() @@ -2225,6 +2274,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertRegex(output, 'udpcsum') self.assertRegex(output, 'udp6zerocsumrx') + touch_network_unit('25-geneve.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('geneve99:degraded') + def test_ipip_tunnel(self): copy_network_unit('12-dummy.netdev', '25-ipip.network', '25-ipip-tunnel.netdev', '25-tunnel.network', @@ -2825,6 +2878,13 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): print(output) self.assertRegex(output, 'encap fou encap-sport auto encap-dport 55556') + touch_network_unit( + '25-fou-ipproto-ipip.netdev', '25-fou-ipproto-gre.netdev', + '25-fou-ipip.netdev', '25-fou-sit.netdev', + '25-fou-gre.netdev', '25-fou-gretap.netdev') + networkctl_reload() + self.wait_online('ipiptun96:off', 'sittun96:off', 'gretun96:off', 'gretap96:off', setup_state='unmanaged') + def test_vxlan(self): copy_network_unit('11-dummy.netdev', '25-vxlan-test1.network', '25-vxlan.netdev', '25-vxlan.network', @@ -2929,6 +2989,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.wait_online('nlmon99:carrier') self.networkctl_check_unit('nlmon99', '25-nlmon', '26-netdev-link-local-addressing-yes') + touch_network_unit('25-nlmon.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('nlmon99:carrier') + @expectedFailureIfModuleIsNotAvailable('ifb') def test_ifb(self): copy_network_unit('25-ifb.netdev', '26-netdev-link-local-addressing-yes.network') @@ -2937,6 +3001,10 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.wait_online('ifb99:degraded') self.networkctl_check_unit('ifb99', '25-ifb', '26-netdev-link-local-addressing-yes') + touch_network_unit('25-ifb.netdev', '26-netdev-link-local-addressing-yes.network') + networkctl_reload() + self.wait_online('ifb99:degraded') + @unittest.skipUnless(os.cpu_count() >= 2, reason="CPU count should be >= 2 to pass this test") def test_rps_cpu_1(self): copy_network_unit('12-dummy.netdev', '12-dummy.network', '25-rps-cpu-1.link') @@ -3093,6 +3161,12 @@ class NetworkdL2TPTests(unittest.TestCase, Utilities): self.assertRegex(output, "Peer session 18, tunnel 11") self.assertRegex(output, "interface name: l2tp-ses2") + touch_network_unit( + '11-dummy.netdev', '25-l2tp-dummy.network', + '25-l2tp-udp.netdev', '25-l2tp.network') + networkctl_reload() + self.wait_online('test1:routable', 'l2tp-ses1:degraded', 'l2tp-ses2:degraded') + @expectedFailureIfModuleIsNotAvailable('l2tp_eth', 'l2tp_ip', 'l2tp_netlink') def test_l2tp_ip(self): copy_network_unit('11-dummy.netdev', '25-l2tp-dummy.network', @@ -3122,6 +3196,12 @@ class NetworkdL2TPTests(unittest.TestCase, Utilities): self.assertRegex(output, "Peer session 28, tunnel 12") self.assertRegex(output, "interface name: l2tp-ses4") + touch_network_unit( + '11-dummy.netdev', '25-l2tp-dummy.network', + '25-l2tp-ip.netdev', '25-l2tp.network') + networkctl_reload() + self.wait_online('test1:routable', 'l2tp-ses3:degraded', 'l2tp-ses4:degraded') + class NetworkdNetworkTests(unittest.TestCase, Utilities): def setUp(self): @@ -5365,6 +5445,15 @@ class NetworkdBondTests(unittest.TestCase, Utilities): self.wait_online('dummy98:enslaved', 'bond199:degraded') self.assertNotIn('dummy98: Bringing link down', read_networkd_log(since=since)) + # test for reloading. + touch_network_unit( + '23-active-slave.network', + '23-bond199.network', + '25-bond-active-backup-slave.netdev', + '12-dummy.netdev') + networkctl_reload() + self.wait_online('dummy98:enslaved', 'bond199:degraded') + def test_bond_primary_slave(self): copy_network_unit('23-primary-slave.network', '23-bond199.network', '25-bond-active-backup-slave.netdev', '12-dummy.netdev') start_networkd() @@ -5636,11 +5725,7 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): self.check_bridge_port_attr('bridge99', 'dummy98', 'bpdu_guard', '0') self.check_bridge_port_attr('bridge99', 'dummy98', 'root_block', '0') - def test_bridge_property(self): - copy_network_unit('11-dummy.netdev', '12-dummy.netdev', '26-bridge.netdev', - '26-bridge-slave-interface-1.network', '26-bridge-slave-interface-2.network', - '25-bridge99.network') - start_networkd() + def check_bridge_property(self): self.wait_online('dummy98:enslaved', 'test1:enslaved', 'bridge99:routable') output = check_output('ip -d link show bridge99') @@ -5683,6 +5768,24 @@ class NetworkdBridgeTests(unittest.TestCase, Utilities): print(output) self.check_bridge_port_attr('bridge99', 'test1', 'priority', '0') + def test_bridge_property(self): + copy_network_unit('11-dummy.netdev', '12-dummy.netdev', '26-bridge.netdev', + '26-bridge-slave-interface-1.network', '26-bridge-slave-interface-2.network', + '25-bridge99.network') + start_networkd() + self.check_bridge_property() + + # test reload + touch_network_unit( + '11-dummy.netdev', + '12-dummy.netdev', + '26-bridge.netdev', + '26-bridge-slave-interface-1.network', + '26-bridge-slave-interface-2.network', + '25-bridge99.network') + networkctl_reload() + self.check_bridge_property() + check_output('ip address add 192.168.0.16/24 dev bridge99') output = check_output('ip addr show bridge99') print(output) From f1ca3479ec366fe49bbe51b9c1a46500d6aa3d5f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 12 Nov 2024 19:43:57 +0900 Subject: [PATCH 10/10] networkd-test.py: show current status when wait-online failed For easier debugging on failure. --- test/networkd-test.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/test/networkd-test.py b/test/networkd-test.py index a698627454..929290eab7 100755 --- a/test/networkd-test.py +++ b/test/networkd-test.py @@ -193,6 +193,27 @@ class NetworkdTestingUtilities: class BridgeTest(NetworkdTestingUtilities, unittest.TestCase): """Provide common methods for testing networkd against servers.""" + def wait_online(self): + try: + subprocess.check_call([NETWORKD_WAIT_ONLINE, '--interface', 'port1', '--interface', 'port2', '--timeout=10']) + except (AssertionError, subprocess.CalledProcessError): + # show networkd status, journal, and DHCP server log on failure + print('---- interface status ----') + sys.stdout.flush() + subprocess.call(['ip', 'a', 'show', 'dev', 'mybridge']) + subprocess.call(['ip', 'a', 'show', 'dev', 'port1']) + subprocess.call(['ip', 'a', 'show', 'dev', 'port2']) + print('---- networkctl status ----') + sys.stdout.flush() + rc = subprocess.call(['networkctl', '-n', '0', 'status', 'mybridge', 'port1', 'port2']) + if rc != 0: + print(f"'networkctl status' exited with an unexpected code {rc}") + print('---- journal ----') + subprocess.check_output(['journalctl', '--sync']) + sys.stdout.flush() + subprocess.call(['journalctl', '-b', '--no-pager', '--quiet', '-I', '-u', 'systemd-networkd.service']) + raise + def setUp(self): self.write_network('50-port1.netdev', '''\ [NetDev] @@ -233,6 +254,7 @@ Gateway=192.168.250.1 ''') subprocess.call(['systemctl', 'reset-failed', 'systemd-networkd', 'systemd-resolved']) subprocess.check_call(['systemctl', 'start', 'systemd-networkd']) + self.wait_online() def tearDown(self): subprocess.check_call(['systemctl', 'stop', 'systemd-networkd.socket']) @@ -255,8 +277,7 @@ Priority=28 ''') subprocess.check_call(['ip', 'link', 'set', 'dev', 'port1', 'down']) subprocess.check_call(['systemctl', 'restart', 'systemd-networkd']) - subprocess.check_call([NETWORKD_WAIT_ONLINE, '--interface', - 'port1', '--timeout=5']) + self.wait_online() self.assertEqual(self.read_attr('port1', 'brport/priority'), '28') def test_bridge_port_priority_set_zero(self): @@ -268,8 +289,7 @@ Priority=0 ''') subprocess.check_call(['ip', 'link', 'set', 'dev', 'port2', 'down']) subprocess.check_call(['systemctl', 'restart', 'systemd-networkd']) - subprocess.check_call([NETWORKD_WAIT_ONLINE, '--interface', - 'port2', '--timeout=5']) + self.wait_online() self.assertEqual(self.read_attr('port2', 'brport/priority'), '0') def test_bridge_port_property(self): @@ -288,8 +308,7 @@ Priority=23 ''') subprocess.check_call(['ip', 'link', 'set', 'dev', 'port2', 'down']) subprocess.check_call(['systemctl', 'restart', 'systemd-networkd']) - subprocess.check_call([NETWORKD_WAIT_ONLINE, '--interface', - 'port2', '--timeout=5']) + self.wait_online() self.assertEqual(self.read_attr('port2', 'brport/priority'), '23') self.assertEqual(self.read_attr('port2', 'brport/hairpin_mode'), '1')