From 00b363bb8130180750867f45223523af610ca11c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 00:59:31 +0900 Subject: [PATCH 01/10] network: drop outdated comment All Route objects are managed by Manager since 8d01e44c1f0e00b414d36bd1b46ecff548242208. --- src/network/networkd-manager.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 9290255ad0..96cedbec38 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -705,10 +705,6 @@ Manager* manager_free(Manager *m) { sd_netlink_unref(m->genl); sd_resolve_unref(m->resolve); - /* reject (e.g. unreachable) type routes are managed by Manager, but may be referenced by a - * link. E.g., DHCP6 with prefix delegation creates unreachable routes, and they are referenced - * by the upstream link. And the links may be referenced by netlink slots. Hence, two - * set_free() must be called after the above sd_netlink_unref(). */ m->routes = set_free(m->routes); m->nexthops_by_id = hashmap_free(m->nexthops_by_id); From b5f2d7a1eccf5305aa2f9f4b9e7a71a14c804334 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 01:15:31 +0900 Subject: [PATCH 02/10] network/nexthop: do not share NextHop.nexthops and NextHop.routes with duplicated object Otherwise, these may be freed twice. But, fortunately, when this function is called, both are NULL. So, this should not change any behavior. But for safety. --- src/network/networkd-nexthop.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index c639b01c22..eb557e580e 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -261,6 +261,8 @@ static int nexthop_dup(const NextHop *src, NextHop **ret) { dest->network = NULL; dest->section = NULL; dest->group = NULL; + dest->nexthops = NULL; + dest->routes = NULL; HASHMAP_FOREACH(nhg, src->group) { _cleanup_free_ struct nexthop_grp *g = NULL; From 1168489cd405249cb46907081a94548f401ea3f0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 01:21:28 +0900 Subject: [PATCH 03/10] network/ndisc: constify several arguments and add several assertions Follow-up for 0f8afaf94dd29126981219b3ea2b3bc315cc2dd0. No functional change, just for safety. --- src/network/networkd-ndisc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 33e86fb04e..e778300c70 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -215,7 +215,7 @@ static int ndisc_remove_unused_nexthops(Link *link) { #define NDISC_NEXTHOP_APP_ID SD_ID128_MAKE(76,d2,0f,1f,76,1e,44,d1,97,3a,52,5c,05,68,b5,0d) -static uint32_t ndisc_generate_nexthop_id(NextHop *nexthop, Link *link, sd_id128_t app_id, uint64_t trial) { +static uint32_t ndisc_generate_nexthop_id(const NextHop *nexthop, Link *link, sd_id128_t app_id, uint64_t trial) { assert(nexthop); assert(link); @@ -232,7 +232,7 @@ static uint32_t ndisc_generate_nexthop_id(NextHop *nexthop, Link *link, sd_id128 return (uint32_t) ((result & 0xffffffff) ^ (result >> 32)); } -static bool ndisc_nexthop_equal(NextHop *a, NextHop *b) { +static bool ndisc_nexthop_equal(const NextHop *a, const NextHop *b) { assert(a); assert(b); @@ -250,9 +250,11 @@ static bool ndisc_nexthop_equal(NextHop *a, NextHop *b) { return true; } -static bool ndisc_take_nexthop_id(NextHop *nexthop, NextHop *existing, Manager *manager) { +static bool ndisc_take_nexthop_id(NextHop *nexthop, const NextHop *existing, Manager *manager) { assert(nexthop); + assert(nexthop->id == 0); assert(existing); + assert(existing->id > 0); assert(manager); if (!ndisc_nexthop_equal(nexthop, existing)) @@ -300,7 +302,7 @@ static int ndisc_nexthop_find_id(NextHop *nexthop, Link *link) { return false; } -static int ndisc_nexthop_new(Route *route, Link *link, NextHop **ret) { +static int ndisc_nexthop_new(const Route *route, Link *link, NextHop **ret) { _cleanup_(nexthop_unrefp) NextHop *nexthop = NULL; int r; From 96fef18ca683e60a1a31b2c1ab88bbecea60996d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 01:57:44 +0900 Subject: [PATCH 04/10] network/ndisc: unref Route objects that depend on the nexthop No functional change, as when this function is called, the set will be freed and contained Route objects will be unref()ed anyway soon later by nexthop_detach() -> nexthop_free(). Even though, when the routes are forgotten from the Manager, then it is not necessary to keep them by the nexthop. Let's unref earlier. --- src/network/networkd-nexthop.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index eb557e580e..8e101ad779 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -493,8 +493,11 @@ static void nexthop_forget_dependents(NextHop *nexthop, Manager *manager) { /* If a nexthop is removed, the kernel silently removes routes that depend on the removed nexthop. * Let's forget them. */ - Route *route; - SET_FOREACH(route, nexthop->routes) { + for (;;) { + _cleanup_(route_unrefp) Route *route = set_steal_first(nexthop->routes); + if (!route) + break; + Request *req; if (route_get_request(manager, route, &req) >= 0) route_enter_removed(req->userdata); @@ -503,6 +506,8 @@ static void nexthop_forget_dependents(NextHop *nexthop, Manager *manager) { log_route_debug(route, "Forgetting silently removed", manager); route_detach(route); } + + nexthop->routes = set_free(nexthop->routes); } static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveRequest *rreq) { From 724a296b4f62666f631040f311ed9874e35acb12 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 02:09:08 +0900 Subject: [PATCH 05/10] network/nexthop: drop outdated comment and add one debugging log All NextHop objects are managed by Manager since 352eba2e49453a1b784ffbdb9509ba3f8a945b59. --- src/network/networkd-nexthop.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 8e101ad779..ab4bafba50 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -1199,10 +1199,12 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, else nexthop->ifindex = (int) ifindex; - /* All blackhole or group nexthops are managed by Manager. Note that the linux kernel does not - * set NHA_OID attribute when NHA_BLACKHOLE or NHA_GROUP is set. Just for safety. */ - if (!nexthop_bound_to_link(nexthop)) + /* The linux kernel does not set NHA_OID attribute when NHA_BLACKHOLE or NHA_GROUP is set. + * But let's check that for safety. */ + if (!nexthop_bound_to_link(nexthop) && nexthop->ifindex != 0) { + log_debug("rtnl: received blackhole or group nexthop with NHA_OIF attribute, ignoring the attribute."); nexthop->ifindex = 0; + } nexthop_enter_configured(nexthop); if (req) From 290a507f7c56d11ddbe6138ae15246b2debf48dc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 29 Nov 2024 04:14:56 +0900 Subject: [PATCH 06/10] network/nexthop: ignore foreign nexthops when ManageForeignNextHops=no --- src/network/networkd-nexthop.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index ab4bafba50..e27ecf05f9 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -1144,6 +1144,11 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, /* If we did not know the nexthop, then save it. */ if (!nexthop) { + if (!req && !m->manage_foreign_nexthops) { + log_nexthop_debug(&(const NextHop) { .id = id }, "Ignoring received", m); + return 0; + } + r = nexthop_add_new(m, id, &nexthop); if (r < 0) { log_warning_errno(r, "Failed to add received nexthop, ignoring: %m"); From a4feabd85d4d136d68ee9c8438eeac86bfd174f6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Nov 2024 02:35:29 +0900 Subject: [PATCH 07/10] network: introduce address_forget() and friends and use it where applicable No functional change, just refactoring. --- src/network/networkd-address.c | 69 ++++++++++---------- src/network/networkd-neighbor.c | 61 +++++++++-------- src/network/networkd-nexthop.c | 76 ++++++++++------------ src/network/networkd-route.c | 58 ++++++++--------- src/network/networkd-routing-policy-rule.c | 52 ++++++++------- 5 files changed, 158 insertions(+), 158 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index 0b7fbec28b..bc5bfb8e83 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -1129,6 +1129,23 @@ void log_address_debug(const Address *address, const char *str, const Link *link address->family == AF_INET ? strna(address->label) : ""); } +static void address_forget(Link *link, Address *address, bool removed_by_us, const char *msg) { + assert(link); + assert(address); + assert(msg); + + Request *req; + if (address_get_request(link, address, &req) >= 0) + address_enter_removed(req->userdata); + + if (!address->link && address_get(link, address, &address) < 0) + return; + + address_enter_removed(address); + log_address_debug(address, msg, link); + (void) address_drop(address, removed_by_us); +} + static int address_set_netlink_message(const Address *address, sd_netlink_message *m, Link *link) { uint32_t flags; int r; @@ -1181,16 +1198,8 @@ static int address_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Remov (r == -EADDRNOTAVAIL || !address->link) ? LOG_DEBUG : LOG_WARNING, r, "Could not drop address"); - if (address->link) { - /* If the address cannot be removed, then assume the address is already removed. */ - log_address_debug(address, "Forgetting", link); - - Request *req; - if (address_get_request(link, address, &req) >= 0) - address_enter_removed(req->userdata); - - (void) address_drop(address, /* removed_by_us = */ true); - } + /* If the address cannot be removed, then assume the address is already removed. */ + address_forget(link, address, /* removed_by_us = */ true, "Forgetting"); } return 1; @@ -1775,14 +1784,7 @@ int link_request_static_addresses(Link *link) { } int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(address_unrefp) Address *tmp = NULL; - struct ifa_cacheinfo cinfo; - Link *link; - uint16_t type; - Address *address = NULL; - Request *req = NULL; - bool is_new = false, update_dhcp4; - int ifindex, r; + int r; assert(rtnl); assert(message); @@ -1796,6 +1798,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + uint16_t type; r = sd_netlink_message_get_type(message, &type); if (r < 0) { log_warning_errno(r, "rtnl: could not get message type, ignoring: %m"); @@ -1805,6 +1808,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + int ifindex; r = sd_rtnl_message_addr_get_ifindex(message, &ifindex); if (r < 0) { log_warning_errno(r, "rtnl: could not get ifindex from message, ignoring: %m"); @@ -1814,6 +1818,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + Link *link; r = link_get_by_index(m, ifindex, &link); if (r < 0) { /* when enumerating we might be out of sync, but we will get the address again, so just @@ -1823,6 +1828,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + _cleanup_(address_unrefp) Address *tmp = NULL; r = address_new(&tmp); if (r < 0) return log_oom(); @@ -1890,28 +1896,22 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, assert_not_reached(); } - update_dhcp4 = tmp->family == AF_INET6; - - /* Then, find the managed Address and Request objects corresponding to the received address. */ + /* Then, find the managed Address object corresponding to the received address. */ + Address *address = NULL; (void) address_get(link, tmp, &address); - (void) address_get_request(link, tmp, &req); if (type == RTM_DELADDR) { - if (address) { - bool removed_by_us = FLAGS_SET(address->state, NETWORK_CONFIG_STATE_REMOVING); - - address_enter_removed(address); - log_address_debug(address, "Forgetting removed", link); - (void) address_drop(address, removed_by_us); - } else + if (address) + address_forget(link, address, + /* removed_by_us = */ FLAGS_SET(address->state, NETWORK_CONFIG_STATE_REMOVING), + "Forgetting removed"); + else log_address_debug(tmp, "Kernel removed unknown", link); - if (req) - address_enter_removed(req->userdata); - goto finalize; } + bool is_new = false; if (!address) { /* If we did not know the address, then save it. */ r = address_attach(link, tmp); @@ -1931,6 +1931,8 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, } /* Also update information that cannot be obtained through netlink notification. */ + Request *req = NULL; + (void) address_get_request(link, tmp, &req); if (req && req->waiting_reply) { Address *a = ASSERT_PTR(req->userdata); @@ -1978,6 +1980,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, } else if (r < 0) log_link_debug_errno(link, r, "rtnl: failed to read IFA_FLAGS attribute, ignoring: %m"); + struct ifa_cacheinfo cinfo; r = sd_netlink_message_read_cache_info(message, IFA_CACHEINFO, &cinfo); if (r >= 0) address_set_lifetime(m, address, &cinfo); @@ -2000,7 +2003,7 @@ int manager_rtnl_process_address(sd_netlink *rtnl, sd_netlink_message *message, link_enter_failed(link); finalize: - if (update_dhcp4) { + if (tmp->family == AF_INET6) { r = dhcp4_update_ipv6_connectivity(link); if (r < 0) { log_link_warning_errno(link, r, "Failed to notify IPv6 connectivity to DHCPv4 client: %m"); diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 83275f2fc8..c953f89daa 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -247,6 +247,23 @@ static void log_neighbor_debug(const Neighbor *neighbor, const char *str, const IN_ADDR_TO_STRING(neighbor->dst_addr.family, &neighbor->dst_addr.address)); } +static void neighbor_forget(Link *link, Neighbor *neighbor, const char *msg) { + assert(link); + assert(neighbor); + assert(msg); + + Request *req; + if (neighbor_get_request(link, neighbor, &req) >= 0) + neighbor_enter_removed(req->userdata); + + if (!neighbor->link && neighbor_get(link, neighbor, &neighbor) < 0) + return; + + neighbor_enter_removed(neighbor); + log_neighbor_debug(neighbor, "Forgetting", link); + neighbor_detach(neighbor); +} + static int neighbor_configure(Neighbor *neighbor, Link *link, Request *req) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; int r; @@ -421,16 +438,8 @@ static int neighbor_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Remo (r == -ESRCH || !neighbor->link) ? LOG_DEBUG : LOG_WARNING, r, "Could not remove neighbor"); - if (neighbor->link) { - /* If the neighbor cannot be removed, then assume the neighbor is already removed. */ - log_neighbor_debug(neighbor, "Forgetting", link); - - Request *req; - if (neighbor_get_request(link, neighbor, &req) >= 0) - neighbor_enter_removed(req->userdata); - - neighbor_detach(neighbor); - } + /* If the neighbor cannot be removed, then assume the neighbor is already removed. */ + neighbor_forget(link, neighbor, "Forgetting"); } return 1; @@ -529,13 +538,7 @@ int link_drop_static_neighbors(Link *link) { } int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(neighbor_unrefp) Neighbor *tmp = NULL; - Neighbor *neighbor = NULL; - Request *req = NULL; - uint16_t type, state; - bool is_new = false; - int ifindex, r; - Link *link; + int r; assert(rtnl); assert(message); @@ -549,6 +552,7 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + uint16_t type; r = sd_netlink_message_get_type(message, &type); if (r < 0) { log_warning_errno(r, "rtnl: could not get message type, ignoring: %m"); @@ -558,6 +562,7 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + uint16_t state; r = sd_rtnl_message_neigh_get_state(message, &state); if (r < 0) { log_warning_errno(r, "rtnl: received neighbor message with invalid state, ignoring: %m"); @@ -566,6 +571,7 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, /* Currently, we are interested in only static neighbors. */ return 0; + int ifindex; r = sd_rtnl_message_neigh_get_ifindex(message, &ifindex); if (r < 0) { log_warning_errno(r, "rtnl: could not get ifindex from message, ignoring: %m"); @@ -575,12 +581,14 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + Link *link; r = link_get_by_index(m, ifindex, &link); if (r < 0) /* when enumerating we might be out of sync, but we will get the neighbor again. Also, * kernel sends messages about neighbors after a link is removed. So, just ignore it. */ return 0; + _cleanup_(neighbor_unrefp) Neighbor *tmp = NULL; r = neighbor_new(&tmp); if (r < 0) return log_oom(); @@ -604,25 +612,20 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - /* Then, find the managed Neighbor and Request objects corresponding to the netlink notification. */ + /* Then, find the managed Neighbor object corresponding to the netlink notification. */ + Neighbor *neighbor = NULL; (void) neighbor_get(link, tmp, &neighbor); - (void) neighbor_get_request(link, tmp, &req); if (type == RTM_DELNEIGH) { - if (neighbor) { - neighbor_enter_removed(neighbor); - log_neighbor_debug(neighbor, "Forgetting removed", link); - neighbor_detach(neighbor); - } else + if (neighbor) + neighbor_forget(link, neighbor, "Forgetting removed"); + else log_neighbor_debug(tmp, "Kernel removed unknown", link); - - if (req) - neighbor_enter_removed(req->userdata); - return 0; } /* If we did not know the neighbor, then save it. */ + bool is_new = false; if (!neighbor) { r = neighbor_attach(link, tmp); if (r < 0) { @@ -634,6 +637,8 @@ int manager_rtnl_process_neighbor(sd_netlink *rtnl, sd_netlink_message *message, } /* Also update information that cannot be obtained through netlink notification. */ + Request *req = NULL; + (void) neighbor_get_request(link, tmp, &req); if (req && req->waiting_reply) { Neighbor *n = ASSERT_PTR(req->userdata); diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index e27ecf05f9..7aa7bd6f6d 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -510,6 +510,24 @@ static void nexthop_forget_dependents(NextHop *nexthop, Manager *manager) { nexthop->routes = set_free(nexthop->routes); } +static void nexthop_forget(Manager *manager, NextHop *nexthop, const char *msg) { + assert(manager); + assert(nexthop); + assert(msg); + + Request *req; + if (nexthop_get_request_by_id(manager, nexthop->id, &req) >= 0) + nexthop_enter_removed(req->userdata); + + if (!nexthop->manager && nexthop_get_by_id(manager, nexthop->id, &nexthop) < 0) + return; + + nexthop_enter_removed(nexthop); + log_nexthop_debug(nexthop, msg, manager); + nexthop_forget_dependents(nexthop, nexthop->manager); + nexthop_detach(nexthop); +} + static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveRequest *rreq) { int r; @@ -525,18 +543,8 @@ static int nexthop_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Remov (r == -ENOENT || !nexthop->manager) ? LOG_DEBUG : LOG_WARNING, r, "Could not drop nexthop, ignoring"); - nexthop_forget_dependents(nexthop, manager); - - if (nexthop->manager) { - /* If the nexthop cannot be removed, then assume the nexthop is already removed. */ - log_nexthop_debug(nexthop, "Forgetting", manager); - - Request *req; - if (nexthop_get_request_by_id(manager, nexthop->id, &req) >= 0) - nexthop_enter_removed(req->userdata); - - nexthop_detach(nexthop); - } + /* If the nexthop cannot be removed, then assume the nexthop is already removed. */ + nexthop_forget(manager, nexthop, "Forgetting"); } return 1; @@ -969,20 +977,6 @@ int link_drop_nexthops(Link *link, bool only_static) { return r; } -static void nexthop_forget_one(NextHop *nexthop) { - assert(nexthop); - assert(nexthop->manager); - - Request *req; - if (nexthop_get_request_by_id(nexthop->manager, nexthop->id, &req) >= 0) - nexthop_enter_removed(req->userdata); - - nexthop_enter_removed(nexthop); - log_nexthop_debug(nexthop, "Forgetting silently removed", nexthop->manager); - nexthop_forget_dependents(nexthop, nexthop->manager); - nexthop_detach(nexthop); -} - void link_forget_nexthops(Link *link) { assert(link); assert(link->manager); @@ -999,7 +993,7 @@ void link_forget_nexthops(Link *link) { if (nexthop->family != AF_INET) continue; - nexthop_forget_one(nexthop); + nexthop_forget(link->manager, nexthop, "Forgetting silently removed"); } /* Remove all group nexthops their all members are removed in the above. */ @@ -1020,7 +1014,7 @@ void link_forget_nexthops(Link *link) { if (!hashmap_isempty(nexthop->group)) continue; /* At least one group member still exists. */ - nexthop_forget_one(nexthop); + nexthop_forget(link->manager, nexthop, "Forgetting silently removed"); } } @@ -1084,11 +1078,6 @@ static int nexthop_update_group(NextHop *nexthop, sd_netlink_message *message) { } int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - uint16_t type; - uint32_t id, ifindex; - NextHop *nexthop = NULL; - Request *req = NULL; - bool is_new = false; int r; assert(rtnl); @@ -1103,6 +1092,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + uint16_t type; r = sd_netlink_message_get_type(message, &type); if (r < 0) { log_warning_errno(r, "rtnl: could not get message type, ignoring: %m"); @@ -1112,6 +1102,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + uint32_t id; r = sd_netlink_message_read_u32(message, NHA_ID, &id); if (r == -ENODATA) { log_warning_errno(r, "rtnl: received nexthop message without NHA_ID attribute, ignoring: %m"); @@ -1124,25 +1115,23 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } + NextHop *nexthop = NULL; (void) nexthop_get_by_id(m, id, &nexthop); - (void) nexthop_get_request_by_id(m, id, &req); if (type == RTM_DELNEXTHOP) { - if (nexthop) { - nexthop_enter_removed(nexthop); - log_nexthop_debug(nexthop, "Forgetting removed", m); - nexthop_forget_dependents(nexthop, m); - nexthop_detach(nexthop); - } else + if (nexthop) + nexthop_forget(m, nexthop, "Forgetting removed"); + else log_nexthop_debug(&(const NextHop) { .id = id }, "Kernel removed unknown", m); - if (req) - nexthop_enter_removed(req->userdata); - return 0; } + Request *req = NULL; + (void) nexthop_get_request_by_id(m, id, &req); + /* If we did not know the nexthop, then save it. */ + bool is_new = false; if (!nexthop) { if (!req && !m->manage_foreign_nexthops) { log_nexthop_debug(&(const NextHop) { .id = id }, "Ignoring received", m); @@ -1194,6 +1183,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, else nexthop->blackhole = r; + uint32_t ifindex; r = sd_netlink_message_read_u32(message, NHA_OIF, &ifindex); if (r == -ENODATA) nexthop->ifindex = 0; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 0f3f79ec4f..f6386e0426 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -460,6 +460,23 @@ void log_route_debug(const Route *route, const char *str, Manager *manager) { strna(proto), strna(scope), strna(route_type_to_string(route->type)), strna(flags)); } +static void route_forget(Manager *manager, Route *route, const char *msg) { + assert(manager); + assert(route); + assert(msg); + + Request *req; + if (route_get_request(manager, route, &req) >= 0) + route_enter_removed(req->userdata); + + if (!route->manager && route_get(manager, route, &route) < 0) + return; + + route_enter_removed(route); + log_route_debug(route, msg, manager); + route_detach(route); +} + static int route_set_netlink_message(const Route *route, sd_netlink_message *m) { int r; @@ -564,16 +581,8 @@ static int route_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, RemoveR LOG_DEBUG : LOG_WARNING, r, "Could not drop route, ignoring"); - if (route->manager) { - /* If the route cannot be removed, then assume the route is already removed. */ - log_route_debug(route, "Forgetting", manager); - - Request *req; - if (route_get_request(manager, route, &req) >= 0) - route_enter_removed(req->userdata); - - route_detach(route); - } + /* If the route cannot be removed, then assume the route is already removed. */ + route_forget(manager, route, "Forgetting"); } return 1; @@ -1088,7 +1097,6 @@ static int process_route_one( Route *tmp, const struct rta_cacheinfo *cacheinfo) { - Request *req = NULL; Route *route = NULL; Link *link = NULL; bool is_new = false, update_dhcp4; @@ -1099,13 +1107,15 @@ static int process_route_one( assert(IN_SET(type, RTM_NEWROUTE, RTM_DELROUTE)); (void) route_get(manager, tmp, &route); - (void) route_get_request(manager, tmp, &req); (void) route_get_link(manager, tmp, &link); update_dhcp4 = link && tmp->family == AF_INET6 && tmp->dst_prefixlen == 0; switch (type) { - case RTM_NEWROUTE: + case RTM_NEWROUTE: { + Request *req = NULL; + (void) route_get_request(manager, tmp, &req); + if (!route) { if (!manager->manage_foreign_routes && !(req && req->waiting_reply)) { route_enter_configured(tmp); @@ -1159,20 +1169,14 @@ static int process_route_one( (void) route_setup_timer(route, cacheinfo); break; - + } case RTM_DELROUTE: - if (route) { - route_enter_removed(route); - log_route_debug(route, "Forgetting removed", manager); - route_detach(route); - } else + if (route) + route_forget(manager, route, "Forgetting removed"); + else log_route_debug(tmp, manager->manage_foreign_routes ? "Kernel removed unknown" : "Ignoring received", manager); - - if (req) - route_enter_removed(req->userdata); - break; default: @@ -1574,13 +1578,7 @@ void link_forget_routes(Link *link) { if (!IN_SET(route->type, RTN_UNICAST, RTN_BROADCAST, RTN_ANYCAST, RTN_MULTICAST)) continue; - Request *req; - if (route_get_request(link->manager, route, &req) >= 0) - route_enter_removed(req->userdata); - - route_enter_removed(route); - log_route_debug(route, "Forgetting silently removed", link->manager); - route_detach(route); + route_forget(link->manager, route, "Forgetting silently removed"); } } diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 1a04af6359..87dcaa4fc4 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -550,6 +550,23 @@ static void log_routing_policy_rule_debug(const RoutingPolicyRule *rule, const c strna(rule->iif), strna(rule->oif), strna(table)); } +static void routing_policy_rule_forget(Manager *manager, RoutingPolicyRule *rule, const char *msg) { + assert(manager); + assert(rule); + assert(msg); + + Request *req; + if (routing_policy_rule_get_request(manager, rule, rule->family, &req) >= 0) + routing_policy_rule_enter_removed(req->userdata); + + if (!rule->manager && routing_policy_rule_get(manager, rule, rule->family, &rule) < 0) + return; + + routing_policy_rule_enter_removed(rule); + log_routing_policy_rule_debug(rule, "Forgetting", NULL, manager); + routing_policy_rule_detach(rule); +} + static int routing_policy_rule_set_netlink_message(const RoutingPolicyRule *rule, sd_netlink_message *m) { int r; @@ -708,16 +725,8 @@ static int routing_policy_rule_remove_handler(sd_netlink *rtnl, sd_netlink_messa (r == -ENOENT || !rule->manager) ? LOG_DEBUG : LOG_WARNING, r, "Could not drop routing policy rule, ignoring"); - if (rule->manager) { - /* If the rule cannot be removed, then assume the rule is already removed. */ - log_routing_policy_rule_debug(rule, "Forgetting", NULL, manager); - - Request *req; - if (routing_policy_rule_get_request(manager, rule, rule->family, &req) >= 0) - routing_policy_rule_enter_removed(req->userdata); - - routing_policy_rule_detach(rule); - } + /* If the rule cannot be removed, then assume the rule is already removed. */ + routing_policy_rule_forget(manager, rule, "Forgetting"); } return 1; @@ -1046,10 +1055,6 @@ static bool routing_policy_rule_is_created_by_kernel(const RoutingPolicyRule *ru } int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Manager *m) { - _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; - RoutingPolicyRule *rule = NULL; - Request *req = NULL; - uint16_t type; int r; assert(rtnl); @@ -1063,6 +1068,7 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man return 0; } + uint16_t type; r = sd_netlink_message_get_type(message, &type); if (r < 0) { log_warning_errno(r, "rtnl: could not get message type, ignoring: %m"); @@ -1072,6 +1078,7 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man return 0; } + _cleanup_(routing_policy_rule_unrefp) RoutingPolicyRule *tmp = NULL; r = routing_policy_rule_new(&tmp); if (r < 0) { log_oom(); @@ -1240,23 +1247,20 @@ int manager_rtnl_process_rule(sd_netlink *rtnl, sd_netlink_message *message, Man return 0; } + RoutingPolicyRule *rule = NULL; (void) routing_policy_rule_get(m, tmp, tmp->family, &rule); - (void) routing_policy_rule_get_request(m, tmp, tmp->family, &req); if (type == RTM_DELRULE) { - if (rule) { - routing_policy_rule_enter_removed(rule); - log_routing_policy_rule_debug(rule, "Forgetting removed", NULL, m); - routing_policy_rule_detach(rule); - } else + if (rule) + routing_policy_rule_forget(m, rule, "Forgetting removed"); + else log_routing_policy_rule_debug(tmp, "Kernel removed unknown", NULL, m); - - if (req) - routing_policy_rule_enter_removed(req->userdata); - return 0; } + Request *req = NULL; + (void) routing_policy_rule_get_request(m, tmp, tmp->family, &req); + bool is_new = false; if (!rule) { if (!req && !m->manage_foreign_rules) { From d49312307e97bf5e0e62513b89578323e72f7332 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Nov 2024 09:02:15 +0900 Subject: [PATCH 08/10] network/nexthop: fix argument name Fixes copy-and-paste error in b5b42b516e791aae8b723866be94a7c3e6e99829. --- src/network/networkd-nexthop.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-nexthop.h b/src/network/networkd-nexthop.h index 1466e2702f..00120d04af 100644 --- a/src/network/networkd-nexthop.h +++ b/src/network/networkd-nexthop.h @@ -22,7 +22,7 @@ typedef int (*nexthop_netlink_handler_t)( sd_netlink_message *m, Request *req, Link *link, - NextHop *address); + NextHop *nexthop); struct NextHop { Network *network; From 246b0a4d26dce8b713a73ccf9ce5ec7837a6939f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 29 Nov 2024 04:05:58 +0900 Subject: [PATCH 09/10] network/nexthop: replace unreachable condition with assertion --- src/network/networkd-nexthop.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 7aa7bd6f6d..18b130b959 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -420,8 +420,7 @@ static int nexthop_add_new(Manager *manager, uint32_t id, NextHop **ret) { r = hashmap_ensure_put(&manager->nexthops_by_id, &nexthop_hash_ops, UINT32_TO_PTR(nexthop->id), nexthop); if (r < 0) return r; - if (r == 0) - return -EEXIST; + assert(r > 0); nexthop->manager = manager; From bfe63cb00c908969fc00d02fe7420c9e3b78bbbf Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 29 Nov 2024 04:20:28 +0900 Subject: [PATCH 10/10] network: add missing template to networkd.conf --- src/network/networkd.conf | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/network/networkd.conf b/src/network/networkd.conf index 06d436245e..857df0899f 100644 --- a/src/network/networkd.conf +++ b/src/network/networkd.conf @@ -23,9 +23,15 @@ #ManageForeignRoutes=yes #ManageForeignNextHops=yes #RouteTable= +#IPv4Forwarding= +#IPv6Forwarding= #IPv6PrivacyExtensions=no #UseDomains=no +#[IPv6AddressLabel] +#Prefix= +#Label= + [IPv6AcceptRA] #UseDomains=