* please help backporting some patches to stable release 24.11.7
From: luca.boccassi @ 2026-07-06 10:27 UTC (permalink / raw)
To: dpdk dev
Cc: dpdk stable, Ajit Khaparde, Anatoly Burakov, Andrew Rybchenko,
Anurag Mandal, Beilei Xing, Bruce Richardson, Ciara Loftus,
Claudia Cauli, Dakota Sicher, David Marchand, Ferruh Yigit,
Jian Wang, Jiawen Wu, Junfeng Guo, Konstantin Ananyev,
Marat Khalili, Maxime Peim, Mingjin Ye, Mohammad Shuab Siddique,
Qi Zhang, Somnath Kotur, Thomas Monjalon, Ting Xu, Tyler Retzlaff,
Vladimir Medvedkin, Xiaoyun Li, Zaiyu Wang
Hi commit authors (and maintainers),
Despite being selected by the DPDK maintenance tool ./devtools/git-log-fixes.sh
I didn't apply following commits from DPDK main to 24.11
stable branch, as conflicts or build errors occur.
Can authors check your patches in the following list and either:
- Backport your patches to the 24.11 branch, or
- Indicate that the patch should not be backported
Please do either of the above by 2026/07/10.
You can find the a temporary work-in-progress branch of the coming 24.11.7
release at:
https://github.com/bluca/dpdk-stable
It is recommended to backport on top of that to minimize further conflicts or
misunderstandings.
Some notes on stable backports:
A backport should contain a reference to the DPDK main branch commit
in it's commit message in the following fashion:
[ upstream commit <commit's dpdk main branch SHA-1 checksum> ]
For example:
https://git.dpdk.org/dpdk-stable/commit/?h=18.11&id=d90e6ae6f936ecdc2fd3811ff9f26aec7f3c06eb
When sending the backported patch, please indicate the target branch in the
subject line, as we have multiple branches, for example:
[PATCH 24.11] foo/bar: fix baz
With git format-patch, this can be achieved by appending the parameter:
--subject-prefix='PATCH 24.11'
Send the backported patch to "stable@dpdk.org" but not "dev@dpdk.org".
FYI, branch 24.11 is located at tree:
https://git.dpdk.org/dpdk-stable
Thanks.
Luca Boccassi
---
ece7d7eef0 Anurag Mandal net/iavf: fix duplicate VF reset during PF reset recovery
8e038dee42 Bruce Richardson net/idpf: fix Tx of large mbuf segments
edff93f638 Dakota Sicher net/bnxt: skip timed Tx pacing setup when unsupported
1c39f3f240 Marat Khalili bpf/validate: fix BPF_JMP empty range handling
7f8ce8d13f Marat Khalili bpf/validate: fix EBPF_JSLT | BPF_X evaluation
b482a764c6 Maxime Peim eal: fix index for non-EAL lcores
26e7538e00 Zaiyu Wang net/txgbe: fix link flow control config for Sapphire
e586adbac0 Zaiyu Wang net/txgbe: fix mass of unknown interrupts
^ permalink raw reply
* Re: [PATCH] net/ice: gate send on timestamp offload to E830
From: Bruce Richardson @ 2026-07-06 10:18 UTC (permalink / raw)
To: Soumyadeep Hore; +Cc: dev, aman.deep.singh, manoj.kumar.subbarao
In-Reply-To: <20260704115806.372175-1-soumyadeep.hore@intel.com>
On Sat, Jul 04, 2026 at 05:28:06PM +0530, Soumyadeep Hore wrote:
> The SEND_ON_TIMESTAMP TX offload capability was unconditionally
> advertised for all ice devices in non-safe mode. However, only E830
> hardware supports TxPP (Tx Packet Pacing) via the TXTIME queue
> mechanism. E810 and other ice devices correctly reject the offload
> at queue setup time (ice_tx_queue_setup checks hw->phy_model), but
> advertising the capability misleads applications into thinking the
> feature is available.
>
> Gate the SEND_ON_TIMESTAMP capability advertisement on
> hw->phy_model == ICE_PHY_E830 so that only devices which actually
> support TxPP report the capability.
>
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---
Fixes: 0b6ff09a1f19 ("net/intel: support Tx packet pacing for E830")
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net-intel.
Thanks,
/Bruce
> drivers/net/intel/ice/ice_ethdev.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 0f2e7aee14..030d53aded 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -4568,8 +4568,10 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
> RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO |
> RTE_ETH_TX_OFFLOAD_GRE_TNL_TSO |
> RTE_ETH_TX_OFFLOAD_IPIP_TNL_TSO |
> - RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO |
> - RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP;
> + RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO;
> + if (hw->phy_model == ICE_PHY_E830)
> + dev_info->tx_offload_capa |=
> + RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP;
> dev_info->flow_type_rss_offloads |= ICE_RSS_OFFLOAD_ALL;
> }
>
> --
> 2.34.1
>
^ permalink raw reply
* [PATCH v2] crypto/virtio: cookies are allocated from mempool
From: Radu Nicolau @ 2026-07-06 9:57 UTC (permalink / raw)
To: dev
Cc: maxime.coquelin, david.marchand, Radu Nicolau, jianjay.zhou,
Yu Jiang, Fan Zhang
In-Reply-To: <20260703144931.2541180-1-radu.nicolau@intel.com>
The Rx/Tx functions allocate cookies as needed, no need to
allocate and free from heap.
Fixes: 6f0175ff53e0 ("crypto/virtio: support basic PMD ops")
Cc: jianjay.zhou@huawei.com
Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Tested-by: Yu Jiang <yux.jiang@intel.com>
---
v2: removed redundant mempool free
drivers/crypto/virtio/virtio_cryptodev.c | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_cryptodev.c b/drivers/crypto/virtio/virtio_cryptodev.c
index 6f079f15f6..d70651646e 100644
--- a/drivers/crypto/virtio/virtio_cryptodev.c
+++ b/drivers/crypto/virtio/virtio_cryptodev.c
@@ -68,7 +68,6 @@ void
virtio_crypto_queue_release(struct virtqueue *vq)
{
struct virtio_crypto_hw *hw;
- uint16_t i;
PMD_INIT_FUNC_TRACE();
@@ -80,9 +79,6 @@ virtio_crypto_queue_release(struct virtqueue *vq)
hw->vqs[vq->vq_queue_index] = NULL;
rte_memzone_free(vq->mz);
rte_mempool_free(vq->mpool);
- for (i = 0; i < vq->vq_nentries; i++)
- rte_free(vq->vq_descx[i].cookie);
-
rte_free(vq);
}
}
@@ -102,8 +98,6 @@ virtio_crypto_queue_setup(struct rte_cryptodev *dev,
unsigned int vq_size;
struct virtio_crypto_hw *hw = dev->data->dev_private;
struct virtqueue *vq = NULL;
- uint32_t i = 0;
- uint32_t j;
PMD_INIT_FUNC_TRACE();
@@ -175,29 +169,12 @@ virtio_crypto_queue_setup(struct rte_cryptodev *dev,
"Cannot create mempool");
goto mpool_create_err;
}
- for (i = 0; i < nb_desc; i++) {
- vq->vq_descx[i].cookie =
- rte_zmalloc("crypto PMD op cookie pointer",
- sizeof(struct virtio_crypto_op_cookie),
- RTE_CACHE_LINE_SIZE);
- if (vq->vq_descx[i].cookie == NULL) {
- VIRTIO_CRYPTO_DRV_LOG_ERR("Failed to "
- "alloc mem for cookie");
- goto cookie_alloc_err;
- }
- }
}
*pvq = vq;
return 0;
-cookie_alloc_err:
- rte_mempool_free(vq->mpool);
- if (i != 0) {
- for (j = 0; j < i; j++)
- rte_free(vq->vq_descx[j].cookie);
- }
mpool_create_err:
rte_free(vq);
return -ENOMEM;
--
2.52.0
^ permalink raw reply related
* RE: [PATCH] test/bpf_validate: use unit_test_suite_runner
From: Marat Khalili @ 2026-07-06 9:27 UTC (permalink / raw)
To: Marat Khalili, Konstantin Ananyev
Cc: dev@dpdk.org, thomas@monjalon.net, stephen@networkplumber.org
In-Reply-To: <20260703152910.80285-1-marat.khalili@huawei.com>
> > @@ -2267,5 +2183,47 @@ test_mem_ldx_dw_heap(void)
> > });
> > }
> >
> > -REGISTER_ATTIC_TEST(bpf_validate_mem_ldx_dw_heap_autotest,
> > - test_mem_ldx_dw_heap);
> > +static struct
> > +unit_test_suite test_bpf_validate_suite = {
> > + .suite_name = "Test BPF Validate Unit Test Suite",
> > + .unit_test_cases = {
> > + TEST_CASE(test_alu64_add_k),
Received some additional AI comments that are not visible in patchwork (?).
> **Missing blank line before suite definition**
>
> The static structure `test_bpf_validate_suite` should be separated from the
> preceding function `test_mem_ldx_dw_heap()` by a blank line for consistency
> with DPDK formatting conventions.
The model seems to be confused, the structure is clearly separated.
> **Extra space in structure name line**
This one is true BUT seems to be the majority style for some reason.
If fixed, this needs to be fixed in 49 other places.
> The suite name "Test BPF Validate Unit Test Suite" is somewhat redundant
> ("Test" appears twice). [...] This is a minor style preference; the current
> name is acceptable.
I mostly agree with this one, though some (not the majority) of the other
suites also use this style. But as model said, the current name is acceptable.
Net result, I do not plan to send another revision in the absence of other
comments or opinions.
^ permalink raw reply
* [PATCH] net/crc: add 4x folding loop for aarch64 NEON implementation
From: Shreesh Adiga @ 2026-07-06 9:24 UTC (permalink / raw)
To: Wathsala Vithanage; +Cc: dev
In-Reply-To: <20260703073118.788f4324@phoenix.local>
Add a 64-byte loop that maintains 4 fold registers and processes
64 bytes at a time. The 4x fold registers is then reduced to 16 byte
single fold, similar to x86 SSE implementation. This technique is
described in the paper by Intel:
"Fast CRC Computation for Generic Polynomials Using PCLMULQDQ Instruction"
This results in roughly 2x performance improvement due to better ILP
for large input sizes like 1024 observed on Cortex-X925.
Also removed the special handling of len 16 to 31 similar to x86 SSE
implementation.
Signed-off-by: Shreesh Adiga <16567adigashreesh@gmail.com>
---
v2 changes removes special handling for len 16-31 similar to x86
SSE implementation.
lib/net/net_crc_neon.c | 103 +++++++++++++++++++++++------------------
1 file changed, 58 insertions(+), 45 deletions(-)
diff --git a/lib/net/net_crc_neon.c b/lib/net/net_crc_neon.c
index cee75ddd31..d924aed25d 100644
--- a/lib/net/net_crc_neon.c
+++ b/lib/net/net_crc_neon.c
@@ -16,6 +16,7 @@
/** PMULL CRC computation context structure */
struct crc_pmull_ctx {
uint64x2_t rk1_rk2;
+ uint64x2_t rk3_rk4;
uint64x2_t rk5_rk6;
uint64x2_t rk7_rk8;
};
@@ -136,54 +137,69 @@ crc32_eth_calc_pmull(
temp = vreinterpretq_u64_u32(vsetq_lane_u32(crc, vmovq_n_u32(0), 0));
/**
- * Folding all data into single 16 byte data block
- * Assumes: fold holds first 16 bytes of data
+ * Folding all data into 4 parallel 16 byte data block
+ * Later folds 4 parallel blocks into single fold block
*/
- if (unlikely(data_len < 32)) {
- if (unlikely(data_len == 16)) {
- /* 16 bytes */
- fold = vld1q_u64((const uint64_t *)data);
- fold = veorq_u64(fold, temp);
- goto reduction_128_64;
- }
+ if (likely(data_len >= 64)) {
+ uint64x2_t fold1, fold2, fold3, fold4;
+ uint64x2_t temp1, temp2, temp3, temp4;
+ fold1 = vld1q_u64((const uint64_t *)(data + 0));
+ fold2 = vld1q_u64((const uint64_t *)(data + 16));
+ fold3 = vld1q_u64((const uint64_t *)(data + 32));
+ fold4 = vld1q_u64((const uint64_t *)(data + 48));
+ fold1 = veorq_u64(fold1, temp);
+ k = params->rk1_rk2;
- if (unlikely(data_len < 16)) {
- /* 0 to 15 bytes */
- alignas(16) uint8_t buffer[16];
-
- memset(buffer, 0, sizeof(buffer));
- memcpy(buffer, data, data_len);
-
- fold = vld1q_u64((uint64_t *)buffer);
- fold = veorq_u64(fold, temp);
- if (unlikely(data_len < 4)) {
- fold = vshift_bytes_left(fold, 8 - data_len);
- goto barret_reduction;
- }
- fold = vshift_bytes_left(fold, 16 - data_len);
- goto reduction_128_64;
+ for (n = 64; (n + 64) <= data_len; n += 64) {
+ temp1 = vld1q_u64((const uint64_t *)&data[n + 0]);
+ temp2 = vld1q_u64((const uint64_t *)&data[n + 16]);
+ temp3 = vld1q_u64((const uint64_t *)&data[n + 32]);
+ temp4 = vld1q_u64((const uint64_t *)&data[n + 48]);
+ fold1 = crcr32_folding_round(temp1, k, fold1);
+ fold2 = crcr32_folding_round(temp2, k, fold2);
+ fold3 = crcr32_folding_round(temp3, k, fold3);
+ fold4 = crcr32_folding_round(temp4, k, fold4);
}
- /* 17 to 31 bytes */
- fold = vld1q_u64((const uint64_t *)data);
+ k = params->rk3_rk4;
+ fold1 = crcr32_folding_round(fold2, k, fold1);
+ fold1 = crcr32_folding_round(fold3, k, fold1);
+ fold = crcr32_folding_round(fold4, k, fold1);
+ goto single_fold_loop;
+ }
+
+ if (unlikely(data_len < 16)) {
+ /* 0 to 15 bytes */
+ alignas(16) uint8_t buffer[16];
+
+ memset(buffer, 0, sizeof(buffer));
+ memcpy(buffer, data, data_len);
+
+ fold = vld1q_u64((uint64_t *)buffer);
fold = veorq_u64(fold, temp);
- n = 16;
- k = params->rk1_rk2;
- goto partial_bytes;
+ if (unlikely(data_len < 4)) {
+ fold = vshift_bytes_left(fold, 8 - data_len);
+ goto barret_reduction;
+ }
+ fold = vshift_bytes_left(fold, 16 - data_len);
+ goto reduction_128_64;
}
- /** At least 32 bytes in the buffer */
+ /** At least 16 bytes in the buffer */
/** Apply CRC initial value */
fold = vld1q_u64((const uint64_t *)data);
fold = veorq_u64(fold, temp);
- /** Main folding loop - the last 16 bytes is processed separately */
- k = params->rk1_rk2;
- for (n = 16; (n + 16) <= data_len; n += 16) {
+ /** Single folding loop - the last 16 bytes is processed separately */
+ k = params->rk3_rk4;
+ n = 16;
+
+single_fold_loop:
+ for (; (n + 16) <= data_len; n += 16) {
temp = vld1q_u64((const uint64_t *)&data[n]);
fold = crcr32_folding_round(temp, k, fold);
}
-partial_bytes:
+ /** Partial bytes - process last <16 bytes */
if (likely(n < data_len)) {
uint64x2_t last16, a, b, mask;
uint32_t rem = data_len & 15;
@@ -194,15 +210,8 @@ crc32_eth_calc_pmull(
mask = vshift_bytes_left(vdupq_n_u64(-1), 16 - rem);
b = vorrq_u64(b, vandq_u64(mask, last16));
- /* k = rk1 & rk2 */
- temp = vreinterpretq_u64_p128(vmull_p64(
- vgetq_lane_p64(vreinterpretq_p64_u64(a), 1),
- vgetq_lane_p64(vreinterpretq_p64_u64(k), 0)));
- fold = vreinterpretq_u64_p128(vmull_p64(
- vgetq_lane_p64(vreinterpretq_p64_u64(a), 0),
- vgetq_lane_p64(vreinterpretq_p64_u64(k), 1)));
- fold = veorq_u64(fold, temp);
- fold = veorq_u64(fold, b);
+ /* k = rk3 & rk4 */
+ fold = crcr32_folding_round(b, k, a);
}
/** Reduction 128 -> 32 Assumes: fold holds 128bit folded data */
@@ -221,22 +230,26 @@ void
rte_net_crc_neon_init(void)
{
/* Initialize CRC16 data */
- uint64_t ccitt_k1_k2[2] = {0x189aeLLU, 0x8e10LLU};
+ uint64_t ccitt_k1_k2[2] = {0x14ff2LLU, 0x19a3cLLU};
+ uint64_t ccitt_k3_k4[2] = {0x189aeLLU, 0x8e10LLU};
uint64_t ccitt_k5_k6[2] = {0x189aeLLU, 0x114aaLLU};
uint64_t ccitt_k7_k8[2] = {0x11c581910LLU, 0x10811LLU};
/* Initialize CRC32 data */
- uint64_t eth_k1_k2[2] = {0xccaa009eLLU, 0x1751997d0LLU};
+ uint64_t eth_k1_k2[2] = {0x1c6e41596LLU, 0x154442bd4LLU};
+ uint64_t eth_k3_k4[2] = {0xccaa009eLLU, 0x1751997d0LLU};
uint64_t eth_k5_k6[2] = {0xccaa009eLLU, 0x163cd6124LLU};
uint64_t eth_k7_k8[2] = {0x1f7011640LLU, 0x1db710641LLU};
/** Save the params in context structure */
crc16_ccitt_pmull.rk1_rk2 = vld1q_u64(ccitt_k1_k2);
+ crc16_ccitt_pmull.rk3_rk4 = vld1q_u64(ccitt_k3_k4);
crc16_ccitt_pmull.rk5_rk6 = vld1q_u64(ccitt_k5_k6);
crc16_ccitt_pmull.rk7_rk8 = vld1q_u64(ccitt_k7_k8);
/** Save the params in context structure */
crc32_eth_pmull.rk1_rk2 = vld1q_u64(eth_k1_k2);
+ crc32_eth_pmull.rk3_rk4 = vld1q_u64(eth_k3_k4);
crc32_eth_pmull.rk5_rk6 = vld1q_u64(eth_k5_k6);
crc32_eth_pmull.rk7_rk8 = vld1q_u64(eth_k7_k8);
}
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4 1/2] eal: support dmabuf
From: Anders Franzen @ 2026-07-03 19:22 UTC (permalink / raw)
To: dev; +Cc: cburdick
In-Reply-To: <31c2f2b6-fbdb-4d1c-b6d8-5659e0182f8b.ref@yahoo.se>
[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]
Hi Cliff and EAL Maintainers,
While testing the v4 patch series for a side-by-side CPU vs GPU low-latency
forwarding comparison using DPDK 26.03, we uncovered an unintended backward
compatibility break inside the legacy rte_extmem_register() wrapper path.
Currently, legacy applications calling the standard primitive fail
immediately
on the first page iteration with EINVAL. There are two distinct issues:
1. In rte_exmem_register, the legacy wrapper routes through
rte_extmem_register_dmabuf() passing a hardcoded -1 for the file
descriptor.
However, rte_extmem_register_dmabuf() enforces a strict guard check
right at
the entry gate:
if (dmabuf_fd < 0) { return -EINVAL; }
This rejects all legacy CPU host memory allocation calls. The legacy
wrapper
should instead bypass the experimental gate and map straight to the
underlying
static extmem_register() call.
2. In malloc_heap_create_external_seg,
Variable Shadowing/Corruption: Inside the page initialization tracking
loop,
the master list index iterator variable is clobbered by the sub-page
loop counter 'i'.
As a result:
eal_memseg_list_set_dmabuf_info(i, -1, 0);
targets a stale loop counter index instead of preserving the allocated
master memseg list.
This leaves the real segment list sitting at its zero-initialized
default (dmabuf_fd=0),
which poisons subsequent driver lookups (e.g. mlx5 trying to map
standard CPU frames
via reg_dmabuf_mr because fd=0 matches standard stdin instead of a clean
-1).
Fixing it requires caching 'i' into a persistent local tracker variable
(e.g., msl_idx)
immediately following the successful rte_fbarray_init() block, and
passing that
cached tracker to eal_memseg_list_set_dmabuf_info().
[-- Attachment #2: Type: text/html, Size: 7679 bytes --]
^ permalink raw reply
* [PATCH v3] net/virtio-user: fix eventfd sharing in secondary process
From: Samar Yadav @ 2026-07-03 10:25 UTC (permalink / raw)
To: dev; +Cc: chenbox, maxime.coquelin, stable, stephen, tiwei.bie, Samar Yadav
In-Reply-To: <20260629120615.2701697-1-samaryadav5@gmail.com>
virtio_user secondary processes cannot communicate with the vhost
backend: the kick/call eventfds are opened by the primary and never
shared, so a secondary's queue notification writes to an invalid fd
and traffic stalls.
Share the fds over a dedicated virtio-user multiprocess channel. The
primary registers a process-wide MP action that returns a port's
kick/call fds (looked up by port name); a secondary requests them at
probe time, before the port is announced.
The received fds are stored in eth_dev->process_private, which is
per-process, instead of the primary-owned shared dev->kickfds and
dev->callfds arrays; the secondary data path notifies the backend using
its own kickfd. In the primary, the MP handler reads the fd arrays under
dev->mutex, and the teardown path takes the same lock while closing and
freeing them, so the two cannot race.
Also fix the pre-existing pthread_mutex_init(&dev->mutex, NULL) call in
virtio_user_dev_init(): POSIX requires PTHREAD_PROCESS_SHARED for a mutex
stored in shared memory regardless of which processes actually lock it;
use rte_thread_mutex_init_shared() as other multiprocess-aware drivers do.
Fixes: 1c8489da561b ("net/virtio-user: fix multi-process support")
Cc: tiwei.bie@intel.com
Cc: stable@dpdk.org
Signed-off-by: Samar Yadav <samaryadav5@gmail.com>
---
v3:
- Reorder new #includes: system headers first, then DPDK lib headers
(alphabetized), then local virtio_* headers, per project convention.
- Name the "2" in eventfd count/index arithmetic (kick + call per queue)
as VIRTIO_USER_EVENTFDS_PER_QUEUE instead of a bare literal. Leaves
the pre-existing max_queue_pairs * 2 vring math untouched, since that
is a different multiplier (vrings per queue pair) and already matches
this file's existing style.
- Name the 5-second MP reply timeout in virtio_user_sync_eventfds() as
VIRTIO_USER_MP_TIMEOUT_SEC with a comment on the rationale.
v2:
- Use rte_calloc() instead of rte_malloc() for the kick/call fd arrays
in virtio_user_sync_eventfds() so allocation failures are cleaner and
unset entries are zero-initialised before the explicit -1 sentinel loop.
(Stephen Hemminger)
- Fix the pre-existing pthread_mutex_init(&dev->mutex, NULL) call in
virtio_user_dev_init() to use rte_thread_mutex_init_shared(), which
sets PTHREAD_PROCESS_SHARED as POSIX requires for a mutex in shared
memory. Add #include <rte_thread.h> to pull in the declaration.
(Stephen Hemminger)
.mailmap | 1 +
.../net/virtio/virtio_user/virtio_user_dev.c | 59 +++-
.../net/virtio/virtio_user/virtio_user_dev.h | 23 ++
drivers/net/virtio/virtio_user_ethdev.c | 276 +++++++++++++++++-
4 files changed, 350 insertions(+), 9 deletions(-)
diff --git a/.mailmap b/.mailmap
index 4001e5f..8f921d4 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1448,6 +1448,7 @@ Salem Sol <salems@nvidia.com>
Sam Andrew <samandrew@microsoft.com>
Sam Chen <sam.chen@nebula-matrix.com>
Sam Grove <sam.grove@sifive.com>
+Samar Yadav <samaryadav5@gmail.com> <samar.yadav@broadcom.com>
Sameer Vaze <svaze@qti.qualcomm.com>
Sameh Gobriel <sameh.gobriel@intel.com>
Samik Gupta <samik.gupta@broadcom.com>
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index f3df73c..5663b24 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -15,6 +15,7 @@
#include <sys/stat.h>
#include <rte_alarm.h>
+#include <rte_thread.h>
#include <rte_string_fns.h>
#include <rte_eal_memconfig.h>
#include <rte_malloc.h>
@@ -34,6 +35,54 @@ const char * const virtio_user_backend_strings[] = {
[VIRTIO_USER_BACKEND_VHOST_VDPA] = "VHOST_VDPA",
};
+/*
+ * Collect the primary device's kick/call fds (interleaved kick,call per queue)
+ * for sharing with a secondary process. Caller must serialize against the
+ * control path (dev->mutex) so the fd arrays are not freed concurrently.
+ */
+int
+virtio_user_get_eventfds_from_dev(struct virtio_user_dev *dev,
+ int fds[VIRTIO_USER_MAX_EVENTFDS])
+{
+ uint32_t max_queues;
+ int i, total_fds = 0;
+ int kickfd, callfd;
+
+ if (dev == NULL || fds == NULL)
+ return -EINVAL;
+
+ if (dev->kickfds == NULL || dev->callfds == NULL) {
+ PMD_INIT_LOG(ERR, "Device eventfd arrays not initialized");
+ return -EINVAL;
+ }
+
+ max_queues = dev->max_queue_pairs * 2;
+ if (dev->hw_cvq)
+ max_queues += 1;
+
+ if (max_queues * VIRTIO_USER_EVENTFDS_PER_QUEUE > VIRTIO_USER_MAX_EVENTFDS) {
+ PMD_INIT_LOG(ERR,
+ "Device needs %u eventfds, exceeds MP limit %d",
+ max_queues * VIRTIO_USER_EVENTFDS_PER_QUEUE,
+ VIRTIO_USER_MAX_EVENTFDS);
+ return -E2BIG;
+ }
+
+ for (i = 0; i < (int)max_queues; i++) {
+ kickfd = dev->kickfds[i];
+ callfd = dev->callfds[i];
+ if (kickfd < 0 || callfd < 0) {
+ PMD_INIT_LOG(ERR, "Queue %d has invalid fds (kick=%d call=%d)",
+ i, kickfd, callfd);
+ return -EINVAL;
+ }
+ fds[total_fds++] = kickfd;
+ fds[total_fds++] = callfd;
+ }
+
+ return total_fds;
+}
+
static int
virtio_user_uninit_notify_queue(struct virtio_user_dev *dev, uint32_t queue_sel)
{
@@ -733,7 +782,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, uint16_t queues,
{
uint64_t backend_features;
- pthread_mutex_init(&dev->mutex, NULL);
+ rte_thread_mutex_init_shared(&dev->mutex);
strlcpy(dev->path, path, PATH_MAX);
dev->started = 0;
@@ -865,9 +914,15 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
rte_mem_event_callback_unregister(VIRTIO_USER_MEM_EVENT_CLB_NAME, dev);
+ /*
+ * Serialize closing/freeing the kick/call fd arrays against the MP
+ * handler, which reads them under the same lock to share them with
+ * secondary processes.
+ */
+ pthread_mutex_lock(&dev->mutex);
virtio_user_dev_uninit_notify(dev);
-
virtio_user_free_vrings(dev);
+ pthread_mutex_unlock(&dev->mutex);
free(dev->ifname);
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 66400b3..32d0b12 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -11,6 +11,14 @@
#include "../virtio.h"
#include "../virtio_ring.h"
+#include <rte_eal.h>
+
+/* Max eventfds shareable over the MP channel (bounded by SCM_RIGHTS). */
+#define VIRTIO_USER_MAX_EVENTFDS RTE_MP_MAX_FD_NUM
+
+/* Each queue contributes one kick fd and one call fd to the MP channel. */
+#define VIRTIO_USER_EVENTFDS_PER_QUEUE 2
+
enum virtio_user_backend_type {
VIRTIO_USER_BACKEND_UNKNOWN,
VIRTIO_USER_BACKEND_VHOST_USER,
@@ -89,5 +97,20 @@ int virtio_user_dev_get_rss_config(struct virtio_user_dev *dev, void *dst, size_
int length);
void virtio_user_dev_delayed_disconnect_handler(void *param);
int virtio_user_dev_server_reconnect(struct virtio_user_dev *dev);
+
+/**
+ * Collect a primary device's kick/call eventfds for sharing with a
+ * secondary process over the multiprocess channel.
+ *
+ * @param dev
+ * Pointer to the virtio_user device (primary).
+ * @param fds
+ * Output array, must hold at least VIRTIO_USER_MAX_EVENTFDS elements.
+ * @return
+ * Number of fds written on success, negative errno on error.
+ */
+int virtio_user_get_eventfds_from_dev(struct virtio_user_dev *dev,
+ int fds[VIRTIO_USER_MAX_EVENTFDS]);
+
extern const char * const virtio_user_backend_strings[];
#endif
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 747ddde..191f0c2 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -2,6 +2,7 @@
* Copyright(c) 2010-2016 Intel Corporation
*/
+#include <errno.h>
#include <stdint.h>
#include <stdlib.h>
#include <sys/types.h>
@@ -12,13 +13,16 @@
#include <sys/sysmacros.h>
#include <sys/socket.h>
-#include <rte_malloc.h>
-#include <rte_kvargs.h>
-#include <ethdev_vdev.h>
-#include <bus_vdev_driver.h>
#include <rte_alarm.h>
#include <rte_cycles.h>
+#include <rte_errno.h>
#include <rte_io.h>
+#include <rte_kvargs.h>
+#include <rte_malloc.h>
+#include <rte_spinlock.h>
+#include <rte_string_fns.h>
+#include <ethdev_vdev.h>
+#include <bus_vdev_driver.h>
#include "virtio_ethdev.h"
#include "virtio_logs.h"
@@ -28,6 +32,39 @@
#include "virtio_user/virtio_user_dev.h"
#include "virtio_user/vhost.h"
+/* Virtio-user multiprocess communication channel */
+#define VIRTIO_USER_MP_NAME "virtio_user_mp"
+
+/*
+ * How long a secondary waits for the primary to reply with eventfds.
+ * The primary's MP handler does no blocking work, so a slow reply only
+ * means the EAL MP thread is backed up; 5s gives ample margin without
+ * stalling probe for too long if the primary is gone.
+ */
+#define VIRTIO_USER_MP_TIMEOUT_SEC 5
+
+struct virtio_user_mp_param {
+ char port_name[RTE_DEV_NAME_MAX_LEN];
+};
+
+/*
+ * Per-process private data, referenced by eth_dev->process_private which (unlike
+ * dev_private) is NOT shared between primary and secondary processes. A secondary
+ * stores the kick/call fds it receives from the primary here, so it never mutates
+ * the primary-owned shared dev->kickfds/dev->callfds arrays. callfds are kept for
+ * a complete per-process view of the backend fds; only kickfds are used by the
+ * secondary data path today.
+ */
+struct virtio_user_proc_priv {
+ uint32_t nr_queues;
+ int *kickfds;
+ int *callfds;
+};
+
+/* Guards one-time registration of the process-wide MP action. */
+static rte_spinlock_t virtio_user_mp_lock = RTE_SPINLOCK_INITIALIZER;
+static bool virtio_user_mp_registered;
+
#define virtio_user_get_dev(hwp) container_of(hwp, struct virtio_user_dev, hw)
static void
@@ -269,6 +306,26 @@ virtio_user_del_queue(struct virtio_hw *hw, struct virtqueue *vq)
virtio_user_dev_destroy_shadow_cvq(dev);
}
+/*
+ * Return the kick fd to notify the backend for a queue in the running process.
+ * The secondary uses its own fds (process_private); the primary owns dev->kickfds.
+ */
+static int
+virtio_user_get_kickfd(struct virtio_hw *hw, struct virtio_user_dev *dev,
+ uint16_t queue_idx)
+{
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ struct rte_eth_dev *eth_dev = &rte_eth_devices[hw->port_id];
+ struct virtio_user_proc_priv *pp = eth_dev->process_private;
+
+ if (pp == NULL || queue_idx >= pp->nr_queues)
+ return -1;
+ return pp->kickfds[queue_idx];
+ }
+
+ return dev->kickfds[queue_idx];
+}
+
static void
virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
{
@@ -282,8 +339,10 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
}
if (!dev->notify_area) {
- if (write(dev->kickfds[vq->vq_queue_index], ¬ify_data,
- sizeof(notify_data)) < 0)
+ int kickfd = virtio_user_get_kickfd(hw, dev, vq->vq_queue_index);
+
+ if (kickfd < 0 || write(kickfd, ¬ify_data,
+ sizeof(notify_data)) < 0)
PMD_DRV_LOG(ERR, "failed to kick backend: %s",
strerror(errno));
return;
@@ -495,6 +554,166 @@ virtio_user_eth_dev_free(struct rte_eth_dev *eth_dev)
rte_eth_dev_release_port(eth_dev);
}
+/* Close and free a secondary's per-process eventfd storage. */
+static void
+virtio_user_free_proc_priv(struct rte_eth_dev *eth_dev)
+{
+ struct virtio_user_proc_priv *pp = eth_dev->process_private;
+ uint32_t i;
+
+ if (pp == NULL)
+ return;
+
+ for (i = 0; i < pp->nr_queues; i++) {
+ if (pp->kickfds != NULL && pp->kickfds[i] >= 0)
+ close(pp->kickfds[i]);
+ if (pp->callfds != NULL && pp->callfds[i] >= 0)
+ close(pp->callfds[i]);
+ }
+
+ rte_free(pp->kickfds);
+ rte_free(pp->callfds);
+ rte_free(pp);
+ eth_dev->process_private = NULL;
+}
+
+/*
+ * Primary-side MP handler: reply with this port's kick/call eventfds so the
+ * requesting secondary can talk to the vhost backend. Always sends a reply
+ * (num_fds == 0 on error) so the secondary fails fast instead of timing out.
+ */
+static int
+virtio_user_mp_primary_handler(const struct rte_mp_msg *msg, const void *peer)
+{
+ const struct virtio_user_mp_param *param =
+ (const struct virtio_user_mp_param *)msg->param;
+ int eventfds[VIRTIO_USER_MAX_EVENTFDS];
+ struct rte_eth_dev *eth_dev;
+ struct virtio_user_dev *dev;
+ struct rte_mp_msg reply;
+ int num_fds;
+ int i;
+
+ memset(&reply, 0, sizeof(reply));
+ strlcpy(reply.name, msg->name, sizeof(reply.name));
+ reply.len_param = 0;
+ reply.num_fds = 0;
+
+ eth_dev = rte_eth_dev_get_by_name(param->port_name);
+ if (eth_dev == NULL || eth_dev->data->dev_private == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to find virtio_user port: %s",
+ param->port_name);
+ return rte_mp_reply(&reply, peer);
+ }
+
+ dev = eth_dev->data->dev_private;
+
+ /* serialize against control-path changes to the fd arrays */
+ pthread_mutex_lock(&dev->mutex);
+ num_fds = virtio_user_get_eventfds_from_dev(dev, eventfds);
+ if (num_fds >= 0 && num_fds <= RTE_MP_MAX_FD_NUM) {
+ reply.num_fds = num_fds;
+ for (i = 0; i < num_fds; i++)
+ reply.fds[i] = eventfds[i];
+ } else {
+ PMD_INIT_LOG(ERR, "Cannot share eventfds for %s (ret=%d)",
+ param->port_name, num_fds);
+ }
+ pthread_mutex_unlock(&dev->mutex);
+
+ return rte_mp_reply(&reply, peer);
+}
+
+/*
+ * Secondary-side: request the primary's kick/call eventfds and store them in
+ * this process's eth_dev->process_private. The shared dev->kickfds/dev->callfds
+ * arrays (owned by the primary) are never touched.
+ */
+static int
+virtio_user_sync_eventfds(struct rte_eth_dev *eth_dev, struct virtio_user_dev *dev)
+{
+ struct rte_mp_msg mp_req, *mp_rep;
+ struct rte_mp_reply mp_reply = {0};
+ struct virtio_user_mp_param *req_param;
+ struct timespec ts = {.tv_sec = VIRTIO_USER_MP_TIMEOUT_SEC, .tv_nsec = 0};
+ struct virtio_user_proc_priv *pp;
+ uint32_t total_queues, i;
+ int nr_fds, ret = 0;
+
+ if (dev == NULL)
+ return -EINVAL;
+
+ if (rte_eal_process_type() != RTE_PROC_SECONDARY)
+ return -EINVAL;
+
+ total_queues = dev->max_queue_pairs * 2 + (dev->hw_cvq ? 1 : 0);
+
+ pp = rte_zmalloc("virtio_user_proc_priv", sizeof(*pp), 0);
+ if (pp == NULL)
+ return -ENOMEM;
+
+ pp->kickfds = rte_calloc("virtio_user_proc_priv",
+ total_queues, sizeof(int), 0);
+ pp->callfds = rte_calloc("virtio_user_proc_priv",
+ total_queues, sizeof(int), 0);
+ if (pp->kickfds == NULL || pp->callfds == NULL) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
+ for (i = 0; i < total_queues; i++) {
+ pp->kickfds[i] = -1;
+ pp->callfds[i] = -1;
+ }
+
+ memset(&mp_req, 0, sizeof(mp_req));
+ req_param = (struct virtio_user_mp_param *)mp_req.param;
+ strlcpy(req_param->port_name, eth_dev->data->name,
+ sizeof(req_param->port_name));
+ strlcpy(mp_req.name, VIRTIO_USER_MP_NAME, RTE_MP_MAX_NAME_LEN);
+ mp_req.len_param = sizeof(*req_param);
+ mp_req.num_fds = 0;
+
+ if (rte_mp_request_sync(&mp_req, &mp_reply, &ts) < 0 ||
+ mp_reply.nb_received != 1) {
+ PMD_INIT_LOG(ERR, "Failed to request eventfds from primary");
+ free(mp_reply.msgs);
+ ret = -EIO;
+ goto err_free;
+ }
+
+ mp_rep = &mp_reply.msgs[0];
+ nr_fds = mp_rep->num_fds;
+
+ /* a partially-synced device cannot work: treat any mismatch as fatal */
+ if (nr_fds != (int)(total_queues * VIRTIO_USER_EVENTFDS_PER_QUEUE)) {
+ PMD_INIT_LOG(ERR, "Expected %u eventfds, received %d",
+ total_queues * VIRTIO_USER_EVENTFDS_PER_QUEUE, nr_fds);
+ for (i = 0; i < (uint32_t)nr_fds; i++)
+ close(mp_rep->fds[i]);
+ free(mp_reply.msgs);
+ ret = -EPROTO;
+ goto err_free;
+ }
+
+ for (i = 0; i < total_queues; i++) {
+ pp->kickfds[i] = mp_rep->fds[i * VIRTIO_USER_EVENTFDS_PER_QUEUE];
+ pp->callfds[i] = mp_rep->fds[i * VIRTIO_USER_EVENTFDS_PER_QUEUE + 1];
+ }
+ pp->nr_queues = total_queues;
+ free(mp_reply.msgs);
+
+ eth_dev->process_private = pp;
+ PMD_INIT_LOG(DEBUG, "Synced %u queue eventfds for secondary port %s",
+ total_queues, eth_dev->data->name);
+ return 0;
+
+err_free:
+ rte_free(pp->kickfds);
+ rte_free(pp->callfds);
+ rte_free(pp);
+ return ret;
+}
+
/* Dev initialization routine. Invoked once for each virtio vdev at
* EAL init time, see rte_bus_probe().
* Returns 0 on success.
@@ -542,6 +761,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
eth_dev->dev_ops = &virtio_user_secondary_eth_dev_ops;
eth_dev->device = &vdev->device;
+
+ /* populate this process's eventfds before announcing the port */
+ ret = virtio_user_sync_eventfds(eth_dev, dev);
+ if (ret < 0) {
+ PMD_INIT_LOG(ERR,
+ "Failed to sync eventfds in secondary: %d",
+ ret);
+ rte_eth_dev_release_port(eth_dev);
+ return ret;
+ }
+
rte_eth_dev_probing_finish(eth_dev);
return 0;
}
@@ -722,6 +952,36 @@ virtio_user_pmd_probe(struct rte_vdev_device *vdev)
}
}
+ /*
+ * Register the process-wide MP action once so secondaries can fetch a
+ * port's eventfds by name. It is intentionally left registered for the
+ * lifetime of the process (cleaned up at exit): unregistering per device
+ * cannot drain handler calls already dispatched on the EAL MP thread.
+ */
+ rte_spinlock_lock(&virtio_user_mp_lock);
+ if (!virtio_user_mp_registered) {
+ ret = rte_mp_action_register(VIRTIO_USER_MP_NAME,
+ virtio_user_mp_primary_handler);
+ if (ret < 0 && rte_errno != EEXIST) {
+ rte_spinlock_unlock(&virtio_user_mp_lock);
+ if (rte_errno == ENOTSUP) {
+ PMD_INIT_LOG(WARNING,
+ "MP unsupported, secondary eventfd sharing disabled");
+ rte_eth_dev_probing_finish(eth_dev);
+ ret = 0;
+ goto end;
+ }
+ PMD_INIT_LOG(ERR, "Failed to register MP handler: %s",
+ strerror(rte_errno));
+ virtio_user_dev_uninit(dev);
+ virtio_user_eth_dev_free(eth_dev);
+ ret = -1;
+ goto end;
+ }
+ virtio_user_mp_registered = true;
+ }
+ rte_spinlock_unlock(&virtio_user_mp_lock);
+
rte_eth_dev_probing_finish(eth_dev);
ret = 0;
@@ -749,8 +1009,10 @@ virtio_user_pmd_remove(struct rte_vdev_device *vdev)
if (!eth_dev)
return 0;
- if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+ virtio_user_free_proc_priv(eth_dev);
return rte_eth_dev_release_port(eth_dev);
+ }
/* make sure the device is stopped, queues freed */
return rte_eth_dev_close(eth_dev->data->port_id);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] crypto/virtio: cookies are allocated from mempool
From: David Marchand @ 2026-07-06 9:18 UTC (permalink / raw)
To: Radu Nicolau; +Cc: dev, maxime.coquelin, jianjay.zhou, Fan Zhang
In-Reply-To: <20260703144931.2541180-1-radu.nicolau@intel.com>
On Fri, 3 Jul 2026 at 16:50, Radu Nicolau <radu.nicolau@intel.com> wrote:
>
> The Rx/Tx functions allocate cookies as needed, no need to
> allocate and free from heap.
>
> Fixes: 6f0175ff53e0 ("crypto/virtio: support basic PMD ops")
> Cc: jianjay.zhou@huawei.com
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
LGTM, except one nit below.
[snip]
>
> -cookie_alloc_err:
> - rte_mempool_free(vq->mpool);
> - if (i != 0) {
> - for (j = 0; j < i; j++)
> - rte_free(vq->vq_descx[j].cookie);
> - }
> mpool_create_err:
> + rte_mempool_free(vq->mpool);
vq->mpool == NULL if reaching this label.
Calling rte_mempool_free() with NULL is harmless/noop.
Still, there is no need (and I find it confusing) to add
rte_mempool_free under the mpool_create_err label.
> rte_free(vq);
> return -ENOMEM;
> }
> --
> 2.52.0
>
--
David Marchand
^ permalink raw reply
* [DPDK/core Bug 1964] [dpdk26.07-rc2] DPDK build failed with EXTRA_CFLAGS='-O1' on Ubuntu26.04
From: bugzilla @ 2026-07-06 9:17 UTC (permalink / raw)
To: dev
http://bugs.dpdk.org/show_bug.cgi?id=1964
Bug ID: 1964
Summary: [dpdk26.07-rc2] DPDK build failed with
EXTRA_CFLAGS='-O1' on Ubuntu26.04
Product: DPDK
Version: 26.07
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: core
Assignee: dev@dpdk.org
Reporter: daxuex.gao@intel.com
Target Milestone: ---
[Environment]
DPDK version:
commit f77bc8767a952cf833fc9c85d6b5b8d77d39a5da (HEAD -> main, tag: v26.07-rc2,
origin/main, origin/HEAD)
OS: Ubuntu26.04/7.0.0-14-generic
Compiler: gcc (Ubuntu 15.2.0-16ubuntu1) 15.2.0
[Test Setup]
# build cmd:
CC=gcc meson --optimization=1 --werror -Dlibdir=lib --default-library=static
x86_64-native-linuxapp-gcc
ninja -C x86_64-native-linuxapp-gcc
[Output]
# ninja -C x86_64-native-linuxapp-gcc
ninja: Entering directory `x86_64-native-linuxapp-gcc'
[1683/3722] Compiling C object
drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_fdir.c.o
FAILED: drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_fdir.c.o
gcc -Idrivers/libtmp_rte_net_hinic3.a.p -Idrivers -I../drivers
-Idrivers/net/hinic3 -I../drivers/net/hinic3 -Idrivers/net/hinic3/base
-I../drivers/net/hinic3/base -Idrivers/net/hinic3/htn_adapt
-I../drivers/net/hinic3/htn_adapt -Idrivers/net/hinic3/stn_adapt
-I../drivers/net/hinic3/stn_adapt -Ilib/ethdev -I../lib/ethdev -Ilib/eal/common
-I../lib/eal/common -I. -I.. -Iconfig -I../config -Ilib/eal/include
-I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include
-Ilib/eal/x86/include -I../lib/eal/x86/include -I../kernel/linux -Ilib/eal
-I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics
-I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/argparse
-I../lib/argparse -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool
-I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter
-Idrivers/bus/pci -I../drivers/bus/pci -I../drivers/bus/pci/linux -Ilib/pci
-I../lib/pci -Idrivers/bus/vdev -I../drivers/bus/vdev
-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra
-Werror -std=c11 -O1 -include rte_config.h -Wvla -Wcast-qual -Wdeprecated
-Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith
-Wshadow -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings
-Wno-packed-not-aligned -Wno-missing-field-initializers
-fzero-init-padding-bits=all -D_GNU_SOURCE -fPIC -march=native -mrtm
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation
-Wno-address-of-packed-member -DHW_CONVERT_ENDIAN -DRTE_COMPONENT_CLASS=pmd_net
-DRTE_COMPONENT_NAME=hinic3 -DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.hinic3 -MD -MQ
drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_fdir.c.o -MF
drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_fdir.c.o.d -o
drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_fdir.c.o -c
../drivers/net/hinic3/hinic3_fdir.c
../drivers/net/hinic3/hinic3_fdir.c: In function
‘hinic3_enable_rxq_fdir_filter’:
../drivers/net/hinic3/hinic3_fdir.c:1278:13: error: ‘ret’ may be used
uninitialized [-Werror=maybe-uninitialized]
1278 | int ret;
| ^~~
cc1: all warnings being treated as errors
[1694/3722] Compiling C object
drivers/libtmp_rte_net_hinic3.a.p/net_hinic3_hinic3_ethdev.c.o
ninja: build stopped: subcommand failed.
[Expected Result]
Build Passed
[Regression]
Is this issue a regression: (Y/N)N
[First bad commit]
commit 1c963e5976a812771a4f3329e340ff9f9bef4406 (HEAD)
Author: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue Jun 9 08:53:30 2026 -0700
tailq: fix name truncation
The tailq name is defined as 32 bytes, but name would be
silently truncated at 31 bytes. The function strlcpy() size
already accounts for the NUL character at the end.
Bugzilla ID: 1954
Fixes: f9acaf84e923 ("replace snprintf with strlcpy without adding extra
include")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* Re: [PATCH v3 1/6] event/dpaa2: disable channel before closing it
From: Maxime Leroy @ 2026-07-06 9:11 UTC (permalink / raw)
To: Hemant Agrawal; +Cc: dev, hemant.agrawal, sachin.saxena, david.marchand
In-Reply-To: <19dd309f-a2c7-4ccc-9067-e8116239bcb7@oss.nxp.com>
Hi Hemant,
On Fri, Jul 3, 2026 at 7:21 PM Hemant Agrawal
<hemant.agrawal@oss.nxp.com> wrote:
>
>
> On 30-06-2026 20:13, Maxime Leroy wrote:
>
> rte_dpaa2_close_dpcon_device() called dpcon_close() but not
> dpcon_disable(). close only releases the MC control session; it does not
> disable the channel. Cosmetic: an idle DPCON generates nothing and every
> consumer reprograms on setup, so disable before close only to return the
> channel clean, as the symmetric counterpart of the dpcon_enable() done on
> setup.
>
> Signed-off-by: Maxime Leroy <maxime@leroys.fr>
> ---
> drivers/event/dpaa2/dpaa2_hw_dpcon.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/event/dpaa2/dpaa2_hw_dpcon.c b/drivers/event/dpaa2/dpaa2_hw_dpcon.c
> index ea5b0d4b85..f65a63a786 100644
> --- a/drivers/event/dpaa2/dpaa2_hw_dpcon.c
> +++ b/drivers/event/dpaa2/dpaa2_hw_dpcon.c
> @@ -128,6 +128,7 @@ rte_dpaa2_close_dpcon_device(int object_id)
>
> if (dpcon_dev) {
> rte_dpaa2_free_dpcon_dev(dpcon_dev);
> + dpcon_disable(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token);
> dpcon_close(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token);
> TAILQ_REMOVE(&dpcon_dev_list, dpcon_dev, next);
> rte_free(dpcon_dev);
>
> The ordering here is wrong. rte_dpaa2_free_dpcon_dev(dpcon_dev) is called before dpcon_disable(). Once the device is returned to the shared pool via rte_dpaa2_free_dpcon_dev(), another thread can immediately re-allocate the same dpcon_dev pointer. The subsequent dpcon_disable() call would then quiesce a channel already in active use by the new owner — a use-after-free on the pool object.
>
> The correct teardown sequence must be:
>
> dpcon_disable() — quiesce the hardware channel
> dpcon_close() — release the MC control session
> rte_dpaa2_free_dpcon_dev() — return to pool
>
> Please reorder accordingly:
>
> dpcon_disable(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token);
>
> dpcon_close(&dpcon_dev->dpcon, CMD_PRI_LOW, dpcon_dev->token);
>
> TAILQ_REMOVE(&dpcon_dev_list, dpcon_dev, next);
>
> rte_dpaa2_free_dpcon_dev(dpcon_dev);
>
> rte_free(dpcon_dev);
>
>
>
On the ordering: there's no real race. This is the .close handler, run
once at teardown from fslmc_vfio_close_group() on a single thread, not
concurrent with the datapath, so nothing can re-acquire the node
between free and close.
But the patch shouldn't exist at all. The pooled DPAA2 objects use a
reprogram-on-borrow model: the free helpers (rte_dpaa2_free_dpcon_dev,
rte_dpaa2_free_dpci_dev, dpaa2_free_dpbp_dev) only drop in_use, and
nothing un-programs an object on return, the next borrower reprograms.
The sibling DPCI close handler confirms it: it does dpci_close() with
no dpci_disable(). Adding dpcon_disable() here makes DPCON the lone
outlier for no benefit, an enabled channel returned to the pool is
inert (FQ parked at unbind, enable is re-applied idempotently on the
next borrow).
So there's no reason to treat dpcon/CDAN differently. Dropping the patch.
Thanks,
Maxime
^ permalink raw reply
* Re: [PATCH v2] pdump: fix teardown race with opt-in MP request mode
From: Stephen Hemminger @ 2026-07-06 8:25 UTC (permalink / raw)
To: Kumar, Pushpendra1X
Cc: dev, stable, Thomas Monjalon, Pattan, Reshma, Burakov, Anatoly,
Dmitry Kozlyuk, Richardson, Bruce, 14pwcse1224, pushpendra.kumar
In-Reply-To: <SJ0PR11MB5865204AD69038DEF60D76FBDAF22@SJ0PR11MB5865.namprd11.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 43837 bytes --]
Not registering pdump in secondary means there is no capture in secondary.
Still think that is misuse
On Sun, Jul 5, 2026, 23:08 Kumar, Pushpendra1X <pushpendra1x.kumar@intel.com>
wrote:
> Hi Stephen,
>
>
>
> Thanks for taking time to review.
>
>
>
> Just to clarify, the async ENABLE/DISABLE approach was part of v1 and I
>
> dropped that direction in v2.
>
>
>
> The current proposal still uses synchronous request handling. The EAL
>
> change is only intended to allow pdump to ignore peers that do not
>
> register the pdump action, while preserving existing
>
> rte_mp_request_sync() semantics for current users.
>
>
>
> The generation tracking and inflight synchronization added in pdump are
>
> there to prevent teardown races from delayed processing after a DISABLE
>
> request, not to make enable/disable asynchronous. The intent is simply
>
> to avoid releasing shared resources while another secondary may still be
>
> processing a previously forwarded request.
>
> I may be being overly cautious here, but I'd appreciate your thoughts on
>
> whether you consider this protection unnecessary.
>
>
>
> My concern is that the forwarding model currently assumes all
>
> secondaries participate in pdump, while EAL does not enforce or track
>
> that assumption.
>
>
>
> On the other hand, if EAL were to handle this generically, it would need
>
> a mechanism to determine which secondary processes have registered a
>
> given action before forwarding requests. The proposed API remains
>
> generic, while pdump is simply its first user.
>
>
>
> Thanks,
>
> Pushpendra
>
>
>
> *From:* Stephen Hemminger <stephen@networkplumber.org>
> *Sent:* Sunday, July 5, 2026 11:58 PM
> *To:* Kumar, Pushpendra1X <pushpendra1x.kumar@intel.com>
> *Cc:* dev <dev@dpdk.org>; stable <stable@dpdk.org>; Thomas Monjalon <
> thomas@monjalon.net>; Pattan, Reshma <reshma.pattan@intel.com>; Burakov,
> Anatoly <anatoly.burakov@intel.com>; Dmitry Kozlyuk <
> dmitry.kozliuk@gmail.com>; Richardson, Bruce <bruce.richardson@intel.com>;
> 14pwcse1224@uetpeshawar.edu.pk; pushpendra.kumar@tieto.com
> *Subject:* Re: [PATCH v2] pdump: fix teardown race with opt-in MP request
> mode
>
>
>
> Root cause is not calling rte_pdump_init in secondary. This should be
> detected or fixed automatically by EAL. Working around with async adds
> unnecessary complexity
>
>
>
> On Sun, Jul 5, 2026, 01:48 Pushpendra Kumar <pushpendra1x.kumar@intel.com>
> wrote:
>
> This change addresses two pdump teardown failure cases:
>
> 1. A requester secondary can time out if a forwarded secondary is slow,
> unresponsive, or does not implement rte_pdump_init().
> 2. After a DISABLE request times out, the requester secondary receives
> the reply and releases shared resources, which other slow or
> unresponsive secondaries may access, leading to a crash.
>
> Add an opt-in EAL multiprocess request mode that allows a caller to
> ignore peers that do not implement the requested action, while preserving
> the existing rte_mp_request_sync() behavior for legacy users.
>
> pdump uses this new mode when forwarding enable/disable requests and adds
> the following behavior:
> - track request generation so stale callbacks cannot act on a newer
> enable/disable cycle
> - wait for in-flight callbacks to drain before completing disable
> - prevent shared resource teardown from racing with delayed secondary
> processing
> - keep pdump intent-driven without extra consumer-specific workaround
> logic
>
> The EAL change is required because the current mp request path treats
> a missing action as a failure, which cannot satisfy pdump's needs without
> changing behavior for all existing users. The new API is opt-in, so
> existing callers of rte_mp_request_sync() keep their current semantics.
>
> Validation:
> - Baseline-vs-changes benchmark used DPDK-native test-pmd as primary and
> dumpcap as secondary.
> - Both runs captured 1024 packets with the same packet-count exit
> condition.
> - A 5-second perf stat sample on the forwarding primary showed no
> meaningful regression: with changes recorded 12,402,055,320 cycles and
> 17,391,769,084 instructions, versus 12,450,031,900 cycles and
> 17,499,706,053 instructions on the parent baseline.
> - The extra synchronization in the hot path did not show material
> overhead in this sample.
>
> Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
> Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
>
> Cc: stable@dpdk.org
> ---
> This V2 patch addresses two pdump teardown failure scenarios:
>
> - A requester secondary can time out if a forwarded secondary is slow,
> unresponsive, or does not implement rte_pdump_init().
> - A delayed DISABLE reply can race with requester-side teardown, which may
> lead to crashes if peer processes still access shared pdump resources.
>
> To fix this end-to-end:
> - EAL adds an opt-in MP request mode to ignore peers with no matching
> action,
> while preserving legacy rte_mp_request_sync() behavior.
> - pdump uses that mode, tracks generation, and waits for in-flight
> callbacks
> to drain before completing disable.
> - dumpcap and pdump app cleanup paths are fail-closed when disable does not
> complete successfully.
>
> If this direction is accepted, I can split from the next revision into a
> focused EAL patch followed by pdump and app-user updates.
>
> .mailmap | 1 +
> app/dumpcap/main.c | 23 ++-
> app/pdump/main.c | 32 +++-
> doc/guides/rel_notes/release_26_07.rst | 7 +
> lib/eal/common/eal_common_proc.c | 37 +++-
> lib/eal/include/rte_eal.h | 33 ++++
> lib/eal/windows/eal_mp.c | 9 +
> lib/pdump/rte_pdump.c | 241 ++++++++++++++++++++++---
> lib/pdump/rte_pdump.h | 40 +++-
> 9 files changed, 379 insertions(+), 44 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/app/dumpcap/main.c b/app/dumpcap/main.c
> index 46a6cb251e..f421a17034 100644
> --- a/app/dumpcap/main.c
> +++ b/app/dumpcap/main.c
> @@ -521,14 +521,33 @@ static void
> cleanup_pdump_resources(void)
> {
> struct interface *intf;
> + int ret;
> + bool disable_failed = false;
>
> TAILQ_FOREACH(intf, &interfaces, next) {
> - rte_pdump_disable(intf->port,
> - RTE_PDUMP_ALL_QUEUES,
> RTE_PDUMP_FLAG_RXTX);
> + ret = rte_pdump_disable(intf->port,
> + RTE_PDUMP_ALL_QUEUES,
> RTE_PDUMP_FLAG_RXTX);
> + if (ret < 0) {
> + disable_failed = true;
> + fprintf(stderr,
> + "Disable pdump failed on %u:%s: %s\n",
> + intf->port, intf->name,
> + rte_strerror(rte_errno));
> + }
> if (intf->opts.promisc_mode)
> rte_eth_promiscuous_disable(intf->port);
> }
>
> + if (disable_failed) {
> + /*
> + * Fail-closed: if disable did not complete, keep pdump
> action/state
> + * alive and do not uninit shared capture resources.
> + */
> + fprintf(stderr,
> + "Skipping pdump uninit because disable did not
> complete on all interfaces\n");
> + return;
> + }
> +
> rte_pdump_uninit();
> }
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index 1e62c8adc1..4cfb038395 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -451,14 +451,17 @@ print_pdump_stats(void)
> }
> }
>
> -static inline void
> +static inline int
> disable_pdump(struct pdump_tuples *pt)
> {
> if (pt->dump_by_type == DEVICE_ID)
> - rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
> - pt->dir);
> + return rte_pdump_disable_by_deviceid(pt->device_id,
> pt->queue,
> + pt->dir);
> else if (pt->dump_by_type == PORT_ID)
> - rte_pdump_disable(pt->port, pt->queue, pt->dir);
> + return rte_pdump_disable(pt->port, pt->queue, pt->dir);
> +
> + rte_errno = EINVAL;
> + return -EINVAL;
> }
>
> static inline void
> @@ -518,15 +521,30 @@ static void
> cleanup_pdump_resources(void)
> {
> int i;
> + int ret;
> + bool disable_failed = false;
> struct pdump_tuples *pt;
> char name[RTE_ETH_NAME_MAX_LEN];
>
> - /* disable pdump and free the pdump_tuple resources */
> + /* Disable all callbacks first; freeing shared objects before this
> is unsafe. */
> for (i = 0; i < num_tuples; i++) {
> pt = &pdump_t[i];
>
> - /* remove callbacks */
> - disable_pdump(pt);
> + ret = disable_pdump(pt);
> + if (ret < 0) {
> + disable_failed = true;
> + printf("pdump disable failed (tuple=%d, errno=%d:
> %s); "
> + "skip teardown to avoid stale callback
> access\n",
> + i, rte_errno, rte_strerror(rte_errno));
> + }
> + }
> +
> + if (disable_failed)
> + return;
> +
> + /* free the pdump_tuple resources */
> + for (i = 0; i < num_tuples; i++) {
> + pt = &pdump_t[i];
>
> /*
> * transmit rest of the enqueued packets of the rings on to
> diff --git a/doc/guides/rel_notes/release_26_07.rst
> b/doc/guides/rel_notes/release_26_07.rst
> index 4ca0a9ac77..1700a56bc7 100644
> --- a/doc/guides/rel_notes/release_26_07.rst
> +++ b/doc/guides/rel_notes/release_26_07.rst
> @@ -177,6 +177,13 @@ New Features
> Added AGENTS.md file for AI review
> and supporting scripts to review patches and documentation.
>
> +* **Hardened pdump teardown on disable failure.**
> +
> + Hardened pdump request completion and application cleanup behavior so
> timeout
> + or disable failure does not trigger premature pdump resource teardown.
> + pdump now uses an opt-in MP request mode that ignores peers without the
> + pdump action, while preserving legacy EAL MP behavior for all other
> users.
> +
>
> Removed Items
> -------------
> diff --git a/lib/eal/common/eal_common_proc.c
> b/lib/eal/common/eal_common_proc.c
> index 06f151818c..5a458c0417 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -56,6 +56,7 @@ static struct action_entry_list action_entry_list =
> enum mp_type {
> MP_MSG, /* Share message with peers, will not block */
> MP_REQ, /* Request for information, Will block for a reply */
> + MP_REQ_IGN, /* Request where missing action should return ignore */
> MP_REP, /* Response to previously-received request */
> MP_IGN, /* Response telling requester to ignore this response */
> };
> @@ -384,11 +385,11 @@ process_msg(struct mp_msg_internal *m, struct
> sockaddr_un *s)
> pthread_mutex_unlock(&mp_mutex_action);
>
> if (!action) {
> - if (m->type == MP_REQ && !internal_conf->init_complete) {
> - /* if this is a request, and init is not yet
> complete,
> - * and callback wasn't registered, we should tell
> the
> - * requester to ignore our existence because we're
> not
> - * yet ready to process this request.
> + if ((m->type == MP_REQ && !internal_conf->init_complete) ||
> + m->type == MP_REQ_IGN) {
> + /*
> + * Ask requester to ignore this peer when action
> is not
> + * registered either due to early init or explicit
> request policy.
> */
> struct rte_mp_msg dummy;
>
> @@ -936,7 +937,8 @@ mp_request_async(const char *dst, struct rte_mp_msg
> *req,
>
> static int
> mp_request_sync(const char *dst, struct rte_mp_msg *req,
> - struct rte_mp_reply *reply, const struct timespec *ts)
> + struct rte_mp_reply *reply, const struct timespec *ts,
> + enum mp_type req_type)
> {
> int ret;
> pthread_condattr_t attr;
> @@ -959,7 +961,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg
> *req,
> return -1;
> }
>
> - ret = send_msg(dst, req, MP_REQ);
> + ret = send_msg(dst, req, req_type);
> if (ret < 0) {
> EAL_LOG(ERR, "Fail to send request %s:%s",
> dst, req->name);
> @@ -1010,11 +1012,20 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
> int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts)
> +{
> + return rte_mp_request_sync_ex(req, reply, ts, 0);
> +}
> +
> +RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags)
> {
> int dir_fd, ret = -1;
> DIR *mp_dir;
> struct dirent *ent;
> struct timespec now, end;
> + enum mp_type req_type = MP_REQ;
> const struct internal_config *internal_conf =
> eal_get_internal_configuration();
>
> @@ -1027,6 +1038,14 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> if (check_input(req) != 0)
> goto end;
>
> + if (flags & ~RTE_MP_REQ_F_IGNORE_NO_ACTION) {
> + rte_errno = EINVAL;
> + goto end;
> + }
> +
> + if (flags & RTE_MP_REQ_F_IGNORE_NO_ACTION)
> + req_type = MP_REQ_IGN;
> +
> if (internal_conf->no_shconf) {
> EAL_LOG(DEBUG, "No shared files mode enabled, IPC is
> disabled");
> rte_errno = ENOTSUP;
> @@ -1046,7 +1065,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> /* for secondary process, send request to the primary process only
> */
> if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> pthread_mutex_lock(&pending_requests.lock);
> - ret = mp_request_sync(eal_mp_socket_path(), req, reply,
> &end);
> + ret = mp_request_sync(eal_mp_socket_path(), req, reply,
> &end, req_type);
> pthread_mutex_unlock(&pending_requests.lock);
> goto end;
> }
> @@ -1086,7 +1105,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> /* unlocks the mutex while waiting for response,
> * locks on receive
> */
> - if (mp_request_sync(path, req, reply, &end))
> + if (mp_request_sync(path, req, reply, &end, req_type))
> goto unlock_end;
> }
> ret = 0;
> diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
> index 7241f3be5d..415b974945 100644
> --- a/lib/eal/include/rte_eal.h
> +++ b/lib/eal/include/rte_eal.h
> @@ -173,6 +173,15 @@ struct rte_mp_reply {
> struct rte_mp_msg *msgs; /* caller to free */
> };
>
> +/** Request flags for rte_mp_request_sync_ex(). */
> +enum rte_mp_request_flags {
> + /**
> + * Ask peers that do not have a registered action to return MP_IGN
> + * instead of causing timeout for this request.
> + */
> + RTE_MP_REQ_F_IGNORE_NO_ACTION = 1u << 0,
> +};
> +
> /**
> * Action function typedef used by other components.
> *
> @@ -292,6 +301,30 @@ int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts);
>
> +/**
> + * Send a request to peer processes with explicit request flags.
> + *
> + * This API is equivalent to rte_mp_request_sync() with opt-in behavior
> + * controls provided through @p flags.
> + *
> + * @param req
> + * The req argument contains the customized request message.
> + * @param reply
> + * The reply argument will be for storing all the replied messages;
> + * the caller is responsible for free reply->msgs.
> + * @param ts
> + * The ts argument specifies how long we can wait for the peer(s) to
> reply.
> + * @param flags
> + * Bitmask of values from enum rte_mp_request_flags.
> + *
> + * @return
> + * - On success, return 0.
> + * - On failure, return -1, and the reason will be stored in rte_errno.
> + */
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags);
> +
> /**
> * Send a request to the peer process and expect a reply in a separate
> callback.
> *
> diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
> index 6703355318..1066a4d2bc 100644
> --- a/lib/eal/windows/eal_mp.c
> +++ b/lib/eal/windows/eal_mp.c
> @@ -56,10 +56,19 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
> int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts)
> +{
> + return rte_mp_request_sync_ex(req, reply, ts, 0);
> +}
> +
> +RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags)
> {
> RTE_SET_USED(req);
> RTE_SET_USED(reply);
> RTE_SET_USED(ts);
> + RTE_SET_USED(flags);
> EAL_LOG_NOT_IMPLEMENTED();
> return -1;
> }
> diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
> index ac94efe7ff..3764aeb2d3 100644
> --- a/lib/pdump/rte_pdump.c
> +++ b/lib/pdump/rte_pdump.c
> @@ -3,9 +3,12 @@
> */
>
> #include <stdlib.h>
> +#include <errno.h>
>
> #include <eal_export.h>
> +#include <rte_eal.h>
> #include <rte_alarm.h>
> +#include <rte_cycles.h>
> #include <rte_mbuf.h>
> #include <rte_ethdev.h>
> #include <rte_lcore.h>
> @@ -68,6 +71,7 @@ struct pdump_request {
>
> const struct rte_bpf_prm *prm;
> uint32_t snaplen;
> + uint32_t generation;
> };
>
> struct pdump_response {
> @@ -81,6 +85,11 @@ struct pdump_bundle {
> char peer[];
> };
>
> +struct pdump_shared_ctl {
> + RTE_ATOMIC(uint32_t) enabled;
> + RTE_ATOMIC(uint32_t) inflight;
> +};
> +
> static struct pdump_rxtx_cbs {
> struct rte_ring *ring;
> struct rte_mempool *mp;
> @@ -88,11 +97,11 @@ static struct pdump_rxtx_cbs {
> const struct rte_bpf *filter;
> enum pdump_version ver;
> uint32_t snaplen;
> + uint32_t generation;
> RTE_ATOMIC(uint32_t) use_count;
> } rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
> tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
>
> -
> /*
> * The packet capture statistics keep track of packets
> * accepted, filtered and dropped. These are per-queue
> @@ -102,9 +111,59 @@ static const char MZ_RTE_PDUMP_STATS[] =
> "rte_pdump_stats";
> static struct {
> struct rte_pdump_stats
> rx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> struct rte_pdump_stats
> tx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + struct pdump_shared_ctl
> rx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + struct pdump_shared_ctl
> tx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + RTE_ATOMIC(uint32_t) generation;
> const struct rte_memzone *mz;
> } *pdump_stats;
>
> +static int
> +pdump_enter(struct pdump_shared_ctl *ctl, const struct pdump_rxtx_cbs
> *cbs)
> +{
> + uint32_t generation =
> rte_atomic_load_explicit(&pdump_stats->generation,
> +
> rte_memory_order_relaxed);
> +
> + if (rte_atomic_load_explicit(&ctl->enabled,
> rte_memory_order_relaxed) == 0 ||
> + cbs->generation != generation)
> + return 0;
> +
> + rte_atomic_fetch_add_explicit(&ctl->inflight, 1,
> rte_memory_order_acquire);
> +
> + /*
> + * Disable/re-enable may race with callback entry. Re-check
> enabled and
> + * generation after taking inflight reference so disable can wait
> for this
> + * callback to drain and stale callbacks become no-op.
> + */
> + generation = rte_atomic_load_explicit(&pdump_stats->generation,
> + rte_memory_order_relaxed);
> + if (rte_atomic_load_explicit(&ctl->enabled,
> rte_memory_order_relaxed) != 0 &&
> + cbs->generation == generation)
> + return 1;
> +
> + rte_atomic_fetch_sub_explicit(&ctl->inflight, 1,
> rte_memory_order_release);
> + return 0;
> +}
> +
> +static __rte_always_inline void
> +pdump_exit(struct pdump_shared_ctl *ctl)
> +{
> + rte_atomic_fetch_sub_explicit(&ctl->inflight, 1,
> rte_memory_order_release);
> +}
> +
> +static int
> +pdump_wait_inflight(struct pdump_shared_ctl *ctl)
> +{
> + uint64_t end_tsc = rte_get_timer_cycles() + MP_TIMEOUT_S *
> rte_get_timer_hz();
> +
> + while (rte_atomic_load_explicit(&ctl->inflight,
> rte_memory_order_acquire) != 0) {
> + if (rte_get_timer_cycles() > end_tsc)
> + return -ETIMEDOUT;
> + rte_pause();
> + }
> +
> + return 0;
> +}
> +
> static void
> pdump_cb_wait(struct pdump_rxtx_cbs *cbs)
> {
> @@ -224,12 +283,17 @@ pdump_rx(uint16_t port, uint16_t queue,
> uint16_t max_pkts __rte_unused, void *user_params)
> {
> struct pdump_rxtx_cbs *cbs = user_params;
> + struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][queue];
> struct rte_pdump_stats *stats = &pdump_stats->rx[port][queue];
>
> + if (!pdump_enter(ctl, cbs))
> + return nb_pkts;
> +
> pdump_cb_hold(cbs);
> pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_IN,
> pkts, nb_pkts, cbs, stats);
> pdump_cb_release(cbs);
> + pdump_exit(ctl);
>
> return nb_pkts;
> }
> @@ -239,12 +303,17 @@ pdump_tx(uint16_t port, uint16_t queue,
> struct rte_mbuf **pkts, uint16_t nb_pkts, void
> *user_params)
> {
> struct pdump_rxtx_cbs *cbs = user_params;
> + struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][queue];
> struct rte_pdump_stats *stats = &pdump_stats->tx[port][queue];
>
> + if (!pdump_enter(ctl, cbs))
> + return nb_pkts;
> +
> pdump_cb_hold(cbs);
> pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_OUT,
> pkts, nb_pkts, cbs, stats);
> pdump_cb_release(cbs);
> + pdump_exit(ctl);
>
> return nb_pkts;
> }
> @@ -255,20 +324,43 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> uint16_t end_q, uint16_t port, uint16_t queue,
> struct rte_ring *ring, struct rte_mempool *mp,
> struct rte_bpf *filter,
> - uint16_t operation, uint32_t snaplen)
> + uint16_t operation, uint32_t snaplen,
> + uint32_t generation)
> {
> uint16_t qid;
>
> qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
> for (; qid < end_q; qid++) {
> struct pdump_rxtx_cbs *cbs = &rx_cbs[port][qid];
> + struct pdump_shared_ctl *ctl =
> &pdump_stats->rx_ctl[port][qid];
>
> if (operation == ENABLE) {
> + int ret;
> +
> if (cbs->cb) {
> - PDUMP_LOG_LINE(ERR,
> - "rx callback for port=%d queue=%d,
> already exists",
> - port, qid);
> - return -EEXIST;
> + if (cbs->generation == generation &&
> + rte_atomic_load_explicit(&ctl->enabled,
> + rte_memory_order_relaxed) !=
> 0) {
> + PDUMP_LOG_LINE(ERR,
> + "rx callback for port=%d
> queue=%d, already exists",
> + port, qid);
> + return -EEXIST;
> + }
> +
> + PDUMP_LOG_LINE(DEBUG,
> + "reconciling stale rx callback for
> port=%d queue=%d"
> + " old_gen=%u new_gen=%u",
> + port, qid, cbs->generation,
> generation);
> +
> + ret = rte_eth_remove_rx_callback(port,
> qid, cbs->cb);
> + if (ret < 0) {
> + PDUMP_LOG_LINE(ERR,
> + "failed to reconcile stale
> rx callback, errno=%d",
> + -ret);
> + return ret;
> + }
> + pdump_cb_wait(cbs);
> + cbs->cb = NULL;
> }
> cbs->use_count = 0;
> cbs->ver = ver;
> @@ -276,6 +368,7 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> cbs->mp = mp;
> cbs->snaplen = snaplen;
> cbs->filter = filter;
> + cbs->generation = generation;
>
> cbs->cb = rte_eth_add_first_rx_callback(port, qid,
> pdump_rx,
> cbs);
> @@ -286,10 +379,15 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> return rte_errno;
> }
>
> + rte_atomic_store_explicit(&ctl->inflight, 0,
> rte_memory_order_relaxed);
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> rte_memory_order_release);
> +
> memset(&pdump_stats->rx[port][qid], 0,
> sizeof(struct rte_pdump_stats));
> } else if (operation == DISABLE) {
> int ret;
>
> + rte_atomic_store_explicit(&ctl->enabled, 0,
> rte_memory_order_release);
> +
> if (cbs->cb == NULL) {
> PDUMP_LOG_LINE(ERR,
> "no existing rx callback for
> port=%d queue=%d",
> @@ -298,6 +396,11 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> }
> ret = rte_eth_remove_rx_callback(port, qid,
> cbs->cb);
> if (ret < 0) {
> + /* Keep state coherent: callback is still
> registered,
> + * so restore enabled.
> + */
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> + rte_memory_order_release);
> PDUMP_LOG_LINE(ERR,
> "failed to remove rx callback,
> errno=%d",
> -ret);
> @@ -305,6 +408,10 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> }
> pdump_cb_wait(cbs);
> cbs->cb = NULL;
> +
> + ret = pdump_wait_inflight(ctl);
> + if (ret < 0)
> + return ret;
> }
> }
>
> @@ -316,7 +423,8 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> uint16_t end_q, uint16_t port, uint16_t queue,
> struct rte_ring *ring, struct rte_mempool *mp,
> struct rte_bpf *filter,
> - uint16_t operation, uint32_t snaplen)
> + uint16_t operation, uint32_t snaplen,
> + uint32_t generation)
> {
>
> uint16_t qid;
> @@ -324,13 +432,36 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
> for (; qid < end_q; qid++) {
> struct pdump_rxtx_cbs *cbs = &tx_cbs[port][qid];
> + struct pdump_shared_ctl *ctl =
> &pdump_stats->tx_ctl[port][qid];
>
> if (operation == ENABLE) {
> + int ret;
> +
> if (cbs->cb) {
> - PDUMP_LOG_LINE(ERR,
> - "tx callback for port=%d queue=%d,
> already exists",
> - port, qid);
> - return -EEXIST;
> + if (cbs->generation == generation &&
> + rte_atomic_load_explicit(&ctl->enabled,
> + rte_memory_order_relaxed) !=
> 0) {
> + PDUMP_LOG_LINE(ERR,
> + "tx callback for port=%d
> queue=%d, already exists",
> + port, qid);
> + return -EEXIST;
> + }
> +
> + PDUMP_LOG_LINE(DEBUG,
> + "reconciling stale tx callback for
> port=%d queue=%d"
> + " old_gen=%u new_gen=%u",
> + port, qid, cbs->generation,
> generation);
> +
> + ret = rte_eth_remove_tx_callback(port,
> qid, cbs->cb);
> + if (ret < 0) {
> + PDUMP_LOG_LINE(ERR,
> + "failed to reconcile stale
> tx callback, errno=%d",
> + -ret);
> + return ret;
> + }
> +
> + pdump_cb_wait(cbs);
> + cbs->cb = NULL;
> }
> cbs->use_count = 0;
> cbs->ver = ver;
> @@ -338,6 +469,7 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> cbs->mp = mp;
> cbs->snaplen = snaplen;
> cbs->filter = filter;
> + cbs->generation = generation;
>
> cbs->cb = rte_eth_add_tx_callback(port, qid,
> pdump_tx,
> cbs);
> @@ -347,10 +479,15 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> rte_errno);
> return rte_errno;
> }
> +
> + rte_atomic_store_explicit(&ctl->inflight, 0,
> rte_memory_order_relaxed);
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> rte_memory_order_release);
> memset(&pdump_stats->tx[port][qid], 0,
> sizeof(struct rte_pdump_stats));
> } else if (operation == DISABLE) {
> int ret;
>
> + rte_atomic_store_explicit(&ctl->enabled, 0,
> rte_memory_order_release);
> +
> if (cbs->cb == NULL) {
> PDUMP_LOG_LINE(ERR,
> "no existing tx callback for
> port=%d queue=%d",
> @@ -359,6 +496,11 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> }
> ret = rte_eth_remove_tx_callback(port, qid,
> cbs->cb);
> if (ret < 0) {
> + /* Keep state coherent: callback is still
> registered,
> + * so restore enabled.
> + */
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> + rte_memory_order_release);
> PDUMP_LOG_LINE(ERR,
> "failed to remove tx callback,
> errno=%d",
> -ret);
> @@ -367,6 +509,10 @@ pdump_register_tx_callbacks(enum pdump_version ver,
>
> pdump_cb_wait(cbs);
> cbs->cb = NULL;
> +
> + ret = pdump_wait_inflight(ctl);
> + if (ret < 0)
> + return ret;
> }
> }
>
> @@ -459,7 +605,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_rx_q : queue
> + 1;
> ret = pdump_register_rx_callbacks(p->ver, end_q, port,
> queue,
> ring, mp, filter,
> - operation, p->snaplen);
> + operation, p->snaplen,
> + p->generation);
> if (ret < 0)
> return ret;
> }
> @@ -469,7 +616,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_tx_q : queue
> + 1;
> ret = pdump_register_tx_callbacks(p->ver, end_q, port,
> queue,
> ring, mp, filter,
> - operation, p->snaplen);
> + operation, p->snaplen,
> + p->generation);
> if (ret < 0)
> return ret;
> }
> @@ -477,12 +625,14 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> return ret;
> }
>
> -static void
> -pdump_request_to_secondary(const struct pdump_request *req)
> +static int
> +pdump_request_to_secondary_sync(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};
> + int ret = 0;
> + uint16_t i;
>
> PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary",
> pdump_opname(req->op));
>
> @@ -490,14 +640,41 @@ 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");
> + if (rte_mp_request_sync_ex(&mp_req, &mp_reply, &ts,
> + RTE_MP_REQ_F_IGNORE_NO_ACTION) != 0) {
> + PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed: %s",
> + strerror(rte_errno));
> + return -rte_errno;
> + }
>
> - else if (mp_reply.nb_sent != mp_reply.nb_received)
> + 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);
> + ret = -ETIMEDOUT;
> + }
> +
> + for (i = 0; i < mp_reply.nb_received; i++) {
> + struct rte_mp_msg *mp_rep = &mp_reply.msgs[i];
> + const struct pdump_response *resp;
> +
> + if (mp_rep->len_param != sizeof(*resp)) {
> + PDUMP_LOG_LINE(ERR, "invalid secondary reply size
> %u", mp_rep->len_param);
> + if (ret == 0)
> + ret = -EINVAL;
> + continue;
> + }
> +
> + resp = (const struct pdump_response *)mp_rep->param;
> + if (resp->err_value != 0) {
> + PDUMP_LOG_LINE(ERR, "secondary reply failed: op=%u
> err=%d",
> + resp->res_op, resp->err_value);
> + if (ret == 0)
> + ret = resp->err_value;
> + }
> + }
>
> free(mp_reply.msgs);
> + return ret;
> }
>
> /* Allocate temporary storage for passing state to the alarm thread for
> deferred handling */
> @@ -556,7 +733,10 @@ pdump_handle_primary_request(const struct rte_mp_msg
> *mp_msg, const void *peer)
> PDUMP_LOG_LINE(DEBUG, "secondary pdump %s",
> pdump_opname(req->op));
>
> /* Can just do it now, no need for interrupt thread */
> - ret = set_pdump_rxtx_cbs(req);
> + if (req->op == ENABLE || req->op == DISABLE)
> + ret = set_pdump_rxtx_cbs(req);
> + else
> + ret = -EINVAL;
> }
>
> return pdump_send_response(req, ret, peer);
> @@ -569,17 +749,29 @@ __pdump_request(void *param)
> {
> struct pdump_bundle *bundle = param;
> struct rte_mp_msg *msg = &bundle->msg;
> - const struct pdump_request *req =
> - (const struct pdump_request *)msg->param;
> + struct pdump_request *req =
> + (struct pdump_request *)msg->param;
> int ret;
>
> PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
>
> + if (req->op == DISABLE) {
> + req->generation =
> rte_atomic_fetch_add_explicit(&pdump_stats->generation, 1,
> +
> rte_memory_order_acq_rel) + 1;
> + } else if (req->op == ENABLE) {
> + req->generation =
> rte_atomic_load_explicit(&pdump_stats->generation,
> +
> rte_memory_order_acquire);
> + }
> +
> ret = set_pdump_rxtx_cbs(req);
>
> - /* Primary process is responsible for broadcasting request to all
> secondaries */
> + /*
> + * Primary process is responsible for broadcasting the request to
> all
> + * secondaries. The sync request uses opt-in ignore-missing-action
> mode so
> + * pdump does not depend on unrelated secondary processes.
> + */
> if (ret == 0)
> - pdump_request_to_secondary(req);
> + ret = pdump_request_to_secondary_sync(req);
>
> pdump_send_response(req, ret, bundle->peer);
> free(bundle);
> @@ -659,6 +851,9 @@ rte_pdump_init(void)
>
> pdump_stats = mz->addr;
> pdump_stats->mz = mz;
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> + rte_atomic_store_explicit(&pdump_stats->generation, 1,
> + rte_memory_order_release);
>
> return 0;
> }
> diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h
> index 1e32d46097..77e5914420 100644
> --- a/lib/pdump/rte_pdump.h
> +++ b/lib/pdump/rte_pdump.h
> @@ -64,7 +64,7 @@ rte_pdump_uninit(void);
> * queues of a given port.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> * @param ring
> * ring on which captured packets will be enqueued for user.
> * @param mp
> @@ -72,6 +72,11 @@ rte_pdump_uninit(void);
> * @param filter
> * Unused should be NULL.
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -103,6 +108,11 @@ rte_pdump_enable(uint16_t port, uint16_t queue,
> uint32_t flags,
> * @param prm
> * Use BPF program to run to filter packes (can be NULL)
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -124,7 +134,14 @@ rte_pdump_enable_bpf(uint16_t port_id, uint16_t queue,
> * queues of a given port.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> + *
> + * @note
> + * A disable failure (including timeout/no response) means teardown is
> not
> + * complete across pdump-enabled peers. The caller must not release
> shared
> + * pdump resources and must not uninitialize pdump for that capture
> session
> + * until disable succeeds. Releasing resources on disable failure can
> lead to
> + * crashes in peer processes still accessing those resources.
> *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> @@ -153,6 +170,11 @@ rte_pdump_disable(uint16_t port, uint16_t queue,
> uint32_t flags);
> * @param filter
> * unused should be NULL
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -186,6 +208,11 @@ rte_pdump_enable_by_deviceid(char *device_id,
> uint16_t queue,
> * @param filter
> * Use BPF program to run to filter packes (can be NULL)
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -210,7 +237,14 @@ rte_pdump_enable_bpf_by_deviceid(const char
> *device_id, uint16_t queue,
> * queues of a given device id.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> + *
> + * @note
> + * A disable failure (including timeout/no response) means teardown is
> not
> + * complete across pdump-enabled peers. The caller must not release
> shared
> + * pdump resources and must not uninitialize pdump for that capture
> session
> + * until disable succeeds. Releasing resources on disable failure can
> lead to
> + * crashes in peer processes still accessing those resources.
> *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> --
> 2.43.0
>
>
[-- Attachment #2: Type: text/html, Size: 57594 bytes --]
^ permalink raw reply
* TB meeting MoM April 15 2026
From: Jerin Jacob @ 2026-07-06 7:50 UTC (permalink / raw)
To: dpdk-dev
MINUTES OF TECHNICAL BOARD MEETING
Date: Apr 15, 2026
MEMBERS ATTENDING
- Nathan
- Bruce
- Thomas
- Hemant
- Jerin (Chair)
- Aaron
- Kevin
- Maxime
- Stephen
NOTE: The technical board meetings every second Wednesday at
https://meet.jit.si/DPDK at 3 pm UTC.
Meetings are public, and DPDK community members are welcome to attend.
1. SUMMIT PLANNING — BIRDS OF A FEATHER (BOF) SESSIONS
A discussion was held on organizing BOF sessions for the upcoming summit.
Decisions reached:
- A physical whiteboard will be placed at the conference venue for
attendees to suggest and vote on BOF topics.
- The tech board will pre-seed the whiteboard with 2–3 initial topic
ideas, with a third held as fallback.
- Attendees will be split into groups by topic interest rather than
attending all sessions simultaneously.
- Voting on session topics will use a polling tool (e.g., Polly).
Jerin will create a Slack/email thread to brainstorm and collect BOF
topic candidates ahead of the summit.
Open point: Whether to run single unified discussions or parallel
breakout groups was raised by Thomas but not fully resolved.
2. JOINT MEETING PLANNING — MAY 11TH, STOCKHOLM
- A joint meeting planning session is scheduled for April 28th at
10:00 AM US Eastern. Topics will include roadmap feasibility and
project provisioning.
- The actual joint meeting will be held on May 11th in Stockholm.
- Remote participation will be available for non-voting silver members.
- Two attendance confirmations received to date.
- The May 11th tech board meeting is CANCELLED due to time constraints
and travel overlap.
3. GOVERNING BOARD PRESENTATION
- Hemant will represent both the tech board and governing board at the
upcoming presentation.
- Bruce and Nathan confirmed most updates will reflect status quo,
including ongoing AI integration work.
- Bruce recommended highlighting the BOF session planning as a notable
development for the governing board.
- Hemant is preparing slides/materials and will share with the tech
board for review within 1–2 days.
- Hemant to send pending Tech Board meeting Minutues if any.
4. DEVELOPMENT WISHLIST
- Thomas requested the governing board allocate budget for a community
development wishlist.
- All board members (tech and governing) are encouraged to submit
ideas for future development projects.
- Jerin to follow up with Prasun to collect detailed development
ideas, pending the context email from Thomas.
5. WEBSITE REBRAND PROPOSAL
- Nathan announced a proposal from Ben Thomas to rebrand and revamp
the project website, with the goal of increasing contributors and
reviewers.
- The website has not been updated in 6–7 years; the governing board
provided positive feedback on the wireframe and UX design.
- The proposal will be submitted to LF Creative Services by end of
this week; cost estimates expected soon.
- Nathan will circulate the redesign brief to the governing board by
Friday or Monday for review.
- A formal vote will take place at the next governing board meeting in
two weeks.
^ permalink raw reply
* [DPDK/examples Bug 1963] hotplug_mp: attach the share device failed in secondary processe
From: bugzilla @ 2026-07-06 6:48 UTC (permalink / raw)
To: dev
http://bugs.dpdk.org/show_bug.cgi?id=1963
Bug ID: 1963
Summary: hotplug_mp: attach the share device failed in
secondary processe
Product: DPDK
Version: 25.07
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: examples
Assignee: dev@dpdk.org
Reporter: yex.chen@intel.com
Target Milestone: ---
Environment
===========
DPDK version: f77bc8767a952cf833fc9c85d6b5b8d77d39a5da
version: 26.07.0-rc2
OS: Rhel9.6 (Plow)/5.14.0-570.12.1.el9_6.x86_64
Compiler: gcc version 11.5.0 20240719
Hardware platform: Intel(R) Xeon(R) 6767P
NIC hardware: Intel Corporation Ethernet Controller XL710 for 40GbE QSFP+
[8086:1583]
NIC firmware:
driver: vfio-pci
kdriver: i40e-2.30.18
firmware-version: 9.57 0x8001037b 1.4002.0
Test Setup
Steps to reproduce
==================
1. Build hotplug_mp
CC=gcc meson -Dlibdir=lib --default-library=static x86_64-native-linuxapp-gcc
-Dmax_lcores=256
ninja -C x86_64-native-linuxapp-gcc
meson configure -Dexamples=multi_process/hotplug_mp x86_64-native-linuxapp-gcc
ninja -C x86_64-native-linuxapp-gcc
2. bind pf to i40e
usertools/dpdk-devbind.py --force --bind=i40e 0000:da:00.0
3. Start dpdk-hotplug_mp as primary then secondary
x86_64-native-linuxapp-gcc/examples/dpdk-hotplug_mp -l 1-2 -n 4 -a
0000:da:00.0 --proc-type=auto
4. Bind the port to vfio-pci
usertools/dpdk-devbind.py --force --bind=vfio-pci 0000:da:00.0
5. Attach the physical device from primary, check primary and secondary
processes attach the share device
example> attach 0000:da:00.0
example> list
Results:
========
the primary processe attach the share device successfully,
but the secondary processe attach the share device failed
example> list
list all etherdev
Expected Result:
================
the primary and secondary processes attach the share device successfully
example> list
list all etherdev
0 0000:da:00.0
bad commit:
===========
commit: 1a2e26d8701d36c82fc2e405abb5e80d73df54bb
Author: David Marchand <david.marchand@redhat.com>
Date: Wed Jun 24 14:07:56 2026 +0200
dev: skip multi-process in hotplug
If multi-process is disabled ({}no-shconf/{-}-in-memory),
there is no need to build a multi-process message.
Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
--
You are receiving this mail because:
You are the assignee for the bug.
^ permalink raw reply
* [PATCH v2] devtools: support local opencode agent for patch review
From: datshan @ 2026-07-06 6:29 UTC (permalink / raw)
To: thomas, stephen; +Cc: aconole, dev
In-Reply-To: <20260703091902.525837-1-datshan@qq.com>
From: Chengwen Feng <fengchengwen@huawei.com>
Currently review-patch.py only supports cloud AI providers
(Anthropic, OpenAI, xAI, Google) via REST API, requiring API keys.
Add a --via option that invokes the locally installed opencode CLI as
the review runner instead of making HTTP calls. opencode reads
AGENTS.md from the DPDK project directory automatically, needing no
configuration beyond opencode on PATH.
The --via and -p/--provider options are independent -- via routes to
the local agent mode while -p continues to use the cloud API path.
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
v2: Address comments from Stephen, including: not arise FileNotFoundError
and more clarify.
---
devtools/ai/review-patch.py | 231 ++++++++++++++++++++++++++----------
1 file changed, 169 insertions(+), 62 deletions(-)
diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..5d023ee689 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -3,9 +3,10 @@
# Copyright(c) 2026 Stephen Hemminger
"""
-Review DPDK patches using AI providers.
+Review DPDK patches using AI providers or a local agent tool.
Supported providers: Anthropic Claude, OpenAI ChatGPT, xAI Grok, Google Gemini
+Supported agent: OpenCode (--via opencode)
"""
import argparse
@@ -551,6 +552,109 @@ def build_google_request(
}
+def _call_opencode(
+ model: str,
+ system_prompt: str,
+ patch_content: str,
+ patch_name: str,
+ agents_path: str = "",
+ output_format: str = "text",
+ verbose: bool = False,
+ timeout: int = 300,
+) -> tuple[str, TokenUsage]:
+ """Call local opencode CLI for review."""
+ format_instruction = FORMAT_INSTRUCTIONS.get(output_format, "")
+ user_prompt = (
+ f"Review the attached DPDK patch file '{patch_name}'.\n\n"
+ f"Focus on correctness bugs, C coding style, API requirements, "
+ f"and other guideline violations. "
+ f"Commit message format and SPDX/copyright are checked by "
+ f"checkpatches.sh -- do NOT flag those.\n\n"
+ f"{format_instruction}"
+ )
+
+ with tempfile.NamedTemporaryFile(
+ mode="w", suffix=".patch", delete=False, prefix="review_"
+ ) as f:
+ f.write(patch_content)
+ patch_temp = f.name
+
+ try:
+ full_message = system_prompt + "\n\n" + user_prompt
+
+ cmd = [
+ "opencode", "run", "--format", "json",
+ "--dir", str(Path(__file__).resolve().parent.parent.parent),
+ ]
+ if model:
+ cmd.extend(["--model", model])
+ if verbose:
+ cmd.append("--print-logs")
+ cmd.append(full_message)
+ cmd.extend(["--file", patch_temp])
+ if agents_path:
+ cmd.extend(["--file", agents_path])
+
+ if verbose:
+ print(f"Running: {' '.join(cmd)}", file=sys.stderr)
+
+ try:
+ result = subprocess.run(
+ cmd,
+ capture_output=True,
+ text=True,
+ timeout=timeout,
+ )
+ except FileNotFoundError:
+ error("opencode not found. Install from https://opencode.ai")
+ except subprocess.TimeoutExpired:
+ error(f"opencode timed out after {timeout} seconds")
+
+ if result.returncode != 0:
+ error(
+ f"opencode exited with code {result.returncode}: "
+ f"{result.stderr[:500]}"
+ )
+
+ finally:
+ os.unlink(patch_temp)
+
+ text_parts = []
+ usage = TokenUsage()
+ steps = 0
+ for line in result.stdout.splitlines():
+ stripped = line.strip()
+ if not stripped:
+ continue
+ try:
+ event = json.loads(stripped)
+ except json.JSONDecodeError:
+ continue
+
+ event_type = event.get("type", "")
+ if event_type == "text":
+ part_text = event.get("part", {}).get("text", "")
+ if part_text:
+ text_parts.append(part_text)
+ elif event_type == "step_finish":
+ steps += 1
+ tokens = event.get("part", {}).get("tokens", {})
+ if tokens:
+ usage.input_tokens += tokens.get("input", 0)
+ usage.output_tokens += tokens.get("output", 0)
+ cache = tokens.get("cache", {})
+ usage.cache_creation_tokens += cache.get("write", 0)
+ usage.cache_read_tokens += cache.get("read", 0)
+
+ usage.api_calls = 1 if steps > 0 else 0
+ review_text = "\n".join(text_parts)
+
+ if not review_text:
+ error("No review text received from opencode")
+
+ return review_text, usage
+
+
def call_api(
provider: str,
api_key: str,
@@ -740,6 +844,7 @@ def main() -> None:
Examples:
%(prog)s patch.patch # Review with default settings
%(prog)s -p openai my-patch.patch # Use OpenAI ChatGPT
+ %(prog)s --via opencode my-patch.patch # Use local opencode agent
%(prog)s -f markdown patch.patch # Output as Markdown
%(prog)s -f json -o review.json patch.patch # Save JSON to file
%(prog)s -f html -o review.html patch.patch # Save HTML to file
@@ -786,7 +891,13 @@ def main() -> None:
"--provider",
choices=PROVIDERS.keys(),
default="anthropic",
- help="AI provider (default: anthropic)",
+ help="Cloud AI provider (default: anthropic)",
+ )
+ parser.add_argument(
+ "--via",
+ choices=["opencode"],
+ default=None,
+ help="Use a local agent tool instead of a cloud API (e.g. --via opencode)",
)
parser.add_argument(
"-a",
@@ -926,14 +1037,19 @@ def main() -> None:
if not args.patch_file:
parser.error("patch_file is required")
- # Get provider config
- config = PROVIDERS[args.provider]
- model = args.model or config["default_model"]
-
- # Get API key
- api_key = os.environ.get(config["env_var"])
- if not api_key:
- error(f"{config['env_var']} environment variable not set")
+ # Get provider config or set up local agent runner
+ via = args.via
+ if via:
+ model = args.model or ""
+ api_key = ""
+ provider_name = "OpenCode"
+ else:
+ config = PROVIDERS[args.provider]
+ model = args.model or config["default_model"]
+ api_key = os.environ.get(config["env_var"])
+ if not api_key:
+ error(f"{config['env_var']} environment variable not set")
+ provider_name = config["name"]
# Validate files
agents_path = Path(args.agents)
@@ -971,17 +1087,42 @@ def main() -> None:
patch_content = patch_path.read_text(encoding="utf-8", errors="replace")
patch_name = patch_path.name
- # Determine max tokens for this provider
- max_input_tokens = args.max_tokens or PROVIDER_INPUT_LIMITS.get(
- args.provider, 100000
- )
+ # Dispatch to agent or provider
+ def _run_review(patch_body: str, patch_label: str) -> tuple[str, TokenUsage]:
+ if via:
+ return _call_opencode(
+ model, system_prompt,
+ patch_body, patch_label,
+ str(agents_path),
+ args.output_format, args.verbose, args.timeout,
+ )
+ return call_api(
+ args.provider, api_key, model, args.tokens,
+ system_prompt, agents_content,
+ patch_body, patch_label,
+ args.output_format, args.verbose, args.timeout,
+ )
- # Estimate token count
- estimated_tokens = estimate_tokens(patch_content + agents_content)
+ # Determine max tokens (cloud API only)
+ max_input_tokens = 0
+ if via:
+ estimated_tokens = 1
+ else:
+ max_input_tokens = args.max_tokens or PROVIDER_INPUT_LIMITS.get(
+ args.provider, 100000
+ )
+ estimated_tokens = estimate_tokens(patch_content + agents_content)
# Accumulate token usage across all API calls
total_usage = TokenUsage()
+ if via and args.large_file != "error":
+ print(
+ "Warning: --large-file is ignored in --via mode; "
+ "opencode handles large files automatically",
+ file=sys.stderr,
+ )
+
# Parse patch range if specified
patch_start, patch_end = None, None
if args.patch_range:
@@ -1039,19 +1180,7 @@ def main() -> None:
patch_label = f"Patch {i}/{total_patches}"
print(f"\nReviewing {patch_label}...", file=sys.stderr)
- review_text, call_usage = call_api(
- args.provider,
- api_key,
- model,
- args.tokens,
- system_prompt,
- agents_content,
- patch,
- f"{patch_name} ({patch_label})",
- args.output_format,
- args.verbose,
- args.timeout,
- )
+ review_text, call_usage = _run_review(patch, f"{patch_name} ({patch_label})")
total_usage.add(call_usage)
all_reviews.append((patch_label, review_text))
@@ -1063,10 +1192,10 @@ def main() -> None:
# Skip the normal API call
estimated_tokens = 0 # Bypass size check since we've already processed
- # Check if content is too large
+ # Check if content is too large (cloud API only)
is_large = estimated_tokens > max_input_tokens
- if is_large:
+ if is_large and not via:
print(
f"Warning: Estimated {estimated_tokens:,} tokens exceeds limit of "
f"{max_input_tokens:,}",
@@ -1109,19 +1238,7 @@ def main() -> None:
chunk_label = f"Chunk {chunk_num}/{total_chunks}"
print(f"Reviewing {chunk_label}...", file=sys.stderr)
- review_text, call_usage = call_api(
- args.provider,
- api_key,
- model,
- args.tokens,
- system_prompt,
- agents_content,
- chunk,
- f"{patch_name} ({chunk_label})",
- args.output_format,
- args.verbose,
- args.timeout,
- )
+ review_text, call_usage = _run_review(chunk, f"{patch_name} ({chunk_label})")
total_usage.add(call_usage)
all_reviews.append((chunk_label, review_text))
@@ -1135,7 +1252,10 @@ def main() -> None:
if args.verbose:
print("=== Request ===", file=sys.stderr)
- print(f"Provider: {args.provider}", file=sys.stderr)
+ if via:
+ print(f"Runner: {args.via}", file=sys.stderr)
+ else:
+ print(f"Provider: {args.provider}", file=sys.stderr)
print(f"Model: {model}", file=sys.stderr)
print(f"Review date: {review_date}", file=sys.stderr)
if args.release:
@@ -1162,26 +1282,13 @@ def main() -> None:
# Call API (unless already processed via chunks/split)
if estimated_tokens > 0: # Not already processed
- review_text, call_usage = call_api(
- args.provider,
- api_key,
- model,
- args.tokens,
- system_prompt,
- agents_content,
- patch_content,
- patch_name,
- args.output_format,
- args.verbose,
- args.timeout,
- )
+ review_text, call_usage = _run_review(patch_content, patch_name)
total_usage.add(call_usage)
if not review_text:
- error(f"No response received from {args.provider}")
+ error(f"No response received from {provider_name}")
# Format output based on requested format
- provider_name = config["name"]
if args.output_format == "json":
# For JSON, try to parse and add metadata
@@ -1205,7 +1312,7 @@ def main() -> None:
output_data = {
"metadata": {
"patch_file": patch_name,
- "provider": args.provider,
+ "provider": args.via or args.provider,
"provider_name": provider_name,
"model": model,
"review_date": review_date,
@@ -1260,7 +1367,7 @@ def main() -> None:
print_token_summary(
total_usage,
- args.provider,
+ args.via or args.provider,
model,
args.show_tokens or args.verbose,
)
--
2.54.0
^ permalink raw reply related
* RE: [PATCH] crypto/virtio: cookies are allocated from mempool
From: Jiang, YuX @ 2026-07-06 3:00 UTC (permalink / raw)
To: Nicolau, Radu, dev@dpdk.org
Cc: maxime.coquelin@redhat.com, Nicolau, Radu,
jianjay.zhou@huawei.com, Fan Zhang
In-Reply-To: <20260703144931.2541180-1-radu.nicolau@intel.com>
> -----Original Message-----
> From: Radu Nicolau <radu.nicolau@intel.com>
> Sent: Friday, July 3, 2026 10:50 PM
> To: dev@dpdk.org
> Cc: maxime.coquelin@redhat.com; Nicolau, Radu <radu.nicolau@intel.com>;
> jianjay.zhou@huawei.com; Fan Zhang <fanzhang.oss@gmail.com>
> Subject: [PATCH] crypto/virtio: cookies are allocated from mempool
>
> The Rx/Tx functions allocate cookies as needed, no need to allocate and free from
> heap.
>
> Fixes: 6f0175ff53e0 ("crypto/virtio: support basic PMD ops")
> Cc: jianjay.zhou@huawei.com
>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
> drivers/crypto/virtio/virtio_cryptodev.c | 24 +-----------------------
> 1 file changed, 1 insertion(+), 23 deletions(-)
>
Tested-by: Yu Jiang <yux.jiang@intel.com>
Best Regards
Yu Jiang
^ permalink raw reply
* RE: [PATCH v2] pdump: fix teardown race with opt-in MP request mode
From: Kumar, Pushpendra1X @ 2026-07-05 21:08 UTC (permalink / raw)
To: Stephen Hemminger
Cc: dev, stable, Thomas Monjalon, Pattan, Reshma, Burakov, Anatoly,
Dmitry Kozlyuk, Richardson, Bruce, 14pwcse1224@uetpeshawar.edu.pk,
pushpendra.kumar@tieto.com
In-Reply-To: <CAOaVG15QiN6z8queMZmrFNN5xdii4r5G08R8smhLcnjSht7MRA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 42925 bytes --]
Hi Stephen,
Thanks for taking time to review.
Just to clarify, the async ENABLE/DISABLE approach was part of v1 and I
dropped that direction in v2.
The current proposal still uses synchronous request handling. The EAL
change is only intended to allow pdump to ignore peers that do not
register the pdump action, while preserving existing
rte_mp_request_sync() semantics for current users.
The generation tracking and inflight synchronization added in pdump are
there to prevent teardown races from delayed processing after a DISABLE
request, not to make enable/disable asynchronous. The intent is simply
to avoid releasing shared resources while another secondary may still be
processing a previously forwarded request.
I may be being overly cautious here, but I'd appreciate your thoughts on
whether you consider this protection unnecessary.
My concern is that the forwarding model currently assumes all
secondaries participate in pdump, while EAL does not enforce or track
that assumption.
On the other hand, if EAL were to handle this generically, it would need
a mechanism to determine which secondary processes have registered a
given action before forwarding requests. The proposed API remains
generic, while pdump is simply its first user.
Thanks,
Pushpendra
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Sunday, July 5, 2026 11:58 PM
To: Kumar, Pushpendra1X <pushpendra1x.kumar@intel.com>
Cc: dev <dev@dpdk.org>; stable <stable@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Pattan, Reshma <reshma.pattan@intel.com>; Burakov, Anatoly <anatoly.burakov@intel.com>; Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>; Richardson, Bruce <bruce.richardson@intel.com>; 14pwcse1224@uetpeshawar.edu.pk; pushpendra.kumar@tieto.com
Subject: Re: [PATCH v2] pdump: fix teardown race with opt-in MP request mode
Root cause is not calling rte_pdump_init in secondary. This should be detected or fixed automatically by EAL. Working around with async adds unnecessary complexity
On Sun, Jul 5, 2026, 01:48 Pushpendra Kumar <pushpendra1x.kumar@intel.com<mailto:pushpendra1x.kumar@intel.com>> wrote:
This change addresses two pdump teardown failure cases:
1. A requester secondary can time out if a forwarded secondary is slow,
unresponsive, or does not implement rte_pdump_init().
2. After a DISABLE request times out, the requester secondary receives
the reply and releases shared resources, which other slow or
unresponsive secondaries may access, leading to a crash.
Add an opt-in EAL multiprocess request mode that allows a caller to
ignore peers that do not implement the requested action, while preserving
the existing rte_mp_request_sync() behavior for legacy users.
pdump uses this new mode when forwarding enable/disable requests and adds
the following behavior:
- track request generation so stale callbacks cannot act on a newer
enable/disable cycle
- wait for in-flight callbacks to drain before completing disable
- prevent shared resource teardown from racing with delayed secondary
processing
- keep pdump intent-driven without extra consumer-specific workaround
logic
The EAL change is required because the current mp request path treats
a missing action as a failure, which cannot satisfy pdump's needs without
changing behavior for all existing users. The new API is opt-in, so
existing callers of rte_mp_request_sync() keep their current semantics.
Validation:
- Baseline-vs-changes benchmark used DPDK-native test-pmd as primary and
dumpcap as secondary.
- Both runs captured 1024 packets with the same packet-count exit
condition.
- A 5-second perf stat sample on the forwarding primary showed no
meaningful regression: with changes recorded 12,402,055,320 cycles and
17,391,769,084 instructions, versus 12,450,031,900 cycles and
17,499,706,053 instructions on the parent baseline.
- The extra synchronization in the hot path did not show material
overhead in this sample.
Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com<mailto:pushpendra1x.kumar@intel.com>>
Cc: stable@dpdk.org<mailto:stable@dpdk.org>
---
This V2 patch addresses two pdump teardown failure scenarios:
- A requester secondary can time out if a forwarded secondary is slow,
unresponsive, or does not implement rte_pdump_init().
- A delayed DISABLE reply can race with requester-side teardown, which may
lead to crashes if peer processes still access shared pdump resources.
To fix this end-to-end:
- EAL adds an opt-in MP request mode to ignore peers with no matching action,
while preserving legacy rte_mp_request_sync() behavior.
- pdump uses that mode, tracks generation, and waits for in-flight callbacks
to drain before completing disable.
- dumpcap and pdump app cleanup paths are fail-closed when disable does not
complete successfully.
If this direction is accepted, I can split from the next revision into a
focused EAL patch followed by pdump and app-user updates.
.mailmap | 1 +
app/dumpcap/main.c | 23 ++-
app/pdump/main.c | 32 +++-
doc/guides/rel_notes/release_26_07.rst | 7 +
lib/eal/common/eal_common_proc.c | 37 +++-
lib/eal/include/rte_eal.h | 33 ++++
lib/eal/windows/eal_mp.c | 9 +
lib/pdump/rte_pdump.c | 241 ++++++++++++++++++++++---
lib/pdump/rte_pdump.h | 40 +++-
9 files changed, 379 insertions(+), 44 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<mailto:przemyslaw.gierszynski@intel.com>>
Przemyslaw Patynowski <przemyslawx.patynowski@intel.com<mailto:przemyslawx.patynowski@intel.com>>
Przemyslaw Zegan <przemyslawx.zegan@intel.com<mailto:przemyslawx.zegan@intel.com>>
Pu Xu <583493798@qq.com<mailto:583493798@qq.com>>
+Pushpendra Kumar <pushpendra1x.kumar@intel.com<mailto:pushpendra1x.kumar@intel.com>>
Qi Fu <qi.fu@intel.com<mailto:qi.fu@intel.com>>
Qi Zhang <qi.z.zhang@intel.com<mailto:qi.z.zhang@intel.com>>
Qian Hao <qi_an_hao@126.com<mailto:qi_an_hao@126.com>>
diff --git a/app/dumpcap/main.c b/app/dumpcap/main.c
index 46a6cb251e..f421a17034 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -521,14 +521,33 @@ static void
cleanup_pdump_resources(void)
{
struct interface *intf;
+ int ret;
+ bool disable_failed = false;
TAILQ_FOREACH(intf, &interfaces, next) {
- rte_pdump_disable(intf->port,
- RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ ret = rte_pdump_disable(intf->port,
+ RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ if (ret < 0) {
+ disable_failed = true;
+ fprintf(stderr,
+ "Disable pdump failed on %u:%s: %s\n",
+ intf->port, intf->name,
+ rte_strerror(rte_errno));
+ }
if (intf->opts.promisc_mode)
rte_eth_promiscuous_disable(intf->port);
}
+ if (disable_failed) {
+ /*
+ * Fail-closed: if disable did not complete, keep pdump action/state
+ * alive and do not uninit shared capture resources.
+ */
+ fprintf(stderr,
+ "Skipping pdump uninit because disable did not complete on all interfaces\n");
+ return;
+ }
+
rte_pdump_uninit();
}
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 1e62c8adc1..4cfb038395 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -451,14 +451,17 @@ print_pdump_stats(void)
}
}
-static inline void
+static inline int
disable_pdump(struct pdump_tuples *pt)
{
if (pt->dump_by_type == DEVICE_ID)
- rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
- pt->dir);
+ return rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
+ pt->dir);
else if (pt->dump_by_type == PORT_ID)
- rte_pdump_disable(pt->port, pt->queue, pt->dir);
+ return rte_pdump_disable(pt->port, pt->queue, pt->dir);
+
+ rte_errno = EINVAL;
+ return -EINVAL;
}
static inline void
@@ -518,15 +521,30 @@ static void
cleanup_pdump_resources(void)
{
int i;
+ int ret;
+ bool disable_failed = false;
struct pdump_tuples *pt;
char name[RTE_ETH_NAME_MAX_LEN];
- /* disable pdump and free the pdump_tuple resources */
+ /* Disable all callbacks first; freeing shared objects before this is unsafe. */
for (i = 0; i < num_tuples; i++) {
pt = &pdump_t[i];
- /* remove callbacks */
- disable_pdump(pt);
+ ret = disable_pdump(pt);
+ if (ret < 0) {
+ disable_failed = true;
+ printf("pdump disable failed (tuple=%d, errno=%d: %s); "
+ "skip teardown to avoid stale callback access\n",
+ i, rte_errno, rte_strerror(rte_errno));
+ }
+ }
+
+ if (disable_failed)
+ return;
+
+ /* free the pdump_tuple resources */
+ for (i = 0; i < num_tuples; i++) {
+ pt = &pdump_t[i];
/*
* transmit rest of the enqueued packets of the rings on to
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 4ca0a9ac77..1700a56bc7 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -177,6 +177,13 @@ New Features
Added AGENTS.md file for AI review
and supporting scripts to review patches and documentation.
+* **Hardened pdump teardown on disable failure.**
+
+ Hardened pdump request completion and application cleanup behavior so timeout
+ or disable failure does not trigger premature pdump resource teardown.
+ pdump now uses an opt-in MP request mode that ignores peers without the
+ pdump action, while preserving legacy EAL MP behavior for all other users.
+
Removed Items
-------------
diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 06f151818c..5a458c0417 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -56,6 +56,7 @@ static struct action_entry_list action_entry_list =
enum mp_type {
MP_MSG, /* Share message with peers, will not block */
MP_REQ, /* Request for information, Will block for a reply */
+ MP_REQ_IGN, /* Request where missing action should return ignore */
MP_REP, /* Response to previously-received request */
MP_IGN, /* Response telling requester to ignore this response */
};
@@ -384,11 +385,11 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
pthread_mutex_unlock(&mp_mutex_action);
if (!action) {
- if (m->type == MP_REQ && !internal_conf->init_complete) {
- /* if this is a request, and init is not yet complete,
- * and callback wasn't registered, we should tell the
- * requester to ignore our existence because we're not
- * yet ready to process this request.
+ if ((m->type == MP_REQ && !internal_conf->init_complete) ||
+ m->type == MP_REQ_IGN) {
+ /*
+ * Ask requester to ignore this peer when action is not
+ * registered either due to early init or explicit request policy.
*/
struct rte_mp_msg dummy;
@@ -936,7 +937,8 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
static int
mp_request_sync(const char *dst, struct rte_mp_msg *req,
- struct rte_mp_reply *reply, const struct timespec *ts)
+ struct rte_mp_reply *reply, const struct timespec *ts,
+ enum mp_type req_type)
{
int ret;
pthread_condattr_t attr;
@@ -959,7 +961,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
return -1;
}
- ret = send_msg(dst, req, MP_REQ);
+ ret = send_msg(dst, req, req_type);
if (ret < 0) {
EAL_LOG(ERR, "Fail to send request %s:%s",
dst, req->name);
@@ -1010,11 +1012,20 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
int dir_fd, ret = -1;
DIR *mp_dir;
struct dirent *ent;
struct timespec now, end;
+ enum mp_type req_type = MP_REQ;
const struct internal_config *internal_conf =
eal_get_internal_configuration();
@@ -1027,6 +1038,14 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
if (check_input(req) != 0)
goto end;
+ if (flags & ~RTE_MP_REQ_F_IGNORE_NO_ACTION) {
+ rte_errno = EINVAL;
+ goto end;
+ }
+
+ if (flags & RTE_MP_REQ_F_IGNORE_NO_ACTION)
+ req_type = MP_REQ_IGN;
+
if (internal_conf->no_shconf) {
EAL_LOG(DEBUG, "No shared files mode enabled, IPC is disabled");
rte_errno = ENOTSUP;
@@ -1046,7 +1065,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* for secondary process, send request to the primary process only */
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
pthread_mutex_lock(&pending_requests.lock);
- ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end);
+ ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end, req_type);
pthread_mutex_unlock(&pending_requests.lock);
goto end;
}
@@ -1086,7 +1105,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* unlocks the mutex while waiting for response,
* locks on receive
*/
- if (mp_request_sync(path, req, reply, &end))
+ if (mp_request_sync(path, req, reply, &end, req_type))
goto unlock_end;
}
ret = 0;
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 7241f3be5d..415b974945 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -173,6 +173,15 @@ struct rte_mp_reply {
struct rte_mp_msg *msgs; /* caller to free */
};
+/** Request flags for rte_mp_request_sync_ex(). */
+enum rte_mp_request_flags {
+ /**
+ * Ask peers that do not have a registered action to return MP_IGN
+ * instead of causing timeout for this request.
+ */
+ RTE_MP_REQ_F_IGNORE_NO_ACTION = 1u << 0,
+};
+
/**
* Action function typedef used by other components.
*
@@ -292,6 +301,30 @@ int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts);
+/**
+ * Send a request to peer processes with explicit request flags.
+ *
+ * This API is equivalent to rte_mp_request_sync() with opt-in behavior
+ * controls provided through @p flags.
+ *
+ * @param req
+ * The req argument contains the customized request message.
+ * @param reply
+ * The reply argument will be for storing all the replied messages;
+ * the caller is responsible for free reply->msgs.
+ * @param ts
+ * The ts argument specifies how long we can wait for the peer(s) to reply.
+ * @param flags
+ * Bitmask of values from enum rte_mp_request_flags.
+ *
+ * @return
+ * - On success, return 0.
+ * - On failure, return -1, and the reason will be stored in rte_errno.
+ */
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags);
+
/**
* Send a request to the peer process and expect a reply in a separate callback.
*
diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
index 6703355318..1066a4d2bc 100644
--- a/lib/eal/windows/eal_mp.c
+++ b/lib/eal/windows/eal_mp.c
@@ -56,10 +56,19 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
RTE_SET_USED(req);
RTE_SET_USED(reply);
RTE_SET_USED(ts);
+ RTE_SET_USED(flags);
EAL_LOG_NOT_IMPLEMENTED();
return -1;
}
diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index ac94efe7ff..3764aeb2d3 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -3,9 +3,12 @@
*/
#include <stdlib.h>
+#include <errno.h>
#include <eal_export.h>
+#include <rte_eal.h>
#include <rte_alarm.h>
+#include <rte_cycles.h>
#include <rte_mbuf.h>
#include <rte_ethdev.h>
#include <rte_lcore.h>
@@ -68,6 +71,7 @@ struct pdump_request {
const struct rte_bpf_prm *prm;
uint32_t snaplen;
+ uint32_t generation;
};
struct pdump_response {
@@ -81,6 +85,11 @@ struct pdump_bundle {
char peer[];
};
+struct pdump_shared_ctl {
+ RTE_ATOMIC(uint32_t) enabled;
+ RTE_ATOMIC(uint32_t) inflight;
+};
+
static struct pdump_rxtx_cbs {
struct rte_ring *ring;
struct rte_mempool *mp;
@@ -88,11 +97,11 @@ static struct pdump_rxtx_cbs {
const struct rte_bpf *filter;
enum pdump_version ver;
uint32_t snaplen;
+ uint32_t generation;
RTE_ATOMIC(uint32_t) use_count;
} rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
-
/*
* The packet capture statistics keep track of packets
* accepted, filtered and dropped. These are per-queue
@@ -102,9 +111,59 @@ static const char MZ_RTE_PDUMP_STATS[] = "rte_pdump_stats";
static struct {
struct rte_pdump_stats rx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
struct rte_pdump_stats tx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl rx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl tx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ RTE_ATOMIC(uint32_t) generation;
const struct rte_memzone *mz;
} *pdump_stats;
+static int
+pdump_enter(struct pdump_shared_ctl *ctl, const struct pdump_rxtx_cbs *cbs)
+{
+ uint32_t generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_relaxed);
+
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) == 0 ||
+ cbs->generation != generation)
+ return 0;
+
+ rte_atomic_fetch_add_explicit(&ctl->inflight, 1, rte_memory_order_acquire);
+
+ /*
+ * Disable/re-enable may race with callback entry. Re-check enabled and
+ * generation after taking inflight reference so disable can wait for this
+ * callback to drain and stale callbacks become no-op.
+ */
+ generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_relaxed);
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) != 0 &&
+ cbs->generation == generation)
+ return 1;
+
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+ return 0;
+}
+
+static __rte_always_inline void
+pdump_exit(struct pdump_shared_ctl *ctl)
+{
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+}
+
+static int
+pdump_wait_inflight(struct pdump_shared_ctl *ctl)
+{
+ uint64_t end_tsc = rte_get_timer_cycles() + MP_TIMEOUT_S * rte_get_timer_hz();
+
+ while (rte_atomic_load_explicit(&ctl->inflight, rte_memory_order_acquire) != 0) {
+ if (rte_get_timer_cycles() > end_tsc)
+ return -ETIMEDOUT;
+ rte_pause();
+ }
+
+ return 0;
+}
+
static void
pdump_cb_wait(struct pdump_rxtx_cbs *cbs)
{
@@ -224,12 +283,17 @@ pdump_rx(uint16_t port, uint16_t queue,
uint16_t max_pkts __rte_unused, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->rx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_IN,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -239,12 +303,17 @@ pdump_tx(uint16_t port, uint16_t queue,
struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->tx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_OUT,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -255,20 +324,43 @@ pdump_register_rx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &rx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "rx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "rx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale rx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale rx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -276,6 +368,7 @@ pdump_register_rx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_first_rx_callback(port, qid,
pdump_rx, cbs);
@@ -286,10 +379,15 @@ pdump_register_rx_callbacks(enum pdump_version ver,
return rte_errno;
}
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
+
memset(&pdump_stats->rx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing rx callback for port=%d queue=%d",
@@ -298,6 +396,11 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove rx callback, errno=%d",
-ret);
@@ -305,6 +408,10 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -316,7 +423,8 @@ pdump_register_tx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
@@ -324,13 +432,36 @@ pdump_register_tx_callbacks(enum pdump_version ver,
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &tx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "tx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "tx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale tx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale tx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -338,6 +469,7 @@ pdump_register_tx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_tx_callback(port, qid, pdump_tx,
cbs);
@@ -347,10 +479,15 @@ pdump_register_tx_callbacks(enum pdump_version ver,
rte_errno);
return rte_errno;
}
+
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
memset(&pdump_stats->tx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing tx callback for port=%d queue=%d",
@@ -359,6 +496,11 @@ pdump_register_tx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove tx callback, errno=%d",
-ret);
@@ -367,6 +509,10 @@ pdump_register_tx_callbacks(enum pdump_version ver,
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -459,7 +605,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_rx_q : queue + 1;
ret = pdump_register_rx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
@@ -469,7 +616,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_tx_q : queue + 1;
ret = pdump_register_tx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
@@ -477,12 +625,14 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
return ret;
}
-static void
-pdump_request_to_secondary(const struct pdump_request *req)
+static int
+pdump_request_to_secondary_sync(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};
+ int ret = 0;
+ uint16_t i;
PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary", pdump_opname(req->op));
@@ -490,14 +640,41 @@ pdump_request_to_secondary(const struct pdump_request *req)
strlcpy(mp_req.name<http://mp_req.name>, PDUMP_MP, sizeof(mp_req.name<http://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");
+ if (rte_mp_request_sync_ex(&mp_req, &mp_reply, &ts,
+ RTE_MP_REQ_F_IGNORE_NO_ACTION) != 0) {
+ PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed: %s",
+ strerror(rte_errno));
+ return -rte_errno;
+ }
- else if (mp_reply.nb_sent != mp_reply.nb_received)
+ 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);
+ ret = -ETIMEDOUT;
+ }
+
+ for (i = 0; i < mp_reply.nb_received; i++) {
+ struct rte_mp_msg *mp_rep = &mp_reply.msgs[i];
+ const struct pdump_response *resp;
+
+ if (mp_rep->len_param != sizeof(*resp)) {
+ PDUMP_LOG_LINE(ERR, "invalid secondary reply size %u", mp_rep->len_param);
+ if (ret == 0)
+ ret = -EINVAL;
+ continue;
+ }
+
+ resp = (const struct pdump_response *)mp_rep->param;
+ if (resp->err_value != 0) {
+ PDUMP_LOG_LINE(ERR, "secondary reply failed: op=%u err=%d",
+ resp->res_op, resp->err_value);
+ if (ret == 0)
+ ret = resp->err_value;
+ }
+ }
free(mp_reply.msgs);
+ return ret;
}
/* Allocate temporary storage for passing state to the alarm thread for deferred handling */
@@ -556,7 +733,10 @@ pdump_handle_primary_request(const struct rte_mp_msg *mp_msg, const void *peer)
PDUMP_LOG_LINE(DEBUG, "secondary pdump %s", pdump_opname(req->op));
/* Can just do it now, no need for interrupt thread */
- ret = set_pdump_rxtx_cbs(req);
+ if (req->op == ENABLE || req->op == DISABLE)
+ ret = set_pdump_rxtx_cbs(req);
+ else
+ ret = -EINVAL;
}
return pdump_send_response(req, ret, peer);
@@ -569,17 +749,29 @@ __pdump_request(void *param)
{
struct pdump_bundle *bundle = param;
struct rte_mp_msg *msg = &bundle->msg;
- const struct pdump_request *req =
- (const struct pdump_request *)msg->param;
+ struct pdump_request *req =
+ (struct pdump_request *)msg->param;
int ret;
PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
+ if (req->op == DISABLE) {
+ req->generation = rte_atomic_fetch_add_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_acq_rel) + 1;
+ } else if (req->op == ENABLE) {
+ req->generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_acquire);
+ }
+
ret = set_pdump_rxtx_cbs(req);
- /* Primary process is responsible for broadcasting request to all secondaries */
+ /*
+ * Primary process is responsible for broadcasting the request to all
+ * secondaries. The sync request uses opt-in ignore-missing-action mode so
+ * pdump does not depend on unrelated secondary processes.
+ */
if (ret == 0)
- pdump_request_to_secondary(req);
+ ret = pdump_request_to_secondary_sync(req);
pdump_send_response(req, ret, bundle->peer);
free(bundle);
@@ -659,6 +851,9 @@ rte_pdump_init(void)
pdump_stats = mz->addr;
pdump_stats->mz = mz;
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+ rte_atomic_store_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_release);
return 0;
}
diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h
index 1e32d46097..77e5914420 100644
--- a/lib/pdump/rte_pdump.h
+++ b/lib/pdump/rte_pdump.h
@@ -64,7 +64,7 @@ rte_pdump_uninit(void);
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
* @param ring
* ring on which captured packets will be enqueued for user.
* @param mp
@@ -72,6 +72,11 @@ rte_pdump_uninit(void);
* @param filter
* Unused should be NULL.
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -103,6 +108,11 @@ rte_pdump_enable(uint16_t port, uint16_t queue, uint32_t flags,
* @param prm
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -124,7 +134,14 @@ rte_pdump_enable_bpf(uint16_t port_id, uint16_t queue,
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
@@ -153,6 +170,11 @@ rte_pdump_disable(uint16_t port, uint16_t queue, uint32_t flags);
* @param filter
* unused should be NULL
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -186,6 +208,11 @@ rte_pdump_enable_by_deviceid(char *device_id, uint16_t queue,
* @param filter
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -210,7 +237,14 @@ rte_pdump_enable_bpf_by_deviceid(const char *device_id, uint16_t queue,
* queues of a given device id.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
--
2.43.0
[-- Attachment #2: Type: text/html, Size: 78222 bytes --]
^ permalink raw reply related
* Re: [PATCH v2] pdump: fix teardown race with opt-in MP request mode
From: Stephen Hemminger @ 2026-07-05 18:27 UTC (permalink / raw)
To: Pushpendra Kumar
Cc: dev, stable, Thomas Monjalon, Reshma Pattan, Anatoly Burakov,
Dmitry Kozlyuk, Bruce Richardson, 14pwcse1224, pushpendra.kumar
In-Reply-To: <20260705084415.1389340-1-pushpendra1x.kumar@intel.com>
[-- Attachment #1: Type: text/plain, Size: 41652 bytes --]
Root cause is not calling rte_pdump_init in secondary. This should be
detected or fixed automatically by EAL. Working around with async adds
unnecessary complexity
On Sun, Jul 5, 2026, 01:48 Pushpendra Kumar <pushpendra1x.kumar@intel.com>
wrote:
> This change addresses two pdump teardown failure cases:
>
> 1. A requester secondary can time out if a forwarded secondary is slow,
> unresponsive, or does not implement rte_pdump_init().
> 2. After a DISABLE request times out, the requester secondary receives
> the reply and releases shared resources, which other slow or
> unresponsive secondaries may access, leading to a crash.
>
> Add an opt-in EAL multiprocess request mode that allows a caller to
> ignore peers that do not implement the requested action, while preserving
> the existing rte_mp_request_sync() behavior for legacy users.
>
> pdump uses this new mode when forwarding enable/disable requests and adds
> the following behavior:
> - track request generation so stale callbacks cannot act on a newer
> enable/disable cycle
> - wait for in-flight callbacks to drain before completing disable
> - prevent shared resource teardown from racing with delayed secondary
> processing
> - keep pdump intent-driven without extra consumer-specific workaround
> logic
>
> The EAL change is required because the current mp request path treats
> a missing action as a failure, which cannot satisfy pdump's needs without
> changing behavior for all existing users. The new API is opt-in, so
> existing callers of rte_mp_request_sync() keep their current semantics.
>
> Validation:
> - Baseline-vs-changes benchmark used DPDK-native test-pmd as primary and
> dumpcap as secondary.
> - Both runs captured 1024 packets with the same packet-count exit
> condition.
> - A 5-second perf stat sample on the forwarding primary showed no
> meaningful regression: with changes recorded 12,402,055,320 cycles and
> 17,391,769,084 instructions, versus 12,450,031,900 cycles and
> 17,499,706,053 instructions on the parent baseline.
> - The extra synchronization in the hot path did not show material
> overhead in this sample.
>
> Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
> Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
>
> Cc: stable@dpdk.org
> ---
> This V2 patch addresses two pdump teardown failure scenarios:
>
> - A requester secondary can time out if a forwarded secondary is slow,
> unresponsive, or does not implement rte_pdump_init().
> - A delayed DISABLE reply can race with requester-side teardown, which may
> lead to crashes if peer processes still access shared pdump resources.
>
> To fix this end-to-end:
> - EAL adds an opt-in MP request mode to ignore peers with no matching
> action,
> while preserving legacy rte_mp_request_sync() behavior.
> - pdump uses that mode, tracks generation, and waits for in-flight
> callbacks
> to drain before completing disable.
> - dumpcap and pdump app cleanup paths are fail-closed when disable does not
> complete successfully.
>
> If this direction is accepted, I can split from the next revision into a
> focused EAL patch followed by pdump and app-user updates.
>
> .mailmap | 1 +
> app/dumpcap/main.c | 23 ++-
> app/pdump/main.c | 32 +++-
> doc/guides/rel_notes/release_26_07.rst | 7 +
> lib/eal/common/eal_common_proc.c | 37 +++-
> lib/eal/include/rte_eal.h | 33 ++++
> lib/eal/windows/eal_mp.c | 9 +
> lib/pdump/rte_pdump.c | 241 ++++++++++++++++++++++---
> lib/pdump/rte_pdump.h | 40 +++-
> 9 files changed, 379 insertions(+), 44 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/app/dumpcap/main.c b/app/dumpcap/main.c
> index 46a6cb251e..f421a17034 100644
> --- a/app/dumpcap/main.c
> +++ b/app/dumpcap/main.c
> @@ -521,14 +521,33 @@ static void
> cleanup_pdump_resources(void)
> {
> struct interface *intf;
> + int ret;
> + bool disable_failed = false;
>
> TAILQ_FOREACH(intf, &interfaces, next) {
> - rte_pdump_disable(intf->port,
> - RTE_PDUMP_ALL_QUEUES,
> RTE_PDUMP_FLAG_RXTX);
> + ret = rte_pdump_disable(intf->port,
> + RTE_PDUMP_ALL_QUEUES,
> RTE_PDUMP_FLAG_RXTX);
> + if (ret < 0) {
> + disable_failed = true;
> + fprintf(stderr,
> + "Disable pdump failed on %u:%s: %s\n",
> + intf->port, intf->name,
> + rte_strerror(rte_errno));
> + }
> if (intf->opts.promisc_mode)
> rte_eth_promiscuous_disable(intf->port);
> }
>
> + if (disable_failed) {
> + /*
> + * Fail-closed: if disable did not complete, keep pdump
> action/state
> + * alive and do not uninit shared capture resources.
> + */
> + fprintf(stderr,
> + "Skipping pdump uninit because disable did not
> complete on all interfaces\n");
> + return;
> + }
> +
> rte_pdump_uninit();
> }
>
> diff --git a/app/pdump/main.c b/app/pdump/main.c
> index 1e62c8adc1..4cfb038395 100644
> --- a/app/pdump/main.c
> +++ b/app/pdump/main.c
> @@ -451,14 +451,17 @@ print_pdump_stats(void)
> }
> }
>
> -static inline void
> +static inline int
> disable_pdump(struct pdump_tuples *pt)
> {
> if (pt->dump_by_type == DEVICE_ID)
> - rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
> - pt->dir);
> + return rte_pdump_disable_by_deviceid(pt->device_id,
> pt->queue,
> + pt->dir);
> else if (pt->dump_by_type == PORT_ID)
> - rte_pdump_disable(pt->port, pt->queue, pt->dir);
> + return rte_pdump_disable(pt->port, pt->queue, pt->dir);
> +
> + rte_errno = EINVAL;
> + return -EINVAL;
> }
>
> static inline void
> @@ -518,15 +521,30 @@ static void
> cleanup_pdump_resources(void)
> {
> int i;
> + int ret;
> + bool disable_failed = false;
> struct pdump_tuples *pt;
> char name[RTE_ETH_NAME_MAX_LEN];
>
> - /* disable pdump and free the pdump_tuple resources */
> + /* Disable all callbacks first; freeing shared objects before this
> is unsafe. */
> for (i = 0; i < num_tuples; i++) {
> pt = &pdump_t[i];
>
> - /* remove callbacks */
> - disable_pdump(pt);
> + ret = disable_pdump(pt);
> + if (ret < 0) {
> + disable_failed = true;
> + printf("pdump disable failed (tuple=%d, errno=%d:
> %s); "
> + "skip teardown to avoid stale callback
> access\n",
> + i, rte_errno, rte_strerror(rte_errno));
> + }
> + }
> +
> + if (disable_failed)
> + return;
> +
> + /* free the pdump_tuple resources */
> + for (i = 0; i < num_tuples; i++) {
> + pt = &pdump_t[i];
>
> /*
> * transmit rest of the enqueued packets of the rings on to
> diff --git a/doc/guides/rel_notes/release_26_07.rst
> b/doc/guides/rel_notes/release_26_07.rst
> index 4ca0a9ac77..1700a56bc7 100644
> --- a/doc/guides/rel_notes/release_26_07.rst
> +++ b/doc/guides/rel_notes/release_26_07.rst
> @@ -177,6 +177,13 @@ New Features
> Added AGENTS.md file for AI review
> and supporting scripts to review patches and documentation.
>
> +* **Hardened pdump teardown on disable failure.**
> +
> + Hardened pdump request completion and application cleanup behavior so
> timeout
> + or disable failure does not trigger premature pdump resource teardown.
> + pdump now uses an opt-in MP request mode that ignores peers without the
> + pdump action, while preserving legacy EAL MP behavior for all other
> users.
> +
>
> Removed Items
> -------------
> diff --git a/lib/eal/common/eal_common_proc.c
> b/lib/eal/common/eal_common_proc.c
> index 06f151818c..5a458c0417 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -56,6 +56,7 @@ static struct action_entry_list action_entry_list =
> enum mp_type {
> MP_MSG, /* Share message with peers, will not block */
> MP_REQ, /* Request for information, Will block for a reply */
> + MP_REQ_IGN, /* Request where missing action should return ignore */
> MP_REP, /* Response to previously-received request */
> MP_IGN, /* Response telling requester to ignore this response */
> };
> @@ -384,11 +385,11 @@ process_msg(struct mp_msg_internal *m, struct
> sockaddr_un *s)
> pthread_mutex_unlock(&mp_mutex_action);
>
> if (!action) {
> - if (m->type == MP_REQ && !internal_conf->init_complete) {
> - /* if this is a request, and init is not yet
> complete,
> - * and callback wasn't registered, we should tell
> the
> - * requester to ignore our existence because we're
> not
> - * yet ready to process this request.
> + if ((m->type == MP_REQ && !internal_conf->init_complete) ||
> + m->type == MP_REQ_IGN) {
> + /*
> + * Ask requester to ignore this peer when action
> is not
> + * registered either due to early init or explicit
> request policy.
> */
> struct rte_mp_msg dummy;
>
> @@ -936,7 +937,8 @@ mp_request_async(const char *dst, struct rte_mp_msg
> *req,
>
> static int
> mp_request_sync(const char *dst, struct rte_mp_msg *req,
> - struct rte_mp_reply *reply, const struct timespec *ts)
> + struct rte_mp_reply *reply, const struct timespec *ts,
> + enum mp_type req_type)
> {
> int ret;
> pthread_condattr_t attr;
> @@ -959,7 +961,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg
> *req,
> return -1;
> }
>
> - ret = send_msg(dst, req, MP_REQ);
> + ret = send_msg(dst, req, req_type);
> if (ret < 0) {
> EAL_LOG(ERR, "Fail to send request %s:%s",
> dst, req->name);
> @@ -1010,11 +1012,20 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
> int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts)
> +{
> + return rte_mp_request_sync_ex(req, reply, ts, 0);
> +}
> +
> +RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags)
> {
> int dir_fd, ret = -1;
> DIR *mp_dir;
> struct dirent *ent;
> struct timespec now, end;
> + enum mp_type req_type = MP_REQ;
> const struct internal_config *internal_conf =
> eal_get_internal_configuration();
>
> @@ -1027,6 +1038,14 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> if (check_input(req) != 0)
> goto end;
>
> + if (flags & ~RTE_MP_REQ_F_IGNORE_NO_ACTION) {
> + rte_errno = EINVAL;
> + goto end;
> + }
> +
> + if (flags & RTE_MP_REQ_F_IGNORE_NO_ACTION)
> + req_type = MP_REQ_IGN;
> +
> if (internal_conf->no_shconf) {
> EAL_LOG(DEBUG, "No shared files mode enabled, IPC is
> disabled");
> rte_errno = ENOTSUP;
> @@ -1046,7 +1065,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> /* for secondary process, send request to the primary process only
> */
> if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> pthread_mutex_lock(&pending_requests.lock);
> - ret = mp_request_sync(eal_mp_socket_path(), req, reply,
> &end);
> + ret = mp_request_sync(eal_mp_socket_path(), req, reply,
> &end, req_type);
> pthread_mutex_unlock(&pending_requests.lock);
> goto end;
> }
> @@ -1086,7 +1105,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct
> rte_mp_reply *reply,
> /* unlocks the mutex while waiting for response,
> * locks on receive
> */
> - if (mp_request_sync(path, req, reply, &end))
> + if (mp_request_sync(path, req, reply, &end, req_type))
> goto unlock_end;
> }
> ret = 0;
> diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
> index 7241f3be5d..415b974945 100644
> --- a/lib/eal/include/rte_eal.h
> +++ b/lib/eal/include/rte_eal.h
> @@ -173,6 +173,15 @@ struct rte_mp_reply {
> struct rte_mp_msg *msgs; /* caller to free */
> };
>
> +/** Request flags for rte_mp_request_sync_ex(). */
> +enum rte_mp_request_flags {
> + /**
> + * Ask peers that do not have a registered action to return MP_IGN
> + * instead of causing timeout for this request.
> + */
> + RTE_MP_REQ_F_IGNORE_NO_ACTION = 1u << 0,
> +};
> +
> /**
> * Action function typedef used by other components.
> *
> @@ -292,6 +301,30 @@ int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts);
>
> +/**
> + * Send a request to peer processes with explicit request flags.
> + *
> + * This API is equivalent to rte_mp_request_sync() with opt-in behavior
> + * controls provided through @p flags.
> + *
> + * @param req
> + * The req argument contains the customized request message.
> + * @param reply
> + * The reply argument will be for storing all the replied messages;
> + * the caller is responsible for free reply->msgs.
> + * @param ts
> + * The ts argument specifies how long we can wait for the peer(s) to
> reply.
> + * @param flags
> + * Bitmask of values from enum rte_mp_request_flags.
> + *
> + * @return
> + * - On success, return 0.
> + * - On failure, return -1, and the reason will be stored in rte_errno.
> + */
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags);
> +
> /**
> * Send a request to the peer process and expect a reply in a separate
> callback.
> *
> diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
> index 6703355318..1066a4d2bc 100644
> --- a/lib/eal/windows/eal_mp.c
> +++ b/lib/eal/windows/eal_mp.c
> @@ -56,10 +56,19 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
> int
> rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> const struct timespec *ts)
> +{
> + return rte_mp_request_sync_ex(req, reply, ts, 0);
> +}
> +
> +RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
> +int
> +rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
> + const struct timespec *ts, uint32_t flags)
> {
> RTE_SET_USED(req);
> RTE_SET_USED(reply);
> RTE_SET_USED(ts);
> + RTE_SET_USED(flags);
> EAL_LOG_NOT_IMPLEMENTED();
> return -1;
> }
> diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
> index ac94efe7ff..3764aeb2d3 100644
> --- a/lib/pdump/rte_pdump.c
> +++ b/lib/pdump/rte_pdump.c
> @@ -3,9 +3,12 @@
> */
>
> #include <stdlib.h>
> +#include <errno.h>
>
> #include <eal_export.h>
> +#include <rte_eal.h>
> #include <rte_alarm.h>
> +#include <rte_cycles.h>
> #include <rte_mbuf.h>
> #include <rte_ethdev.h>
> #include <rte_lcore.h>
> @@ -68,6 +71,7 @@ struct pdump_request {
>
> const struct rte_bpf_prm *prm;
> uint32_t snaplen;
> + uint32_t generation;
> };
>
> struct pdump_response {
> @@ -81,6 +85,11 @@ struct pdump_bundle {
> char peer[];
> };
>
> +struct pdump_shared_ctl {
> + RTE_ATOMIC(uint32_t) enabled;
> + RTE_ATOMIC(uint32_t) inflight;
> +};
> +
> static struct pdump_rxtx_cbs {
> struct rte_ring *ring;
> struct rte_mempool *mp;
> @@ -88,11 +97,11 @@ static struct pdump_rxtx_cbs {
> const struct rte_bpf *filter;
> enum pdump_version ver;
> uint32_t snaplen;
> + uint32_t generation;
> RTE_ATOMIC(uint32_t) use_count;
> } rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
> tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
>
> -
> /*
> * The packet capture statistics keep track of packets
> * accepted, filtered and dropped. These are per-queue
> @@ -102,9 +111,59 @@ static const char MZ_RTE_PDUMP_STATS[] =
> "rte_pdump_stats";
> static struct {
> struct rte_pdump_stats
> rx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> struct rte_pdump_stats
> tx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + struct pdump_shared_ctl
> rx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + struct pdump_shared_ctl
> tx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
> + RTE_ATOMIC(uint32_t) generation;
> const struct rte_memzone *mz;
> } *pdump_stats;
>
> +static int
> +pdump_enter(struct pdump_shared_ctl *ctl, const struct pdump_rxtx_cbs
> *cbs)
> +{
> + uint32_t generation =
> rte_atomic_load_explicit(&pdump_stats->generation,
> +
> rte_memory_order_relaxed);
> +
> + if (rte_atomic_load_explicit(&ctl->enabled,
> rte_memory_order_relaxed) == 0 ||
> + cbs->generation != generation)
> + return 0;
> +
> + rte_atomic_fetch_add_explicit(&ctl->inflight, 1,
> rte_memory_order_acquire);
> +
> + /*
> + * Disable/re-enable may race with callback entry. Re-check
> enabled and
> + * generation after taking inflight reference so disable can wait
> for this
> + * callback to drain and stale callbacks become no-op.
> + */
> + generation = rte_atomic_load_explicit(&pdump_stats->generation,
> + rte_memory_order_relaxed);
> + if (rte_atomic_load_explicit(&ctl->enabled,
> rte_memory_order_relaxed) != 0 &&
> + cbs->generation == generation)
> + return 1;
> +
> + rte_atomic_fetch_sub_explicit(&ctl->inflight, 1,
> rte_memory_order_release);
> + return 0;
> +}
> +
> +static __rte_always_inline void
> +pdump_exit(struct pdump_shared_ctl *ctl)
> +{
> + rte_atomic_fetch_sub_explicit(&ctl->inflight, 1,
> rte_memory_order_release);
> +}
> +
> +static int
> +pdump_wait_inflight(struct pdump_shared_ctl *ctl)
> +{
> + uint64_t end_tsc = rte_get_timer_cycles() + MP_TIMEOUT_S *
> rte_get_timer_hz();
> +
> + while (rte_atomic_load_explicit(&ctl->inflight,
> rte_memory_order_acquire) != 0) {
> + if (rte_get_timer_cycles() > end_tsc)
> + return -ETIMEDOUT;
> + rte_pause();
> + }
> +
> + return 0;
> +}
> +
> static void
> pdump_cb_wait(struct pdump_rxtx_cbs *cbs)
> {
> @@ -224,12 +283,17 @@ pdump_rx(uint16_t port, uint16_t queue,
> uint16_t max_pkts __rte_unused, void *user_params)
> {
> struct pdump_rxtx_cbs *cbs = user_params;
> + struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][queue];
> struct rte_pdump_stats *stats = &pdump_stats->rx[port][queue];
>
> + if (!pdump_enter(ctl, cbs))
> + return nb_pkts;
> +
> pdump_cb_hold(cbs);
> pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_IN,
> pkts, nb_pkts, cbs, stats);
> pdump_cb_release(cbs);
> + pdump_exit(ctl);
>
> return nb_pkts;
> }
> @@ -239,12 +303,17 @@ pdump_tx(uint16_t port, uint16_t queue,
> struct rte_mbuf **pkts, uint16_t nb_pkts, void
> *user_params)
> {
> struct pdump_rxtx_cbs *cbs = user_params;
> + struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][queue];
> struct rte_pdump_stats *stats = &pdump_stats->tx[port][queue];
>
> + if (!pdump_enter(ctl, cbs))
> + return nb_pkts;
> +
> pdump_cb_hold(cbs);
> pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_OUT,
> pkts, nb_pkts, cbs, stats);
> pdump_cb_release(cbs);
> + pdump_exit(ctl);
>
> return nb_pkts;
> }
> @@ -255,20 +324,43 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> uint16_t end_q, uint16_t port, uint16_t queue,
> struct rte_ring *ring, struct rte_mempool *mp,
> struct rte_bpf *filter,
> - uint16_t operation, uint32_t snaplen)
> + uint16_t operation, uint32_t snaplen,
> + uint32_t generation)
> {
> uint16_t qid;
>
> qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
> for (; qid < end_q; qid++) {
> struct pdump_rxtx_cbs *cbs = &rx_cbs[port][qid];
> + struct pdump_shared_ctl *ctl =
> &pdump_stats->rx_ctl[port][qid];
>
> if (operation == ENABLE) {
> + int ret;
> +
> if (cbs->cb) {
> - PDUMP_LOG_LINE(ERR,
> - "rx callback for port=%d queue=%d,
> already exists",
> - port, qid);
> - return -EEXIST;
> + if (cbs->generation == generation &&
> + rte_atomic_load_explicit(&ctl->enabled,
> + rte_memory_order_relaxed) !=
> 0) {
> + PDUMP_LOG_LINE(ERR,
> + "rx callback for port=%d
> queue=%d, already exists",
> + port, qid);
> + return -EEXIST;
> + }
> +
> + PDUMP_LOG_LINE(DEBUG,
> + "reconciling stale rx callback for
> port=%d queue=%d"
> + " old_gen=%u new_gen=%u",
> + port, qid, cbs->generation,
> generation);
> +
> + ret = rte_eth_remove_rx_callback(port,
> qid, cbs->cb);
> + if (ret < 0) {
> + PDUMP_LOG_LINE(ERR,
> + "failed to reconcile stale
> rx callback, errno=%d",
> + -ret);
> + return ret;
> + }
> + pdump_cb_wait(cbs);
> + cbs->cb = NULL;
> }
> cbs->use_count = 0;
> cbs->ver = ver;
> @@ -276,6 +368,7 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> cbs->mp = mp;
> cbs->snaplen = snaplen;
> cbs->filter = filter;
> + cbs->generation = generation;
>
> cbs->cb = rte_eth_add_first_rx_callback(port, qid,
> pdump_rx,
> cbs);
> @@ -286,10 +379,15 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> return rte_errno;
> }
>
> + rte_atomic_store_explicit(&ctl->inflight, 0,
> rte_memory_order_relaxed);
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> rte_memory_order_release);
> +
> memset(&pdump_stats->rx[port][qid], 0,
> sizeof(struct rte_pdump_stats));
> } else if (operation == DISABLE) {
> int ret;
>
> + rte_atomic_store_explicit(&ctl->enabled, 0,
> rte_memory_order_release);
> +
> if (cbs->cb == NULL) {
> PDUMP_LOG_LINE(ERR,
> "no existing rx callback for
> port=%d queue=%d",
> @@ -298,6 +396,11 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> }
> ret = rte_eth_remove_rx_callback(port, qid,
> cbs->cb);
> if (ret < 0) {
> + /* Keep state coherent: callback is still
> registered,
> + * so restore enabled.
> + */
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> + rte_memory_order_release);
> PDUMP_LOG_LINE(ERR,
> "failed to remove rx callback,
> errno=%d",
> -ret);
> @@ -305,6 +408,10 @@ pdump_register_rx_callbacks(enum pdump_version ver,
> }
> pdump_cb_wait(cbs);
> cbs->cb = NULL;
> +
> + ret = pdump_wait_inflight(ctl);
> + if (ret < 0)
> + return ret;
> }
> }
>
> @@ -316,7 +423,8 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> uint16_t end_q, uint16_t port, uint16_t queue,
> struct rte_ring *ring, struct rte_mempool *mp,
> struct rte_bpf *filter,
> - uint16_t operation, uint32_t snaplen)
> + uint16_t operation, uint32_t snaplen,
> + uint32_t generation)
> {
>
> uint16_t qid;
> @@ -324,13 +432,36 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
> for (; qid < end_q; qid++) {
> struct pdump_rxtx_cbs *cbs = &tx_cbs[port][qid];
> + struct pdump_shared_ctl *ctl =
> &pdump_stats->tx_ctl[port][qid];
>
> if (operation == ENABLE) {
> + int ret;
> +
> if (cbs->cb) {
> - PDUMP_LOG_LINE(ERR,
> - "tx callback for port=%d queue=%d,
> already exists",
> - port, qid);
> - return -EEXIST;
> + if (cbs->generation == generation &&
> + rte_atomic_load_explicit(&ctl->enabled,
> + rte_memory_order_relaxed) !=
> 0) {
> + PDUMP_LOG_LINE(ERR,
> + "tx callback for port=%d
> queue=%d, already exists",
> + port, qid);
> + return -EEXIST;
> + }
> +
> + PDUMP_LOG_LINE(DEBUG,
> + "reconciling stale tx callback for
> port=%d queue=%d"
> + " old_gen=%u new_gen=%u",
> + port, qid, cbs->generation,
> generation);
> +
> + ret = rte_eth_remove_tx_callback(port,
> qid, cbs->cb);
> + if (ret < 0) {
> + PDUMP_LOG_LINE(ERR,
> + "failed to reconcile stale
> tx callback, errno=%d",
> + -ret);
> + return ret;
> + }
> +
> + pdump_cb_wait(cbs);
> + cbs->cb = NULL;
> }
> cbs->use_count = 0;
> cbs->ver = ver;
> @@ -338,6 +469,7 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> cbs->mp = mp;
> cbs->snaplen = snaplen;
> cbs->filter = filter;
> + cbs->generation = generation;
>
> cbs->cb = rte_eth_add_tx_callback(port, qid,
> pdump_tx,
> cbs);
> @@ -347,10 +479,15 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> rte_errno);
> return rte_errno;
> }
> +
> + rte_atomic_store_explicit(&ctl->inflight, 0,
> rte_memory_order_relaxed);
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> rte_memory_order_release);
> memset(&pdump_stats->tx[port][qid], 0,
> sizeof(struct rte_pdump_stats));
> } else if (operation == DISABLE) {
> int ret;
>
> + rte_atomic_store_explicit(&ctl->enabled, 0,
> rte_memory_order_release);
> +
> if (cbs->cb == NULL) {
> PDUMP_LOG_LINE(ERR,
> "no existing tx callback for
> port=%d queue=%d",
> @@ -359,6 +496,11 @@ pdump_register_tx_callbacks(enum pdump_version ver,
> }
> ret = rte_eth_remove_tx_callback(port, qid,
> cbs->cb);
> if (ret < 0) {
> + /* Keep state coherent: callback is still
> registered,
> + * so restore enabled.
> + */
> + rte_atomic_store_explicit(&ctl->enabled, 1,
> + rte_memory_order_release);
> PDUMP_LOG_LINE(ERR,
> "failed to remove tx callback,
> errno=%d",
> -ret);
> @@ -367,6 +509,10 @@ pdump_register_tx_callbacks(enum pdump_version ver,
>
> pdump_cb_wait(cbs);
> cbs->cb = NULL;
> +
> + ret = pdump_wait_inflight(ctl);
> + if (ret < 0)
> + return ret;
> }
> }
>
> @@ -459,7 +605,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_rx_q : queue
> + 1;
> ret = pdump_register_rx_callbacks(p->ver, end_q, port,
> queue,
> ring, mp, filter,
> - operation, p->snaplen);
> + operation, p->snaplen,
> + p->generation);
> if (ret < 0)
> return ret;
> }
> @@ -469,7 +616,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_tx_q : queue
> + 1;
> ret = pdump_register_tx_callbacks(p->ver, end_q, port,
> queue,
> ring, mp, filter,
> - operation, p->snaplen);
> + operation, p->snaplen,
> + p->generation);
> if (ret < 0)
> return ret;
> }
> @@ -477,12 +625,14 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
> return ret;
> }
>
> -static void
> -pdump_request_to_secondary(const struct pdump_request *req)
> +static int
> +pdump_request_to_secondary_sync(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};
> + int ret = 0;
> + uint16_t i;
>
> PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary",
> pdump_opname(req->op));
>
> @@ -490,14 +640,41 @@ 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");
> + if (rte_mp_request_sync_ex(&mp_req, &mp_reply, &ts,
> + RTE_MP_REQ_F_IGNORE_NO_ACTION) != 0) {
> + PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed: %s",
> + strerror(rte_errno));
> + return -rte_errno;
> + }
>
> - else if (mp_reply.nb_sent != mp_reply.nb_received)
> + 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);
> + ret = -ETIMEDOUT;
> + }
> +
> + for (i = 0; i < mp_reply.nb_received; i++) {
> + struct rte_mp_msg *mp_rep = &mp_reply.msgs[i];
> + const struct pdump_response *resp;
> +
> + if (mp_rep->len_param != sizeof(*resp)) {
> + PDUMP_LOG_LINE(ERR, "invalid secondary reply size
> %u", mp_rep->len_param);
> + if (ret == 0)
> + ret = -EINVAL;
> + continue;
> + }
> +
> + resp = (const struct pdump_response *)mp_rep->param;
> + if (resp->err_value != 0) {
> + PDUMP_LOG_LINE(ERR, "secondary reply failed: op=%u
> err=%d",
> + resp->res_op, resp->err_value);
> + if (ret == 0)
> + ret = resp->err_value;
> + }
> + }
>
> free(mp_reply.msgs);
> + return ret;
> }
>
> /* Allocate temporary storage for passing state to the alarm thread for
> deferred handling */
> @@ -556,7 +733,10 @@ pdump_handle_primary_request(const struct rte_mp_msg
> *mp_msg, const void *peer)
> PDUMP_LOG_LINE(DEBUG, "secondary pdump %s",
> pdump_opname(req->op));
>
> /* Can just do it now, no need for interrupt thread */
> - ret = set_pdump_rxtx_cbs(req);
> + if (req->op == ENABLE || req->op == DISABLE)
> + ret = set_pdump_rxtx_cbs(req);
> + else
> + ret = -EINVAL;
> }
>
> return pdump_send_response(req, ret, peer);
> @@ -569,17 +749,29 @@ __pdump_request(void *param)
> {
> struct pdump_bundle *bundle = param;
> struct rte_mp_msg *msg = &bundle->msg;
> - const struct pdump_request *req =
> - (const struct pdump_request *)msg->param;
> + struct pdump_request *req =
> + (struct pdump_request *)msg->param;
> int ret;
>
> PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
>
> + if (req->op == DISABLE) {
> + req->generation =
> rte_atomic_fetch_add_explicit(&pdump_stats->generation, 1,
> +
> rte_memory_order_acq_rel) + 1;
> + } else if (req->op == ENABLE) {
> + req->generation =
> rte_atomic_load_explicit(&pdump_stats->generation,
> +
> rte_memory_order_acquire);
> + }
> +
> ret = set_pdump_rxtx_cbs(req);
>
> - /* Primary process is responsible for broadcasting request to all
> secondaries */
> + /*
> + * Primary process is responsible for broadcasting the request to
> all
> + * secondaries. The sync request uses opt-in ignore-missing-action
> mode so
> + * pdump does not depend on unrelated secondary processes.
> + */
> if (ret == 0)
> - pdump_request_to_secondary(req);
> + ret = pdump_request_to_secondary_sync(req);
>
> pdump_send_response(req, ret, bundle->peer);
> free(bundle);
> @@ -659,6 +851,9 @@ rte_pdump_init(void)
>
> pdump_stats = mz->addr;
> pdump_stats->mz = mz;
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> + rte_atomic_store_explicit(&pdump_stats->generation, 1,
> + rte_memory_order_release);
>
> return 0;
> }
> diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h
> index 1e32d46097..77e5914420 100644
> --- a/lib/pdump/rte_pdump.h
> +++ b/lib/pdump/rte_pdump.h
> @@ -64,7 +64,7 @@ rte_pdump_uninit(void);
> * queues of a given port.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> * @param ring
> * ring on which captured packets will be enqueued for user.
> * @param mp
> @@ -72,6 +72,11 @@ rte_pdump_uninit(void);
> * @param filter
> * Unused should be NULL.
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -103,6 +108,11 @@ rte_pdump_enable(uint16_t port, uint16_t queue,
> uint32_t flags,
> * @param prm
> * Use BPF program to run to filter packes (can be NULL)
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -124,7 +134,14 @@ rte_pdump_enable_bpf(uint16_t port_id, uint16_t queue,
> * queues of a given port.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> + *
> + * @note
> + * A disable failure (including timeout/no response) means teardown is
> not
> + * complete across pdump-enabled peers. The caller must not release
> shared
> + * pdump resources and must not uninitialize pdump for that capture
> session
> + * until disable succeeds. Releasing resources on disable failure can
> lead to
> + * crashes in peer processes still accessing those resources.
> *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> @@ -153,6 +170,11 @@ rte_pdump_disable(uint16_t port, uint16_t queue,
> uint32_t flags);
> * @param filter
> * unused should be NULL
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -186,6 +208,11 @@ rte_pdump_enable_by_deviceid(char *device_id,
> uint16_t queue,
> * @param filter
> * Use BPF program to run to filter packes (can be NULL)
> *
> + * @note
> + * In applications that enable capture on multiple interfaces, enable
> may be
> + * partially applied before an error is returned. Callers should
> explicitly
> + * unwind partial enable state.
> + *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> */
> @@ -210,7 +237,14 @@ rte_pdump_enable_bpf_by_deviceid(const char
> *device_id, uint16_t queue,
> * queues of a given device id.
> * @param flags
> * flags specifies
> RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
> - * on which packet capturing should be enabled for a given port and
> queue.
> + * on which packet capturing should be disabled for a given port and
> queue.
> + *
> + * @note
> + * A disable failure (including timeout/no response) means teardown is
> not
> + * complete across pdump-enabled peers. The caller must not release
> shared
> + * pdump resources and must not uninitialize pdump for that capture
> session
> + * until disable succeeds. Releasing resources on disable failure can
> lead to
> + * crashes in peer processes still accessing those resources.
> *
> * @return
> * 0 on success, -1 on error, rte_errno is set accordingly.
> --
> 2.43.0
>
>
[-- Attachment #2: Type: text/html, Size: 51805 bytes --]
^ permalink raw reply
* [PATCH v3] pdump: fix teardown race with opt-in MP request mode
From: Pushpendra Kumar @ 2026-07-05 17:07 UTC (permalink / raw)
To: dev
Cc: stable, reshma.pattan, stephen, anatoly.burakov, dmitry.kozliuk,
thomas, Pushpendra Kumar
In-Reply-To: <20260705084415.1389340-1-pushpendra1x.kumar@intel.com>
This change addresses two pdump teardown failure cases:
1. A requester secondary can time out if a forwarded secondary is slow,
unresponsive, or does not implement rte_pdump_init().
2. After a DISABLE request times out, the requester secondary receives
the reply and releases shared resources, which other slow or
unresponsive secondaries may access, leading to a crash.
Add an opt-in EAL multiprocess request mode that allows a caller to
ignore peers that do not implement the requested action, while preserving
the existing rte_mp_request_sync() behavior for legacy users.
pdump uses this new mode when forwarding enable/disable requests and adds
the following behavior:
- track request generation so stale callbacks cannot act on a newer
enable/disable cycle
- wait for in-flight callbacks to drain before completing disable
- prevent shared resource teardown from racing with delayed secondary
processing
- keep pdump intent-driven without extra consumer-specific workaround
logic
The EAL change is required because the current mp request path treats
a missing action as a failure, which cannot satisfy pdump's needs without
changing behavior for all existing users. The new API is opt-in, so
existing callers of rte_mp_request_sync() keep their current semantics.
Validation:
- Baseline-vs-changes benchmark used DPDK-native test-pmd as primary and
dumpcap as secondary.
- Both runs captured 1024 packets with the same packet-count exit
condition.
- A 5-second perf stat sample on the forwarding primary showed no
meaningful regression: with changes recorded 12,402,055,320 cycles and
17,391,769,084 instructions, versus 12,450,031,900 cycles and
17,499,706,053 instructions on the parent baseline.
- This validates that the primary forwarding path is not materially
affected.
- Secondary callback-path overhead for pdump_enter/pdump_exit was not
separately microbenchmarked in this patch.
Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
Cc: stable@dpdk.org
---
v3:
- strengthen `pdump_enter()` generation loads to use acquire ordering
- clarify why `pdump_enter()` re-checks generation after taking `inflight`
- document the distinct roles of `use_count` and `inflight`
- document bounded disable timeout and retry expectation
- document first-error return policy in `pdump_request_to_secondary_sync()`
- document that unknown `rte_mp_request_sync_ex()` flags are rejected intentionally
- document why generation starts at 1 on primary init
- clarify fail-closed cleanup behavior and same-process recovery limitation
- reword release notes to emphasize opt-in EAL API/flag semantics and unchanged legacy behavior
- clarify commit-message validation scope: perf numbers cover the forwarding primary path only
Not changed:
- no automatic RX/TX rollback on partial enable failure
Current pdump enable intentionally remains partial-apply as documented in the public API. Adding rollback would change semantics toward transactional enable and introduce additional failure handling if rollback itself fails.
- no non-NULL `cb` after successful callback removal on timeout
After successful `rte_eth_remove_*x_callback()`, `cb` must continue to reflect actual ethdev registration state. Leaving it non-NULL would make local state inconsistent and could trigger incorrect later remove/reconcile behavior.
- no RX/TX helper refactor in this correctness patch
This area is concurrency-sensitive and RX/TX callback APIs are not identical, so keeping the logic local reduces regression risk.
If you want a shorter version for the email intro instead of the `---` block, use:
v3 updates mainly tighten documentation and clarify intent around the accepted fixes:
acquire ordering in `pdump_enter()`, explicit comments for generation re-check, `use_count` vs `inflight`, bounded timeout/retry, first-error aggregation, unknown-flag rejection, and generation initialization; fail-closed cleanup/recovery notes; release-note wording for the new opt-in EAL API; and commit-message clarification that current perf numbers cover the forwarding primary path only.
.mailmap | 1 +
app/dumpcap/main.c | 24 ++-
app/pdump/main.c | 37 +++-
doc/guides/rel_notes/release_26_07.rst | 9 +
lib/eal/common/eal_common_proc.c | 41 +++-
lib/eal/include/rte_eal.h | 33 +++
lib/eal/windows/eal_mp.c | 9 +
lib/pdump/rte_pdump.c | 266 ++++++++++++++++++++++---
lib/pdump/rte_pdump.h | 42 +++-
9 files changed, 417 insertions(+), 45 deletions(-)
diff --git a/.mailmap b/.mailmap
index 05a55c0bd6..3bfe0c4df0 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1338,6 +1338,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/app/dumpcap/main.c b/app/dumpcap/main.c
index 46a6cb251e..c6ae916075 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -521,14 +521,34 @@ static void
cleanup_pdump_resources(void)
{
struct interface *intf;
+ int ret;
+ bool disable_failed = false;
TAILQ_FOREACH(intf, &interfaces, next) {
- rte_pdump_disable(intf->port,
- RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ ret = rte_pdump_disable(intf->port,
+ RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ if (ret < 0) {
+ disable_failed = true;
+ fprintf(stderr,
+ "Disable pdump failed on %u:%s: %s\n",
+ intf->port, intf->name,
+ rte_strerror(rte_errno));
+ }
if (intf->opts.promisc_mode)
rte_eth_promiscuous_disable(intf->port);
}
+ if (disable_failed) {
+ /*
+ * Fail-closed: if disable did not complete, keep pdump action/state
+ * alive and do not uninit shared capture resources. Recovery in the
+ * same process instance is not guaranteed until disable succeeds.
+ */
+ fprintf(stderr,
+ "Skipping pdump uninit because disable did not complete on all interfaces\n");
+ return;
+ }
+
rte_pdump_uninit();
}
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 1e62c8adc1..8f33d2f603 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -451,14 +451,17 @@ print_pdump_stats(void)
}
}
-static inline void
+static inline int
disable_pdump(struct pdump_tuples *pt)
{
if (pt->dump_by_type == DEVICE_ID)
- rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
- pt->dir);
+ return rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
+ pt->dir);
else if (pt->dump_by_type == PORT_ID)
- rte_pdump_disable(pt->port, pt->queue, pt->dir);
+ return rte_pdump_disable(pt->port, pt->queue, pt->dir);
+
+ rte_errno = EINVAL;
+ return -EINVAL;
}
static inline void
@@ -518,15 +521,35 @@ static void
cleanup_pdump_resources(void)
{
int i;
+ int ret;
+ bool disable_failed = false;
struct pdump_tuples *pt;
char name[RTE_ETH_NAME_MAX_LEN];
- /* disable pdump and free the pdump_tuple resources */
+ /* Disable all callbacks first; freeing shared objects before this is unsafe. */
for (i = 0; i < num_tuples; i++) {
pt = &pdump_t[i];
- /* remove callbacks */
- disable_pdump(pt);
+ ret = disable_pdump(pt);
+ if (ret < 0) {
+ disable_failed = true;
+ printf("pdump disable failed (tuple=%d, errno=%d: %s); "
+ "skip teardown to avoid stale callback access\n",
+ i, rte_errno, rte_strerror(rte_errno));
+ }
+ }
+
+ /*
+ * Fail-closed: do not uninit/free shared pdump resources after disable
+ * failure. Recovery in the same process instance is not guaranteed until
+ * disable succeeds.
+ */
+ if (disable_failed)
+ return;
+
+ /* free the pdump_tuple resources */
+ for (i = 0; i < num_tuples; i++) {
+ pt = &pdump_t[i];
/*
* transmit rest of the enqueued packets of the rings on to
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 8b1bdada1a..51fc74e7ef 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -250,6 +250,15 @@ New Features
Added AGENTS.md file for AI review
and supporting scripts to review patches and documentation.
+* **Hardened pdump teardown on disable failure.**
+
+ Hardened pdump request completion and application cleanup behavior so timeout
+ or disable failure does not trigger premature pdump resource teardown.
+ EAL now provides ``rte_mp_request_sync_ex()`` with
+ ``RTE_MP_REQ_F_IGNORE_NO_ACTION`` to ignore peers that do not implement
+ the requested action. pdump uses this opt-in mode, while legacy
+ ``rte_mp_request_sync()`` behavior remains unchanged.
+
Removed Items
-------------
diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 5133eaaa47..fbfecd82d3 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -56,6 +56,7 @@ static struct action_entry_list action_entry_list =
enum mp_type {
MP_MSG, /* Share message with peers, will not block */
MP_REQ, /* Request for information, Will block for a reply */
+ MP_REQ_IGN, /* Request where missing action should return ignore */
MP_REP, /* Response to previously-received request */
MP_IGN, /* Response telling requester to ignore this response */
};
@@ -399,11 +400,11 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
pthread_mutex_unlock(&mp_mutex_action);
if (!action) {
- if (m->type == MP_REQ && !internal_conf->init_complete) {
- /* if this is a request, and init is not yet complete,
- * and callback wasn't registered, we should tell the
- * requester to ignore our existence because we're not
- * yet ready to process this request.
+ if ((m->type == MP_REQ && !internal_conf->init_complete) ||
+ m->type == MP_REQ_IGN) {
+ /*
+ * Ask requester to ignore this peer when action is not
+ * registered either due to early init or explicit request policy.
*/
struct rte_mp_msg dummy;
@@ -947,7 +948,8 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
static int
mp_request_sync(const char *dst, struct rte_mp_msg *req,
- struct rte_mp_reply *reply, const struct timespec *ts)
+ struct rte_mp_reply *reply, const struct timespec *ts,
+ enum mp_type req_type)
{
int ret;
pthread_condattr_t attr;
@@ -970,7 +972,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
return -1;
}
- ret = send_msg(dst, req, MP_REQ);
+ ret = send_msg(dst, req, req_type);
if (ret < 0) {
EAL_LOG(ERR, "Fail to send request %s:%s",
dst, req->name);
@@ -1021,11 +1023,20 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
int dir_fd, ret = -1;
DIR *mp_dir;
struct dirent *ent;
struct timespec now, end;
+ enum mp_type req_type = MP_REQ;
const struct internal_config *internal_conf =
eal_get_internal_configuration();
@@ -1038,6 +1049,18 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
if (check_input(req) != 0)
goto end;
+ /*
+ * Reject unknown flags for safety instead of silently ignoring them.
+ * Future flags require corresponding support in this implementation.
+ */
+ if (flags & ~RTE_MP_REQ_F_IGNORE_NO_ACTION) {
+ rte_errno = EINVAL;
+ goto end;
+ }
+
+ if (flags & RTE_MP_REQ_F_IGNORE_NO_ACTION)
+ req_type = MP_REQ_IGN;
+
if (internal_conf->no_shconf) {
EAL_LOG(DEBUG, "No shared files mode enabled, IPC is disabled");
rte_errno = ENOTSUP;
@@ -1057,7 +1080,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* for secondary process, send request to the primary process only */
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
pthread_mutex_lock(&pending_requests.lock);
- ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end);
+ ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end, req_type);
pthread_mutex_unlock(&pending_requests.lock);
goto end;
}
@@ -1097,7 +1120,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* unlocks the mutex while waiting for response,
* locks on receive
*/
- if (mp_request_sync(path, req, reply, &end))
+ if (mp_request_sync(path, req, reply, &end, req_type))
goto unlock_end;
}
ret = 0;
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 7241f3be5d..415b974945 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -173,6 +173,15 @@ struct rte_mp_reply {
struct rte_mp_msg *msgs; /* caller to free */
};
+/** Request flags for rte_mp_request_sync_ex(). */
+enum rte_mp_request_flags {
+ /**
+ * Ask peers that do not have a registered action to return MP_IGN
+ * instead of causing timeout for this request.
+ */
+ RTE_MP_REQ_F_IGNORE_NO_ACTION = 1u << 0,
+};
+
/**
* Action function typedef used by other components.
*
@@ -292,6 +301,30 @@ int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts);
+/**
+ * Send a request to peer processes with explicit request flags.
+ *
+ * This API is equivalent to rte_mp_request_sync() with opt-in behavior
+ * controls provided through @p flags.
+ *
+ * @param req
+ * The req argument contains the customized request message.
+ * @param reply
+ * The reply argument will be for storing all the replied messages;
+ * the caller is responsible for free reply->msgs.
+ * @param ts
+ * The ts argument specifies how long we can wait for the peer(s) to reply.
+ * @param flags
+ * Bitmask of values from enum rte_mp_request_flags.
+ *
+ * @return
+ * - On success, return 0.
+ * - On failure, return -1, and the reason will be stored in rte_errno.
+ */
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags);
+
/**
* Send a request to the peer process and expect a reply in a separate callback.
*
diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
index 6703355318..1066a4d2bc 100644
--- a/lib/eal/windows/eal_mp.c
+++ b/lib/eal/windows/eal_mp.c
@@ -56,10 +56,19 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
RTE_SET_USED(req);
RTE_SET_USED(reply);
RTE_SET_USED(ts);
+ RTE_SET_USED(flags);
EAL_LOG_NOT_IMPLEMENTED();
return -1;
}
diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index ac94efe7ff..f55f43774c 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -3,9 +3,12 @@
*/
#include <stdlib.h>
+#include <errno.h>
#include <eal_export.h>
+#include <rte_eal.h>
#include <rte_alarm.h>
+#include <rte_cycles.h>
#include <rte_mbuf.h>
#include <rte_ethdev.h>
#include <rte_lcore.h>
@@ -29,7 +32,10 @@ RTE_LOG_REGISTER_DEFAULT(pdump_logtype, NOTICE);
#define PDUMP_BURST_SIZE 32u
-/* Overly generous timeout for secondary to respond */
+/*
+ * Shared timeout budget for both MP request/reply and inflight callback drain
+ * during disable. Kept fixed for now to avoid API/config scope changes.
+ */
#define MP_TIMEOUT_S 5
enum pdump_operation {
@@ -68,6 +74,7 @@ struct pdump_request {
const struct rte_bpf_prm *prm;
uint32_t snaplen;
+ uint32_t generation;
};
struct pdump_response {
@@ -81,6 +88,11 @@ struct pdump_bundle {
char peer[];
};
+struct pdump_shared_ctl {
+ RTE_ATOMIC(uint32_t) enabled;
+ RTE_ATOMIC(uint32_t) inflight;
+};
+
static struct pdump_rxtx_cbs {
struct rte_ring *ring;
struct rte_mempool *mp;
@@ -88,11 +100,11 @@ static struct pdump_rxtx_cbs {
const struct rte_bpf *filter;
enum pdump_version ver;
uint32_t snaplen;
+ uint32_t generation;
RTE_ATOMIC(uint32_t) use_count;
} rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
-
/*
* The packet capture statistics keep track of packets
* accepted, filtered and dropped. These are per-queue
@@ -102,12 +114,73 @@ static const char MZ_RTE_PDUMP_STATS[] = "rte_pdump_stats";
static struct {
struct rte_pdump_stats rx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
struct rte_pdump_stats tx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl rx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl tx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ RTE_ATOMIC(uint32_t) generation;
const struct rte_memzone *mz;
} *pdump_stats;
+static int
+pdump_enter(struct pdump_shared_ctl *ctl, const struct pdump_rxtx_cbs *cbs)
+{
+ uint32_t generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_acquire);
+
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) == 0 ||
+ cbs->generation != generation)
+ return 0;
+
+ rte_atomic_fetch_add_explicit(&ctl->inflight, 1, rte_memory_order_acquire);
+
+ /*
+ * Disable/re-enable may race with callback entry. Re-check enabled and
+ * generation after taking inflight reference so disable can wait for this
+ * callback to drain and stale callbacks become no-op. The second generation
+ * load is intentional: it detects an epoch change that happens between the
+ * first check and inflight increment.
+ */
+ generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_acquire);
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) != 0 &&
+ cbs->generation == generation)
+ return 1;
+
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+ return 0;
+}
+
+static __rte_always_inline void
+pdump_exit(struct pdump_shared_ctl *ctl)
+{
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+}
+
+static int
+pdump_wait_inflight(struct pdump_shared_ctl *ctl)
+{
+ uint64_t end_tsc = rte_get_timer_cycles() + MP_TIMEOUT_S * rte_get_timer_hz();
+
+ /*
+ * Use the same timeout budget as MP sync requests. Under heavy load this
+ * can time out and fail closed, requiring disable retry by the caller.
+ */
+
+ while (rte_atomic_load_explicit(&ctl->inflight, rte_memory_order_acquire) != 0) {
+ if (rte_get_timer_cycles() > end_tsc)
+ return -ETIMEDOUT;
+ rte_pause();
+ }
+
+ return 0;
+}
+
static void
pdump_cb_wait(struct pdump_rxtx_cbs *cbs)
{
+ /*
+ * use_count protects callback-owned data (for example filter lifetime),
+ * while ctl->inflight protects callback entry/teardown synchronization.
+ */
/* make sure the data loads happens before the use count load */
rte_atomic_thread_fence(rte_memory_order_acquire);
@@ -224,12 +297,17 @@ pdump_rx(uint16_t port, uint16_t queue,
uint16_t max_pkts __rte_unused, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->rx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_IN,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -239,12 +317,17 @@ pdump_tx(uint16_t port, uint16_t queue,
struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->tx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_OUT,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -255,20 +338,43 @@ pdump_register_rx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &rx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "rx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "rx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale rx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale rx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -276,6 +382,7 @@ pdump_register_rx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_first_rx_callback(port, qid,
pdump_rx, cbs);
@@ -286,10 +393,15 @@ pdump_register_rx_callbacks(enum pdump_version ver,
return rte_errno;
}
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
+
memset(&pdump_stats->rx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing rx callback for port=%d queue=%d",
@@ -298,6 +410,11 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove rx callback, errno=%d",
-ret);
@@ -305,6 +422,10 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -316,7 +437,8 @@ pdump_register_tx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
@@ -324,13 +446,36 @@ pdump_register_tx_callbacks(enum pdump_version ver,
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &tx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "tx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "tx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale tx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale tx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -338,6 +483,7 @@ pdump_register_tx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_tx_callback(port, qid, pdump_tx,
cbs);
@@ -347,10 +493,15 @@ pdump_register_tx_callbacks(enum pdump_version ver,
rte_errno);
return rte_errno;
}
+
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
memset(&pdump_stats->tx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing tx callback for port=%d queue=%d",
@@ -359,6 +510,11 @@ pdump_register_tx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove tx callback, errno=%d",
-ret);
@@ -367,6 +523,10 @@ pdump_register_tx_callbacks(enum pdump_version ver,
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -459,17 +619,23 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_rx_q : queue + 1;
ret = pdump_register_rx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
+ /*
+ * Intentional partial-apply behavior: if RX succeeds and TX fails,
+ * RX remains enabled. API notes require caller-side unwind.
+ */
/* register TX callback */
if (flags & RTE_PDUMP_FLAG_TX) {
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_tx_q : queue + 1;
ret = pdump_register_tx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
@@ -477,12 +643,14 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
return ret;
}
-static void
-pdump_request_to_secondary(const struct pdump_request *req)
+static int
+pdump_request_to_secondary_sync(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};
+ int ret = 0;
+ uint16_t i;
PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary", pdump_opname(req->op));
@@ -490,14 +658,45 @@ 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");
+ if (rte_mp_request_sync_ex(&mp_req, &mp_reply, &ts,
+ RTE_MP_REQ_F_IGNORE_NO_ACTION) != 0) {
+ PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed: %s",
+ strerror(rte_errno));
+ return -rte_errno;
+ }
- else if (mp_reply.nb_sent != mp_reply.nb_received)
+ 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);
+ ret = -ETIMEDOUT;
+ }
+
+ for (i = 0; i < mp_reply.nb_received; i++) {
+ struct rte_mp_msg *mp_rep = &mp_reply.msgs[i];
+ const struct pdump_response *resp;
+
+ if (mp_rep->len_param != sizeof(*resp)) {
+ PDUMP_LOG_LINE(ERR, "invalid secondary reply size %u", mp_rep->len_param);
+ if (ret == 0)
+ ret = -EINVAL;
+ continue;
+ }
+
+ resp = (const struct pdump_response *)mp_rep->param;
+ if (resp->err_value != 0) {
+ PDUMP_LOG_LINE(ERR, "secondary reply failed: op=%u err=%d",
+ resp->res_op, resp->err_value);
+ /*
+ * Return the first failure to keep a single representative
+ * status; all per-secondary errors are still logged above.
+ */
+ if (ret == 0)
+ ret = resp->err_value;
+ }
+ }
free(mp_reply.msgs);
+ return ret;
}
/* Allocate temporary storage for passing state to the alarm thread for deferred handling */
@@ -556,7 +755,10 @@ pdump_handle_primary_request(const struct rte_mp_msg *mp_msg, const void *peer)
PDUMP_LOG_LINE(DEBUG, "secondary pdump %s", pdump_opname(req->op));
/* Can just do it now, no need for interrupt thread */
- ret = set_pdump_rxtx_cbs(req);
+ if (req->op == ENABLE || req->op == DISABLE)
+ ret = set_pdump_rxtx_cbs(req);
+ else
+ ret = -EINVAL;
}
return pdump_send_response(req, ret, peer);
@@ -569,17 +771,29 @@ __pdump_request(void *param)
{
struct pdump_bundle *bundle = param;
struct rte_mp_msg *msg = &bundle->msg;
- const struct pdump_request *req =
- (const struct pdump_request *)msg->param;
+ struct pdump_request *req =
+ (struct pdump_request *)msg->param;
int ret;
PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
+ if (req->op == DISABLE) {
+ req->generation = rte_atomic_fetch_add_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_acq_rel) + 1;
+ } else if (req->op == ENABLE) {
+ req->generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_acquire);
+ }
+
ret = set_pdump_rxtx_cbs(req);
- /* Primary process is responsible for broadcasting request to all secondaries */
+ /*
+ * Primary process is responsible for broadcasting the request to all
+ * secondaries. The sync request uses opt-in ignore-missing-action mode so
+ * pdump does not depend on unrelated secondary processes.
+ */
if (ret == 0)
- pdump_request_to_secondary(req);
+ ret = pdump_request_to_secondary_sync(req);
pdump_send_response(req, ret, bundle->peer);
free(bundle);
@@ -659,6 +873,10 @@ rte_pdump_init(void)
pdump_stats = mz->addr;
pdump_stats->mz = mz;
+ /* Start at generation 1 so zero remains the default pre-init value. */
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+ rte_atomic_store_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_release);
return 0;
}
diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h
index 1e32d46097..52d3c0ca93 100644
--- a/lib/pdump/rte_pdump.h
+++ b/lib/pdump/rte_pdump.h
@@ -64,7 +64,7 @@ rte_pdump_uninit(void);
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
* @param ring
* ring on which captured packets will be enqueued for user.
* @param mp
@@ -72,6 +72,11 @@ rte_pdump_uninit(void);
* @param filter
* Unused should be NULL.
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -103,6 +108,11 @@ rte_pdump_enable(uint16_t port, uint16_t queue, uint32_t flags,
* @param prm
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -124,7 +134,15 @@ rte_pdump_enable_bpf(uint16_t port_id, uint16_t queue,
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources. Timeout is
+ * bounded; callers should retry disable before attempting teardown.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
@@ -153,6 +171,11 @@ rte_pdump_disable(uint16_t port, uint16_t queue, uint32_t flags);
* @param filter
* unused should be NULL
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -186,6 +209,11 @@ rte_pdump_enable_by_deviceid(char *device_id, uint16_t queue,
* @param filter
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -210,7 +238,15 @@ rte_pdump_enable_bpf_by_deviceid(const char *device_id, uint16_t queue,
* queues of a given device id.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources. Timeout is
+ * bounded; callers should retry disable before attempting teardown.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
--
2.43.0
^ permalink raw reply related
* [PATCH v2] pdump: fix teardown race with opt-in MP request mode
From: Pushpendra Kumar @ 2026-07-05 8:44 UTC (permalink / raw)
To: dev
Cc: stable, thomas, reshma.pattan, stephen, anatoly.burakov,
dmitry.kozliuk, bruce.richardson, 14pwcse1224, pushpendra.kumar,
Pushpendra Kumar
In-Reply-To: <20260701060257.562895-1-pushpendra1x.kumar@intel.com>
This change addresses two pdump teardown failure cases:
1. A requester secondary can time out if a forwarded secondary is slow,
unresponsive, or does not implement rte_pdump_init().
2. After a DISABLE request times out, the requester secondary receives
the reply and releases shared resources, which other slow or
unresponsive secondaries may access, leading to a crash.
Add an opt-in EAL multiprocess request mode that allows a caller to
ignore peers that do not implement the requested action, while preserving
the existing rte_mp_request_sync() behavior for legacy users.
pdump uses this new mode when forwarding enable/disable requests and adds
the following behavior:
- track request generation so stale callbacks cannot act on a newer
enable/disable cycle
- wait for in-flight callbacks to drain before completing disable
- prevent shared resource teardown from racing with delayed secondary
processing
- keep pdump intent-driven without extra consumer-specific workaround
logic
The EAL change is required because the current mp request path treats
a missing action as a failure, which cannot satisfy pdump's needs without
changing behavior for all existing users. The new API is opt-in, so
existing callers of rte_mp_request_sync() keep their current semantics.
Validation:
- Baseline-vs-changes benchmark used DPDK-native test-pmd as primary and
dumpcap as secondary.
- Both runs captured 1024 packets with the same packet-count exit
condition.
- A 5-second perf stat sample on the forwarding primary showed no
meaningful regression: with changes recorded 12,402,055,320 cycles and
17,391,769,084 instructions, versus 12,450,031,900 cycles and
17,499,706,053 instructions on the parent baseline.
- The extra synchronization in the hot path did not show material
overhead in this sample.
Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
Cc: stable@dpdk.org
---
This V2 patch addresses two pdump teardown failure scenarios:
- A requester secondary can time out if a forwarded secondary is slow,
unresponsive, or does not implement rte_pdump_init().
- A delayed DISABLE reply can race with requester-side teardown, which may
lead to crashes if peer processes still access shared pdump resources.
To fix this end-to-end:
- EAL adds an opt-in MP request mode to ignore peers with no matching action,
while preserving legacy rte_mp_request_sync() behavior.
- pdump uses that mode, tracks generation, and waits for in-flight callbacks
to drain before completing disable.
- dumpcap and pdump app cleanup paths are fail-closed when disable does not
complete successfully.
If this direction is accepted, I can split from the next revision into a
focused EAL patch followed by pdump and app-user updates.
.mailmap | 1 +
app/dumpcap/main.c | 23 ++-
app/pdump/main.c | 32 +++-
doc/guides/rel_notes/release_26_07.rst | 7 +
lib/eal/common/eal_common_proc.c | 37 +++-
lib/eal/include/rte_eal.h | 33 ++++
lib/eal/windows/eal_mp.c | 9 +
lib/pdump/rte_pdump.c | 241 ++++++++++++++++++++++---
lib/pdump/rte_pdump.h | 40 +++-
9 files changed, 379 insertions(+), 44 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/app/dumpcap/main.c b/app/dumpcap/main.c
index 46a6cb251e..f421a17034 100644
--- a/app/dumpcap/main.c
+++ b/app/dumpcap/main.c
@@ -521,14 +521,33 @@ static void
cleanup_pdump_resources(void)
{
struct interface *intf;
+ int ret;
+ bool disable_failed = false;
TAILQ_FOREACH(intf, &interfaces, next) {
- rte_pdump_disable(intf->port,
- RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ ret = rte_pdump_disable(intf->port,
+ RTE_PDUMP_ALL_QUEUES, RTE_PDUMP_FLAG_RXTX);
+ if (ret < 0) {
+ disable_failed = true;
+ fprintf(stderr,
+ "Disable pdump failed on %u:%s: %s\n",
+ intf->port, intf->name,
+ rte_strerror(rte_errno));
+ }
if (intf->opts.promisc_mode)
rte_eth_promiscuous_disable(intf->port);
}
+ if (disable_failed) {
+ /*
+ * Fail-closed: if disable did not complete, keep pdump action/state
+ * alive and do not uninit shared capture resources.
+ */
+ fprintf(stderr,
+ "Skipping pdump uninit because disable did not complete on all interfaces\n");
+ return;
+ }
+
rte_pdump_uninit();
}
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 1e62c8adc1..4cfb038395 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -451,14 +451,17 @@ print_pdump_stats(void)
}
}
-static inline void
+static inline int
disable_pdump(struct pdump_tuples *pt)
{
if (pt->dump_by_type == DEVICE_ID)
- rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
- pt->dir);
+ return rte_pdump_disable_by_deviceid(pt->device_id, pt->queue,
+ pt->dir);
else if (pt->dump_by_type == PORT_ID)
- rte_pdump_disable(pt->port, pt->queue, pt->dir);
+ return rte_pdump_disable(pt->port, pt->queue, pt->dir);
+
+ rte_errno = EINVAL;
+ return -EINVAL;
}
static inline void
@@ -518,15 +521,30 @@ static void
cleanup_pdump_resources(void)
{
int i;
+ int ret;
+ bool disable_failed = false;
struct pdump_tuples *pt;
char name[RTE_ETH_NAME_MAX_LEN];
- /* disable pdump and free the pdump_tuple resources */
+ /* Disable all callbacks first; freeing shared objects before this is unsafe. */
for (i = 0; i < num_tuples; i++) {
pt = &pdump_t[i];
- /* remove callbacks */
- disable_pdump(pt);
+ ret = disable_pdump(pt);
+ if (ret < 0) {
+ disable_failed = true;
+ printf("pdump disable failed (tuple=%d, errno=%d: %s); "
+ "skip teardown to avoid stale callback access\n",
+ i, rte_errno, rte_strerror(rte_errno));
+ }
+ }
+
+ if (disable_failed)
+ return;
+
+ /* free the pdump_tuple resources */
+ for (i = 0; i < num_tuples; i++) {
+ pt = &pdump_t[i];
/*
* transmit rest of the enqueued packets of the rings on to
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 4ca0a9ac77..1700a56bc7 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -177,6 +177,13 @@ New Features
Added AGENTS.md file for AI review
and supporting scripts to review patches and documentation.
+* **Hardened pdump teardown on disable failure.**
+
+ Hardened pdump request completion and application cleanup behavior so timeout
+ or disable failure does not trigger premature pdump resource teardown.
+ pdump now uses an opt-in MP request mode that ignores peers without the
+ pdump action, while preserving legacy EAL MP behavior for all other users.
+
Removed Items
-------------
diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
index 06f151818c..5a458c0417 100644
--- a/lib/eal/common/eal_common_proc.c
+++ b/lib/eal/common/eal_common_proc.c
@@ -56,6 +56,7 @@ static struct action_entry_list action_entry_list =
enum mp_type {
MP_MSG, /* Share message with peers, will not block */
MP_REQ, /* Request for information, Will block for a reply */
+ MP_REQ_IGN, /* Request where missing action should return ignore */
MP_REP, /* Response to previously-received request */
MP_IGN, /* Response telling requester to ignore this response */
};
@@ -384,11 +385,11 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
pthread_mutex_unlock(&mp_mutex_action);
if (!action) {
- if (m->type == MP_REQ && !internal_conf->init_complete) {
- /* if this is a request, and init is not yet complete,
- * and callback wasn't registered, we should tell the
- * requester to ignore our existence because we're not
- * yet ready to process this request.
+ if ((m->type == MP_REQ && !internal_conf->init_complete) ||
+ m->type == MP_REQ_IGN) {
+ /*
+ * Ask requester to ignore this peer when action is not
+ * registered either due to early init or explicit request policy.
*/
struct rte_mp_msg dummy;
@@ -936,7 +937,8 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,
static int
mp_request_sync(const char *dst, struct rte_mp_msg *req,
- struct rte_mp_reply *reply, const struct timespec *ts)
+ struct rte_mp_reply *reply, const struct timespec *ts,
+ enum mp_type req_type)
{
int ret;
pthread_condattr_t attr;
@@ -959,7 +961,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req,
return -1;
}
- ret = send_msg(dst, req, MP_REQ);
+ ret = send_msg(dst, req, req_type);
if (ret < 0) {
EAL_LOG(ERR, "Fail to send request %s:%s",
dst, req->name);
@@ -1010,11 +1012,20 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
int dir_fd, ret = -1;
DIR *mp_dir;
struct dirent *ent;
struct timespec now, end;
+ enum mp_type req_type = MP_REQ;
const struct internal_config *internal_conf =
eal_get_internal_configuration();
@@ -1027,6 +1038,14 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
if (check_input(req) != 0)
goto end;
+ if (flags & ~RTE_MP_REQ_F_IGNORE_NO_ACTION) {
+ rte_errno = EINVAL;
+ goto end;
+ }
+
+ if (flags & RTE_MP_REQ_F_IGNORE_NO_ACTION)
+ req_type = MP_REQ_IGN;
+
if (internal_conf->no_shconf) {
EAL_LOG(DEBUG, "No shared files mode enabled, IPC is disabled");
rte_errno = ENOTSUP;
@@ -1046,7 +1065,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* for secondary process, send request to the primary process only */
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
pthread_mutex_lock(&pending_requests.lock);
- ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end);
+ ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end, req_type);
pthread_mutex_unlock(&pending_requests.lock);
goto end;
}
@@ -1086,7 +1105,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
/* unlocks the mutex while waiting for response,
* locks on receive
*/
- if (mp_request_sync(path, req, reply, &end))
+ if (mp_request_sync(path, req, reply, &end, req_type))
goto unlock_end;
}
ret = 0;
diff --git a/lib/eal/include/rte_eal.h b/lib/eal/include/rte_eal.h
index 7241f3be5d..415b974945 100644
--- a/lib/eal/include/rte_eal.h
+++ b/lib/eal/include/rte_eal.h
@@ -173,6 +173,15 @@ struct rte_mp_reply {
struct rte_mp_msg *msgs; /* caller to free */
};
+/** Request flags for rte_mp_request_sync_ex(). */
+enum rte_mp_request_flags {
+ /**
+ * Ask peers that do not have a registered action to return MP_IGN
+ * instead of causing timeout for this request.
+ */
+ RTE_MP_REQ_F_IGNORE_NO_ACTION = 1u << 0,
+};
+
/**
* Action function typedef used by other components.
*
@@ -292,6 +301,30 @@ int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts);
+/**
+ * Send a request to peer processes with explicit request flags.
+ *
+ * This API is equivalent to rte_mp_request_sync() with opt-in behavior
+ * controls provided through @p flags.
+ *
+ * @param req
+ * The req argument contains the customized request message.
+ * @param reply
+ * The reply argument will be for storing all the replied messages;
+ * the caller is responsible for free reply->msgs.
+ * @param ts
+ * The ts argument specifies how long we can wait for the peer(s) to reply.
+ * @param flags
+ * Bitmask of values from enum rte_mp_request_flags.
+ *
+ * @return
+ * - On success, return 0.
+ * - On failure, return -1, and the reason will be stored in rte_errno.
+ */
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags);
+
/**
* Send a request to the peer process and expect a reply in a separate callback.
*
diff --git a/lib/eal/windows/eal_mp.c b/lib/eal/windows/eal_mp.c
index 6703355318..1066a4d2bc 100644
--- a/lib/eal/windows/eal_mp.c
+++ b/lib/eal/windows/eal_mp.c
@@ -56,10 +56,19 @@ RTE_EXPORT_SYMBOL(rte_mp_request_sync)
int
rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply,
const struct timespec *ts)
+{
+ return rte_mp_request_sync_ex(req, reply, ts, 0);
+}
+
+RTE_EXPORT_SYMBOL(rte_mp_request_sync_ex)
+int
+rte_mp_request_sync_ex(struct rte_mp_msg *req, struct rte_mp_reply *reply,
+ const struct timespec *ts, uint32_t flags)
{
RTE_SET_USED(req);
RTE_SET_USED(reply);
RTE_SET_USED(ts);
+ RTE_SET_USED(flags);
EAL_LOG_NOT_IMPLEMENTED();
return -1;
}
diff --git a/lib/pdump/rte_pdump.c b/lib/pdump/rte_pdump.c
index ac94efe7ff..3764aeb2d3 100644
--- a/lib/pdump/rte_pdump.c
+++ b/lib/pdump/rte_pdump.c
@@ -3,9 +3,12 @@
*/
#include <stdlib.h>
+#include <errno.h>
#include <eal_export.h>
+#include <rte_eal.h>
#include <rte_alarm.h>
+#include <rte_cycles.h>
#include <rte_mbuf.h>
#include <rte_ethdev.h>
#include <rte_lcore.h>
@@ -68,6 +71,7 @@ struct pdump_request {
const struct rte_bpf_prm *prm;
uint32_t snaplen;
+ uint32_t generation;
};
struct pdump_response {
@@ -81,6 +85,11 @@ struct pdump_bundle {
char peer[];
};
+struct pdump_shared_ctl {
+ RTE_ATOMIC(uint32_t) enabled;
+ RTE_ATOMIC(uint32_t) inflight;
+};
+
static struct pdump_rxtx_cbs {
struct rte_ring *ring;
struct rte_mempool *mp;
@@ -88,11 +97,11 @@ static struct pdump_rxtx_cbs {
const struct rte_bpf *filter;
enum pdump_version ver;
uint32_t snaplen;
+ uint32_t generation;
RTE_ATOMIC(uint32_t) use_count;
} rx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT],
tx_cbs[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
-
/*
* The packet capture statistics keep track of packets
* accepted, filtered and dropped. These are per-queue
@@ -102,9 +111,59 @@ static const char MZ_RTE_PDUMP_STATS[] = "rte_pdump_stats";
static struct {
struct rte_pdump_stats rx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
struct rte_pdump_stats tx[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl rx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ struct pdump_shared_ctl tx_ctl[RTE_MAX_ETHPORTS][RTE_MAX_QUEUES_PER_PORT];
+ RTE_ATOMIC(uint32_t) generation;
const struct rte_memzone *mz;
} *pdump_stats;
+static int
+pdump_enter(struct pdump_shared_ctl *ctl, const struct pdump_rxtx_cbs *cbs)
+{
+ uint32_t generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_relaxed);
+
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) == 0 ||
+ cbs->generation != generation)
+ return 0;
+
+ rte_atomic_fetch_add_explicit(&ctl->inflight, 1, rte_memory_order_acquire);
+
+ /*
+ * Disable/re-enable may race with callback entry. Re-check enabled and
+ * generation after taking inflight reference so disable can wait for this
+ * callback to drain and stale callbacks become no-op.
+ */
+ generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_relaxed);
+ if (rte_atomic_load_explicit(&ctl->enabled, rte_memory_order_relaxed) != 0 &&
+ cbs->generation == generation)
+ return 1;
+
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+ return 0;
+}
+
+static __rte_always_inline void
+pdump_exit(struct pdump_shared_ctl *ctl)
+{
+ rte_atomic_fetch_sub_explicit(&ctl->inflight, 1, rte_memory_order_release);
+}
+
+static int
+pdump_wait_inflight(struct pdump_shared_ctl *ctl)
+{
+ uint64_t end_tsc = rte_get_timer_cycles() + MP_TIMEOUT_S * rte_get_timer_hz();
+
+ while (rte_atomic_load_explicit(&ctl->inflight, rte_memory_order_acquire) != 0) {
+ if (rte_get_timer_cycles() > end_tsc)
+ return -ETIMEDOUT;
+ rte_pause();
+ }
+
+ return 0;
+}
+
static void
pdump_cb_wait(struct pdump_rxtx_cbs *cbs)
{
@@ -224,12 +283,17 @@ pdump_rx(uint16_t port, uint16_t queue,
uint16_t max_pkts __rte_unused, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->rx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_IN,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -239,12 +303,17 @@ pdump_tx(uint16_t port, uint16_t queue,
struct rte_mbuf **pkts, uint16_t nb_pkts, void *user_params)
{
struct pdump_rxtx_cbs *cbs = user_params;
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][queue];
struct rte_pdump_stats *stats = &pdump_stats->tx[port][queue];
+ if (!pdump_enter(ctl, cbs))
+ return nb_pkts;
+
pdump_cb_hold(cbs);
pdump_copy(port, queue, RTE_PCAPNG_DIRECTION_OUT,
pkts, nb_pkts, cbs, stats);
pdump_cb_release(cbs);
+ pdump_exit(ctl);
return nb_pkts;
}
@@ -255,20 +324,43 @@ pdump_register_rx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &rx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->rx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "rx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "rx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale rx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale rx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -276,6 +368,7 @@ pdump_register_rx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_first_rx_callback(port, qid,
pdump_rx, cbs);
@@ -286,10 +379,15 @@ pdump_register_rx_callbacks(enum pdump_version ver,
return rte_errno;
}
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
+
memset(&pdump_stats->rx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing rx callback for port=%d queue=%d",
@@ -298,6 +396,11 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_rx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove rx callback, errno=%d",
-ret);
@@ -305,6 +408,10 @@ pdump_register_rx_callbacks(enum pdump_version ver,
}
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -316,7 +423,8 @@ pdump_register_tx_callbacks(enum pdump_version ver,
uint16_t end_q, uint16_t port, uint16_t queue,
struct rte_ring *ring, struct rte_mempool *mp,
struct rte_bpf *filter,
- uint16_t operation, uint32_t snaplen)
+ uint16_t operation, uint32_t snaplen,
+ uint32_t generation)
{
uint16_t qid;
@@ -324,13 +432,36 @@ pdump_register_tx_callbacks(enum pdump_version ver,
qid = (queue == RTE_PDUMP_ALL_QUEUES) ? 0 : queue;
for (; qid < end_q; qid++) {
struct pdump_rxtx_cbs *cbs = &tx_cbs[port][qid];
+ struct pdump_shared_ctl *ctl = &pdump_stats->tx_ctl[port][qid];
if (operation == ENABLE) {
+ int ret;
+
if (cbs->cb) {
- PDUMP_LOG_LINE(ERR,
- "tx callback for port=%d queue=%d, already exists",
- port, qid);
- return -EEXIST;
+ if (cbs->generation == generation &&
+ rte_atomic_load_explicit(&ctl->enabled,
+ rte_memory_order_relaxed) != 0) {
+ PDUMP_LOG_LINE(ERR,
+ "tx callback for port=%d queue=%d, already exists",
+ port, qid);
+ return -EEXIST;
+ }
+
+ PDUMP_LOG_LINE(DEBUG,
+ "reconciling stale tx callback for port=%d queue=%d"
+ " old_gen=%u new_gen=%u",
+ port, qid, cbs->generation, generation);
+
+ ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
+ if (ret < 0) {
+ PDUMP_LOG_LINE(ERR,
+ "failed to reconcile stale tx callback, errno=%d",
+ -ret);
+ return ret;
+ }
+
+ pdump_cb_wait(cbs);
+ cbs->cb = NULL;
}
cbs->use_count = 0;
cbs->ver = ver;
@@ -338,6 +469,7 @@ pdump_register_tx_callbacks(enum pdump_version ver,
cbs->mp = mp;
cbs->snaplen = snaplen;
cbs->filter = filter;
+ cbs->generation = generation;
cbs->cb = rte_eth_add_tx_callback(port, qid, pdump_tx,
cbs);
@@ -347,10 +479,15 @@ pdump_register_tx_callbacks(enum pdump_version ver,
rte_errno);
return rte_errno;
}
+
+ rte_atomic_store_explicit(&ctl->inflight, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&ctl->enabled, 1, rte_memory_order_release);
memset(&pdump_stats->tx[port][qid], 0, sizeof(struct rte_pdump_stats));
} else if (operation == DISABLE) {
int ret;
+ rte_atomic_store_explicit(&ctl->enabled, 0, rte_memory_order_release);
+
if (cbs->cb == NULL) {
PDUMP_LOG_LINE(ERR,
"no existing tx callback for port=%d queue=%d",
@@ -359,6 +496,11 @@ pdump_register_tx_callbacks(enum pdump_version ver,
}
ret = rte_eth_remove_tx_callback(port, qid, cbs->cb);
if (ret < 0) {
+ /* Keep state coherent: callback is still registered,
+ * so restore enabled.
+ */
+ rte_atomic_store_explicit(&ctl->enabled, 1,
+ rte_memory_order_release);
PDUMP_LOG_LINE(ERR,
"failed to remove tx callback, errno=%d",
-ret);
@@ -367,6 +509,10 @@ pdump_register_tx_callbacks(enum pdump_version ver,
pdump_cb_wait(cbs);
cbs->cb = NULL;
+
+ ret = pdump_wait_inflight(ctl);
+ if (ret < 0)
+ return ret;
}
}
@@ -459,7 +605,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_rx_q : queue + 1;
ret = pdump_register_rx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
@@ -469,7 +616,8 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
end_q = (queue == RTE_PDUMP_ALL_QUEUES) ? nb_tx_q : queue + 1;
ret = pdump_register_tx_callbacks(p->ver, end_q, port, queue,
ring, mp, filter,
- operation, p->snaplen);
+ operation, p->snaplen,
+ p->generation);
if (ret < 0)
return ret;
}
@@ -477,12 +625,14 @@ set_pdump_rxtx_cbs(const struct pdump_request *p)
return ret;
}
-static void
-pdump_request_to_secondary(const struct pdump_request *req)
+static int
+pdump_request_to_secondary_sync(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};
+ int ret = 0;
+ uint16_t i;
PDUMP_LOG_LINE(DEBUG, "forward req %s to secondary", pdump_opname(req->op));
@@ -490,14 +640,41 @@ 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");
+ if (rte_mp_request_sync_ex(&mp_req, &mp_reply, &ts,
+ RTE_MP_REQ_F_IGNORE_NO_ACTION) != 0) {
+ PDUMP_LOG_LINE(ERR, "rte_mp_request_sync failed: %s",
+ strerror(rte_errno));
+ return -rte_errno;
+ }
- else if (mp_reply.nb_sent != mp_reply.nb_received)
+ 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);
+ ret = -ETIMEDOUT;
+ }
+
+ for (i = 0; i < mp_reply.nb_received; i++) {
+ struct rte_mp_msg *mp_rep = &mp_reply.msgs[i];
+ const struct pdump_response *resp;
+
+ if (mp_rep->len_param != sizeof(*resp)) {
+ PDUMP_LOG_LINE(ERR, "invalid secondary reply size %u", mp_rep->len_param);
+ if (ret == 0)
+ ret = -EINVAL;
+ continue;
+ }
+
+ resp = (const struct pdump_response *)mp_rep->param;
+ if (resp->err_value != 0) {
+ PDUMP_LOG_LINE(ERR, "secondary reply failed: op=%u err=%d",
+ resp->res_op, resp->err_value);
+ if (ret == 0)
+ ret = resp->err_value;
+ }
+ }
free(mp_reply.msgs);
+ return ret;
}
/* Allocate temporary storage for passing state to the alarm thread for deferred handling */
@@ -556,7 +733,10 @@ pdump_handle_primary_request(const struct rte_mp_msg *mp_msg, const void *peer)
PDUMP_LOG_LINE(DEBUG, "secondary pdump %s", pdump_opname(req->op));
/* Can just do it now, no need for interrupt thread */
- ret = set_pdump_rxtx_cbs(req);
+ if (req->op == ENABLE || req->op == DISABLE)
+ ret = set_pdump_rxtx_cbs(req);
+ else
+ ret = -EINVAL;
}
return pdump_send_response(req, ret, peer);
@@ -569,17 +749,29 @@ __pdump_request(void *param)
{
struct pdump_bundle *bundle = param;
struct rte_mp_msg *msg = &bundle->msg;
- const struct pdump_request *req =
- (const struct pdump_request *)msg->param;
+ struct pdump_request *req =
+ (struct pdump_request *)msg->param;
int ret;
PDUMP_LOG_LINE(DEBUG, "primary pdump %s", pdump_opname(req->op));
+ if (req->op == DISABLE) {
+ req->generation = rte_atomic_fetch_add_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_acq_rel) + 1;
+ } else if (req->op == ENABLE) {
+ req->generation = rte_atomic_load_explicit(&pdump_stats->generation,
+ rte_memory_order_acquire);
+ }
+
ret = set_pdump_rxtx_cbs(req);
- /* Primary process is responsible for broadcasting request to all secondaries */
+ /*
+ * Primary process is responsible for broadcasting the request to all
+ * secondaries. The sync request uses opt-in ignore-missing-action mode so
+ * pdump does not depend on unrelated secondary processes.
+ */
if (ret == 0)
- pdump_request_to_secondary(req);
+ ret = pdump_request_to_secondary_sync(req);
pdump_send_response(req, ret, bundle->peer);
free(bundle);
@@ -659,6 +851,9 @@ rte_pdump_init(void)
pdump_stats = mz->addr;
pdump_stats->mz = mz;
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+ rte_atomic_store_explicit(&pdump_stats->generation, 1,
+ rte_memory_order_release);
return 0;
}
diff --git a/lib/pdump/rte_pdump.h b/lib/pdump/rte_pdump.h
index 1e32d46097..77e5914420 100644
--- a/lib/pdump/rte_pdump.h
+++ b/lib/pdump/rte_pdump.h
@@ -64,7 +64,7 @@ rte_pdump_uninit(void);
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
* @param ring
* ring on which captured packets will be enqueued for user.
* @param mp
@@ -72,6 +72,11 @@ rte_pdump_uninit(void);
* @param filter
* Unused should be NULL.
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -103,6 +108,11 @@ rte_pdump_enable(uint16_t port, uint16_t queue, uint32_t flags,
* @param prm
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -124,7 +134,14 @@ rte_pdump_enable_bpf(uint16_t port_id, uint16_t queue,
* queues of a given port.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
@@ -153,6 +170,11 @@ rte_pdump_disable(uint16_t port, uint16_t queue, uint32_t flags);
* @param filter
* unused should be NULL
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -186,6 +208,11 @@ rte_pdump_enable_by_deviceid(char *device_id, uint16_t queue,
* @param filter
* Use BPF program to run to filter packes (can be NULL)
*
+ * @note
+ * In applications that enable capture on multiple interfaces, enable may be
+ * partially applied before an error is returned. Callers should explicitly
+ * unwind partial enable state.
+ *
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
*/
@@ -210,7 +237,14 @@ rte_pdump_enable_bpf_by_deviceid(const char *device_id, uint16_t queue,
* queues of a given device id.
* @param flags
* flags specifies RTE_PDUMP_FLAG_RX/RTE_PDUMP_FLAG_TX/RTE_PDUMP_FLAG_RXTX
- * on which packet capturing should be enabled for a given port and queue.
+ * on which packet capturing should be disabled for a given port and queue.
+ *
+ * @note
+ * A disable failure (including timeout/no response) means teardown is not
+ * complete across pdump-enabled peers. The caller must not release shared
+ * pdump resources and must not uninitialize pdump for that capture session
+ * until disable succeeds. Releasing resources on disable failure can lead to
+ * crashes in peer processes still accessing those resources.
*
* @return
* 0 on success, -1 on error, rte_errno is set accordingly.
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] devtools: support local opencode agent for patch review
From: Stephen Hemminger @ 2026-07-04 16:46 UTC (permalink / raw)
To: datshan; +Cc: thomas, aconole, dev
In-Reply-To: <tencent_4A299B76E71AE508AF5D403AFDA4B248B708@qq.com>
On Fri, 3 Jul 2026 17:19:02 +0800
datshan@qq.com wrote:
> From: Chengwen Feng <fengchengwen@huawei.com>
>
> Currently review-patch.py only supports cloud AI providers
> (Anthropic, OpenAI, xAI, Google) via REST API, requiring API keys.
>
> Add a --via option that invokes the locally installed opencode CLI as
> the review runner instead of making HTTP calls. opencode reads
> AGENTS.md from the DPDK project directory automatically, needing no
> configuration beyond opencode on PATH.
>
> The --via and -p/--provider options are independent -- via routes to
> the local agent mode while -p continues to use the cloud API path.
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
So most of this script was AI generated, turned back to AI
to look at this change...
Thanks for the patch - the opencode integration looks sound. The
--format json parsing matches what the CLI actually emits, and the
_run_review refactor is a clean way to share the dispatch. A few items:
Bug:
- subprocess.run() can raise TimeoutExpired, but only FileNotFoundError
is caught, so a timeout gives a raw traceback. Catch
subprocess.TimeoutExpired and route it through error() like the cloud
path does.
Please clarify / fix:
- -a/--agents is silently ignored in --via mode; opencode auto-loads
AGENTS.md from --dir, which you set to the repo root while AGENTS.md
lives under devtools/ai/. Does opencode actually find the guidelines?
If not the review runs without them.
- JSON metadata still reports "provider": args.provider (stays
"anthropic" under --via); use (args.via or args.provider) as you
already did for the token summary.
- --large-file modes are silently skipped in via mode - intended? A
warning would help.
Nits: non-ASCII em-dash in a prompt string (use --); the trailing "--"
before the message is unnecessary and may drop the prompt; redundant
import tempfile; unused max_tokens param; f-string with no placeholder;
run black on the full_message assignment.
^ permalink raw reply
* Re: [PATCH] bpf: fix unitialized warning
From: Stephen Hemminger @ 2026-07-04 16:12 UTC (permalink / raw)
To: dev; +Cc: Konstantin Ananyev, Marat Khalili
In-Reply-To: <20260704040322.157058-1-stephen@networkplumber.org>
On Fri, 3 Jul 2026 21:03:22 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> Coverity complains unitialized use of structure.
>
> Coverity ID: 504611
> Fixes: 17509d474226 ("bpf/validate: fix BPF_ADD of pointer to a scalar")
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Ignore AI review of this patch. The CI AI review is using weak AI
model and no tooling; it ends up hallucinating about code that doesn't exist.
Better AI reviewing the reviewer sees...
Coverity 504611 is a true positive. eval_fill_max_bound() sets u, s,
v.type, and mask, but not v.size or v.buf_size. In eval_add() the
pointer+pointer path does eval_fill_max_bound(&rs_buf, msk); *rd =
rs_buf; on a fresh local, so the struct copy propagates the
uninitialized v.size/v.buf_size. Zero-init at declaration closes
exactly that.
The AI review is quoting code that isn't there. eval_add() has no
rs_buf = *rs; and no eval_apply_mask(&rs_buf, ...) call. That snippet
looks reconstructed from the diff hunk header (eval_apply_mask, the
preceding function) rather than the function actually being patched.
Its central claim, that rs_buf is only touched via rs = &rs_buf inside
the if, is wrong: the uninitialized read is the *rd = rs_buf copy, and
rs then reads it after the block.
^ permalink raw reply
* Re: [RFC] net/bonding: support limited datapath in secondary process
From: Stephen Hemminger @ 2026-07-04 16:09 UTC (permalink / raw)
To: Weijun Pan; +Cc: dev, chas3, humin29
In-Reply-To: <CAMEmXsxzEfsu1boqPrYtTbuf3j9VM0vb8JBzC-NKm4YXJFVPNw@mail.gmail.com>
On Sat, 4 Jul 2026 11:00:42 -0500
Weijun Pan <wpan3636@gmail.com> wrote:
> Hi,
>
> I am looking into Bugzilla 1900 and real secondary-process datapath
> support for the bonding PMD.
>
> The current secondary attach path is safe by default: secondary Rx/Tx
> uses blackhole burst functions instead of installing real per-mode
> bonding burst functions. This avoids crashes because the internal
> bonding state is not valid in the secondary process.
>
> However, this also means that a secondary process cannot perform real
> Rx/Tx through a bonding device. This is the issue described in
> Bugzilla 1900.
>
> I would like to discuss whether a limited secondary datapath model
> would be acceptable, without fully sharing all bonding state across
> processes.
>
> The proposed model is:
>
> * the primary process owns the bonding control plane;
> * the primary process owns the LACP state machine for mode 4 / 802.3ad;
> * secondary processes cannot change bond mode, add/remove members, or
> enable/disable dedicated LACP queues;
> * the primary process publishes only a minimal shared datapath state snapshot;
> * secondary processes only read this shared state for Rx/Tx decisions;
> * blackhole burst functions remain the fallback when real secondary
> datapath support is not available.
>
> For mode 4, the shared snapshot could initially include only the state
> required by the datapath, such as the active member list and
> per-member collecting/distributing state.
>
> This would make the secondary process a datapath consumer of
> primary-maintained bonding state, rather than a second owner of the
> bonding control plane.
>
> Questions:
>
> 1. Should real secondary datapath support be considered at all, or
> should bonding secondary support remain limited to safe blackhole
> Rx/Tx?
> 2. If real secondary datapath support is considered, is this limited
> secondary datapath model acceptable for the bonding PMD?
>
> I would like to work on this if the direction sounds reasonable.
>
> Thanks,
> Weijun
It would be great to allow secondary to do Rx/Tx.
But it needs to be safe against primary changing datapath.
The important part is that is better to return an error or blackhole
data rather than causing crashes. If you require primary to do all
control plane; then all secondary requests need to return an error.
^ permalink raw reply
* [RFC] net/bonding: support limited datapath in secondary process
From: Weijun Pan @ 2026-07-04 16:00 UTC (permalink / raw)
To: dev; +Cc: chas3, humin29, stephen
Hi,
I am looking into Bugzilla 1900 and real secondary-process datapath
support for the bonding PMD.
The current secondary attach path is safe by default: secondary Rx/Tx
uses blackhole burst functions instead of installing real per-mode
bonding burst functions. This avoids crashes because the internal
bonding state is not valid in the secondary process.
However, this also means that a secondary process cannot perform real
Rx/Tx through a bonding device. This is the issue described in
Bugzilla 1900.
I would like to discuss whether a limited secondary datapath model
would be acceptable, without fully sharing all bonding state across
processes.
The proposed model is:
* the primary process owns the bonding control plane;
* the primary process owns the LACP state machine for mode 4 / 802.3ad;
* secondary processes cannot change bond mode, add/remove members, or
enable/disable dedicated LACP queues;
* the primary process publishes only a minimal shared datapath state snapshot;
* secondary processes only read this shared state for Rx/Tx decisions;
* blackhole burst functions remain the fallback when real secondary
datapath support is not available.
For mode 4, the shared snapshot could initially include only the state
required by the datapath, such as the active member list and
per-member collecting/distributing state.
This would make the secondary process a datapath consumer of
primary-maintained bonding state, rather than a second owner of the
bonding control plane.
Questions:
1. Should real secondary datapath support be considered at all, or
should bonding secondary support remain limited to safe blackhole
Rx/Tx?
2. If real secondary datapath support is considered, is this limited
secondary datapath model acceptable for the bonding PMD?
I would like to work on this if the direction sounds reasonable.
Thanks,
Weijun
^ permalink raw reply
* [PATCH] doc: announce QinQ offloading operations in iavf PMD
From: Anurag Mandal @ 2026-07-04 13:17 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, vladimir.medvedkin, Anurag Mandal
The iavf PMD supports hardware offloading for all QinQ operations.
QinQ tag stripping and insertion offloads have been added for both
outer VLAN TPIDs: 0x88a8 (IEEE 802.1ad) & 0x8100 (IEEE 802.1Q).
Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
doc/guides/nics/features/iavf.ini | 2 +-
doc/guides/rel_notes/release_26_07.rst | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/doc/guides/nics/features/iavf.ini b/doc/guides/nics/features/iavf.ini
index 0ba6f7dfd7..e324a7aecb 100644
--- a/doc/guides/nics/features/iavf.ini
+++ b/doc/guides/nics/features/iavf.ini
@@ -30,7 +30,7 @@ Traffic manager = Y
Inline crypto = P
CRC offload = Y
VLAN offload = P
-QinQ offload = P
+QinQ offload = Y
L3 checksum offload = Y
L4 checksum offload = Y
Timestamp offload = Y
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index cf79eb57bf..d864019244 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -139,6 +139,7 @@ New Features
* Added support for transmitting LLDP packets based on mbuf packet type.
* Implemented AVX2 context descriptor transmit paths.
+ * Added support for QinQ offloading operations.
* **Updated Intel ice driver.**
--
2.34.1
^ permalink raw reply related
* [PATCH] net/ice: gate send on timestamp offload to E830
From: Soumyadeep Hore @ 2026-07-04 11:58 UTC (permalink / raw)
To: dev, aman.deep.singh, manoj.kumar.subbarao, bruce.richardson
The SEND_ON_TIMESTAMP TX offload capability was unconditionally
advertised for all ice devices in non-safe mode. However, only E830
hardware supports TxPP (Tx Packet Pacing) via the TXTIME queue
mechanism. E810 and other ice devices correctly reject the offload
at queue setup time (ice_tx_queue_setup checks hw->phy_model), but
advertising the capability misleads applications into thinking the
feature is available.
Gate the SEND_ON_TIMESTAMP capability advertisement on
hw->phy_model == ICE_PHY_E830 so that only devices which actually
support TxPP report the capability.
Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
---
drivers/net/intel/ice/ice_ethdev.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 0f2e7aee14..030d53aded 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -4568,8 +4568,10 @@ ice_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
RTE_ETH_TX_OFFLOAD_VXLAN_TNL_TSO |
RTE_ETH_TX_OFFLOAD_GRE_TNL_TSO |
RTE_ETH_TX_OFFLOAD_IPIP_TNL_TSO |
- RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO |
- RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP;
+ RTE_ETH_TX_OFFLOAD_GENEVE_TNL_TSO;
+ if (hw->phy_model == ICE_PHY_E830)
+ dev_info->tx_offload_capa |=
+ RTE_ETH_TX_OFFLOAD_SEND_ON_TIMESTAMP;
dev_info->flow_type_rss_offloads |= ICE_RSS_OFFLOAD_ALL;
}
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox