From d9eee312a7232c83d7494139d4d1bc8831d57455 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 4 Feb 2021 00:40:08 +0900 Subject: [PATCH 1/5] network: make address_configure() or friends return 1 when the address is new --- src/network/networkd-address.c | 15 ++++++----- src/network/networkd-neighbor.c | 7 +++-- src/network/networkd-nexthop.c | 7 ++--- src/network/networkd-route.c | 31 ++++++++++++---------- src/network/networkd-routing-policy-rule.c | 7 ++--- 5 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/network/networkd-address.c b/src/network/networkd-address.c index d7fbb6eac5..499aa5be0a 100644 --- a/src/network/networkd-address.c +++ b/src/network/networkd-address.c @@ -326,6 +326,7 @@ static int address_add_foreign(Link *link, const Address *in, Address **ret) { } static int address_add(Link *link, const Address *in, Address **ret) { + bool is_new = false; Address *address; int r; @@ -338,6 +339,7 @@ static int address_add(Link *link, const Address *in, Address **ret) { r = address_add_internal(link, &link->addresses, in, &address); if (r < 0) return r; + is_new = true; } else if (r == 0) { /* Take over a foreign address */ r = set_ensure_put(&link->addresses, &address_hash_ops, address); @@ -353,8 +355,7 @@ static int address_add(Link *link, const Address *in, Address **ret) { if (ret) *ret = address; - - return 0; + return is_new; } static int address_update(Address *address, const Address *src) { @@ -811,7 +812,7 @@ int address_configure( Address *acquired_address, *a; uint32_t flags; bool update; - int r; + int r, k; assert(address); assert(IN_SET(address->family, AF_INET, AF_INET6)); @@ -889,9 +890,9 @@ int address_configure( if (r < 0) return log_link_error_errno(link, r, "Could not append IFA_CACHEINFO attribute: %m"); - r = address_add(link, address, &a); - if (r < 0) - return log_link_error_errno(link, r, "Could not add address: %m"); + k = address_add(link, address, &a); + if (k < 0) + return log_link_error_errno(link, k, "Could not add address: %m"); r = address_set_masquerade(a, true); if (r < 0) @@ -914,7 +915,7 @@ int address_configure( if (ret) *ret = a; - return 1; + return k; } static int static_address_ready_callback(Address *address) { diff --git a/src/network/networkd-neighbor.c b/src/network/networkd-neighbor.c index 22b716b14a..f54a2f9c99 100644 --- a/src/network/networkd-neighbor.c +++ b/src/network/networkd-neighbor.c @@ -170,6 +170,7 @@ static int neighbor_add_internal(Link *link, Set **neighbors, const Neighbor *in } static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { + bool is_new = false; Neighbor *neighbor; int r; @@ -179,6 +180,7 @@ static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { r = neighbor_add_internal(link, &link->neighbors, in, &neighbor); if (r < 0) return r; + is_new = true; } else if (r == 0) { /* Neighbor is foreign, claim it as recognized */ r = set_ensure_put(&link->neighbors, &neighbor_hash_ops, neighbor); @@ -188,12 +190,13 @@ static int neighbor_add(Link *link, const Neighbor *in, Neighbor **ret) { set_remove(link->neighbors_foreign, neighbor); } else if (r == 1) { /* Neighbor already exists */ + ; } else return r; if (ret) *ret = neighbor; - return 0; + return is_new; } static int neighbor_add_foreign(Link *link, const Neighbor *in, Neighbor **ret) { @@ -279,7 +282,7 @@ static int neighbor_configure(Neighbor *neighbor, Link *link) { if (r < 0) return log_link_error_errno(link, r, "Could not add neighbor: %m"); - return 0; + return r; } int link_set_neighbors(Link *link) { diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index ea9c252cc7..ab0562e9e8 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -188,6 +188,7 @@ static int nexthop_add_foreign(Link *link, NextHop *in, NextHop **ret) { } static int nexthop_add(Link *link, NextHop *in, NextHop **ret) { + bool is_new = false; NextHop *nexthop; int r; @@ -197,6 +198,7 @@ static int nexthop_add(Link *link, NextHop *in, NextHop **ret) { r = nexthop_add_internal(link, &link->nexthops, in, &nexthop); if (r < 0) return r; + is_new = true; } else if (r == 0) { /* Take over a foreign nexthop */ r = set_ensure_put(&link->nexthops, &nexthop_hash_ops, nexthop); @@ -212,8 +214,7 @@ static int nexthop_add(Link *link, NextHop *in, NextHop **ret) { if (ret) *ret = nexthop; - - return 0; + return is_new; } static int nexthop_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -297,7 +298,7 @@ static int nexthop_configure(NextHop *nexthop, Link *link) { if (r < 0) return log_link_error_errno(link, r, "Could not add nexthop: %m"); - return 1; + return r; } int link_set_nexthop(Link *link) { diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index ef977dac5c..5a908dbeb0 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -549,6 +549,7 @@ static int route_add_foreign(Manager *manager, Link *link, const Route *in, Rout static int route_add(Manager *manager, Link *link, const Route *in, const MultipathRoute *m, Route **ret) { _cleanup_(route_freep) Route *tmp = NULL; + bool is_new = false; Route *route; int r; @@ -572,6 +573,7 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip r = route_add_internal(manager, link, link ? &link->routes : &manager->routes, in, &route); if (r < 0) return r; + is_new = true; } else if (r == 0) { /* Take over a foreign route */ if (link) { @@ -595,8 +597,7 @@ static int route_add(Manager *manager, Link *link, const Route *in, const Multip if (ret) *ret = route; - - return 0; + return is_new; } static bool route_type_is_reject(const Route *route) { @@ -949,15 +950,15 @@ static int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdat static int route_add_and_setup_timer(Link *link, const Route *route, const MultipathRoute *m, Route **ret) { _cleanup_(sd_event_source_unrefp) sd_event_source *expire = NULL; Route *nr; - int r; + int r, k; assert(link); assert(route); if (route_type_is_reject(route)) - r = route_add(link->manager, NULL, route, NULL, &nr); + k = route_add(link->manager, NULL, route, NULL, &nr); else if (!m || m->ifindex == 0 || m->ifindex == link->ifindex) - r = route_add(NULL, link, route, m, &nr); + k = route_add(NULL, link, route, m, &nr); else { Link *link_gw; @@ -965,10 +966,10 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi if (r < 0) return log_link_error_errno(link, r, "Failed to get link with ifindex %d: %m", m->ifindex); - r = route_add(NULL, link_gw, route, m, &nr); + k = route_add(NULL, link_gw, route, m, &nr); } - if (r < 0) - return log_link_error_errno(link, r, "Could not add route: %m"); + if (k < 0) + return log_link_error_errno(link, k, "Could not add route: %m"); /* TODO: drop expiration handling once it can be pushed into the kernel */ if (nr->lifetime != USEC_INFINITY && !kernel_route_expiration_supported()) { @@ -984,7 +985,7 @@ static int route_add_and_setup_timer(Link *link, const Route *route, const Multi if (ret) *ret = nr; - return 0; + return k; } static int append_nexthop_one(const Route *route, const MultipathRoute *m, struct rtattr **rta, size_t offset) { @@ -1075,7 +1076,7 @@ int route_configure( Route **ret) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; - int r; + int r, k = 0; assert(link); assert(link->manager); @@ -1165,9 +1166,9 @@ int route_configure( if (ordered_set_isempty(route->multipath_routes)) { Route *nr; - r = route_add_and_setup_timer(link, route, NULL, &nr); - if (r < 0) - return r; + k = route_add_and_setup_timer(link, route, NULL, &nr); + if (k < 0) + return k; if (ret) *ret = nr; @@ -1180,6 +1181,8 @@ int route_configure( r = route_add_and_setup_timer(link, route, m, NULL); if (r < 0) return r; + if (r > 0) + k = 1; } } @@ -1190,7 +1193,7 @@ int route_configure( link_ref(link); - return 0; + return k; } static int route_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index 7802a1e55a..ed966f3c17 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -326,6 +326,7 @@ static int routing_policy_rule_get(Manager *m, const RoutingPolicyRule *rule, Ro static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, int family, RoutingPolicyRule **ret) { _cleanup_(routing_policy_rule_freep) RoutingPolicyRule *rule = NULL; RoutingPolicyRule *existing; + bool is_new = false; int r; assert(m); @@ -353,6 +354,7 @@ static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, int rule->manager = m; existing = TAKE_PTR(rule); + is_new = true; } else if (r == 0) { /* Take over a foreign rule. */ r = set_ensure_put(&m->rules, &routing_policy_rule_hash_ops, existing); @@ -369,8 +371,7 @@ static int routing_policy_rule_add(Manager *m, const RoutingPolicyRule *in, int if (ret) *ret = existing; - - return 0; + return is_new; } static int routing_policy_rule_consume_foreign(Manager *m, RoutingPolicyRule *rule) { @@ -635,7 +636,7 @@ static int routing_policy_rule_configure_internal(const RoutingPolicyRule *rule, if (r < 0) return log_link_error_errno(link, r, "Could not add rule: %m"); - return 1; + return r; } static int routing_policy_rule_configure(const RoutingPolicyRule *rule, Link *link) { From 0ef9f3c76ff37a0ea7d2c44ad7c659d1e8d052f1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 4 Feb 2021 00:44:03 +0900 Subject: [PATCH 2/5] network: set return value at the end of the function The later netlink_call_async() call may fail. We should not touch the return value when the function failed. --- src/network/networkd-route.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 5a908dbeb0..e461845351 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -1077,6 +1077,7 @@ int route_configure( _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; int r, k = 0; + Route *nr; assert(link); assert(link->manager); @@ -1164,14 +1165,9 @@ int route_configure( return log_link_error_errno(link, r, "Could not append RTA_MULTIPATH attribute: %m"); if (ordered_set_isempty(route->multipath_routes)) { - Route *nr; - k = route_add_and_setup_timer(link, route, NULL, &nr); if (k < 0) return k; - - if (ret) - *ret = nr; } else { MultipathRoute *m; @@ -1193,6 +1189,9 @@ int route_configure( link_ref(link); + if (ret) + *ret = nr; + return k; } From b54aed29c61b9b86dc9cfa214c946586ea1f4972 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 4 Feb 2021 00:52:28 +0900 Subject: [PATCH 3/5] network: dhcp6: fix condtion check --- src/network/networkd-dhcp6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 451b552345..0498d5af56 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -137,7 +137,7 @@ static int dhcp6_pd_remove_old(Link *link, bool force) { assert(link); assert(link->manager); - if (!force && (link->dhcp6_pd_address_messages != 0 || link->dhcp6_pd_route_configured != 0)) + if (!force && (link->dhcp6_pd_address_messages != 0 || link->dhcp6_pd_route_messages != 0)) return 0; if (set_isempty(link->dhcp6_pd_addresses_old) && set_isempty(link->dhcp6_pd_routes_old)) From 24b445c2cea4d4e883b74ce501068dbbb81a3b51 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 4 Feb 2021 01:00:42 +0900 Subject: [PATCH 4/5] network: ndisc: change link state into "configuring" only when a new address or route will be assigned --- src/network/networkd-ndisc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 47b35bd543..4b9485b6b2 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -343,6 +343,8 @@ static int ndisc_route_configure(Route *route, Link *link, sd_ndisc_router *rt) r = route_configure(route, link, ndisc_route_handler, &ret); if (r < 0) return log_link_error_errno(link, r, "Failed to set NDisc route: %m"); + if (r > 0) + link->ndisc_routes_configured = false; link->ndisc_routes_messages++; @@ -437,6 +439,8 @@ static int ndisc_address_configure(Address *address, Link *link, sd_ndisc_router r = address_configure(address, link, ndisc_address_handler, &ret); if (r < 0) return log_link_error_errno(link, r, "Failed to set NDisc SLAAC address: %m"); + if (r > 0) + link->ndisc_addresses_configured = false; link->ndisc_addresses_messages++; @@ -1213,9 +1217,6 @@ static int ndisc_router_handler(Link *link, sd_ndisc_router *rt) { return 0; } - link->ndisc_addresses_configured = false; - link->ndisc_routes_configured = false; - SET_FOREACH(na, link->ndisc_addresses) if (IN6_ARE_ADDR_EQUAL(&na->router, &router.in6)) na->marked = true; From 111ce984190f98de24a1a3b83b259827bb23fc69 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 4 Feb 2021 01:00:59 +0900 Subject: [PATCH 5/5] network: dhcp6: change link state into "configuring" only when a new address or route will be assigned --- src/network/networkd-dhcp6.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c index 0498d5af56..326c9f3fcf 100644 --- a/src/network/networkd-dhcp6.c +++ b/src/network/networkd-dhcp6.c @@ -281,6 +281,8 @@ static int dhcp6_set_pd_route(Link *link, const union in_addr_union *prefix, con r = route_configure(route, link, dhcp6_pd_route_handler, &ret); if (r < 0) return log_link_error_errno(link, r, "Failed to set DHCPv6 prefix route: %m"); + if (r > 0) + link->dhcp6_pd_route_configured = false; link->dhcp6_pd_route_messages++; @@ -394,6 +396,8 @@ static int dhcp6_set_pd_address( r = address_configure(address, link, dhcp6_pd_address_handler, &ret); if (r < 0) return log_link_error_errno(link, r, "Failed to set DHCPv6 delegated prefix address: %m"); + if (r > 0) + link->dhcp6_pd_address_configured = false; link->dhcp6_pd_address_messages++; @@ -576,8 +580,6 @@ static int dhcp6_pd_prepare(Link *link) { if (!link_dhcp6_pd_is_enabled(link)) return 0; - link->dhcp6_pd_address_configured = false; - link->dhcp6_pd_route_configured = false; link->dhcp6_pd_prefixes_assigned = true; while ((address = set_steal_first(link->dhcp6_pd_addresses))) { @@ -816,6 +818,8 @@ static int dhcp6_set_unreachable_route(Link *link, const union in_addr_union *ad if (r < 0) return log_link_error_errno(link, r, "Failed to set unreachable route for DHCPv6 delegated subnet %s/%u: %m", strna(buf), prefixlen); + if (r > 0) + link->dhcp6_route_configured = false; link->dhcp6_route_messages++; @@ -1038,6 +1042,8 @@ static int dhcp6_update_address( if (r < 0) return log_link_error_errno(link, r, "Failed to set DHCPv6 address %s/%u: %m", strnull(buffer), addr->prefixlen); + if (r > 0) + link->dhcp6_address_configured = false; link->dhcp6_address_messages++; @@ -1103,9 +1109,6 @@ static int dhcp6_lease_ip_acquired(sd_dhcp6_client *client, Link *link) { Route *rt; int r; - link->dhcp6_address_configured = false; - link->dhcp6_route_configured = false; - while ((a = set_steal_first(link->dhcp6_addresses))) { r = set_ensure_put(&link->dhcp6_addresses_old, &address_hash_ops, a); if (r < 0)