From 798e5dc8ae9051da827f45c1920a2a40562cd083 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Aug 2021 14:10:45 +0900 Subject: [PATCH 1/5] network: introduce a helper function netdev_is_stacked_and_independent() --- src/network/netdev/netdev.c | 80 +++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index c3ab6aecdb..ebd99a4c8d 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -167,6 +167,42 @@ bool netdev_is_managed(NetDev *netdev) { return hashmap_get(netdev->manager->netdevs, netdev->ifname) == netdev; } +static bool netdev_is_stacked_and_independent(NetDev *netdev) { + assert(netdev); + + if (!IN_SET(netdev_get_create_type(netdev), NETDEV_CREATE_STACKED, NETDEV_CREATE_AFTER_CONFIGURED)) + return false; + + switch (netdev->kind) { + case NETDEV_KIND_ERSPAN: + return ERSPAN(netdev)->independent; + case NETDEV_KIND_GRE: + return GRE(netdev)->independent; + case NETDEV_KIND_GRETAP: + return GRETAP(netdev)->independent; + case NETDEV_KIND_IP6GRE: + return IP6GRE(netdev)->independent; + case NETDEV_KIND_IP6GRETAP: + return IP6GRETAP(netdev)->independent; + case NETDEV_KIND_IP6TNL: + return IP6TNL(netdev)->independent; + case NETDEV_KIND_IPIP: + return IPIP(netdev)->independent; + case NETDEV_KIND_SIT: + return SIT(netdev)->independent; + case NETDEV_KIND_VTI: + return VTI(netdev)->independent; + case NETDEV_KIND_VTI6: + return VTI6(netdev)->independent; + case NETDEV_KIND_VXLAN: + return VXLAN(netdev)->independent; + case NETDEV_KIND_XFRM: + return XFRM(netdev)->independent; + default: + return false; + } +} + static void netdev_detach_from_manager(NetDev *netdev) { if (netdev->ifname && netdev->manager) hashmap_remove(netdev->manager->netdevs, netdev->ifname); @@ -671,7 +707,6 @@ int link_request_to_crate_stacked_netdev(Link *link, NetDev *netdev) { int netdev_load_one(Manager *manager, const char *filename) { _cleanup_(netdev_unrefp) NetDev *netdev_raw = NULL, *netdev = NULL; const char *dropin_dirname; - bool independent = false; int r; assert(manager); @@ -792,48 +827,7 @@ int netdev_load_one(Manager *manager, const char *filename) { return r; } - switch (netdev->kind) { - case NETDEV_KIND_IPIP: - independent = IPIP(netdev)->independent; - break; - case NETDEV_KIND_GRE: - independent = GRE(netdev)->independent; - break; - case NETDEV_KIND_GRETAP: - independent = GRETAP(netdev)->independent; - break; - case NETDEV_KIND_IP6GRE: - independent = IP6GRE(netdev)->independent; - break; - case NETDEV_KIND_IP6GRETAP: - independent = IP6GRETAP(netdev)->independent; - break; - case NETDEV_KIND_SIT: - independent = SIT(netdev)->independent; - break; - case NETDEV_KIND_VTI: - independent = VTI(netdev)->independent; - break; - case NETDEV_KIND_VTI6: - independent = VTI6(netdev)->independent; - break; - case NETDEV_KIND_IP6TNL: - independent = IP6TNL(netdev)->independent; - break; - case NETDEV_KIND_ERSPAN: - independent = ERSPAN(netdev)->independent; - break; - case NETDEV_KIND_XFRM: - independent = XFRM(netdev)->independent; - break; - case NETDEV_KIND_VXLAN: - independent = VXLAN(netdev)->independent; - break; - default: - break; - } - - if (independent) { + if (netdev_is_stacked_and_independent(netdev)) { r = netdev_create(netdev, NULL, NULL); if (r < 0) return r; From 8f65304c5d68ec10ab7cbe685e92261b84c73914 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Aug 2021 14:23:54 +0900 Subject: [PATCH 2/5] network: use netdev_enter_failed() instead of netdev_drop() on error Preparation for later commits to support reconfiguring netdevs. --- src/network/netdev/bareudp.c | 2 +- src/network/netdev/fou-tunnel.c | 2 +- src/network/netdev/geneve.c | 2 +- src/network/netdev/l2tp-tunnel.c | 2 +- src/network/netdev/macsec.c | 8 ++++---- src/network/netdev/netdev.c | 5 ++--- src/network/netdev/netdev.h | 1 + 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/network/netdev/bareudp.c b/src/network/netdev/bareudp.c index 22c0e49d94..e210625ac5 100644 --- a/src/network/netdev/bareudp.c +++ b/src/network/netdev/bareudp.c @@ -29,7 +29,7 @@ static int bare_udp_netdev_create_handler(sd_netlink *rtnl, sd_netlink_message * log_netdev_info(netdev, "BareUDP netdev exists, using existing without changing its parameters."); else if (r < 0) { log_netdev_warning_errno(netdev, r, "BareUDP netdev could not be created: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/fou-tunnel.c b/src/network/netdev/fou-tunnel.c index 0ed0df7579..e01de0f151 100644 --- a/src/network/netdev/fou-tunnel.c +++ b/src/network/netdev/fou-tunnel.c @@ -108,7 +108,7 @@ static int fou_tunnel_create_handler(sd_netlink *rtnl, sd_netlink_message *m, Ne 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"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/geneve.c b/src/network/netdev/geneve.c index fd0b5119e7..b0baa53edc 100644 --- a/src/network/netdev/geneve.c +++ b/src/network/netdev/geneve.c @@ -37,7 +37,7 @@ static int geneve_netdev_create_handler(sd_netlink *rtnl, sd_netlink_message *m, log_netdev_info(netdev, "Geneve netdev exists, using existing without changing its parameters"); else if (r < 0) { log_netdev_warning_errno(netdev, r, "Geneve netdev could not be created: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/l2tp-tunnel.c b/src/network/netdev/l2tp-tunnel.c index 32ec024558..f651b2f7bf 100644 --- a/src/network/netdev/l2tp-tunnel.c +++ b/src/network/netdev/l2tp-tunnel.c @@ -355,7 +355,7 @@ static int l2tp_create_tunnel_handler(sd_netlink *rtnl, sd_netlink_message *m, N 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"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index 77c5f8c4e7..74e9fdd10b 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -317,7 +317,7 @@ static int macsec_receive_association_handler(sd_netlink *rtnl, sd_netlink_messa else if (r < 0) { log_netdev_warning_errno(netdev, r, "Failed to add receive secure association: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } @@ -375,7 +375,7 @@ static int macsec_receive_channel_handler(sd_netlink *rtnl, sd_netlink_message * else if (r < 0) { log_netdev_warning_errno(netdev, r, "Failed to add receive secure channel: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } @@ -387,7 +387,7 @@ static int macsec_receive_channel_handler(sd_netlink *rtnl, sd_netlink_message * if (r < 0) { log_netdev_warning_errno(netdev, r, "Failed to configure receive security association: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } } @@ -441,7 +441,7 @@ static int macsec_transmit_association_handler(sd_netlink *rtnl, sd_netlink_mess else if (r < 0) { log_netdev_warning_errno(netdev, r, "Failed to add transmit secure association: %m"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index ebd99a4c8d..2a97dbea6e 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -268,9 +268,8 @@ int netdev_get(Manager *manager, const char *name, NetDev **ret) { return 0; } -static int netdev_enter_failed(NetDev *netdev) { +void netdev_enter_failed(NetDev *netdev) { netdev->state = NETDEV_STATE_FAILED; - return 0; } static int netdev_enter_ready(NetDev *netdev) { @@ -302,7 +301,7 @@ static int netdev_create_handler(sd_netlink *rtnl, sd_netlink_message *m, NetDev 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"); - netdev_drop(netdev); + netdev_enter_failed(netdev); return 1; } diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 493ae32b22..29eec8d553 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -184,6 +184,7 @@ extern const NetDevVTable * const netdev_vtable[_NETDEV_KIND_MAX]; int netdev_load(Manager *manager, bool reload); int netdev_load_one(Manager *manager, const char *filename); void netdev_drop(NetDev *netdev); +void netdev_enter_failed(NetDev *netdev); NetDev *netdev_unref(NetDev *netdev); NetDev *netdev_ref(NetDev *netdev); From b14686ff3a68d5e14c806a2a600e155ab07c37bf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Aug 2021 14:38:34 +0900 Subject: [PATCH 3/5] network: adjust log messages, function names, etc. --- src/network/netdev/netdev.c | 14 +++++++------- src/network/netdev/netdev.h | 4 ++-- src/network/networkd-link.c | 2 +- src/network/networkd-queue.c | 13 ++++++------- src/network/networkd-queue.h | 2 +- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 2a97dbea6e..9b4291be0a 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -595,12 +595,12 @@ static bool netdev_is_ready_to_create(NetDev *netdev, Link *link) { return true; } -int request_process_create_stacked_netdev(Request *req) { +int request_process_stacked_netdev(Request *req) { int r; assert(req); assert(req->link); - assert(req->type == REQUEST_TYPE_CREATE_STACKED_NETDEV); + assert(req->type == REQUEST_TYPE_STACKED_NETDEV); assert(req->netdev); assert(req->netlink_handler); @@ -667,7 +667,7 @@ static int link_create_stacked_netdev_after_configured_handler(sd_netlink *rtnl, return 0; } -int link_request_to_crate_stacked_netdev(Link *link, NetDev *netdev) { +int link_request_stacked_netdev(Link *link, NetDev *netdev) { NetDevCreateType create_type; int r; @@ -684,22 +684,22 @@ int link_request_to_crate_stacked_netdev(Link *link, NetDev *netdev) { if (create_type == NETDEV_CREATE_STACKED) { link->stacked_netdevs_created = false; - r = link_queue_request(link, REQUEST_TYPE_CREATE_STACKED_NETDEV, netdev, false, + r = link_queue_request(link, REQUEST_TYPE_STACKED_NETDEV, netdev, false, &link->create_stacked_netdev_messages, link_create_stacked_netdev_handler, NULL); } else { link->stacked_netdevs_after_configured_created = false; - r = link_queue_request(link, REQUEST_TYPE_CREATE_STACKED_NETDEV, netdev, false, + r = link_queue_request(link, REQUEST_TYPE_STACKED_NETDEV, netdev, false, &link->create_stacked_netdev_after_configured_messages, link_create_stacked_netdev_after_configured_handler, NULL); } if (r < 0) - return log_link_error_errno(link, r, "Failed to request to create stacked netdev '%s': %m", + return log_link_error_errno(link, r, "Failed to request stacked netdev '%s': %m", netdev->ifname); - log_link_debug(link, "Requested to create stacked netdev '%s'", netdev->ifname); + log_link_debug(link, "Requested stacked netdev '%s'", netdev->ifname); return 0; } diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 29eec8d553..4614f6566e 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -197,8 +197,8 @@ int netdev_set_ifindex(NetDev *netdev, sd_netlink_message *newlink); int netdev_get_mac(const char *ifname, struct ether_addr **ret); int netdev_join(NetDev *netdev, Link *link, link_netlink_message_handler_t cb); -int request_process_create_stacked_netdev(Request *req); -int link_request_to_crate_stacked_netdev(Link *link, NetDev *netdev); +int request_process_stacked_netdev(Request *req); +int link_request_stacked_netdev(Link *link, NetDev *netdev); const char *netdev_kind_to_string(NetDevKind d) _const_; NetDevKind netdev_kind_from_string(const char *d) _pure_; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 6855189bcc..59313bcdfd 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -594,7 +594,7 @@ static int link_request_stacked_netdevs(Link *link) { link->stacked_netdevs_after_configured_created = false; HASHMAP_FOREACH(netdev, link->network->stacked_netdevs) { - r = link_request_to_crate_stacked_netdev(link, netdev); + r = link_request_stacked_netdev(link, netdev); if (r < 0) return r; } diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 7f2481997d..dcf5f4f052 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -32,8 +32,6 @@ static void request_free_object(RequestType type, void *object) { case REQUEST_TYPE_BRIDGE_MDB: bridge_mdb_free(object); break; - case REQUEST_TYPE_CREATE_STACKED_NETDEV: - break; case REQUEST_TYPE_DHCP_SERVER: case REQUEST_TYPE_DHCP4_CLIENT: case REQUEST_TYPE_DHCP6_CLIENT: @@ -56,6 +54,7 @@ static void request_free_object(RequestType type, void *object) { routing_policy_rule_free(object); break; case REQUEST_TYPE_SET_LINK: + case REQUEST_TYPE_STACKED_NETDEV: case REQUEST_TYPE_UP_DOWN: break; default: @@ -110,7 +109,7 @@ static void request_hash_func(const Request *req, struct siphash *state) { case REQUEST_TYPE_ADDRESS_LABEL: case REQUEST_TYPE_BRIDGE_FDB: case REQUEST_TYPE_BRIDGE_MDB: - case REQUEST_TYPE_CREATE_STACKED_NETDEV: + case REQUEST_TYPE_STACKED_NETDEV: /* TODO: Currently, these types do not have any specific hash and compare functions. * Fortunately, all these objects are 'static', thus we can use the trivial functions. */ trivial_hash_func(req->object, state); @@ -173,7 +172,7 @@ static int request_compare_func(const struct Request *a, const struct Request *b case REQUEST_TYPE_ADDRESS_LABEL: case REQUEST_TYPE_BRIDGE_FDB: case REQUEST_TYPE_BRIDGE_MDB: - case REQUEST_TYPE_CREATE_STACKED_NETDEV: + case REQUEST_TYPE_STACKED_NETDEV: return trivial_compare_func(a->object, b->object); case REQUEST_TYPE_DHCP_SERVER: case REQUEST_TYPE_DHCP4_CLIENT: @@ -306,9 +305,6 @@ int manager_process_requests(sd_event_source *s, void *userdata) { case REQUEST_TYPE_BRIDGE_MDB: r = request_process_bridge_mdb(req); break; - case REQUEST_TYPE_CREATE_STACKED_NETDEV: - r = request_process_create_stacked_netdev(req); - break; case REQUEST_TYPE_DHCP_SERVER: r = request_process_dhcp_server(req); break; @@ -339,6 +335,9 @@ int manager_process_requests(sd_event_source *s, void *userdata) { case REQUEST_TYPE_SET_LINK: r = request_process_set_link(req); break; + case REQUEST_TYPE_STACKED_NETDEV: + r = request_process_stacked_netdev(req); + break; case REQUEST_TYPE_UP_DOWN: r = request_process_link_up_or_down(req); break; diff --git a/src/network/networkd-queue.h b/src/network/networkd-queue.h index 1fdd07d60a..b1c0a9f0d4 100644 --- a/src/network/networkd-queue.h +++ b/src/network/networkd-queue.h @@ -26,7 +26,6 @@ typedef enum RequestType { REQUEST_TYPE_ADDRESS_LABEL, REQUEST_TYPE_BRIDGE_FDB, REQUEST_TYPE_BRIDGE_MDB, - REQUEST_TYPE_CREATE_STACKED_NETDEV, REQUEST_TYPE_DHCP_SERVER, REQUEST_TYPE_DHCP4_CLIENT, REQUEST_TYPE_DHCP6_CLIENT, @@ -37,6 +36,7 @@ typedef enum RequestType { REQUEST_TYPE_ROUTE, REQUEST_TYPE_ROUTING_POLICY_RULE, REQUEST_TYPE_SET_LINK, + REQUEST_TYPE_STACKED_NETDEV, REQUEST_TYPE_UP_DOWN, _REQUEST_TYPE_MAX, _REQUEST_TYPE_INVALID = -EINVAL, From 2f117922d40b0f947cc74a9849636dcf4c97d1f3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Aug 2021 14:31:27 +0900 Subject: [PATCH 4/5] network: recreate stacked netdevs when underlying device is re-added Closes #20430. --- src/network/netdev/netdev.c | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 9b4291be0a..426be8c7fc 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -203,6 +203,18 @@ static bool netdev_is_stacked_and_independent(NetDev *netdev) { } } +static bool netdev_is_stacked(NetDev *netdev) { + assert(netdev); + + if (!IN_SET(netdev_get_create_type(netdev), NETDEV_CREATE_STACKED, NETDEV_CREATE_AFTER_CONFIGURED)) + return false; + + if (netdev_is_stacked_and_independent(netdev)) + return false; + + return true; +} + static void netdev_detach_from_manager(NetDev *netdev) { if (netdev->ifname && netdev->manager) hashmap_remove(netdev->manager->netdevs, netdev->ifname); @@ -238,9 +250,19 @@ static NetDev *netdev_free(NetDev *netdev) { DEFINE_TRIVIAL_REF_UNREF_FUNC(NetDev, netdev, netdev_free); void netdev_drop(NetDev *netdev) { - if (!netdev || netdev->state == NETDEV_STATE_LINGER) + if (!netdev) return; + if (netdev_is_stacked(netdev)) { + /* The netdev may be removed due to the underlying device removal, and the device may + * be re-added later. */ + netdev->state = NETDEV_STATE_LOADING; + netdev->ifindex = 0; + + log_netdev_debug(netdev, "netdev removed"); + return; + } + netdev->state = NETDEV_STATE_LINGER; log_netdev_debug(netdev, "netdev removed"); @@ -668,21 +690,18 @@ static int link_create_stacked_netdev_after_configured_handler(sd_netlink *rtnl, } int link_request_stacked_netdev(Link *link, NetDev *netdev) { - NetDevCreateType create_type; int r; assert(link); assert(netdev); - create_type = netdev_get_create_type(netdev); - if (!IN_SET(create_type, NETDEV_CREATE_STACKED, NETDEV_CREATE_AFTER_CONFIGURED)) + if (!netdev_is_stacked(netdev)) return -EINVAL; - if (netdev->state != NETDEV_STATE_LOADING || netdev->ifindex > 0) - /* Already created (or removed?) */ - return 0; + if (!IN_SET(netdev->state, NETDEV_STATE_LOADING, NETDEV_STATE_FAILED) || netdev->ifindex > 0) + return 0; /* Already created. */ - if (create_type == NETDEV_CREATE_STACKED) { + if (netdev_get_create_type(netdev) == NETDEV_CREATE_STACKED) { link->stacked_netdevs_created = false; r = link_queue_request(link, REQUEST_TYPE_STACKED_NETDEV, netdev, false, &link->create_stacked_netdev_messages, From 1d0c9bd7536803568389799c57cf28f43c713bce Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 12 Aug 2021 14:45:40 +0900 Subject: [PATCH 5/5] test-network: add a testcase for recreating stacked netdevs --- test/test-network/systemd-networkd-tests.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index cccaddbc9b..dffd728881 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -1183,6 +1183,25 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertRegex(output, ' mtu 2000 ') self.assertRegex(output, 'macvlan mode ' + mode + ' ') + rc = call("ip link del test1") + self.assertEqual(rc, 0) + time.sleep(1) + + rc = call("ip link add test1 type dummy") + self.assertEqual(rc, 0) + time.sleep(1) + + self.wait_online(['macvlan99:degraded', 'test1:degraded']) + + output = check_output('ip -d link show test1') + print(output) + self.assertRegex(output, ' mtu 2000 ') + + output = check_output('ip -d link show macvlan99') + print(output) + self.assertRegex(output, ' mtu 2000 ') + self.assertRegex(output, 'macvlan mode ' + mode + ' ') + @expectedFailureIfModuleIsNotAvailable('ipvlan') def test_ipvlan(self): for mode, flag in [['L2', 'private'], ['L3', 'vepa'], ['L3S', 'bridge']]: