All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
To: dev@dpdk.org
Cc: pushpendra.kumar@tieto.com, reshma.pattan@intel.com,
	stephen@networkplumber.org, stable@dpdk.org,
	Pushpendra Kumar <pushpendra1x.kumar@intel.com>
Subject: [PATCH] pdump: fix request timeout on unresponsive secondary
Date: Wed,  1 Jul 2026 11:32:57 +0530	[thread overview]
Message-ID: <20260701060257.562895-1-pushpendra1x.kumar@intel.com> (raw)

When a primary handles a pdump enable/disable request from a secondary
(e.g. dpdk-dumpcap), it applies the callbacks locally and forwards the
request to the other secondaries before replying. The forward used
rte_mp_request_sync(), blocking up to MP_TIMEOUT_S for every secondary
to acknowledge.

A secondary that is attached but has not called rte_pdump_init() never
registers the mp_pdump action and silently drops the message (logging
only "Cannot find action: mp_pdump"). Commit c3ceb8742295 ("pdump:
forward callback enable to secondary process"), which added this
broadcast, appears to assume every secondary calls rte_pdump_init();
when one does not, the primary cannot tell it from a slow or dead peer
and blocks for the full timeout. That delay pushes the reply past the
requester's own timeout, so dpdk-dumpcap reports "Packet dump enable
... failed Connection timed out".

Forward the request asynchronously with rte_mp_request_async() so an
unresponsive secondary can no longer block the reply; a callback logs a
warning if not all secondaries acknowledge. The forward is still issued
before the requester reply, preserving the ordering from commit
928f43e3f9c1 ("pdump: fix race in disabling").

This slightly weakens the happens-before guarantee: the sync call was a
full-acknowledgement barrier, whereas the async path relies on AF_UNIX
SOCK_DGRAM FIFO ordering and the single mp_handle thread processing the
forward before the reply that triggers teardown. This suffices for the
disable race fixed by 928f43e3f9c1, but reviewers should confirm they
are comfortable with the weaker guarantee.

Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
Cc: stable@dpdk.org

Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
---
 .mailmap              |  1 +
 lib/pdump/rte_pdump.c | 23 ++++++++++++++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/.mailmap b/.mailmap
index c5bc728fae..c1313c7148 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1332,6 +1332,7 @@ Przemyslaw Gierszynski <przemyslaw.gierszynski@intel.com>
 Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
 Przemyslaw Zegan <przemyslawx.zegan@intel.com>
 Pu Xu <583493798@qq.com>
+Pushpendra Kumar <pushpendra1x.kumar@intel.com>
 Qi Fu <qi.fu@intel.com>
 Qi Zhang <qi.z.zhang@intel.com>
 Qian Hao <qi_an_hao@126.com>
diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index ac94efe7ff..f2d5b70111 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -477,11 +477,21 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
 	return ret;
 }
 
+/* Async reply handler; warns if any secondary did not respond. */
+static int
+pdump_secondary_reply(const struct rte_mp_msg *request __rte_unused,
+		      const struct rte_mp_reply *reply)
+{
+	if (reply->nb_sent != reply->nb_received)
+		PDUMP_LOG_LINE(ERR, "not all secondary's replied (sent %u recv %u)",
+			       reply->nb_sent, reply->nb_received);
+	return 0;
+}
+
 static void
 pdump_request_to_secondary(const struct pdump_request *req)
 {
 	struct rte_mp_msg mp_req = { };
-	struct rte_mp_reply mp_reply;
 	struct timespec ts = {.tv_sec = MP_TIMEOUT_S, .tv_nsec = 0};
 
 	PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary", pdump_opname(req->op));
@@ -490,14 +500,9 @@ pdump_request_to_secondary(const struct pdump_request *req)
 	strlcpy(mp_req.name, PDUMP_MP, sizeof(mp_req.name));
 	mp_req.len_param = sizeof(*req);
 
-	if (rte_mp_request_sync(&mp_req, &mp_reply, &ts) != 0)
-		PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed");
-
-	else if (mp_reply.nb_sent != mp_reply.nb_received)
-		PDUMP_LOG_LINE(ERR, "not all secondary's replied (sent %u recv %u)",
-			       mp_reply.nb_sent, mp_reply.nb_received);
-
-	free(mp_reply.msgs);
+	/* Forward asynchronously so an unresponsive secondary cannot block the requester reply. */
+	if (rte_mp_request_async(&mp_req, &ts, pdump_secondary_reply) != 0)
+		PDUMP_LOG_LINE(ERR, "rte_mp_request_async failed");
 }
 
 /* Allocate temporary storage for passing state to the alarm thread for deferred handling */
-- 
2.43.0


             reply	other threads:[~2026-07-01  6:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01  6:02 Pushpendra Kumar [this message]
2026-07-02  4:01 ` [PATCH] pdump: fix request timeout on unresponsive secondary Pushpendra Kumar
2026-07-03 15:52 ` Stephen Hemminger
2026-07-05  8:44 ` [PATCH v2] pdump: fix teardown race with opt-in MP request mode Pushpendra Kumar
2026-07-05 17:07   ` [PATCH v3] " Pushpendra Kumar
2026-07-05 18:27   ` [PATCH v2] " Stephen Hemminger
2026-07-05 21:08     ` Kumar, Pushpendra1X
2026-07-06  8:25       ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260701060257.562895-1-pushpendra1x.kumar@intel.com \
    --to=pushpendra1x.kumar@intel.com \
    --cc=dev@dpdk.org \
    --cc=pushpendra.kumar@tieto.com \
    --cc=reshma.pattan@intel.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.