From: Saran Tunyasuvunakool Date: Tue, 12 Dec 2017 15:25:36 +0000 (+0000) Subject: networkd: Fix race condition in [RoutingPolicyRule] handling (#7615) X-Git-Tag: v236~28 X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fsystemd.git;a=commitdiff_plain;h=7715629e9a1b9fa21343468de6bfd80360b415ff networkd: Fix race condition in [RoutingPolicyRule] handling (#7615) The routing policy rule setup logic is moved to the routes setup phase (rather than the addresses setup phase as it is now). Additionally, a call to `link_check_ready` is added to the routing policy rules setup handler. This prevents a race condition with the routes setup handler. Also give each async handler its own message counter to prevent race conditions when logging successes. Fixes: #7614 --- diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 5b8288fb724..6c4711e2e8d 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -739,7 +739,10 @@ void link_check_ready(Link *link) { if (!link->network) return; - if (!link->static_configured) + if (!link->static_routes_configured) + return; + + if (!link->routing_policy_rules_configured) return; if (link_ipv4ll_enabled(link)) @@ -797,10 +800,15 @@ static int link_set_routing_policy_rule(Link *link) { return r; } - link->link_messages++; + link->routing_policy_rule_messages++; } routing_policy_rule_purge(link->manager, link); + if (link->routing_policy_rule_messages == 0) { + link->routing_policy_rules_configured = true; + link_check_ready(link); + } else + log_link_debug(link, "Setting routing policy rules"); return 0; } @@ -809,12 +817,12 @@ static int route_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userdata _cleanup_link_unref_ Link *link = userdata; int r; - assert(link->link_messages > 0); + assert(link->route_messages > 0); assert(IN_SET(link->state, LINK_STATE_SETTING_ADDRESSES, LINK_STATE_SETTING_ROUTES, LINK_STATE_FAILED, LINK_STATE_LINGER)); - link->link_messages--; + link->route_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; @@ -823,9 +831,9 @@ static int route_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userdata if (r < 0 && r != -EEXIST) log_link_warning_errno(link, r, "Could not set route: %m"); - if (link->link_messages == 0) { + if (link->route_messages == 0) { log_link_debug(link, "Routes set"); - link->static_configured = true; + link->static_routes_configured = true; link_check_ready(link); } @@ -850,11 +858,13 @@ static int link_enter_set_routes(Link *link) { return r; } - link->link_messages++; + link->route_messages++; } - if (link->link_messages == 0) { - link->static_configured = true; + (void) link_set_routing_policy_rule(link); + + if (link->route_messages == 0) { + link->static_routes_configured = true; link_check_ready(link); } else log_link_debug(link, "Setting routes"); @@ -888,11 +898,11 @@ static int address_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userda assert(m); assert(link); assert(link->ifname); - assert(link->link_messages > 0); + assert(link->address_messages > 0); assert(IN_SET(link->state, LINK_STATE_SETTING_ADDRESSES, LINK_STATE_FAILED, LINK_STATE_LINGER)); - link->link_messages--; + link->address_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; @@ -903,7 +913,7 @@ static int address_handler(sd_netlink *rtnl, sd_netlink_message *m, void *userda else if (r >= 0) manager_rtnl_process_address(rtnl, m, link->manager); - if (link->link_messages == 0) { + if (link->address_messages == 0) { log_link_debug(link, "Addresses set"); link_enter_set_routes(link); } @@ -919,9 +929,9 @@ static int address_label_handler(sd_netlink *rtnl, sd_netlink_message *m, void * assert(m); assert(link); assert(link->ifname); - assert(link->link_messages > 0); + assert(link->address_label_messages > 0); - link->link_messages--; + link->address_label_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; @@ -932,7 +942,7 @@ static int address_label_handler(sd_netlink *rtnl, sd_netlink_message *m, void * else if (r >= 0) manager_rtnl_process_address(rtnl, m, link->manager); - if (link->link_messages == 0) + if (link->address_label_messages == 0) log_link_debug(link, "Addresses label set"); return 1; @@ -1072,7 +1082,7 @@ static int link_enter_set_addresses(Link *link) { return r; } - link->link_messages++; + link->address_messages++; } LIST_FOREACH(labels, label, link->network->address_labels) { @@ -1083,7 +1093,7 @@ static int link_enter_set_addresses(Link *link) { return r; } - link->link_messages++; + link->address_label_messages++; } /* now that we can figure out a default address for the dhcp server, @@ -1206,7 +1216,7 @@ static int link_enter_set_addresses(Link *link) { log_link_debug(link, "Offering DHCPv4 leases"); } - if (link->link_messages == 0) + if (link->address_messages == 0) link_enter_set_routes(link); else log_link_debug(link, "Setting addresses"); @@ -2699,8 +2709,6 @@ static int link_configure(Link *link) { return r; } - (void) link_set_routing_policy_rule(link); - if (link_has_carrier(link) || link->network->configure_without_carrier) { r = link_acquire_conf(link); if (r < 0) diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 25e49c8329a..8aaaa679ff3 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -86,7 +86,11 @@ typedef struct Link { LinkState state; LinkOperationalState operstate; - unsigned link_messages; + unsigned address_messages; + unsigned address_label_messages; + unsigned route_messages; + unsigned routing_policy_rule_messages; + unsigned routing_policy_rule_remove_messages; unsigned enslaving; Set *addresses; @@ -109,7 +113,8 @@ typedef struct Link { bool ipv4ll_address:1; bool ipv4ll_route:1; - bool static_configured; + bool static_routes_configured; + bool routing_policy_rules_configured; bool setting_mtu; LIST_HEAD(Address, pool_addresses); diff --git a/src/network/networkd-routing-policy-rule.c b/src/network/networkd-routing-policy-rule.c index f4b3b7180b6..1314564c110 100644 --- a/src/network/networkd-routing-policy-rule.c +++ b/src/network/networkd-routing-policy-rule.c @@ -324,7 +324,7 @@ static int routing_policy_rule_remove_handler(sd_netlink *rtnl, sd_netlink_messa assert(link); assert(link->ifname); - link->link_messages--; + link->routing_policy_rule_remove_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; @@ -438,9 +438,9 @@ int link_routing_policy_rule_handler(sd_netlink *rtnl, sd_netlink_message *m, vo assert(m); assert(link); assert(link->ifname); - assert(link->link_messages > 0); + assert(link->routing_policy_rule_messages > 0); - link->link_messages--; + link->routing_policy_rule_messages--; if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; @@ -449,8 +449,11 @@ int link_routing_policy_rule_handler(sd_netlink *rtnl, sd_netlink_message *m, vo if (r < 0 && r != -EEXIST) log_link_warning_errno(link, r, "Could not add routing policy rule: %m"); - if (link->link_messages == 0) + if (link->routing_policy_rule_messages == 0) { log_link_debug(link, "Routing policy rule configured"); + link->routing_policy_rules_configured = true; + link_check_ready(link); + } return 1; } @@ -1057,7 +1060,7 @@ void routing_policy_rule_purge(Manager *m, Link *link) { continue; } - link->link_messages++; + link->routing_policy_rule_remove_messages++; } } }