platform: merge branch 'lr/nl-retry'

https://bugzilla.redhat.com/show_bug.cgi?id=2169512

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1537

src/libnm-platform/nm-linux-platform.c

@@ -328,6 +328,10 @@ struct _ifla_vf_vlan_info {
 
 /*****************************************************************************/
 
+#define RESYNC_RETRIES 50
+
+/*****************************************************************************/
+
 typedef struct {
     guint16 family_id;
 } GenlFamilyData;
@@ -455,7 +459,14 @@ typedef enum _nm_packed {
     WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN = 0,
     WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK,
     WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_UNKNOWN,
+
+    /* The receive buffer of the netlink socket has a large, but limited size.
+     * It can fill up, and we lose messages. When that happens, we may lose a
+     * response that we were waiting for. This error number indicates that we
+     * don't know the response due to a resync. We probably should retry the
+     * request. */
     WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC,
+
     WAIT_FOR_NL_RESPONSE_RESULT_FAILED_POLL,
     WAIT_FOR_NL_RESPONSE_RESULT_FAILED_TIMEOUT,
     WAIT_FOR_NL_RESPONSE_RESULT_FAILED_DISPOSING,
@@ -6694,7 +6705,7 @@ delayed_action_wait_for_response_complete(NMPlatform             *platform,
 
     nm_assert(NM_FLAGS_ANY(priv->delayed_action.flags, ACTION_TYPE));
     nm_assert(idx < priv->delayed_action.list_wait_for_response_x[netlink_protocol]->len);
-    nm_assert(seq_result);
+    nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
     data = delayed_action_get_list_wait_for_resonse(priv, netlink_protocol, idx);
 
@@ -7059,6 +7070,8 @@ delayed_action_schedule_WAIT_FOR_RESPONSE(NMPlatform                        *pla
         .response.out_data = response_out_data,
     };
 
+    nm_assert(!out_seq_result || *out_seq_result == WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
     delayed_action_schedule(
         platform,
         nmp_netlink_protocol_info(netlink_protocol)->delayed_action_type_wait_for_response,
@@ -7423,6 +7436,8 @@ _nl_send_nlmsghdr(NMPlatform                        *platform,
 
     nm_assert(nlhdr);
 
+    nm_assert(out_seq_result && *out_seq_result == WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
     seq              = _nlh_seq_next_get(priv, NMP_NETLINK_ROUTE);
     nlhdr->nlmsg_seq = seq;
 
@@ -7450,7 +7465,7 @@ again:
             errsv = errno;
             if (errsv == EINTR && try_count++ < 100)
                 goto again;
-            _LOGD("netlink: nl-send-nlmsghdr: failed sending message: %s (%d)",
+            _LOGI("netlink: nl-send-nlmsghdr: failed sending message: %s (%d)",
                   nm_strerror_native(errsv),
                   errsv);
             return -nm_errno_from_native(errsv);
@@ -7481,13 +7496,15 @@ _netlink_send_nlmsg(NMPlatform                        *platform,
     guint32                 seq;
     int                     nle;
 
+    nm_assert(!out_seq_result || *out_seq_result == WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
     nlhdr            = nlmsg_hdr(nlmsg);
     seq              = _nlh_seq_next_get(priv, netlink_protocol);
     nlhdr->nlmsg_seq = seq;
 
     nle = nl_send_auto(priv->sk_x[netlink_protocol], nlmsg);
     if (nle < 0) {
-        _LOGD("netlink: nl-send-nlmsg: failed sending message: %s (%d)", nm_strerror(nle), nle);
+        _LOGI("netlink: nl-send-nlmsg: failed sending message: %s (%d)", nm_strerror(nle), nle);
         return nle;
     }
 
@@ -8054,14 +8071,17 @@ do_add_link_with_lookup(NMPlatform            *platform,
                         const NMPlatformLink **out_link)
 {
     const NMPObject        *obj = NULL;
-    WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult seq_result;
     gs_free char           *extack_msg = NULL;
     int                     nle;
     char                    s_buf[256];
     NMPCache               *cache     = nm_platform_get_cache(platform);
+    int                     try_count = 0;
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, nlmsg, &seq_result, &extack_msg);
         if (nle < 0) {
             _LOGE("do-add-link[%s/%s]: failed sending netlink request \"%s\" (%d)",
@@ -8075,7 +8095,7 @@ do_add_link_with_lookup(NMPlatform            *platform,
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
         _NMLOG(seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK ? LOGL_DEBUG : LOGL_WARN,
                "do-add-link[%s/%s]: %s",
@@ -8083,6 +8103,9 @@ do_add_link_with_lookup(NMPlatform            *platform,
                nm_link_type_to_string(link_type),
                wait_for_nl_response_to_string(seq_result, extack_msg, s_buf, sizeof(s_buf)));
 
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
+
     if (out_link) {
         obj       = nmp_cache_lookup_link_full(cache, 0, name, FALSE, link_type, NULL, NULL);
         *out_link = NMP_OBJECT_CAST_LINK(obj);
@@ -8099,10 +8122,11 @@ do_add_addrroute(NMPlatform      *platform,
                  char           **out_extack_msg)
 {
     char                    sbuf1[NM_UTILS_TO_STRING_BUFFER_SIZE];
-    WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult seq_result;
     gs_free char           *extack_msg = NULL;
     int                     nle;
     char                    s_buf[256];
+    int                     try_count = 0;
 
     nm_assert(!out_extack_msg || !*out_extack_msg);
     nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(obj_id),
@@ -8113,6 +8137,8 @@ do_add_addrroute(NMPlatform      *platform,
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, nlmsg, &seq_result, &extack_msg);
         if (nle < 0) {
             _LOGE("do-add-%s[%s]: failure sending netlink request \"%s\" (%d)",
@@ -8126,7 +8152,7 @@ do_add_addrroute(NMPlatform      *platform,
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
         _NMLOG((seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK
                 || (suppress_netlink_failure && seq_result < 0))
@@ -8137,6 +8163,9 @@ do_add_addrroute(NMPlatform      *platform,
                nmp_object_to_string(obj_id, NMP_OBJECT_TO_STRING_ID, sbuf1, sizeof(sbuf1)),
                wait_for_nl_response_to_string(seq_result, extack_msg, s_buf, sizeof(s_buf)));
 
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
+
     if (NMP_OBJECT_GET_TYPE(obj_id) == NMP_OBJECT_TYPE_IP6_ADDRESS) {
         /* In rare cases, the object is not yet ready as we received the ACK from
          * kernel. Need to refetch.
@@ -8157,15 +8186,18 @@ static gboolean
 do_delete_object(NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *nlmsg)
 {
     char                    sbuf1[NM_UTILS_TO_STRING_BUFFER_SIZE];
-    WaitForNlResponseResult seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult seq_result;
     gs_free char           *extack_msg = NULL;
     int                     nle;
     char                    s_buf[256];
     gboolean                success;
     const char             *log_detail = "";
+    int                     try_count  = 0;
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, nlmsg, &seq_result, &extack_msg);
         if (nle < 0) {
             _LOGE("do-delete-%s[%s]: failure sending netlink request \"%s\" (%d)",
@@ -8178,7 +8210,7 @@ do_delete_object(NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *n
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
         success = TRUE;
         if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK) {
@@ -8206,6 +8238,9 @@ do_delete_object(NMPlatform *platform, const NMPObject *obj_id, struct nl_msg *n
                wait_for_nl_response_to_string(seq_result, extack_msg, s_buf, sizeof(s_buf)),
                log_detail);
 
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
+
     if (NM_IN_SET(NMP_OBJECT_GET_TYPE(obj_id),
                   NMP_OBJECT_TYPE_IP6_ADDRESS,
                   NMP_OBJECT_TYPE_QDISC,
@@ -8233,7 +8268,7 @@ do_change_link(NMPlatform           *platform,
 {
     nm_auto_pop_netns NMPNetns *netns = NULL;
     int                         nle;
-    WaitForNlResponseResult     seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult     seq_result;
     gs_free char               *extack_msg = NULL;
     char                        s_buf[256];
     int                         result;
@@ -8241,6 +8276,7 @@ do_change_link(NMPlatform           *platform,
     const char                 *log_detail;
     gs_free char               *log_detail_free = NULL;
     const NMPObject            *obj_cache;
+    int                         try_count = 0;
 
     if (!nm_platform_netns_push(platform, &netns)) {
         log_level  = LOGL_ERR;
@@ -8250,6 +8286,7 @@ do_change_link(NMPlatform           *platform,
     }
 
 retry:
+    seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
     result     = -NME_UNSPEC;
     log_level  = LOGL_WARN;
     log_detail = "";
@@ -8270,7 +8307,7 @@ retry:
 
     delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+    nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
     if (NM_IN_SET(seq_result, WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK, -EEXIST, -EADDRINUSE)) {
         log_level = LOGL_DEBUG;
@@ -8318,7 +8355,7 @@ out:
            wait_for_nl_response_to_string(seq_result, extack_msg, s_buf, sizeof(s_buf)),
            log_detail);
 
-    if (result == -EAGAIN)
+    if (result == -EAGAIN && ++try_count < RESYNC_RETRIES)
         goto retry;
 
     return result;
@@ -9841,9 +9878,10 @@ ip_route_get(NMPlatform   *platform,
 
         delayed_action_handle_all(platform);
 
-        /* Retry, if we failed due to a cache resync. That can happen when the netlink
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
-         * socket fills up and we lost the response. */
+
-    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC && ++try_count < 10);
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
 
     if (seq_result < 0) {
         /* negative seq_result is an errno from kernel. Map it to negative
@@ -9867,25 +9905,33 @@ ip_route_get(NMPlatform   *platform,
 static int
 routing_rule_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformRoutingRule *routing_rule)
 {
-    WaitForNlResponseResult      seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult      seq_result;
     nm_auto_nlmsg struct nl_msg *msg        = NULL;
     gs_free char                *extack_msg = NULL;
     char                         s_buf[256];
     int                          nle;
+    int                          try_count = 0;
 
     msg = _nl_msg_new_routing_rule(RTM_NEWRULE, flags, routing_rule);
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, msg, &seq_result, &extack_msg);
         if (nle < 0) {
-        _LOGE("do-add-rule: failed sending netlink request \"%s\" (%d)", nm_strerror(nle), -nle);
+            _LOGE("do-add-rule: failed sending netlink request \"%s\" (%d)",
+                  nm_strerror(nle),
+                  -nle);
             return -NME_PL_NETLINK;
         }
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
 
     _NMLOG(seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK ? LOGL_DEBUG : LOGL_WARN,
            "do-add-rule: %s",
@@ -9903,11 +9949,12 @@ routing_rule_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformRoutin
 static int
 qdisc_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformQdisc *qdisc)
 {
-    WaitForNlResponseResult      seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult      seq_result;
     gs_free char                *extack_msg = NULL;
     int                          nle;
     char                         s_buf[256];
     nm_auto_nlmsg struct nl_msg *msg       = NULL;
+    int                          try_count = 0;
 
     /* Note: @qdisc must not be copied or kept alive because the lifetime of qdisc.kind
      * is undefined. */
@@ -9916,15 +9963,22 @@ qdisc_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformQdisc *qdisc)
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, msg, &seq_result, &extack_msg);
         if (nle < 0) {
-        _LOGE("do-add-qdisc: failed sending netlink request \"%s\" (%d)", nm_strerror(nle), -nle);
+            _LOGE("do-add-qdisc: failed sending netlink request \"%s\" (%d)",
+                  nm_strerror(nle),
+                  -nle);
             return -NME_PL_NETLINK;
         }
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
 
     _NMLOG(seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK ? LOGL_DEBUG : LOGL_WARN,
            "do-add-qdisc: %s",
@@ -9944,7 +9998,7 @@ tc_delete(NMPlatform *platform,
           guint32     parent,
           gboolean    log_error)
 {
-    WaitForNlResponseResult      seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult      seq_result;
     gs_free char                *extack_msg = NULL;
     int                          nle;
     char                         s_buf[256];
@@ -9954,6 +10008,7 @@ tc_delete(NMPlatform *platform,
                   .tcm_ifindex = ifindex,
                   .tcm_parent  = parent,
     };
+    int try_count = 0;
 
     switch (nlmsg_type) {
     case RTM_DELQDISC:
@@ -9974,6 +10029,8 @@ tc_delete(NMPlatform *platform,
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, msg, &seq_result, &extack_msg);
         if (nle < 0) {
             _NMLOG(log_error ? LOGL_ERR : LOGL_DEBUG,
@@ -9986,7 +10043,10 @@ tc_delete(NMPlatform *platform,
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
+
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
 
     _NMLOG((seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK || !log_error) ? LOGL_DEBUG
                                                                                  : LOGL_WARN,
@@ -10013,11 +10073,12 @@ qdisc_delete(NMPlatform *platform, int ifindex, guint32 parent, gboolean log_err
 static int
 tfilter_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformTfilter *tfilter)
 {
-    WaitForNlResponseResult      seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
+    WaitForNlResponseResult      seq_result;
     gs_free char                *extack_msg = NULL;
     int                          nle;
     char                         s_buf[256];
     nm_auto_nlmsg struct nl_msg *msg       = NULL;
+    int                          try_count = 0;
 
     /* Note: @tfilter must not be copied or kept alive because the lifetime of tfilter.kind
      * and tfilter.action.kind is undefined. */
@@ -10026,20 +10087,27 @@ tfilter_add(NMPlatform *platform, NMPNlmFlags flags, const NMPlatformTfilter *tf
 
     event_handler_read_netlink(platform, NMP_NETLINK_ROUTE, FALSE);
 
+    do {
+        seq_result = WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN;
         nle        = _netlink_send_nlmsg_rtnl(platform, msg, &seq_result, &extack_msg);
         if (nle < 0) {
-        _LOGE("do-add-tfilter: failed sending netlink request \"%s\" (%d)", nm_strerror(nle), -nle);
+            _LOGE("do-add-tfilter: failed sending netlink request \"%s\" (%d)",
+                  nm_strerror(nle),
+                  -nle);
             return -NME_PL_NETLINK;
         }
 
         delayed_action_handle_all(platform);
 
-    nm_assert(seq_result);
+        nm_assert(seq_result != WAIT_FOR_NL_RESPONSE_RESULT_UNKNOWN);
 
         _NMLOG(seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK ? LOGL_DEBUG : LOGL_WARN,
                "do-add-tfilter: %s",
                wait_for_nl_response_to_string(seq_result, extack_msg, s_buf, sizeof(s_buf)));
 
+    } while (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_FAILED_RESYNC
+             && ++try_count < RESYNC_RETRIES);
+
     if (seq_result == WAIT_FOR_NL_RESPONSE_RESULT_RESPONSE_OK)
         return 0;