From a5a0a4eb5651e3d27ee820dbf9828f84d1818243 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 Jan 2024 13:10:43 +0900 Subject: [PATCH 1/5] network/route: use specified error message Previously, specified error message was not used. --- src/network/networkd-route.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 0553a66343..4e7a12ce1c 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1131,7 +1131,7 @@ int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Li r = sd_netlink_message_get_errno(m); if (r < 0 && r != -EEXIST) { - log_link_message_warning_errno(link, m, r, "Could not set route"); + log_link_message_warning_errno(link, m, r, error_msg); link_enter_failed(link); return 0; } From f86575ca066dd8a6f73a36e061273d8a641ea205 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 Jan 2024 13:36:22 +0900 Subject: [PATCH 2/5] network/route: unconditionally call route_setup_timer() for managed routes For foreign routes, we do not set lifetime, as it is foreign. So, this should not change any behavior. Preparation for later commits. --- src/network/networkd-route.c | 45 +++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 4e7a12ce1c..78774f60ed 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1534,7 +1534,7 @@ static int process_route_one( _cleanup_(route_freep) Route *tmp = in; Route *route = NULL; - bool update_dhcp4; + bool is_new = false, update_dhcp4; int r; assert(manager); @@ -1549,32 +1549,35 @@ static int process_route_one( switch (type) { case RTM_NEWROUTE: - if (route) { - route->flags = tmp->flags; - route_enter_configured(route); - log_route_debug(route, "Received remembered", link, manager); + if (!route) { + if (!manager->manage_foreign_routes) { + route_enter_configured(tmp); + log_route_debug(tmp, "Ignoring received", link, manager); + return 0; + } - r = route_setup_timer(route, cacheinfo); - if (r < 0) - log_link_warning_errno(link, r, "Failed to configure expiration timer for route, ignoring: %m"); - if (r > 0) - log_route_debug(route, "Configured expiration timer for", link, manager); - - } else if (!manager->manage_foreign_routes) { - route_enter_configured(tmp); - log_route_debug(tmp, "Ignoring received", link, manager); - - } else { - /* A route appeared that we did not request */ - route_enter_configured(tmp); - log_route_debug(tmp, "Received new", link, manager); + /* If we do not know the route, then save it. */ r = route_add(manager, link, tmp); if (r < 0) { log_link_warning_errno(link, r, "Failed to remember foreign route, ignoring: %m"); return 0; } - TAKE_PTR(tmp); - } + + route = TAKE_PTR(tmp); + is_new = true; + + } else + /* Update remembered route with the received notification. */ + route->flags = tmp->flags; + + route_enter_configured(route); + log_route_debug(route, is_new ? "Received new" : "Received remembered", link, manager); + + r = route_setup_timer(route, cacheinfo); + if (r < 0) + log_link_warning_errno(link, r, "Failed to configure expiration timer for route, ignoring: %m"); + if (r > 0) + log_route_debug(route, "Configured expiration timer for", link, manager); break; From 14436bf5235a64682839fa0f9fd1f2d85f0102a3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 Jan 2024 13:44:15 +0900 Subject: [PATCH 3/5] network/route: save if the route expiration is managed by the kernel Otherwise, our own expiration timer will be setup on updating a route. See comment in link_request_route(). --- src/network/networkd-route.c | 35 +++++++++++++---------------------- src/network/networkd-route.h | 14 ++++++++------ 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 78774f60ed..83dfea1392 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1014,26 +1014,26 @@ static int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdat } static int route_setup_timer(Route *route, const struct rta_cacheinfo *cacheinfo) { - Manager *manager; int r; assert(route); - assert(route->manager || (route->link && route->link->manager)); - - manager = route->manager ?: route->link->manager; - - if (route->lifetime_usec == USEC_INFINITY) - return 0; if (cacheinfo && cacheinfo->rta_expires != 0) - /* Assume that non-zero rta_expires means kernel will handle the route expiration. */ - return 0; + route->expiration_managed_by_kernel = true; + if (route->lifetime_usec == USEC_INFINITY || /* We do not request expiration for the route. */ + route->expiration_managed_by_kernel) { /* We have received nonzero expiration previously. The expiration is managed by the kernel. */ + route->expire = sd_event_source_disable_unref(route->expire); + return 0; + } + + Manager *manager = ASSERT_PTR(route->manager ?: ASSERT_PTR(route->link)->manager); r = event_reset_time(manager->event, &route->expire, CLOCK_BOOTTIME, route->lifetime_usec, 0, route_expire_handler, route, 0, "route-expiration", true); if (r < 0) - return r; + return log_link_warning_errno(route->link, r, "Failed to configure expiration timer for route, ignoring: %m"); + log_route_debug(route, "Configured expiration timer for", route->link, manager); return 1; } @@ -1379,15 +1379,10 @@ int link_request_route( if (consume_object) route_free(route); - if (existing->expire) { + if (existing->expire) /* When re-configuring an existing route, kernel does not send RTM_NEWROUTE * message, so we need to update the timer here. */ - r = route_setup_timer(existing, NULL); - if (r < 0) - log_link_warning_errno(link, r, "Failed to update expiration timer for route, ignoring: %m"); - if (r > 0) - log_route_debug(existing, "Updated expiration timer for", link, link->manager); - } + (void) route_setup_timer(existing, NULL); } log_route_debug(existing, "Requesting", link, link->manager); @@ -1573,11 +1568,7 @@ static int process_route_one( route_enter_configured(route); log_route_debug(route, is_new ? "Received new" : "Received remembered", link, manager); - r = route_setup_timer(route, cacheinfo); - if (r < 0) - log_link_warning_errno(link, r, "Failed to configure expiration timer for route, ignoring: %m"); - if (r > 0) - log_route_debug(route, "Configured expiration timer for", link, manager); + (void) route_setup_timer(route, cacheinfo); break; diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 33d1e643cc..0baf199ddb 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -53,6 +53,14 @@ struct Route { /* metrics (RTA_METRICS) */ RouteMetric metric; + /* This is an absolute point in time, and NOT a timespan/duration. + * Must be specified with clock_boottime_or_monotonic(). */ + usec_t lifetime_usec; /* RTA_EXPIRES (IPv6 only) */ + /* Used when kernel does not support RTA_EXPIRES attribute. */ + sd_event_source *expire; + bool expiration_managed_by_kernel:1; /* RTA_CACHEINFO has nonzero rta_expires */ + + /* Only used by conf persers and route_section_verify(). */ bool scope_set:1; bool table_set:1; bool priority_set:1; @@ -65,12 +73,6 @@ struct Route { union in_addr_union src; union in_addr_union prefsrc; OrderedSet *multipath_routes; - - /* This is an absolute point in time, and NOT a timespan/duration. - * Must be specified with clock_boottime_or_monotonic(). */ - usec_t lifetime_usec; - /* Used when kernel does not support RTA_EXPIRES attribute. */ - sd_event_source *expire; }; extern const struct hash_ops route_hash_ops; From 2c0b49baabc52e16a6c7acbd5a129e766fcb565d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 Jan 2024 16:20:39 +0900 Subject: [PATCH 4/5] network/route: update expiration timer only when we know the route exists --- src/network/networkd-dhcp-prefix-delegation.c | 6 ++--- src/network/networkd-ndisc.c | 2 +- src/network/networkd-route.c | 23 ++++++++++++------- src/network/networkd-route.h | 2 +- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index 2158c77393..168b45349e 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -265,7 +265,7 @@ static int dhcp_pd_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Reques assert(link); - r = route_configure_handler_internal(rtnl, m, link, "Failed to add prefix route for DHCP delegated subnet prefix"); + r = route_configure_handler_internal(rtnl, m, link, route, "Failed to add prefix route for DHCP delegated subnet prefix"); if (r <= 0) return r; @@ -625,7 +625,7 @@ static int dhcp4_unreachable_route_handler(sd_netlink *rtnl, sd_netlink_message assert(link); - r = route_configure_handler_internal(rtnl, m, link, "Failed to set unreachable route for DHCPv4 delegated prefix"); + r = route_configure_handler_internal(rtnl, m, link, route, "Failed to set unreachable route for DHCPv4 delegated prefix"); if (r <= 0) return r; @@ -641,7 +641,7 @@ static int dhcp6_unreachable_route_handler(sd_netlink *rtnl, sd_netlink_message assert(link); - r = route_configure_handler_internal(rtnl, m, link, "Failed to set unreachable route for DHCPv6 delegated prefix"); + r = route_configure_handler_internal(rtnl, m, link, route, "Failed to set unreachable route for DHCPv6 delegated prefix"); if (r <= 0) return r; diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 61dc7acb83..4e38cd2764 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -139,7 +139,7 @@ static int ndisc_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request assert(link); - r = route_configure_handler_internal(rtnl, m, link, "Could not set NDisc route"); + r = route_configure_handler_internal(rtnl, m, link, route, "Could not set NDisc route"); if (r <= 0) return r; diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 83dfea1392..f03c871200 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1122,15 +1122,27 @@ static int append_nexthops(const Link *link, const Route *route, sd_netlink_mess return 0; } -int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, const char *error_msg) { +int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, Route *route, const char *error_msg) { int r; assert(m); assert(link); + assert(link->manager); + assert(route); assert(error_msg); r = sd_netlink_message_get_errno(m); - if (r < 0 && r != -EEXIST) { + if (r == -EEXIST) { + Route *existing; + + if (route_get(link->manager, link, route, &existing) >= 0) { + /* When re-configuring an existing route, kernel does not send RTM_NEWROUTE + * notification, so we need to update the timer here. */ + existing->lifetime_usec = route->lifetime_usec; + (void) route_setup_timer(existing, NULL); + } + + } else if (r < 0) { log_link_message_warning_errno(link, m, r, error_msg); link_enter_failed(link); return 0; @@ -1378,11 +1390,6 @@ int link_request_route( existing->lifetime_usec = route->lifetime_usec; if (consume_object) route_free(route); - - if (existing->expire) - /* When re-configuring an existing route, kernel does not send RTM_NEWROUTE - * message, so we need to update the timer here. */ - (void) route_setup_timer(existing, NULL); } log_route_debug(existing, "Requesting", link, link->manager); @@ -1404,7 +1411,7 @@ static int static_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request assert(link); - r = route_configure_handler_internal(rtnl, m, link, "Could not set route"); + r = route_configure_handler_internal(rtnl, m, link, route, "Could not set route"); if (r <= 0) return r; diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 0baf199ddb..3d3dd6ea05 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -83,7 +83,7 @@ Route *route_free(Route *route); DEFINE_SECTION_CLEANUP_FUNCTIONS(Route, route_free); int route_dup(const Route *src, Route **ret); -int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, const char *error_msg); +int route_configure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, Route *route, const char *error_msg); int route_remove(Route *route); int route_remove_and_drop(Route *route); From dd0385c1f2392c0eb3043c251b70bf3812c187c5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 9 Jan 2024 16:22:42 +0900 Subject: [PATCH 5/5] network/dhcp4: use route_configure_handler_internal() at one more place --- src/network/networkd-dhcp4.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 8987b76c52..44ed272387 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -342,12 +342,9 @@ static int dhcp4_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request 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 set DHCPv4 route"); - link_enter_failed(link); - return 1; - } + r = route_configure_handler_internal(rtnl, m, link, route, "Could not set DHCPv4 route"); + if (r <= 0) + return r; r = dhcp4_check_ready(link); if (r < 0)