network: do not process requests which conditionalized with link flags while the...

E.g. nexthop requires IFF_UP flag, but the currently stored flag may be
outdated if we called link_down(). This makes such requests pending if
at least one of the flags are updating.

diff --git a/src/network/networkd-bridge-mdb.c b/src/network/networkd-bridge-mdb.c
index 3bf1e068f00d5bd084d4aa045fb67dd252cd9dc3..faf3dbd0b7bfd7c827391c8a107ff33b605788f4 100644 (file)
--- a/src/network/networkd-bridge-mdb.c
+++ b/src/network/networkd-bridge-mdb.c
@@ -258,6 +258,9 @@ static bool bridge_mdb_is_ready_to_configure(Link *link) {
         if (!IN_SET(master->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED))
                 return false;
 
+        if (master->set_flags_messages > 0)
+                return false;
+
         if (!link_has_carrier(master))
                 return false;
 

diff --git a/src/network/networkd-dhcp-common.c b/src/network/networkd-dhcp-common.c
index 8357b2b99dffa83bd4b2e4c1a82374a0719a0909..249d780887eeca0b972bfa1ab29049dc10e26f43 100644 (file)
--- a/src/network/networkd-dhcp-common.c
+++ b/src/network/networkd-dhcp-common.c
@@ -116,6 +116,9 @@ static int link_configure_and_start_dhcp_delayed(Link *link) {
                         return r;
         }
 
+        if (link->set_flags_messages > 0)
+                return 0;
+
         if (!link_has_carrier(link))
                 return 0;
 

diff --git a/src/network/networkd-dhcp-server.c b/src/network/networkd-dhcp-server.c
index 055aa82ba249e00a44fe34442340a47f9d15433a..8c0cd37f3fa4099e69d3c5237ca67a743d7c59b6 100644 (file)
--- a/src/network/networkd-dhcp-server.c
+++ b/src/network/networkd-dhcp-server.c
@@ -499,6 +499,9 @@ static bool dhcp_server_is_ready_to_configure(Link *link) {
         if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED))
                 return false;
 
+        if (link->set_flags_messages > 0)
+                return false;
+
         if (!link_has_carrier(link))
                 return false;
 

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 66a3740e93dbc0f6fce069f3282d016f0ac56b61..6933c8890f9ce59099d8aef05721154d5087985a 100644 (file)
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -148,8 +148,13 @@ bool link_is_ready_to_configure(Link *link, bool allow_unmanaged) {
         if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED))
                 return false;
 
-        if (!link_has_carrier(link) && !link->network->configure_without_carrier)
-                return false;
+        if (!link->network->configure_without_carrier) {
+                if (link->set_flags_messages > 0)
+                        return false;
+
+                if (!link_has_carrier(link))
+                        return false;
+        }
 
         if (link->set_link_messages > 0)
                 return false;

diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h
index 647c60e6847c5e06fcc483fcee0ca941e0c8317e..780bdee18524664941a66577f9cdd6893c0cb624 100644 (file)
--- a/src/network/networkd-link.h
+++ b/src/network/networkd-link.h
@@ -98,6 +98,7 @@ typedef struct Link {
         unsigned tc_messages;
         unsigned sr_iov_messages;
         unsigned set_link_messages;
+        unsigned set_flags_messages;
         unsigned create_stacked_netdev_messages;
         unsigned create_stacked_netdev_after_configured_messages;
 

diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c
index 954790f832c9218f8a145ab748ce71e27d86879b..4bc29ca0577cae676df205bff9e0fae193521b2b 100644 (file)
--- a/src/network/networkd-nexthop.c
+++ b/src/network/networkd-nexthop.c
@@ -757,9 +757,11 @@ static bool nexthop_is_ready_to_configure(Link *link, const NextHop *nexthop) {
         } else {
                 Link *l;
 
-                /* TODO: fdb nexthop does not require IFF_UP. The condition below needs to be updated
+                /* TODO: fdb nexthop does not require IFF_UP. The conditions below needs to be updated
                  * when fdb nexthop support is added. See rtm_to_nh_config() in net/ipv4/nexthop.c of
                  * kernel. */
+                if (link->set_flags_messages > 0)
+                        return false;
                 if (!FLAGS_SET(link->flags, IFF_UP))
                         return false;
 

diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c
index 00fed02403a013629c428372717e7b92ba9daa33..2c3803e02c2cf77ec4ed3c29b028c13c1a461e0c 100644 (file)
--- a/src/network/networkd-setlink.c
+++ b/src/network/networkd-setlink.c
@@ -38,6 +38,15 @@ static int get_link_master_handler(sd_netlink *rtnl, sd_netlink_message *m, Link
         return 0;
 }
 
+static int get_link_update_flag_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
+        assert(link);
+        assert(link->set_flags_messages > 0);
+
+        link->set_flags_messages--;
+
+        return get_link_default_handler(rtnl, m, link);
+}
+
 static int set_link_handler_internal(
                 sd_netlink *rtnl,
                 sd_netlink_message *m,
@@ -56,7 +65,7 @@ static int set_link_handler_internal(
         link->set_link_messages--;
 
         if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER))
-                return 0;
+                goto on_error;
 
         r = sd_netlink_message_get_errno(m);
         if (r < 0) {
@@ -67,7 +76,7 @@ static int set_link_handler_internal(
 
                 if (!ignore)
                         link_enter_failed(link);
-                return 0;
+                goto on_error;
         }
 
         log_link_debug(link, "%s set.", set_link_operation_to_string(op));
@@ -76,7 +85,7 @@ static int set_link_handler_internal(
                 r = link_call_getlink(link, get_link_handler);
                 if (r < 0) {
                         link_enter_failed(link);
-                        return 0;
+                        goto on_error;
                 }
         }
 
@@ -84,6 +93,14 @@ static int set_link_handler_internal(
                 link_check_ready(link);
 
         return 1;
+
+on_error:
+        if (op == SET_LINK_FLAGS) {
+                assert(link->set_flags_messages > 0);
+                link->set_flags_messages--;
+        }
+
+        return 0;
 }
 
 static int link_set_addrgen_mode_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
@@ -119,7 +136,7 @@ static int link_set_can_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *l
 }
 
 static int link_set_flags_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
-        return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_default_handler);
+        return set_link_handler_internal(rtnl, m, link, SET_LINK_FLAGS, false, get_link_update_flag_handler);
 }
 
 static int link_set_group_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) {
@@ -455,6 +472,8 @@ static bool link_is_ready_to_call_set_link(Request *req) {
                         return false;
                 break;
         case SET_LINK_CAN:
+                /* Do not check link->set_flgas_messages here, as it is ok even if link->flags
+                 * is outdated, and checking the counter causes a deadlock. */
                 if (FLAGS_SET(link->flags, IFF_UP)) {
                         /* The CAN interface must be down to configure bitrate, etc... */
                         r = link_down(link);
@@ -478,6 +497,8 @@ static bool link_is_ready_to_call_set_link(Request *req) {
                                 return false;
                         m = link->network->bond->ifindex;
 
+                        /* Do not check link->set_flgas_messages here, as it is ok even if link->flags
+                         * is outdated, and checking the counter causes a deadlock. */
                         if (FLAGS_SET(link->flags, IFF_UP)) {
                                 /* link must be down when joining to bond master. */
                                 r = link_down(link);
@@ -523,6 +544,9 @@ int request_process_set_link(Request *req) {
                 return log_link_error_errno(req->link, r, "Failed to set %s: %m",
                                             set_link_operation_to_string(req->set_link_operation));
 
+        if (req->set_link_operation == SET_LINK_FLAGS)
+                req->link->set_flags_messages++;
+
         return 1;
 }
 
@@ -770,7 +794,7 @@ static int link_up_or_down_handler_internal(sd_netlink *rtnl, sd_netlink_message
         assert(link);
 
         if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER))
-                return 0;
+                goto on_error;
 
         r = sd_netlink_message_get_errno(m);
         if (r < 0)
@@ -778,11 +802,23 @@ static int link_up_or_down_handler_internal(sd_netlink *rtnl, sd_netlink_message
                                                "Could not bring up interface, ignoring" :
                                                "Could not bring down interface, ignoring");
 
+        r = link_call_getlink(link, get_link_update_flag_handler);
+        if (r < 0) {
+                link_enter_failed(link);
+                goto on_error;
+        }
+
         if (check_ready) {
                 link->activated = true;
                 link_check_ready(link);
         }
 
+        return 1;
+
+on_error:
+        assert(link->set_flags_messages > 0);
+        link->set_flags_messages--;
+
         return 0;
 }
 
@@ -906,7 +942,7 @@ int link_request_to_activate(Link *link) {
 
         link->activated = false;
 
-        r = link_queue_request(link, REQUEST_TYPE_ACTIVATE_LINK, NULL, false, NULL,
+        r = link_queue_request(link, REQUEST_TYPE_ACTIVATE_LINK, NULL, false, &link->set_flags_messages,
                                up ? link_activate_up_handler : link_activate_down_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request to activate link: %m");
@@ -963,7 +999,7 @@ int link_request_to_bring_up_or_down(Link *link, bool up) {
 
         assert(link);
 
-        r = link_queue_request(link, REQUEST_TYPE_UP_DOWN, NULL, false, NULL,
+        r = link_queue_request(link, REQUEST_TYPE_UP_DOWN, NULL, false, &link->set_flags_messages,
                                up ? link_up_handler : link_down_handler, &req);
         if (r < 0)
                 return log_link_error_errno(link, r, "Failed to request to bring %s link: %m",