* [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
* Re: [PATCH] test/bpf_validate: use unit_test_suite_runner
From: Thomas Monjalon @ 2026-07-04 11:18 UTC (permalink / raw)
To: dev; +Cc: Marat Khalili
In-Reply-To: <20260703152910.80285-1-marat.khalili@huawei.com>
Recheck-request: iol-unit-amd64-testing, iol-unit-arm64-testing, github-robot, github-robot: build
^ permalink raw reply
* [PATCH] bpf: fix unitialized warning
From: Stephen Hemminger @ 2026-07-04 4:03 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Konstantin Ananyev, Marat Khalili
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>
---
lib/bpf/bpf_validate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/bpf/bpf_validate.c b/lib/bpf/bpf_validate.c
index f9960088a2..44db85a5a3 100644
--- a/lib/bpf/bpf_validate.c
+++ b/lib/bpf/bpf_validate.c
@@ -659,7 +659,7 @@ eval_apply_mask(struct bpf_reg_val *rv, uint64_t mask)
static void
eval_add(struct bpf_reg_val *rd, const struct bpf_reg_val *rs, uint64_t msk)
{
- struct bpf_reg_val rs_buf;
+ struct bpf_reg_val rs_buf = { 0 };
struct bpf_reg_val rv;
if (RTE_BPF_ARG_PTR_TYPE(rs->v.type) != 0) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 0/6] net/dpaa2: NAPI-style Rx queue interrupts
From: Stephen Hemminger @ 2026-07-03 20:26 UTC (permalink / raw)
To: Maxime Leroy; +Cc: dev, Hemant Agrawal, Sachin Saxena, David Marchand
In-Reply-To: <CAHHRULUxDHRCQC-0fuxfjx9KvUPpJ4-QAEDr3mQQuy1jJgAGgA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6335 bytes --]
Right I pulled it back
On Fri, Jul 3, 2026, 11:56 Maxime Leroy <maxime@leroys.fr> wrote:
> Hi Stephen,
>
> Le ven. 3 juil. 2026, 18:03, Stephen Hemminger <stephen@networkplumber.org>
> a écrit :
>
>> On Tue, 30 Jun 2026 16:43:23 +0200
>> Maxime Leroy <maxime@leroys.fr> wrote:
>>
>> > This series lets a dpaa2 worker sleep on a queue's data-availability
>> > notification instead of busy-polling, exposed through the generic
>> > rte_eth_dev_rx_intr_* API (NAPI-style: poll while frames keep coming,
>> > arm the interrupt and sleep when the queue runs dry).
>> >
>> > Why it is not a trivial .rx_queue_intr_enable
>> > ----------------------------------------------
>> > A worker wakes on its software portal's DQRI, which fires when the
>> > portal's DQRR holds frames. The default dpaa2 Rx burst pulls frames
>> > from the FQ with a volatile dequeue and cannot be interrupt-driven; to
>> > wake on the DQRI the FQ must instead be pushed to the portal's DQRR.
>> >
>> > The natural dpni_set_queue with a notification destination would have to
>> > target the worker's portal, but that portal is only known once a worker
>> > affines, after dev_start, and that MC command holds the global MC lock
>> > long enough to wedge the firmware while traffic runs. So the bind cannot
>> > be done late, against the polling lcore.
>> >
>> > Design
>> > ------
>> > Each Rx FQ is bound to its own DPCON channel, statically, at dev_start
>> > while the dpni is still disabled (no knowledge of the polling lcore). A
>> > worker later subscribes its own ethrx portal to the channel and arms the
>> > DQRI in rx_queue_intr_enable, a one-shot per-portal op, never the
>> wedging
>> > set_queue. On a wakeup the worker drains each of its queues by a
>> volatile
>> > dequeue on the queue's own DPCON channel (one FQ per channel, so no
>> > per-frame demux); it polls all its queues, the same scheduling contract
>> > as plain DPDK polling. A queue can be re-homed to another lcore at
>> > runtime with no set_queue and no port stop.
>> >
>> > This reuses the event PMD's pushed/DQRR model but with one DPCON per FQ
>> > and static affinity (no QBMan scheduling), so the DPCON allocator is
>> > moved from the event driver to the fslmc bus and shared.
>> >
>> > Patch 1 disables the DPCON channel before closing it, an event/dpaa2 fix
>> > the shared allocator depends on. Patches 2 to 4 move the DPCON allocator
>> > to the fslmc bus, make the portal DQRI epoll optional, and add the
>> > dpcon_set_notification MC command. Patch 5 adds the interrupt support
>> > proper; patch 6 pins each DPIO's MSI to the lcore that arms it, a
>> latency
>> > optimisation.
>> >
>> > Tested on LX2160A (lx2160acex7).
>> >
>> > v3:
>> > - Reworked the Rx drain. Both versions bind one DPCON per FQ, but v2
>> > drained the shared portal DQRR and demuxed frames to their FQ by
>> > fqd_ctx, stashing foreign frames in a per-queue FIFO. v3 drains each
>> > queue with a volatile dequeue on its own channel (one FQ per channel),
>> > which drops the demux and stash code.
>> > - Dropped the rx_queue_count fix; it is applied to main.
>> > - Dropped the software VLAN strip patch; an independent net/dpaa2
>> cleanup,
>> > sent standalone and now applied to next-net.
>> > - Dropped the Depends-on: the ethdev fast-path ops fix is now in main.
>> > - Split the dpcon_set_notification MC command into its own patch.
>> > - Added an event/dpaa2 fix to disable the DPCON channel before close,
>> > needed once the allocator is shared.
>> > - Dropped the DQRI holdoff-tuning patch; the immediate-DQRI holdoff is
>> now
>> > set inline in the arm path.
>> > - Added a patch reusing the event driver's MSI-affinity helper (exposed
>> > from its RTE_EVENT_DPAA2 guard) to pin the portal MSI to the lcore
>> that
>> > arms it, so a CDAN wake lands on the worker's own core.
>> >
>> > v2:
>> > - Dropped the RSS RETA patch, an independent net/dpaa2 change the
>> > interrupt path does not require; it will be sent as its own series.
>> > - Dropped the ethdev fast-path ops fix; it is now a standalone series.
>> > - Dropped the eal/interrupts -EEXIST fix, applied to main by David
>> > Marchand.
>> > - Declared qbman_swp_interrupt_set_inhibit and qbman_swp_dqrr_size
>> > __rte_internal (David Marchand).
>> > - Minor formatting cleanup in the Rx interrupt setup.
>> >
>> > Maxime Leroy (6):
>> > event/dpaa2: disable channel before closing it
>> > bus/fslmc: move DPCON management from event driver to bus
>> > bus/fslmc/dpio: make the portal DQRI epoll optional
>> > bus/fslmc/mc: implement dpcon_set_notification
>> > net/dpaa2: support Rx queue interrupts
>> > net/dpaa2: pin Rx queue interrupt to the polling core
>> >
>> > doc/guides/nics/dpaa2.rst | 21 +
>> > doc/guides/nics/features/dpaa2.ini | 1 +
>> > doc/guides/rel_notes/release_26_07.rst | 1 +
>> > drivers/bus/fslmc/mc/dpcon.c | 31 ++
>> > drivers/bus/fslmc/mc/fsl_dpcon.h | 18 +
>> > drivers/bus/fslmc/meson.build | 1 +
>> > .../fslmc/portal}/dpaa2_hw_dpcon.c | 17 +-
>> > drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 112 ++++--
>> > drivers/bus/fslmc/portal/dpaa2_hw_dpio.h | 12 +
>> > drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 12 +
>> > .../fslmc/qbman/include/fsl_qbman_portal.h | 5 +
>> > drivers/bus/fslmc/qbman/qbman_portal.c | 5 +
>> > drivers/event/dpaa2/dpaa2_eventdev.h | 3 -
>> > drivers/event/dpaa2/meson.build | 1 -
>> > drivers/net/dpaa2/dpaa2_ethdev.c | 372 +++++++++++++++++-
>> > drivers/net/dpaa2/dpaa2_ethdev.h | 4 +
>> > drivers/net/dpaa2/dpaa2_rxtx.c | 104 +++--
>> > 17 files changed, 649 insertions(+), 71 deletions(-)
>> > rename drivers/{event/dpaa2 => bus/fslmc/portal}/dpaa2_hw_dpcon.c (88%)
>> >
>> >
>> > base-commit: 030328f5f920a87dabde54dacd4f5ac411ddcac9
>>
>> Looks good, applied to net-next
>
>
> Thanks for the merge.
>
> But this patchset have not been acked by Hemant. Right now, Hemant have
> asked some fixes on this serie. Hemant also said it needs some time to test
> it.
>
>
[-- Attachment #2: Type: text/html, Size: 7967 bytes --]
^ permalink raw reply
* Re: [PATCH v3 4/6] net/ice: timestamp all received packets when PTP is enabled
From: Dawid Wesierski @ 2026-07-03 20:00 UTC (permalink / raw)
To: dawid.wesierski; +Cc: dev, bruce.richardson
In-Reply-To: <20260630120657.1046588-5-dawid.wesierski@intel.com>
I agree with Bruce's.
Mislabeling all packets with the PTP flag (RTE_MBUF_F_RX_IEEE1588_PTP) is incorrect.
The goal is to provide hardware timestamps for all received traffic (for high-resolution packet capture),
the existing RTE_ETH_RX_OFFLOAD_TIMESTAMP offload flag is the appropriate way to achieve this.
I am testing the "proper" way and it seems to be working, will drop this patch from the series.
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Spolka oswiadcza, ze posiada status duzego przedsiebiorcy w rozumieniu ustawy z dnia 8 marca 2013 r. o przeciwdzialaniu nadmiernym opoznieniom w transakcjach handlowych.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.
^ permalink raw reply
* Re: [PATCH v3 0/6] net/dpaa2: NAPI-style Rx queue interrupts
From: Maxime Leroy @ 2026-07-03 18:56 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Hemant Agrawal, Sachin Saxena, David Marchand
In-Reply-To: <20260703090345.3e4099cd@phoenix.local>
[-- Attachment #1: Type: text/plain, Size: 6090 bytes --]
Hi Stephen,
Le ven. 3 juil. 2026, 18:03, Stephen Hemminger <stephen@networkplumber.org>
a écrit :
> On Tue, 30 Jun 2026 16:43:23 +0200
> Maxime Leroy <maxime@leroys.fr> wrote:
>
> > This series lets a dpaa2 worker sleep on a queue's data-availability
> > notification instead of busy-polling, exposed through the generic
> > rte_eth_dev_rx_intr_* API (NAPI-style: poll while frames keep coming,
> > arm the interrupt and sleep when the queue runs dry).
> >
> > Why it is not a trivial .rx_queue_intr_enable
> > ----------------------------------------------
> > A worker wakes on its software portal's DQRI, which fires when the
> > portal's DQRR holds frames. The default dpaa2 Rx burst pulls frames
> > from the FQ with a volatile dequeue and cannot be interrupt-driven; to
> > wake on the DQRI the FQ must instead be pushed to the portal's DQRR.
> >
> > The natural dpni_set_queue with a notification destination would have to
> > target the worker's portal, but that portal is only known once a worker
> > affines, after dev_start, and that MC command holds the global MC lock
> > long enough to wedge the firmware while traffic runs. So the bind cannot
> > be done late, against the polling lcore.
> >
> > Design
> > ------
> > Each Rx FQ is bound to its own DPCON channel, statically, at dev_start
> > while the dpni is still disabled (no knowledge of the polling lcore). A
> > worker later subscribes its own ethrx portal to the channel and arms the
> > DQRI in rx_queue_intr_enable, a one-shot per-portal op, never the wedging
> > set_queue. On a wakeup the worker drains each of its queues by a volatile
> > dequeue on the queue's own DPCON channel (one FQ per channel, so no
> > per-frame demux); it polls all its queues, the same scheduling contract
> > as plain DPDK polling. A queue can be re-homed to another lcore at
> > runtime with no set_queue and no port stop.
> >
> > This reuses the event PMD's pushed/DQRR model but with one DPCON per FQ
> > and static affinity (no QBMan scheduling), so the DPCON allocator is
> > moved from the event driver to the fslmc bus and shared.
> >
> > Patch 1 disables the DPCON channel before closing it, an event/dpaa2 fix
> > the shared allocator depends on. Patches 2 to 4 move the DPCON allocator
> > to the fslmc bus, make the portal DQRI epoll optional, and add the
> > dpcon_set_notification MC command. Patch 5 adds the interrupt support
> > proper; patch 6 pins each DPIO's MSI to the lcore that arms it, a latency
> > optimisation.
> >
> > Tested on LX2160A (lx2160acex7).
> >
> > v3:
> > - Reworked the Rx drain. Both versions bind one DPCON per FQ, but v2
> > drained the shared portal DQRR and demuxed frames to their FQ by
> > fqd_ctx, stashing foreign frames in a per-queue FIFO. v3 drains each
> > queue with a volatile dequeue on its own channel (one FQ per channel),
> > which drops the demux and stash code.
> > - Dropped the rx_queue_count fix; it is applied to main.
> > - Dropped the software VLAN strip patch; an independent net/dpaa2
> cleanup,
> > sent standalone and now applied to next-net.
> > - Dropped the Depends-on: the ethdev fast-path ops fix is now in main.
> > - Split the dpcon_set_notification MC command into its own patch.
> > - Added an event/dpaa2 fix to disable the DPCON channel before close,
> > needed once the allocator is shared.
> > - Dropped the DQRI holdoff-tuning patch; the immediate-DQRI holdoff is
> now
> > set inline in the arm path.
> > - Added a patch reusing the event driver's MSI-affinity helper (exposed
> > from its RTE_EVENT_DPAA2 guard) to pin the portal MSI to the lcore that
> > arms it, so a CDAN wake lands on the worker's own core.
> >
> > v2:
> > - Dropped the RSS RETA patch, an independent net/dpaa2 change the
> > interrupt path does not require; it will be sent as its own series.
> > - Dropped the ethdev fast-path ops fix; it is now a standalone series.
> > - Dropped the eal/interrupts -EEXIST fix, applied to main by David
> > Marchand.
> > - Declared qbman_swp_interrupt_set_inhibit and qbman_swp_dqrr_size
> > __rte_internal (David Marchand).
> > - Minor formatting cleanup in the Rx interrupt setup.
> >
> > Maxime Leroy (6):
> > event/dpaa2: disable channel before closing it
> > bus/fslmc: move DPCON management from event driver to bus
> > bus/fslmc/dpio: make the portal DQRI epoll optional
> > bus/fslmc/mc: implement dpcon_set_notification
> > net/dpaa2: support Rx queue interrupts
> > net/dpaa2: pin Rx queue interrupt to the polling core
> >
> > doc/guides/nics/dpaa2.rst | 21 +
> > doc/guides/nics/features/dpaa2.ini | 1 +
> > doc/guides/rel_notes/release_26_07.rst | 1 +
> > drivers/bus/fslmc/mc/dpcon.c | 31 ++
> > drivers/bus/fslmc/mc/fsl_dpcon.h | 18 +
> > drivers/bus/fslmc/meson.build | 1 +
> > .../fslmc/portal}/dpaa2_hw_dpcon.c | 17 +-
> > drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 112 ++++--
> > drivers/bus/fslmc/portal/dpaa2_hw_dpio.h | 12 +
> > drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 12 +
> > .../fslmc/qbman/include/fsl_qbman_portal.h | 5 +
> > drivers/bus/fslmc/qbman/qbman_portal.c | 5 +
> > drivers/event/dpaa2/dpaa2_eventdev.h | 3 -
> > drivers/event/dpaa2/meson.build | 1 -
> > drivers/net/dpaa2/dpaa2_ethdev.c | 372 +++++++++++++++++-
> > drivers/net/dpaa2/dpaa2_ethdev.h | 4 +
> > drivers/net/dpaa2/dpaa2_rxtx.c | 104 +++--
> > 17 files changed, 649 insertions(+), 71 deletions(-)
> > rename drivers/{event/dpaa2 => bus/fslmc/portal}/dpaa2_hw_dpcon.c (88%)
> >
> >
> > base-commit: 030328f5f920a87dabde54dacd4f5ac411ddcac9
>
> Looks good, applied to net-next
Thanks for the merge.
But this patchset have not been acked by Hemant. Right now, Hemant have
asked some fixes on this serie. Hemant also said it needs some time to test
it.
[-- Attachment #2: Type: text/html, Size: 7539 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/6] event/dpaa2: disable channel before closing it
From: Hemant Agrawal @ 2026-07-03 17:21 UTC (permalink / raw)
To: Maxime Leroy, dev; +Cc: hemant.agrawal, sachin.saxena, david.marchand
In-Reply-To: <20260630144329.457643-2-maxime@leroys.fr>
[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]
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:
1. dpcon_disable() — quiesce the hardware channel
2. dpcon_close() — release the MC control session
3. 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);
[-- Attachment #2: Type: text/html, Size: 3979 bytes --]
^ permalink raw reply
* Re: [PATCH v3 5/6] net/dpaa2: support Rx queue interrupts
From: Hemant Agrawal @ 2026-07-03 17:21 UTC (permalink / raw)
To: Maxime Leroy, dev; +Cc: hemant.agrawal, sachin.saxena, david.marchand
In-Reply-To: <20260630144329.457643-6-maxime@leroys.fr>
[-- Attachment #1: Type: text/plain, Size: 4179 bytes --]
On 30-06-2026 20:13, Maxime Leroy wrote:
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index a68404ee5e..36f8669644 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -5,6 +5,8 @@
>
> #include <time.h>
> #include <net/if.h>
> +#include <unistd.h>
> +#include <errno.h>
>
> #include <eal_export.h>
> #include <rte_mbuf.h>
> @@ -25,6 +27,7 @@
> #include <dpaa2_hw_mempool.h>
> #include <dpaa2_hw_dpio.h>
> #include <mc/fsl_dpmng.h>
> +#include <mc/fsl_dpcon.h>
> #include "dpaa2_ethdev.h"
> #include "dpaa2_sparser.h"
> #include <fsl_qbman_debug.h>
> @@ -658,6 +661,8 @@ dpaa2_clear_queue_active_dps(struct dpaa2_queue *q, int num_lcores)
> }
> }
>
> +static void dpaa2_dev_rx_queue_intr_unbind(struct dpaa2_queue *dpaa2_q);
> +
> static void
> dpaa2_free_rx_tx_queues(struct rte_eth_dev *dev)
> {
> @@ -675,6 +680,12 @@ dpaa2_free_rx_tx_queues(struct rte_eth_dev *dev)
> /* cleaning up queue storage */
> for (i = 0; i < priv->nb_rx_queues; i++) {
> dpaa2_q = priv->rx_vq[i];
> + if (dpaa2_q->napi_dpcon) { /* release the rx-intr channel */
> + dpaa2_dev_rx_queue_intr_unbind(dpaa2_q);
> + rte_dpaa2_free_dpcon_dev(dpaa2_q->napi_dpcon);
> + dpaa2_q->napi_dpcon = NULL;
> + dpaa2_q->napi_sub_dpio = NULL;
> + }
> dpaa2_clear_queue_active_dps(dpaa2_q,
> RTE_MAX_LCORE);
> dpaa2_queue_storage_free(dpaa2_q,
> @@ -880,6 +891,26 @@ dpaa2_eth_dev_configure(struct rte_eth_dev *dev)
> }
> }
>
> + if (dev->data->dev_conf.intr_conf.rxq) {
> + if (!dev->intr_handle)
> + dev->intr_handle = rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
> + if (!dev->intr_handle ||
> + rte_intr_vec_list_alloc(dev->intr_handle, "rxq_intr",
> + dev->data->nb_rx_queues) ||
> + rte_intr_nb_efd_set(dev->intr_handle, dev->data->nb_rx_queues) ||
> + rte_intr_type_set(dev->intr_handle, RTE_INTR_HANDLE_EXT)) {
> + DPAA2_PMD_ERR("Failed to set up rx-queue interrupts");
> + /* capture the error before cleanup may clobber rte_errno */
> + ret = rte_errno ? -rte_errno : -EIO;
> + if (dev->intr_handle) {
> + rte_intr_vec_list_free(dev->intr_handle);
> + rte_intr_instance_free(dev->intr_handle);
> + dev->intr_handle = NULL;
> + }
> + return ret;
> + }
> + }
> +
If |dev_configure()| is called a second time (e.g. after changing
|nb_rx_queues|), |dev->intr_handle| is already non-NULL. The code skips
|rte_intr_instance_alloc()| but calls |rte_intr_vec_list_alloc()| on the
existing handle without first freeing the old vector list. This leaks
the previously allocated vector list and may also cause a size mismatch
if |nb_rx_queues| changed between calls.
Should not we free the old vector list before reallocating:
if (dev->intr_handle)
rte_intr_vec_list_free(dev->intr_handle);
else
dev->intr_handle =
rte_intr_instance_alloc(RTE_INTR_INSTANCE_F_PRIVATE);
/* then always call rte_intr_vec_list_alloc() */
> dpaa2_tm_init(dev);
>
> return 0;
> @@ -898,6 +929,7 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
> {
> struct dpaa2_dev_priv *priv = dev->data->dev_private;
> struct fsl_mc_io *dpni = dev->process_private;
> + bool dpcon_allocated = false;
> struct dpaa2_queue *dpaa2_q;
> struct dpni_queue cfg;
> uint8_t options = 0;
> @@ -938,6 +970,25 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
> dpaa2_q->bp_array = rte_dpaa2_bpid_info;
> dpaa2_q->offloads = rx_conf->offloads;
>
> + /* NAPI: grab a DPCON channel for dev_start to bind this FQ statically */
> + dpaa2_q->napi_sub_dpio = NULL;
> |dpaa2_q->napi_sub_dpio| is declared as |RTE_ATOMIC(struct
> dpaa2_dpio_dev *)| in |dpaa2_hw_pvt.h|, but here it is assigned with a
> plain |= NULL| instead of |rte_atomic_store_explicit(...)|. This is
> inconsistent with the atomic stores used elsewhere (e.g. in
> |dpaa2_dev_rx_queue_intr_unbind|) and may cause memory-ordering issues
> on weakly-ordered architectures (ARM/aarch64).
>
> Please use:
>
> rte_atomic_store_explicit(&dpaa2_q->napi_sub_dpio, NULL,
> rte_memory_order_release);
>
[-- Attachment #2: Type: text/html, Size: 8006 bytes --]
^ permalink raw reply
* [PATCH v2] test/suites: clean up per-test directories
From: Marat Khalili @ 2026-07-03 17:17 UTC (permalink / raw)
Cc: dev, thomas, stephen
In-Reply-To: <20260703144216.43804-1-marat.khalili@huawei.com>
After commit 5ef166838394 ("test: add file-prefix for all fast-tests on
Linux") each test within a suite has its own --file-prefix filled with
the test name to enable parallel execution. It brought a couple of
drawbacks though:
* Solution only applied to one test suite.
* Each working directory occupied ~25MB, with hundreds of tests this
easily overfilled /run filesystems of small VMs. Currently executing
fast-tests suite consumes 2.8GB of RAM, and adding more tests created
problems for github CI.
* While it enabled parallel execution of tests within a single build
directory, --file-prefix argument could no longer be specified
externally (caused duplicate argument error). Specifying custom
--file-prefix was necessary when multiple builds ran in the same
environment, so the corresponding workflow became broken.
Move --file-prefix logic into a separate shell script, preserving
existing argument and cleaning the work directory after the test.
Apply it uniformly to all tests.
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
v2: addressed AI comments and CI failures
* Added missing SPDX header to the new shell script.
* Fixed typo in handling dump_test_names.
* Following AI suggestion changed EUID check to UID check.
* Abandoned attempts to cover telemetry test, it is now out of scope.
app/test/suites/meson.build | 43 +++++-----
app/test/suites/run_test_with_temp_prefix.sh | 84 ++++++++++++++++++++
2 files changed, 108 insertions(+), 19 deletions(-)
create mode 100755 app/test/suites/run_test_with_temp_prefix.sh
diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
index 786c459c24b3..de3c28a64df0 100644
--- a/app/test/suites/meson.build
+++ b/app/test/suites/meson.build
@@ -23,6 +23,15 @@ if not has_hugepage
endif
endif
+if is_linux
+ # use wrapper providing per-test work directories to allow parallel runs
+ test_runner = find_program('run_test_with_temp_prefix.sh')
+ test_args_prefix = [dpdk_test]
+else
+ test_runner = dpdk_test
+ test_args_prefix = []
+endif
+
# process source files to determine the different unit test suites
# - fast_tests
# - perf_tests
@@ -41,14 +50,16 @@ foreach suite:test_suites
if developer_mode
warning('Test "@0@" is not defined in any test suite'.format(t))
endif
- test(t, dpdk_test,
+ test(t, test_runner,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds,
is_parallel: false)
endforeach
elif suite_name == 'stress-tests'
foreach t: suite_tests
- test(t, dpdk_test,
+ test(t, test_runner,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds_stress,
is_parallel: false,
@@ -57,7 +68,8 @@ foreach suite:test_suites
elif suite_name != 'fast-tests'
# simple cases - tests without parameters or special handling
foreach t: suite_tests
- test(t, dpdk_test,
+ test(t, test_runner,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds,
is_parallel: false,
@@ -81,7 +93,7 @@ foreach suite:test_suites
nohuge = params[1] == 'NOHUGE_OK'
asan = params[2] == 'ASAN_OK'
- test_args = []
+ test_args = test_args_prefix
if nohuge
test_args += test_no_huge_args
elif not has_hugepage
@@ -94,25 +106,17 @@ foreach suite:test_suites
test_args += ['-d', dpdk_drivers_build_dir]
endif
- # use unique file-prefix to allow parallel runs
- file_prefix = []
- trace_prefix = []
- if is_linux
- file_prefix = ['--file-prefix=' + test_name.underscorify()]
- trace_prefix = [file_prefix[0] + '_with_traces']
- endif
-
- test(test_name, dpdk_test,
- args : test_args + file_prefix,
+ test(test_name, test_runner,
+ args: test_args,
env: ['DPDK_TEST=' + test_name],
timeout : timeout_seconds_fast,
is_parallel : false,
suite : 'fast-tests')
if not is_windows and test_name == 'trace_autotest'
- trace_extra = ['--trace=.*',
- '--trace-dir=@0@'.format(meson.current_build_dir())]
- test(test_name + '_with_traces', dpdk_test,
- args : test_args + trace_extra + trace_prefix,
+ test_args += ['--trace=.*']
+ test_args += ['--trace-dir=@0@'.format(meson.current_build_dir())]
+ test(test_name + '_with_traces', test_runner,
+ args: test_args,
env: ['DPDK_TEST=' + test_name],
timeout : timeout_seconds_fast,
is_parallel : false,
@@ -165,7 +169,8 @@ dump_test_names = [
'dump_struct_sizes',
]
foreach arg : dump_test_names
- test(arg, dpdk_test,
+ test(arg, test_runner,
+ args: test_args_prefix,
env : ['DPDK_TEST=' + arg],
timeout : timeout_seconds_fast,
is_parallel : false,
diff --git a/app/test/suites/run_test_with_temp_prefix.sh b/app/test/suites/run_test_with_temp_prefix.sh
new file mode 100755
index 000000000000..e7137b29a68f
--- /dev/null
+++ b/app/test/suites/run_test_with_temp_prefix.sh
@@ -0,0 +1,84 @@
+#!/bin/bash
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2026 Huawei Technologies Co., Ltd
+# Run dpdk test with a temporary prefix and clean it up afterwards
+set -eEuo pipefail
+
+# Get value of the long option with name in the first argument, from the rest.
+get_arg_value() {
+ local arg_name="$1"
+ shift 1
+
+ local arg_value=''
+ local grab_next=false
+ local arg
+
+ for arg in "$@"; do
+ if $grab_next; then
+ arg_value="$arg"
+ grab_next=false
+ elif [[ "$arg" == "${arg_name}="* ]]; then
+ arg_value="${arg#*=}"
+ elif [[ "$arg" == "$arg_name" ]]; then
+ grab_next=true
+ fi
+ done
+
+ printf "%s\n" "$arg_value"
+}
+
+# Delete work directory passing through the return code
+delete_work_directory() {
+ local -r test_rc="$1"
+ [[ -z "${WORK_DIRECTORY:-}" ]] || rm -rf "$WORK_DIRECTORY"
+ return "$test_rc"
+}
+
+main() {
+ if [[ $# -eq 0 ]]; then
+ printf "Usage: %s <test_command> [arguments...]\n" "$0" >&2
+ printf "%s %s\n" \
+ "Runs a DPDK test with a temporary file prefix" \
+ "and cleans up the working directory." >&2
+ printf "Ensure the DPDK_TEST environment variable is set.\n" >&2
+ exit 1
+ fi
+ local test_command=("$@")
+
+ if [[ -z "${DPDK_TEST:-}" ]]; then
+ printf "Ensure the DPDK_TEST environment variable is set.\n" >&2
+ exit 1
+ fi
+ local -r dpdk_test="$DPDK_TEST"
+
+ # Make sure file prefix is determined and set in test args
+ local file_prefix="$(get_arg_value --file-prefix "${test_command[@]}")"
+ if [[ -z "$file_prefix" ]]; then
+ # If not yet specified, set --file-prefix to test name
+ file_prefix="$dpdk_test"
+ if [[ -n "$(get_arg_value --trace "${test_command[@]}")" ]]; then
+ # Some tests runs twice, with and without traces
+ file_prefix="${file_prefix}_with_traces"
+ fi
+ test_command+=("--file-prefix=$file_prefix")
+ fi
+
+ # Make sure runtime directory is determined and set in the environment
+ local runtime_directory="${RUNTIME_DIRECTORY:-}"
+ if [[ -z "$runtime_directory" ]]; then
+ # Follow the algorithm in eal_filesystem.c
+ if [[ "$UID" -eq 0 ]]; then
+ runtime_directory='/var/run'
+ else
+ runtime_directory="${XDG_RUNTIME_DIR:-/tmp}"
+ fi
+ export RUNTIME_DIRECTORY="$runtime_directory"
+ fi
+
+ WORK_DIRECTORY="$runtime_directory/dpdk/$file_prefix"
+ trap 'delete_work_directory "$?"' EXIT
+
+ "${test_command[@]}"
+}
+
+main "$@"
--
2.43.0
^ permalink raw reply related
* Re: [RFC] memtank: add memtank library
From: Stephen Hemminger @ 2026-07-03 16:30 UTC (permalink / raw)
To: Konstantin Ananyev; +Cc: dev, hofors, mb
In-Reply-To: <20260610103918.96857-1-konstantin.ananyev@huawei.com>
On Wed, 10 Jun 2026 11:39:17 +0100
Konstantin Ananyev <konstantin.ananyev@huawei.com> wrote:
> Introduce memtank, highly customizable fixed sized object allocator
> for DPDK applications. It offers close to the mempool level performance
> on the fast path.
> Main difference with the mempool is the ability to grow/shrink at runtime
> with user provided grow/shirnk threshold values, plus some extra
> features for higher flexibility.
>
> Key properties:
>
> - relies on user to provide callbacks for actual memory reservations.
> User is free to choose whatever is most suitable way for his scenario,
> i.e: via malloc/rte_malloc/mmap/some custom memory allocator.
> - user defined constructor callback for newly allocated objects.
> - bulk alloc and free APIs.
> - different alloc/free policies (specified by user via flags parameter):
> * lightweight as possible, but can fail
> * more robust, but heavyweight - causes call to user-provided backing
> memory allocator.
> - backing memory grows/shrinks on demand, special API extensions
> to allow user control grow/shrink size, frequency and when/where it
> is going to happen (DP, CP, both, etc.).
> - ability to pre-allocate all objects at memtank creation time
> (mempool like behavior).
> - custom object size and alignment.
> - per object runtime statistics and sanity-checks (boundary violation,
> double free, etc.) can be enabled/disabled at memtank creation time.
>
> Known limitations (subject for further improvements):
>
> - scalability:
> after 8+ lcores conventional mempool (with FIFO) starts to outperform
> memtank (which uses LIFO inside).
> - mempool_cache integration is not part of the library and right now
> has to be implemented by used manually on top of memtank API.
>
> Envisioned usage scenarios within DPDK-based apps:
> various flow/session control structures (TCP PCB, CT, NAT sessions, etc.)
> that needs to be allocated/freed at the data-path.
> Also can be used by 'semi-fastpath' allocations:
> TBL-8 blocks for LPM, hash buckets, etc.
>
> Initial idea is inspired by Linux/Solaris SLAB allocators.
> Also re-used some ideas from my previous work for TLDK project:
> https://github.com/FDio/tldk
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> ---
Lots of good feedback from Fable AI review. Still needs more work.
Thanks for bringing memtank into DPDK proper - dynamic grow/shrink
fills a real gap between mempool and malloc for session-type
structures. The API shape holds up well against the new-library
criteria: handle-based create/destroy, single installed header,
RTE_EXPORT_EXPERIMENTAL_SYMBOL used correctly (26.11 for the next
merge window), and the three callbacks in rte_memtank_prm are the
point of the design rather than a framework smell.
That said, I applied the RFC on 26.07-rc2 and found real bugs.
Items 1-3 were verified at runtime with ASAN/UBSan harnesses
against the applied tree, item 4 with a build.
Errors:
1. Heap buffer overflow in rte_memtank_create() metadata sizing.
memtank_meta_size() reserves no headroom for aligning the user
allocator's return pointer, unlike memchunk_size() which adds
"algn - 1" for exactly this reason. create() then does:
mt = RTE_PTR_ALIGN_CEIL(p, alignof(typeof(*mt)));
which can advance p by up to 63 bytes into an allocation that only
has 48 bytes of slack (the gap between the offset of mtf.free[]
and sizeof(struct memtank_free)). Nothing documents an alignment
contract for prm->alloc(); with any allocator returning less than
16-byte-aligned memory (32-bit glibc malloc guarantees only 8),
put_free() writes the tail of the free[] cache past the end of
the allocation. Verified with ASAN and an 8-byte-aligned alloc
callback:
WRITE of size 7680 ...
#2 copy_objs lib/memtank/memtank.c:101
#3 put_free lib/memtank/memtank.c:131
#4 rte_memtank_free lib/memtank/memtank.c:627
located 0 bytes after 8520-byte region
Fix: mirror memchunk_size(),
sz = sizeof(*mt) + nb_free * sizeof(mt->mtf.free[0]);
sz = RTE_ALIGN_CEIL(sz + alignof(typeof(*mt)) - 1,
alignof(typeof(*mt)));
and document minimum alignment expectations for alloc().
2. Shrink is a no-op: the loop in shrink_chunk() never executes.
memtank.c:234-239:
k = 0;
n = 0;
for (i = 0; i != num && n != k; i += k) {
"n != k" is evaluated before the first iteration with both
variables zero, so the body never runs and the function always
returns 0. Consequently rte_memtank_shrink() and the
RTE_MTANK_FREE_SHRINK flag never release memory. Verified at
runtime: with all chunks on the FULL list and nb_free == max_free,
rte_memtank_shrink() returns 0 and the free() callback is never
invoked; memory is only released by destroy(). The condition
should presumably be "n == k" (continue while the previous batch
was fully satisfied, stop when _shrink_chunk() drains the FULL
list). Note this also means the stress test exercises none of the
shrink paths, despite reporting shrink statistics.
3. Misaligned struct memobj access when obj_align < 8.
check_param() accepts any power-of-two obj_align including 1 and
2, but the per-object trailer (two uint64_t red zones plus a
pointer) is placed at obj + obj_sz - sizeof(struct memobj), which
is only 8-byte aligned when the object stride is. With obj_align=2
and obj_size=5, UBSan traps on the first grow:
memtank.c:86: runtime error: store to misaligned address ...
for type 'struct memobj', which requires 8 byte alignment
Undefined behavior, and a fault on strict-alignment targets. Fix:
round obj_align up to alignof(struct memobj) inside create(), or
reject smaller values in check_param(). The shipped unit test uses
obj_align=2 and only avoids the trap because obj_size=0 keeps the
stride a multiple of 8.
4. Build failure: VLA in the stress test.
test_memtank_stress.c:425:
uint8_t buf[sz];
DPDK's default warning flags include -Wvla, so CI builds with
werror=true fail here. sz can also reach OBJ_SZ_MAX (1MB), which
is unfriendly to thread stacks regardless. Use a fixed-size buffer
with a bounded compare loop, or allocate once per worker.
Warnings:
5. mt->obj_size truncation for very large objects. memobj_size()
returns size_t but is stored into uint32_t (memtank.c:561). With
obj_size near UINT32_MAX the padded per-object size wraps, so
init_chunk() strides with a truncated value while chunk_size is
computed with the full one - corruption instead of EINVAL.
check_param() should bound obj_size.
6. Deprecated API in new test code: rte_atomic64_t/rte_atomic64_*
(test_memtank_stress.c:33-39) and rte_smp_rmb()/rte_smp_mb()
(:542, :616, :678, :686). New code should use
rte_atomic_*_explicit() with rte_memory_order_*. Related: wrk_cmd
(:127) is a plain uint32_t shared between the control lcore and
workers; it should be RTE_ATOMIC(uint32_t) with release store /
acquire load rather than bare access plus full barriers.
7. master/slave terminology throughout the stress test
(struct master_args, MASTER_FLAG_*, test_memtank_master, "launch
on all slaves"). DPDK uses main/worker; this was purged tree-wide
in 20.11 and must not be reintroduced.
8. __rte_cache_aligned placement. memtank.h:40,48,62 and
test_memtank_stress.c:97 use the old trailing form
"} __rte_cache_aligned;"; current convention is
"struct __rte_cache_aligned foo {" (MSVC-compatible position, see
rte_mempool.h). wrk_cmd applies the attribute to a variable - use
"static alignas(RTE_CACHE_LINE_SIZE) uint32_t" there.
9. meson.build declares deps += ['ring', 'telemetry'] but the
library uses neither (the ring is only in the test app, which has
its own dep entry). The empty extra_flags loop is dead boilerplate.
10. Callback contracts are undocumented. alloc/free/init can be
invoked concurrently from multiple lcores (any thread can trigger
grow or shrink), so they must be MT-safe. Worth stating in the
rte_memtank_prm doxygen, along with alignment expectations and
that alloc() need not zero memory.
11. Documentation needs a pass before leaving RFC:
- memtank_lib.rst uses markdown triple backticks (22 occurrences),
which RST renders literally; use ``inline literals``.
- The create/destroy example does not compile: "sruct", semicolons
instead of commas inside the designated initializer, and
"user_define_free" vs the defined "user_defined_free".
- Typos in the rst and header doxygen: "Same a s mempool", "Aled
public API", "ret_memtank_free", "rte_memetank_destroy",
"memntank", "pooll", "susbystem", "udnerlying", "thershold",
"maximimum", "statisitics", "insteance", "intitialize".
- rte_memtank.h:32 says the lists are "(USED, FREE)" but the
code's states are FULL and USED.
- misc.c:233 dump output has a stray comma: "[USED]={,".
- rte_memtank_chunk_free() doxygen for nb_obj says "Number of
objects to allocate".
12. Missing MAINTAINERS entry and release notes for a new library
(fine to defer for an RFC, needed for the real submission).
Info:
13. extern "C" opens before the #includes in rte_memtank.h;
current convention wraps only the declarations, after all includes
(see rte_mempool.h, rte_argparse.h). Include order is also
inverted (stdio.h after the rte_ headers).
14. RTE_MTANK_ALLOC_CHUNK is never tested by the implementation -
rte_memtank_alloc() gates the chunk path on "flags != 0", so any
future flag silently implies chunk allocation. Either test the
flag explicitly or document that any flag enables the chunk path.
^ permalink raw reply
* Re: [PATCH v3 0/6] net/dpaa2: NAPI-style Rx queue interrupts
From: Stephen Hemminger @ 2026-07-03 16:03 UTC (permalink / raw)
To: Maxime Leroy; +Cc: dev, hemant.agrawal, sachin.saxena, david.marchand
In-Reply-To: <20260630144329.457643-1-maxime@leroys.fr>
On Tue, 30 Jun 2026 16:43:23 +0200
Maxime Leroy <maxime@leroys.fr> wrote:
> This series lets a dpaa2 worker sleep on a queue's data-availability
> notification instead of busy-polling, exposed through the generic
> rte_eth_dev_rx_intr_* API (NAPI-style: poll while frames keep coming,
> arm the interrupt and sleep when the queue runs dry).
>
> Why it is not a trivial .rx_queue_intr_enable
> ----------------------------------------------
> A worker wakes on its software portal's DQRI, which fires when the
> portal's DQRR holds frames. The default dpaa2 Rx burst pulls frames
> from the FQ with a volatile dequeue and cannot be interrupt-driven; to
> wake on the DQRI the FQ must instead be pushed to the portal's DQRR.
>
> The natural dpni_set_queue with a notification destination would have to
> target the worker's portal, but that portal is only known once a worker
> affines, after dev_start, and that MC command holds the global MC lock
> long enough to wedge the firmware while traffic runs. So the bind cannot
> be done late, against the polling lcore.
>
> Design
> ------
> Each Rx FQ is bound to its own DPCON channel, statically, at dev_start
> while the dpni is still disabled (no knowledge of the polling lcore). A
> worker later subscribes its own ethrx portal to the channel and arms the
> DQRI in rx_queue_intr_enable, a one-shot per-portal op, never the wedging
> set_queue. On a wakeup the worker drains each of its queues by a volatile
> dequeue on the queue's own DPCON channel (one FQ per channel, so no
> per-frame demux); it polls all its queues, the same scheduling contract
> as plain DPDK polling. A queue can be re-homed to another lcore at
> runtime with no set_queue and no port stop.
>
> This reuses the event PMD's pushed/DQRR model but with one DPCON per FQ
> and static affinity (no QBMan scheduling), so the DPCON allocator is
> moved from the event driver to the fslmc bus and shared.
>
> Patch 1 disables the DPCON channel before closing it, an event/dpaa2 fix
> the shared allocator depends on. Patches 2 to 4 move the DPCON allocator
> to the fslmc bus, make the portal DQRI epoll optional, and add the
> dpcon_set_notification MC command. Patch 5 adds the interrupt support
> proper; patch 6 pins each DPIO's MSI to the lcore that arms it, a latency
> optimisation.
>
> Tested on LX2160A (lx2160acex7).
>
> v3:
> - Reworked the Rx drain. Both versions bind one DPCON per FQ, but v2
> drained the shared portal DQRR and demuxed frames to their FQ by
> fqd_ctx, stashing foreign frames in a per-queue FIFO. v3 drains each
> queue with a volatile dequeue on its own channel (one FQ per channel),
> which drops the demux and stash code.
> - Dropped the rx_queue_count fix; it is applied to main.
> - Dropped the software VLAN strip patch; an independent net/dpaa2 cleanup,
> sent standalone and now applied to next-net.
> - Dropped the Depends-on: the ethdev fast-path ops fix is now in main.
> - Split the dpcon_set_notification MC command into its own patch.
> - Added an event/dpaa2 fix to disable the DPCON channel before close,
> needed once the allocator is shared.
> - Dropped the DQRI holdoff-tuning patch; the immediate-DQRI holdoff is now
> set inline in the arm path.
> - Added a patch reusing the event driver's MSI-affinity helper (exposed
> from its RTE_EVENT_DPAA2 guard) to pin the portal MSI to the lcore that
> arms it, so a CDAN wake lands on the worker's own core.
>
> v2:
> - Dropped the RSS RETA patch, an independent net/dpaa2 change the
> interrupt path does not require; it will be sent as its own series.
> - Dropped the ethdev fast-path ops fix; it is now a standalone series.
> - Dropped the eal/interrupts -EEXIST fix, applied to main by David
> Marchand.
> - Declared qbman_swp_interrupt_set_inhibit and qbman_swp_dqrr_size
> __rte_internal (David Marchand).
> - Minor formatting cleanup in the Rx interrupt setup.
>
> Maxime Leroy (6):
> event/dpaa2: disable channel before closing it
> bus/fslmc: move DPCON management from event driver to bus
> bus/fslmc/dpio: make the portal DQRI epoll optional
> bus/fslmc/mc: implement dpcon_set_notification
> net/dpaa2: support Rx queue interrupts
> net/dpaa2: pin Rx queue interrupt to the polling core
>
> doc/guides/nics/dpaa2.rst | 21 +
> doc/guides/nics/features/dpaa2.ini | 1 +
> doc/guides/rel_notes/release_26_07.rst | 1 +
> drivers/bus/fslmc/mc/dpcon.c | 31 ++
> drivers/bus/fslmc/mc/fsl_dpcon.h | 18 +
> drivers/bus/fslmc/meson.build | 1 +
> .../fslmc/portal}/dpaa2_hw_dpcon.c | 17 +-
> drivers/bus/fslmc/portal/dpaa2_hw_dpio.c | 112 ++++--
> drivers/bus/fslmc/portal/dpaa2_hw_dpio.h | 12 +
> drivers/bus/fslmc/portal/dpaa2_hw_pvt.h | 12 +
> .../fslmc/qbman/include/fsl_qbman_portal.h | 5 +
> drivers/bus/fslmc/qbman/qbman_portal.c | 5 +
> drivers/event/dpaa2/dpaa2_eventdev.h | 3 -
> drivers/event/dpaa2/meson.build | 1 -
> drivers/net/dpaa2/dpaa2_ethdev.c | 372 +++++++++++++++++-
> drivers/net/dpaa2/dpaa2_ethdev.h | 4 +
> drivers/net/dpaa2/dpaa2_rxtx.c | 104 +++--
> 17 files changed, 649 insertions(+), 71 deletions(-)
> rename drivers/{event/dpaa2 => bus/fslmc/portal}/dpaa2_hw_dpcon.c (88%)
>
>
> base-commit: 030328f5f920a87dabde54dacd4f5ac411ddcac9
Looks good, applied to net-next
^ permalink raw reply
* Re: [PATCH] pdump: fix request timeout on unresponsive secondary
From: Stephen Hemminger @ 2026-07-03 15:52 UTC (permalink / raw)
To: Pushpendra Kumar; +Cc: dev, pushpendra.kumar, reshma.pattan, stable
In-Reply-To: <20260701060257.562895-1-pushpendra1x.kumar@intel.com>
On Wed, 1 Jul 2026 11:32:57 +0530
Pushpendra Kumar <pushpendra1x.kumar@intel.com> wrote:
> When a primary handles a pdump enable/disable request from a secondary
> (e.g. dpdk-dumpcap), it applies the callbacks locally and forwards the
> request to the other secondaries before replying. The forward used
> rte_mp_request_sync(), blocking up to MP_TIMEOUT_S for every secondary
> to acknowledge.
>
> A secondary that is attached but has not called rte_pdump_init() never
> registers the mp_pdump action and silently drops the message (logging
> only "Cannot find action: mp_pdump"). Commit c3ceb8742295 ("pdump:
> forward callback enable to secondary process"), which added this
> broadcast, appears to assume every secondary calls rte_pdump_init();
> when one does not, the primary cannot tell it from a slow or dead peer
> and blocks for the full timeout. That delay pushes the reply past the
> requester's own timeout, so dpdk-dumpcap reports "Packet dump enable
> ... failed Connection timed out".
>
> Forward the request asynchronously with rte_mp_request_async() so an
> unresponsive secondary can no longer block the reply; a callback logs a
> warning if not all secondaries acknowledge. The forward is still issued
> before the requester reply, preserving the ordering from commit
> 928f43e3f9c1 ("pdump: fix race in disabling").
>
> This slightly weakens the happens-before guarantee: the sync call was a
> full-acknowledgement barrier, whereas the async path relies on AF_UNIX
> SOCK_DGRAM FIFO ordering and the single mp_handle thread processing the
> forward before the reply that triggers teardown. This suffices for the
> disable race fixed by 928f43e3f9c1, but reviewers should confirm they
> are comfortable with the weaker guarantee.
>
> Fixes: c3ceb8742295 ("pdump: forward callback enable to secondary process")
> Cc: stable@dpdk.org
>
> Signed-off-by: Pushpendra Kumar <pushpendra1x.kumar@intel.com>
> ---
Yes this looks like a real problem but not sure if this the right fix.
More detailed AI review says:
Review — pdump: fix request timeout on unresponsive secondary
Warning: async forward drops the acknowledgement barrier that
928f43e3f9c1 depends on.
The fix in 928f43e3f9c1 was ordering: forward to secondaries
before replying to the requester. But the safety came from
rte_mp_request_sync() being a full-acknowledgement barrier, not
from send order alone. The primary blocked until every secondary
(dumpcap included, since the broadcast walks the whole mp socket
dir) had actually processed the forward and replied — only then
was the disable response sent back to dumpcap.
rte_mp_request_async() only guarantees the forward is sent
before the reply, not processed. And the two messages reach
dumpcap on different paths handled by different threads:
the disable response wakes the thread blocked in
pdump_prepare_client_request()'s rte_mp_request_sync();
the broadcast is handled by dumpcap's separate mp_handle
thread via pdump_handle_primary_request().
Nothing forces mp_handle to run the broadcast before the blocked
thread wakes and tears down. So on disable, dumpcap can free its
ring / drop the mp_pdump action before it processes the forwarded
request — which is exactly the "Cannot find action: mp_pdump" /
missing-response window 928f43e3f9c1 closed. The commit message's
appeal to "AF_UNIX SOCK_DGRAM FIFO ordering" doesn't hold here:
the reply and the forward travel on distinct sockets to distinct
consumers, so there is no FIFO relationship between them.
For enable this is harmless (callbacks are being added, ring
already exists). It's disable where losing the barrier reopens
the race.
Info: the underlying problem is enable vs disable asymmetry.
The timeout bug you're fixing bites hardest on disable (dumpcap
^C), which is also the one path that still needs the barrier — so
"async for enable, keep sync for disable" doesn't fully solve it.
Worth deciding whether the right fix is at the mp layer instead:
the root cause per c3ceb8742295 is a secondary that attached but
never called rte_pdump_init(), so the primary can't distinguish
"didn't register" from "slow/dead." Broadcasting only to
processes that registered the action, or a disable-side handshake
that doesn't block on non-participants, would fix the timeout
without weakening teardown safety.
Info: the "reviewers should confirm they are comfortable with the
weaker guarantee" paragraph reads like an RFC note; it shouldn't
survive into the permanent commit log if this is applied as-is.
^ permalink raw reply
* Re: [PATCH] test/suites: clean up per-test directories
From: Stephen Hemminger @ 2026-07-03 15:49 UTC (permalink / raw)
To: Marat Khalili; +Cc: dev, thomas
In-Reply-To: <20260703144216.43804-1-marat.khalili@huawei.com>
On Fri, 3 Jul 2026 15:42:16 +0100
Marat Khalili <marat.khalili@huawei.com> wrote:
> After commit 5ef166838394 ("test: add file-prefix for all fast-tests on
> Linux") each test within a suite has its own --file-prefix filled with
> the test name to enable parallel execution. It brought a couple of
> drawbacks though:
> * Solution only applied to one test suite.
> * Each working directory occupied ~25MB, with hundreds of tests this
> easily overfilled /run filesystems of small VMs. Currently executing
> fast-tests suite consumes 2.8GB of RAM, and adding more tests created
> problems for github CI.
> * While it enabled parallel execution of tests within a single build
> directory, --file-prefix argument could no longer be specified
> externally (caused duplicate argument error). Specifying custom
> --file-prefix was necessary when multiple builds ran in the same
> environment, so the corresponding workflow became broken.
>
> Move --file-prefix logic into a separate shell script, preserving
> existing argument and cleaning the work directory after the test.
> Apply it uniformly to all tests.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
AI review run manually shows:
Error — app/test/suites/meson.build, dump/debug-tests loop
The loop was changed to pass args : test_args, but test_args at that point is leftover state from the telemetry block above ([dpdk_test_as_arg] + test_no_huge_args + --vdev=... + '-a 0000:00:00.0') — or, if RTE_LIB_TELEMETRY is disabled, whatever the last fast-test iteration happened to leave in it. The debug tests previously ran with no extra args. Every other simple loop correctly uses test_args_prefix; this one should as well.
On Linux the effect is worse than just wrong EAL args: test_args begins with dpdk_test_as_arg (the wrapper's own path), so meson invokes the wrapper with the wrapper path as its first argument. The wrapper then re-execs itself before finally running the real binary — with --no-huge, the skeleton --vdev set, and -a 0000:00:00.0 that were only meant for telemetry_all.
Fix:
foreach arg : dump_test_names
test(arg, dpdk_test,
args : test_args_prefix,
env : ['DPDK_TEST=' + arg],
...
On Windows test_args_prefix is [], which also restores the original no-args behavior there.
Info — uid check diverges from the algorithm it cites. eal_filesystem.c keys the /var/run vs XDG_RUNTIME_DIR choice on getuid() (real uid); the script tests $EUID (effective). These only differ under setuid / sudo-with-preserved-ruid, which won't happen in normal CI, but if the goal is to match EAL exactly, use $UID (or id -ru).
Info — mixed indentation in the wrapper: the two runtime_directory=... assignments inside the root check use tab-plus-spaces, while the rest of the script uses hard tabs.
^ permalink raw reply
* [PATCH v9] graph: add optional profiling stats
From: Morten Brørup @ 2026-07-03 15:43 UTC (permalink / raw)
To: dev, Jerin Jacob, Kiran Kumar K, Nithin Dabilpuram, Zhirun Yan,
Saeed Bishara
Cc: Morten Brørup
In-Reply-To: <20260619202047.2809165-1-mb@smartsharesystems.com>
graph: add optional profiling stats
Added graph node profiling stats, build time configurable by enabling
RTE_GRAPH_PROFILE in rte_config.h.
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v9:
* Fixed comment still mentioning 32 objects.
* Moved sample size array outside loop. (AI)
* Added release note. (AI)
v8:
* Added static const array as local variable, instead of indexing directly
into const array. (AI)
This also eliminates the space required between "} [idx];" weirdness.
* Added build time configurable RTE_GRAPH_PROFILE_BURST_SIZE to replace
the hardcoded burst size of 32. (AI)
v7:
* Use RTE_DIM() in histogram for loop.
* Added static_assert for histogram index values.
* Minor details to please checkpatch.
Although I disagree with requiring a space when indexing into
a constant array "(const type []){values} [idx];",
I have changed the code to comply.
v6:
* Consolidate the four histogram entries into one array. (Saeed Bishara)
* Sample at 32 objs instead of a half burst. (Saeed Bishara)
* Moved stats to different location in rte_node structure. (Jerin)
* Minor details to please checkpatch.
v5:
* Added stats for a half burst and a full burst.
v4:
* Added documentation. (AI)
* Added more comments. (AI)
* Improved dump. (AI)
* Debug shows both cycles/call and cycles/obj.
v3:
* Debug shows cycles/obj instead of cycles/call.
* Fixed missing --in-reply-to.
v2:
* Fixed indentation.
---
config/rte_config.h | 2 ++
doc/guides/prog_guide/graph_lib.rst | 3 ++
doc/guides/rel_notes/release_26_07.rst | 7 ++++
lib/graph/graph_debug.c | 50 ++++++++++++++++++++++++++
lib/graph/node.c | 2 ++
lib/graph/rte_graph_worker_common.h | 32 +++++++++++++++--
6 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/config/rte_config.h b/config/rte_config.h
index 0447cdf2ad..7703a6325b 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -106,6 +106,8 @@
/* rte_graph defines */
#define RTE_GRAPH_BURST_SIZE 256
#define RTE_LIBRTE_GRAPH_STATS 1
+/* RTE_GRAPH_PROFILE is not set */
+#define RTE_GRAPH_PROFILE_BURST_SIZE 32
/****** driver defines ********/
diff --git a/doc/guides/prog_guide/graph_lib.rst b/doc/guides/prog_guide/graph_lib.rst
index 8dd49c19d2..4311c37cc2 100644
--- a/doc/guides/prog_guide/graph_lib.rst
+++ b/doc/guides/prog_guide/graph_lib.rst
@@ -49,6 +49,9 @@ Performance tuning parameters
RTE_GRAPH_BURST_SIZE config option.
The testing shows, on x86 and arm64 servers, The sweet spot is 256 burst
size. While on arm64 embedded SoCs, it is either 64 or 128.
+- Enable the ``RTE_GRAPH_PROFILE`` config option for more profiling details.
+ Set the ``RTE_GRAPH_PROFILE_BURST_SIZE`` config option to sample a specific
+ burst size.
- Disable node statistics (using ``RTE_LIBRTE_GRAPH_STATS`` config option)
if not needed.
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 8b1bdada1a..cb7d48eeac 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -83,6 +83,13 @@ New Features
* The size of the ``struct rte_mempool_cache`` was kept
for API/ABI compatibility purposes.
+* **Added optional graph profiling statistics.**
+
+ Added build-time configurable graph node profiling statistics via
+ ``RTE_GRAPH_PROFILE`` in ``rte_config.h``. When enabled, tracks cycles
+ spent processing bursts of 0, 1, ``RTE_GRAPH_PROFILE_BURST_SIZE``,
+ and ``RTE_GRAPH_BURST_SIZE`` objects per node.
+
* **Added RISC-V vector paths.**
* Increased the default SIMD bitwidth to allow using the vector extension.
diff --git a/lib/graph/graph_debug.c b/lib/graph/graph_debug.c
index e3b8cccdc1..b999fa4140 100644
--- a/lib/graph/graph_debug.c
+++ b/lib/graph/graph_debug.c
@@ -93,6 +93,56 @@ rte_graph_obj_dump(FILE *f, struct rte_graph *g, bool all)
n->dispatch.total_sched_fail);
}
fprintf(f, " total_calls=%" PRId64 "\n", n->total_calls);
+ if (rte_graph_has_stats_feature())
+ fprintf(f, " total_cycles=%" PRIu64 ", avg cycles/call=%.1f\n",
+ n->total_cycles,
+ n->total_calls == 0 ? 0.0 :
+ (double)n->total_cycles / (double)n->total_calls);
+#ifdef RTE_GRAPH_PROFILE
+ static const uint16_t profile_sample_sizes[] = {
+ 0, 1, RTE_GRAPH_PROFILE_BURST_SIZE, RTE_GRAPH_BURST_SIZE};
+ static_assert(RTE_DIM(profile_sample_sizes) == RTE_DIM(n->usage_stats),
+ "usage_stats array size mismatch");
+ int64_t calls_other = n->total_calls;
+ int64_t cycles_other = n->total_cycles;
+ int64_t objs_other = n->total_objs;
+ for (int idx = 0; idx < RTE_DIM(n->usage_stats) + 1; idx++) {
+ uint64_t calls;
+ uint64_t cycles;
+ double objs_per_call;
+ if (idx < RTE_DIM(n->usage_stats)) {
+ uint16_t idx_objs = profile_sample_sizes[idx];
+ fprintf(f, " objs[%u]\n", idx_objs);
+ calls = n->usage_stats[idx].calls;
+ cycles = n->usage_stats[idx].cycles;
+ objs_per_call = (double)idx_objs;
+ calls_other -= calls;
+ cycles_other -= cycles;
+ objs_other -= idx_objs * calls;
+ } else {
+ fprintf(f, " objs[other]\n");
+ if (calls_other > 0 && cycles_other > 0 && objs_other > 0) {
+ calls = calls_other;
+ cycles = cycles_other;
+ objs_per_call = (double)objs_other / (double)calls_other;
+ fprintf(f, " avg objs/call=%.1f\n", objs_per_call);
+ } else {
+ calls = 0;
+ cycles = 0;
+ objs_per_call = 0.0;
+ }
+ }
+ fprintf(f, " calls=%" PRIu64, calls);
+ if (calls != 0)
+ fprintf(f, ", cycles=%" PRIu64 ", avg cycles/call=%.1f",
+ cycles,
+ (double)cycles / (double)calls);
+ if (calls != 0 && objs_per_call != 0.0)
+ fprintf(f, ", avg cycles/obj=%.1f",
+ (double)cycles / (double)calls / objs_per_call);
+ fprintf(f, "\n");
+ }
+#endif
for (i = 0; i < n->nb_edges; i++)
fprintf(f, " edge[%d] <%s>\n", i,
n->nodes[i]->name);
diff --git a/lib/graph/node.c b/lib/graph/node.c
index 1fce3e6632..19b38881ae 100644
--- a/lib/graph/node.c
+++ b/lib/graph/node.c
@@ -110,10 +110,12 @@ __rte_node_register(const struct rte_node_register *reg)
rte_edge_t i;
size_t sz;
+#ifndef RTE_GRAPH_PROFILE
/* Limit Node specific metadata to one cacheline on 64B CL machine */
RTE_BUILD_BUG_ON((offsetof(struct rte_node, nodes) -
offsetof(struct rte_node, ctx)) !=
RTE_CACHE_LINE_MIN_SIZE);
+#endif
graph_spinlock_lock();
diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_worker_common.h
index 4ab53a533e..00e8f5859d 100644
--- a/lib/graph/rte_graph_worker_common.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -121,6 +121,17 @@ struct __rte_cache_aligned rte_node {
rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
/** Fast path area cache line 2. */
+#ifdef RTE_GRAPH_PROFILE
+ /**
+ * Usage when this node processed 0, 1, RTE_GRAPH_PROFILE_BURST_SIZE,
+ * or RTE_GRAPH_BURST_SIZE objects.
+ */
+ struct __rte_cache_aligned {
+ uint64_t calls; /**< Calls done. */
+ uint64_t cycles; /**< Cycles spent. */
+ } usage_stats[4];
+ /** Fast path area cache line 3. */
+#endif
__extension__ struct __rte_cache_aligned {
#define RTE_NODE_CTX_SZ 16
union {
@@ -148,8 +159,10 @@ struct __rte_cache_aligned rte_node {
};
};
+#ifndef RTE_GRAPH_PROFILE
static_assert(offsetof(struct rte_node, nodes) - offsetof(struct rte_node, ctx)
== RTE_CACHE_LINE_MIN_SIZE, "rte_node fast path area must fit in 64 bytes");
+#endif
/**
* @internal
@@ -197,7 +210,7 @@ void __rte_node_stream_alloc_size(struct rte_graph *graph,
static __rte_always_inline void
__rte_node_process(struct rte_graph *graph, struct rte_node *node)
{
- uint64_t start;
+ uint64_t cycles;
uint16_t rc;
void **objs;
@@ -206,11 +219,24 @@ __rte_node_process(struct rte_graph *graph, struct rte_node *node)
rte_prefetch0(objs);
if (rte_graph_has_stats_feature()) {
- start = rte_rdtsc();
+ cycles = -rte_rdtsc();
rc = node->process(graph, node, objs, node->idx);
- node->total_cycles += rte_rdtsc() - start;
+ cycles += rte_rdtsc();
+ node->total_cycles += cycles;
node->total_calls++;
node->total_objs += rc;
+#ifdef RTE_GRAPH_PROFILE
+ if (rc <= 1) {
+ node->usage_stats[rc].calls++;
+ node->usage_stats[rc].cycles += cycles;
+ } else if (rc == RTE_GRAPH_PROFILE_BURST_SIZE) {
+ node->usage_stats[2].calls++;
+ node->usage_stats[2].cycles += cycles;
+ } else if (rc == RTE_GRAPH_BURST_SIZE) {
+ node->usage_stats[3].calls++;
+ node->usage_stats[3].cycles += cycles;
+ }
+#endif
} else {
node->process(graph, node, objs, node->idx);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] test/bpf_validate: use unit_test_suite_runner
From: Stephen Hemminger @ 2026-07-03 15:37 UTC (permalink / raw)
To: Marat Khalili; +Cc: Konstantin Ananyev, dev, thomas
In-Reply-To: <20260703152910.80285-1-marat.khalili@huawei.com>
On Fri, 3 Jul 2026 16:29:09 +0100
Marat Khalili <marat.khalili@huawei.com> wrote:
> Due to github CI running out of tmpfs space after adding 29 new BPF
> validate tests they were temporarily disabled. Restore them as a single
> test using unit_test_suite_runner.
>
> Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
> ---
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply
* [PATCH] test/bpf_validate: use unit_test_suite_runner
From: Marat Khalili @ 2026-07-03 15:29 UTC (permalink / raw)
To: Konstantin Ananyev; +Cc: dev, thomas, stephen
Due to github CI running out of tmpfs space after adding 29 new BPF
validate tests they were temporarily disabled. Restore them as a single
test using unit_test_suite_runner.
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
app/test/test_bpf_validate.c | 130 ++++++++++++-----------------------
1 file changed, 44 insertions(+), 86 deletions(-)
diff --git a/app/test/test_bpf_validate.c b/app/test/test_bpf_validate.c
index 7181303f3ee4..066f1fa156ae 100644
--- a/app/test/test_bpf_validate.c
+++ b/app/test/test_bpf_validate.c
@@ -1291,9 +1291,6 @@ test_alu64_add_k(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_k_autotest,
- test_alu64_add_k);
-
/* 64-bit addition of immediate to a pointer range. */
static int
test_alu64_add_k_pointer(void)
@@ -1309,9 +1306,6 @@ test_alu64_add_k_pointer(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_k_pointer_autotest,
- test_alu64_add_k_pointer);
-
/* 64-bit addition of pointer to a pointer. */
static int
test_alu64_add_x_pointer_pointer(void)
@@ -1327,9 +1321,6 @@ test_alu64_add_x_pointer_pointer(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_x_pointer_pointer_autotest,
- test_alu64_add_x_pointer_pointer);
-
/* 64-bit addition of scalar to a pointer. */
static int
test_alu64_add_x_pointer_scalar(void)
@@ -1345,9 +1336,6 @@ test_alu64_add_x_pointer_scalar(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_x_pointer_scalar_autotest,
- test_alu64_add_x_pointer_scalar);
-
/* 64-bit addition of pointer to a scalar. */
static int
test_alu64_add_x_scalar_pointer(void)
@@ -1363,9 +1351,6 @@ test_alu64_add_x_scalar_pointer(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_x_scalar_pointer_autotest,
- test_alu64_add_x_scalar_pointer);
-
/* 64-bit addition of scalar to a scalar. */
static int
test_alu64_add_x_scalar_scalar(void)
@@ -1381,9 +1366,6 @@ test_alu64_add_x_scalar_scalar(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_add_x_scalar_scalar_autotest,
- test_alu64_add_x_scalar_scalar);
-
/* 64-bit bitwise AND between a scalar range and immediate. */
static int
test_alu64_and_k(void)
@@ -1398,9 +1380,6 @@ test_alu64_and_k(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_and_k_autotest,
- test_alu64_and_k);
-
/* 64-bit division and modulo of UINT64_MAX*2/3. */
static int
test_alu64_div_mod_big_constant(void)
@@ -1442,9 +1421,6 @@ test_alu64_div_mod_big_constant(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_div_mod_big_constant_autotest,
- test_alu64_div_mod_big_constant);
-
/* 64-bit division and modulo of UINT64_MAX/3..UINT64_MAX*2/3 by a constant. */
static int
test_alu64_div_mod_big_range(void)
@@ -1487,9 +1463,6 @@ test_alu64_div_mod_big_range(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_div_mod_big_range_autotest,
- test_alu64_div_mod_big_range);
-
/* 64-bit division and modulo of INT64_MIN by -1. */
static int
test_alu64_div_mod_overflow(void)
@@ -1533,9 +1506,6 @@ test_alu64_div_mod_overflow(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_div_mod_overflow_autotest,
- test_alu64_div_mod_overflow);
-
/* 64-bit left shift by 63. */
static int
test_alu64_lsh_63(void)
@@ -1550,9 +1520,6 @@ test_alu64_lsh_63(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_lsh_63_autotest,
- test_alu64_lsh_63);
-
/* 64-bit multiplication of constant and immediate with overflow. */
static int
test_alu64_mul_k_overflow(void)
@@ -1567,9 +1534,6 @@ test_alu64_mul_k_overflow(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_mul_k_overflow_autotest,
- test_alu64_mul_k_overflow);
-
/* 64-bit mul of small scalar range and immediate. */
static int
test_alu64_mul_k_range_small(void)
@@ -1584,9 +1548,6 @@ test_alu64_mul_k_range_small(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_mul_k_range_small_autotest,
- test_alu64_mul_k_range_small);
-
/* 64-bit negation when interval first element is INT64_MIN. */
static int
test_alu64_neg_int64min_first(void)
@@ -1618,9 +1579,6 @@ test_alu64_neg_int64min_first(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_neg_int64min_first_autotest,
- test_alu64_neg_int64min_first);
-
/* 64-bit negation when interval last element is INT64_MIN. */
static int
test_alu64_neg_int64min_last(void)
@@ -1647,9 +1605,6 @@ test_alu64_neg_int64min_last(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_neg_int64min_last_autotest,
- test_alu64_neg_int64min_last);
-
/* 64-bit negation when interval first element is zero. */
static int
test_alu64_neg_zero_first(void)
@@ -1681,9 +1636,6 @@ test_alu64_neg_zero_first(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_neg_zero_first_autotest,
- test_alu64_neg_zero_first);
-
/* 64-bit negation when interval last element is zero. */
static int
test_alu64_neg_zero_last(void)
@@ -1710,9 +1662,6 @@ test_alu64_neg_zero_last(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_neg_zero_last_autotest,
- test_alu64_neg_zero_last);
-
/* 64-bit bitwise OR between a positive scalar range and negative immediate. */
static int
test_alu64_or_k_negative(void)
@@ -1727,9 +1676,6 @@ test_alu64_or_k_negative(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_or_k_negative_autotest,
- test_alu64_or_k_negative);
-
/* 64-bit bitwise OR between a positive scalar range and positive immediate. */
static int
test_alu64_or_k_positive(void)
@@ -1744,9 +1690,6 @@ test_alu64_or_k_positive(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_or_k_positive_autotest,
- test_alu64_or_k_positive);
-
/* 64-bit difference between two negative ranges.. */
static int
test_alu64_sub_x_src_signed_max_zero(void)
@@ -1761,9 +1704,6 @@ test_alu64_sub_x_src_signed_max_zero(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_sub_x_src_signed_max_zero_autotest,
- test_alu64_sub_x_src_signed_max_zero);
-
/* 64-bit bitwise XOR between a negative scalar range and zero immediate. */
static int
test_alu64_xor_k_negative(void)
@@ -1778,9 +1718,6 @@ test_alu64_xor_k_negative(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_alu64_xor_k_negative_autotest,
- test_alu64_xor_k_negative);
-
/* Jump if greater than immediate. */
static int
test_jmp64_jeq_k(void)
@@ -1796,9 +1733,6 @@ test_jmp64_jeq_k(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_jeq_k_autotest,
- test_jmp64_jeq_k);
-
/* Jump if signed less than another register. */
static int
test_jmp64_jslt_x(void)
@@ -1814,9 +1748,6 @@ test_jmp64_jslt_x(void)
});
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_jslt_x_autotest,
- test_jmp64_jslt_x);
-
/* Jump on ordering comparisons with potential bound overflow. */
static int
test_jmp64_ordering_overflow(void)
@@ -1863,9 +1794,6 @@ test_jmp64_ordering_overflow(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_ordering_overflow_autotest,
- test_jmp64_ordering_overflow);
-
/* Jump on ordering comparisons between two ranges. */
static int
test_jmp64_ordering_ranges(void)
@@ -2022,9 +1950,6 @@ test_jmp64_ordering_ranges(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_ordering_ranges_autotest,
- test_jmp64_ordering_ranges);
-
/* Jump on ordering comparisons with singleton inside the range. */
static int
test_jmp64_ordering_singleton_inside(void)
@@ -2080,9 +2005,6 @@ test_jmp64_ordering_singleton_inside(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_ordering_singleton_inside_autotest,
- test_jmp64_ordering_singleton_inside);
-
/* Jump on ordering comparisons with singleton outside the range. */
static int
test_jmp64_ordering_singleton_outside(void)
@@ -2175,9 +2097,6 @@ test_jmp64_ordering_singleton_outside(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_ordering_singleton_outside_autotest,
- test_jmp64_ordering_singleton_outside);
-
/* Jump on ordering comparisons with ranges "touching" each other. */
static int
test_jmp64_ordering_touching(void)
@@ -2249,9 +2168,6 @@ test_jmp64_ordering_touching(void)
return TEST_SUCCESS;
}
-REGISTER_ATTIC_TEST(bpf_validate_jmp64_ordering_touching_autotest,
- test_jmp64_ordering_touching);
-
/* 64-bit load from heap (should be set to unknown). */
static int
test_mem_ldx_dw_heap(void)
@@ -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),
+ TEST_CASE(test_alu64_add_k_pointer),
+ TEST_CASE(test_alu64_add_x_pointer_pointer),
+ TEST_CASE(test_alu64_add_x_pointer_scalar),
+ TEST_CASE(test_alu64_add_x_scalar_pointer),
+ TEST_CASE(test_alu64_add_x_scalar_scalar),
+ TEST_CASE(test_alu64_and_k),
+ TEST_CASE(test_alu64_div_mod_big_constant),
+ TEST_CASE(test_alu64_div_mod_big_range),
+ TEST_CASE(test_alu64_div_mod_overflow),
+ TEST_CASE(test_alu64_lsh_63),
+ TEST_CASE(test_alu64_mul_k_overflow),
+ TEST_CASE(test_alu64_mul_k_range_small),
+ TEST_CASE(test_alu64_neg_int64min_first),
+ TEST_CASE(test_alu64_neg_int64min_last),
+ TEST_CASE(test_alu64_neg_zero_first),
+ TEST_CASE(test_alu64_neg_zero_last),
+ TEST_CASE(test_alu64_or_k_negative),
+ TEST_CASE(test_alu64_or_k_positive),
+ TEST_CASE(test_alu64_sub_x_src_signed_max_zero),
+ TEST_CASE(test_alu64_xor_k_negative),
+ TEST_CASE(test_jmp64_jeq_k),
+ TEST_CASE(test_jmp64_jslt_x),
+ TEST_CASE(test_jmp64_ordering_overflow),
+ TEST_CASE(test_jmp64_ordering_ranges),
+ TEST_CASE(test_jmp64_ordering_singleton_inside),
+ TEST_CASE(test_jmp64_ordering_singleton_outside),
+ TEST_CASE(test_jmp64_ordering_touching),
+ TEST_CASE(test_mem_ldx_dw_heap),
+ TEST_CASES_END()
+ }
+};
+
+static int
+test_bpf_validate(void)
+{
+ return unit_test_suite_runner(&test_bpf_validate_suite);
+}
+
+REGISTER_FAST_TEST(bpf_validate_autotest, NOHUGE_OK, ASAN_OK, test_bpf_validate);
--
2.43.0
^ permalink raw reply related
* [RFC] review-patch: verify structured findings against the patch
From: Stephen Hemminger @ 2026-07-03 15:03 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
Recent AI reviews have lots of extraneous warnings.
Reviews were misclassified in two ways: findings from a stale
patch revision were reported as current, and classify_review()
pattern-matched prose so a "### Errors" header above a discarded
item counted as an error.
Require the model to return a single JSON object where each error
and warning quotes one line verbatim from the patch. Drop findings
whose evidence or file is not in the diff into a separate
unverified section. Derive the exit code from verified counts
only. Render text/markdown/html locally from the findings.
Stamp output with git patch-id --stable and a final
"X-AI-Review:" verdict line for CI to parse. Retry once on
invalid JSON, then report a contract violation at warning level.
Add --replay for offline testing and tests/run-tests.sh.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
AGENTS.md | 52 ++-
devtools/ai/review-patch.py | 661 ++++++++++++++++++++++++++----------
2 files changed, 521 insertions(+), 192 deletions(-)
diff --git a/AGENTS.md b/AGENTS.md
index af9a7e0772..47a4636307 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -1836,20 +1836,58 @@ devtools/get-maintainer.sh <patch-file>
---
-# Response Format
+# Output Contract
-When you identify an issue:
-1. **State the problem** (1 sentence)
-2. **Why it matters** (1 sentence, only if not obvious)
-3. **Suggested fix** (code snippet or specific action)
+**Deliberation is private.** Work through candidate findings, trace error
+paths, and apply the final checks below BEFORE writing any output. The
+output must contain ONLY findings that survived. Never output:
+- candidate findings followed by "Discard this item" or "(Correction: ...)"
+- reasoning such as "Wait, reviewing the code more carefully..."
+- section headers for severities that have no findings
+- prose verdicts ("No issues found" prose, "all patches are correct")
-Example: This could panic if the string is NULL.
+The output is a single JSON object - no markdown fences, no text before or
+after it:
+
+```
+{
+ "summary": "one line",
+ "errors": [
+ {"issue": "what is wrong", "location": "file:line",
+ "evidence": "one line copied exactly from the patch",
+ "suggestion": "specific fix"}
+ ],
+ "warnings": [same fields as errors],
+ "info": [{"issue": "...", "location": "file:line", "suggestion": "..."}]
+}
+```
+
+**Evidence is mandatory for errors and warnings.** Copy one line verbatim
+from the patch (the added `+` line for new code). For missing-cleanup or
+missing-check findings, quote the line that allocates, opens, or calls.
+The harness verifies evidence against the patch under review: findings
+whose evidence does not appear in the patch are discarded as unverifiable
+(this catches reviews of a stale patch revision and hallucinated line
+references), so quote precisely.
+
+The `issue` field should state the problem in one sentence, add one
+sentence on why it matters only if not obvious, and `suggestion` should be
+a specific fix.
+
+If there are no findings, the entire output is:
+
+```
+{"summary": "No issues found.", "errors": [], "warnings": [], "info": []}
+```
---
## FINAL CHECK BEFORE SUBMITTING REVIEW
-Before outputting your review, do two separate passes:
+Before outputting your review, do two separate passes.
+Both passes are private deliberation:
+the JSON output contains only the findings that survive them,
+with no trace of items that were considered and removed.
### Pass 1: Verify correctness bugs are included
diff --git a/devtools/ai/review-patch.py b/devtools/ai/review-patch.py
index 52601ac156..6623b3d8c2 100755
--- a/devtools/ai/review-patch.py
+++ b/devtools/ai/review-patch.py
@@ -15,8 +15,10 @@
import subprocess
import sys
import tempfile
+from dataclasses import dataclass, field
from datetime import date
from email.message import EmailMessage
+from html import escape
from pathlib import Path
from typing import Any, Iterator
@@ -66,37 +68,31 @@
- Copyright years should reflect when the code was originally written
- Be conservative: reject changes that aren't clearly bug fixes"""
-FORMAT_INSTRUCTIONS = {
- "text": """Provide your review in plain text format.""",
- "markdown": """Provide your review in Markdown format with:
-- Headers (##) for each severity level (Errors, Warnings, Info)
-- Bullet points for individual issues
-- Code blocks (```) for code references
-- Bold (**) for emphasis on key points""",
- "html": """Provide your review in HTML format with:
-- <h2> tags for each severity level (Errors, Warnings, Info)
-- <ul>/<li> for individual issues
-- <pre><code> for code references
-- <strong> for emphasis on key points
-- Use appropriate semantic HTML tags
-- Do NOT include <html>, <head>, or <body> tags - just the content""",
- "json": """Provide your review in JSON format with this structure:
+REVIEW_JSON_INSTRUCTION = """\
+Respond with a single JSON object and nothing else - no markdown fences, \
+no prose before or after, no section headers:
{
- "summary": "Brief one-line summary of the review",
+ "summary": "one line",
"errors": [
- {"issue": "description", "location": "file:line", "suggestion": "fix"}
+ {"issue": "what is wrong", "location": "file:line",
+ "evidence": "one line copied exactly from the patch",
+ "suggestion": "specific fix"}
],
- "warnings": [
- {"issue": "description", "location": "file:line", "suggestion": "fix"}
- ],
- "info": [
- {"issue": "description", "location": "file:line", "suggestion": "fix"}
- ],
- "passed_checks": ["list of checks that passed"],
- "overall_status": "PASS|WARN|FAIL"
-}
-Output ONLY valid JSON, no markdown code fences or other text.""",
+ "warnings": [same fields as errors],
+ "info": [{"issue": "...", "location": "file:line", "suggestion": "..."}]
}
+Rules:
+- Deliberation is private. Output ONLY findings that survive your final
+ checks; never output candidate findings, "Discard this item", or
+ corrections.
+- Empty severities are empty arrays, never prose.
+- "evidence" must be copied verbatim from the patch (the added '+' line for
+ new code). For missing-cleanup or missing-check findings, quote the line
+ that allocates or calls. Findings whose evidence does not appear in the
+ patch are discarded by the harness as unverifiable.
+- If there are no findings:
+ {"summary": "No issues found.", "errors": [], "warnings": [], "info": []}
+"""
USER_PROMPT = """Please review the following DPDK patch file '{patch_name}' \
against the AGENTS.md guidelines. Focus on:
@@ -119,57 +115,213 @@
EXIT_WARNINGS = 2
EXIT_ERRORS = 3
+# Appended to the prompt when the first response was not valid JSON.
+RETRY_NOTE = (
+ "\n\nREMINDER: the previous response was not a single valid JSON "
+ "object. Respond again with ONLY the JSON object in the required schema."
+)
-def classify_review(review_text: str, output_format: str) -> int:
- """Classify review result and return appropriate exit code.
- Returns:
- 0 - clean (no errors or warnings)
- 2 - warnings found (no errors)
- 3 - errors found
- """
- has_errors = False
- has_warnings = False
+@dataclass
+class ReviewSection:
+ """One reviewed unit (whole patch file, one patch, or one chunk)."""
+
+ label: str
+ data: dict[str, Any] | None = None
+ unverified: list[dict[str, Any]] = field(default_factory=list)
+ raw: str | None = None
+
+ @property
+ def contract_violation(self) -> bool:
+ """True when the model response could not be parsed as JSON."""
+ return self.data is None
- if output_format == "json":
+
+def extract_json(text: str) -> dict[str, Any] | None:
+ """Parse a JSON object from model output, tolerating code fences."""
+ candidate = text.strip()
+ if candidate.startswith("```"):
+ candidate = re.sub(r"^```[a-zA-Z]*\n", "", candidate)
+ candidate = re.sub(r"\n```\s*$", "", candidate)
+ try:
+ data = json.loads(candidate)
+ if isinstance(data, dict):
+ return data
+ except json.JSONDecodeError:
+ pass
+ # Fall back to the outermost braces
+ first = candidate.find("{")
+ last = candidate.rfind("}")
+ if first != -1 and last > first:
try:
- data = json.loads(review_text)
- if data.get("errors"):
- has_errors = True
- if data.get("warnings"):
- has_warnings = True
- status = data.get("overall_status", "").upper()
- if status == "FAIL":
- has_errors = True
- elif status == "WARN":
- has_warnings = True
- except (json.JSONDecodeError, AttributeError):
- pass # Fall through to text scanning
-
- if not has_errors and not has_warnings:
- # Scan review text for severity indicators.
- # Match section headers and inline markers across text/markdown/html.
- for line in review_text.splitlines():
- stripped = line.strip().lower()
- # Skip quoted patch context lines
- if stripped.startswith(">") or stripped.startswith("diff --git"):
+ data = json.loads(candidate[first : last + 1])
+ if isinstance(data, dict):
+ return data
+ except json.JSONDecodeError:
+ pass
+ return None
+
+
+def get_patch_ids(patch_content: str) -> list[str]:
+ """Return stable patch ids for the content under review.
+
+ Uses `git patch-id --stable`, which is invariant to format-patch
+ regeneration (timestamps, commit hashes, version prefixes) but changes
+ whenever the diff content changes. This pins a review to the exact
+ revision it reviewed. Returns an empty list if git is unavailable.
+ """
+ try:
+ result = subprocess.run(
+ ["git", "patch-id", "--stable"],
+ input=patch_content,
+ capture_output=True,
+ text=True,
+ check=True,
+ )
+ except (subprocess.CalledProcessError, FileNotFoundError):
+ return []
+ ids = []
+ for line in result.stdout.splitlines():
+ fields = line.split()
+ if fields:
+ ids.append(fields[0])
+ return ids
+
+
+def _normalize_line(line: str) -> str:
+ """Collapse whitespace; drop a leading diff marker if present."""
+ if line[:1] in "+- ":
+ line = line[1:]
+ return " ".join(line.split())
+
+
+def build_patch_line_index(patch_content: str) -> set[str]:
+ """Set of normalized lines appearing in the patch."""
+ index = set()
+ for line in patch_content.splitlines():
+ norm = _normalize_line(line)
+ if norm:
+ index.add(norm)
+ return index
+
+
+def finding_is_verified(
+ finding: dict[str, Any], line_index: set[str], patch_content: str
+) -> bool:
+ """Check a finding's evidence and location against the patch content.
+
+ Guards against stale reviews (findings quoted from a different revision
+ of the patch) and hallucinated line references: every evidence line must
+ appear in the patch, and the file named in location must be part of it.
+ """
+ evidence = str(finding.get("evidence", "")).strip()
+ if not evidence:
+ return False
+ for line in evidence.splitlines():
+ norm = _normalize_line(line)
+ if norm and norm not in line_index:
+ return False
+ location = str(finding.get("location", ""))
+ fname = location.rsplit(":", 1)[0].strip()
+ if fname:
+ # Lenient: the model may shorten the path, so match the basename
+ base = fname.rsplit("/", 1)[-1]
+ if base and base not in patch_content:
+ return False
+ return True
+
+
+def verify_findings(
+ data: dict[str, Any], patch_content: str
+) -> tuple[dict[str, Any], list[dict[str, Any]]]:
+ """Split errors/warnings into verified and unverified findings.
+
+ Info findings pass through unchecked; they are often general
+ suggestions with no specific line to quote.
+ """
+ line_index = build_patch_line_index(patch_content)
+ unverified = []
+ verified = dict(data)
+ for severity in ("errors", "warnings"):
+ kept = []
+ for finding in data.get(severity) or []:
+ if not isinstance(finding, dict):
continue
- if re.match(r"^(#{1,3}\s+)?(\*{0,2})error", stripped) or re.match(
- r"^<h[1-3]>\s*error", stripped
- ):
- has_errors = True
- elif re.match(r"^(#{1,3}\s+)?(\*{0,2})warning", stripped) or re.match(
- r"^<h[1-3]>\s*warning", stripped
- ):
- has_warnings = True
+ if finding_is_verified(finding, line_index, patch_content):
+ kept.append(finding)
+ else:
+ dropped = dict(finding)
+ dropped["severity"] = severity[:-1]
+ unverified.append(dropped)
+ verified[severity] = kept
+ verified["info"] = [f for f in (data.get("info") or []) if isinstance(f, dict)]
+ return verified, unverified
+
+
+def process_review_text(
+ review_text: str, patch_content: str, label: str
+) -> ReviewSection:
+ """Parse and verify one model response into a ReviewSection."""
+ data = extract_json(review_text)
+ if data is None:
+ return ReviewSection(label=label, raw=review_text)
+ verified, unverified = verify_findings(data, patch_content)
+ return ReviewSection(label=label, data=verified, unverified=unverified)
+
+
+def section_counts(sections: list[ReviewSection]) -> dict[str, int]:
+ """Aggregate finding counts across sections."""
+ counts = {
+ "errors": 0,
+ "warnings": 0,
+ "info": 0,
+ "unverified": 0,
+ "violations": 0,
+ }
+ for sec in sections:
+ if sec.contract_violation:
+ counts["violations"] += 1
+ continue
+ counts["errors"] += len(sec.data.get("errors", []))
+ counts["warnings"] += len(sec.data.get("warnings", []))
+ counts["info"] += len(sec.data.get("info", []))
+ counts["unverified"] += len(sec.unverified)
+ return counts
+
+
+def classify_review(counts: dict[str, int]) -> int:
+ """Exit code from verified finding counts.
+
+ Status is derived ONLY from the parsed, verified findings - never from
+ pattern matching on prose (a "### Errors" header above "None found"
+ must not trip the classifier).
- if has_errors:
+ Returns:
+ 0 - clean (no errors or warnings)
+ 2 - warnings, unverified findings, or output-contract violations
+ 3 - errors found
+ """
+ if counts["errors"]:
return EXIT_ERRORS
- if has_warnings:
+ if counts["warnings"] or counts["unverified"] or counts["violations"]:
return EXIT_WARNINGS
return EXIT_CLEAN
+def summary_trailer(counts: dict[str, int], patch_ids: list[str]) -> str:
+ """One-line machine-readable verdict for CI to parse.
+
+ CI labelers should parse this line (or the JSON output), not the prose.
+ """
+ ids = ",".join(pid[:16] for pid in patch_ids) or "unknown"
+ return (
+ f"X-AI-Review: errors={counts['errors']} "
+ f"warnings={counts['warnings']} info={counts['info']} "
+ f"unverified={counts['unverified']} "
+ f"contract-violations={counts['violations']} patch-id={ids}"
+ )
+
+
def is_lts_release(release: str | None) -> bool:
"""Check if a release is an LTS release.
@@ -413,34 +565,82 @@ def get_summary_prompt() -> str:
"""
-def format_combined_reviews(
- reviews: list[tuple[str, str]], output_format: str, patch_name: str
-) -> str:
- """Combine multiple chunk/patch reviews into a single output."""
- if output_format == "json":
- combined = {
- "patch_file": patch_name,
- "sections": [
- {"label": label, "review": review} for label, review in reviews
- ],
- }
- return json.dumps(combined, indent=2)
- elif output_format == "html":
- sections = []
- for label, review in reviews:
- sections.append(f"<h2>{label}</h2>\n{review}")
- return "\n<hr>\n".join(sections)
- elif output_format == "markdown":
- sections = []
- for label, review in reviews:
- sections.append(f"## {label}\n\n{review}")
- return "\n\n---\n\n".join(sections)
- else: # text
- sections = []
- for label, review in reviews:
- sections.append(f"=== {label} ===\n\n{review}")
- separator = "\n\n" + "=" * 60 + "\n\n"
- return separator.join(sections)
+def _finding_lines(num: int, finding: dict[str, Any]) -> list[str]:
+ """Render one finding as indented text lines."""
+ lines = [f" {num}. {finding.get('location') or 'unspecified location'}"]
+ issue = str(finding.get("issue", "")).strip()
+ if issue:
+ lines.append(f" {issue}")
+ for ev_line in str(finding.get("evidence", "")).strip().splitlines():
+ lines.append(f" > {ev_line}")
+ suggestion = str(finding.get("suggestion", "")).strip()
+ if suggestion:
+ lines.append(f" Fix: {suggestion}")
+ return lines
+
+
+def render_section_text(sec: ReviewSection) -> str:
+ """Render one ReviewSection as plain text."""
+ if sec.contract_violation:
+ return (
+ "Output contract violation: the model response was not a valid "
+ "JSON review.\nRaw response follows (unparsed, counted as a "
+ f"warning):\n\n{sec.raw}"
+ )
+ lines: list[str] = []
+ summary = str(sec.data.get("summary", "")).strip()
+ if summary:
+ lines.append(summary)
+ titles = (("errors", "Errors"), ("warnings", "Warnings"), ("info", "Info"))
+ for key, title in titles:
+ findings = sec.data.get(key, [])
+ if not findings:
+ continue
+ lines.append("")
+ lines.append(f"{title}:")
+ for i, finding in enumerate(findings, 1):
+ lines.extend(_finding_lines(i, finding))
+ if sec.unverified:
+ lines.append("")
+ lines.append(
+ "Unverified findings (evidence not found in this patch "
+ "revision; possibly from a stale review - not counted):"
+ )
+ for i, finding in enumerate(sec.unverified, 1):
+ lines.extend(_finding_lines(i, finding))
+ if len(lines) <= 1 and not sec.unverified:
+ return summary or "No issues found."
+ return "\n".join(lines)
+
+
+def render_sections(sections: list[ReviewSection], output_format: str) -> str:
+ """Render all sections in the requested format.
+
+ Markdown and HTML are simple wrappings of the text rendering; findings
+ content comes from the verified structure in all formats.
+ """
+ parts = []
+ for sec in sections:
+ body = render_section_text(sec)
+ if output_format == "markdown":
+ if len(sections) > 1:
+ parts.append(f"## {sec.label}\n\n```\n{body}\n```")
+ else:
+ parts.append(f"```\n{body}\n```")
+ elif output_format == "html":
+ title = f"<h2>{escape(sec.label)}</h2>\n" if len(sections) > 1 else ""
+ parts.append(f"{title}<pre>{escape(body)}</pre>")
+ else: # text
+ if len(sections) > 1:
+ parts.append(f"=== {sec.label} ===\n\n{body}")
+ else:
+ parts.append(body)
+ if output_format == "markdown":
+ return "\n\n---\n\n".join(parts)
+ if output_format == "html":
+ return "\n<hr>\n".join(parts)
+ separator = "\n\n" + "=" * 60 + "\n\n"
+ return separator.join(parts)
def build_system_prompt(review_date: str, release: str | None) -> str:
@@ -466,10 +666,10 @@ def build_anthropic_request(
agents_content: str,
patch_content: str,
patch_name: str,
- output_format: str = "text",
+ retry_note: str = "",
) -> dict[str, Any]:
"""Build request payload for Anthropic API."""
- format_instruction = FORMAT_INSTRUCTIONS.get(output_format, "")
+ format_instruction = REVIEW_JSON_INSTRUCTION + retry_note
user_prompt = USER_PROMPT.format(
patch_name=patch_name, format_instruction=format_instruction
)
@@ -500,10 +700,10 @@ def build_openai_request(
agents_content: str,
patch_content: str,
patch_name: str,
- output_format: str = "text",
+ retry_note: str = "",
) -> dict[str, Any]:
"""Build request payload for OpenAI-compatible APIs."""
- format_instruction = FORMAT_INSTRUCTIONS.get(output_format, "")
+ format_instruction = REVIEW_JSON_INSTRUCTION + retry_note
user_prompt = USER_PROMPT.format(
patch_name=patch_name, format_instruction=format_instruction
)
@@ -527,10 +727,10 @@ def build_google_request(
agents_content: str,
patch_content: str,
patch_name: str,
- output_format: str = "text",
+ retry_note: str = "",
) -> dict[str, Any]:
"""Build request payload for Google Gemini API."""
- format_instruction = FORMAT_INSTRUCTIONS.get(output_format, "")
+ format_instruction = REVIEW_JSON_INSTRUCTION + retry_note
user_prompt = USER_PROMPT.format(
patch_name=patch_name, format_instruction=format_instruction
)
@@ -560,7 +760,7 @@ def call_api(
agents_content: str,
patch_content: str,
patch_name: str,
- output_format: str = "text",
+ retry_note: str = "",
verbose: bool = False,
timeout: int = 300,
) -> tuple[str, TokenUsage]:
@@ -573,7 +773,7 @@ def call_api(
agents_content,
patch_content,
patch_name,
- output_format,
+ retry_note,
)
elif provider == "google":
request_data = build_google_request(
@@ -582,7 +782,7 @@ def call_api(
agents_content,
patch_content,
patch_name,
- output_format,
+ retry_note,
)
else: # openai, xai
request_data = build_openai_request(
@@ -592,7 +792,7 @@ def call_api(
agents_content,
patch_content,
patch_name,
- output_format,
+ retry_note,
)
return send_request(
provider,
@@ -604,6 +804,59 @@ def call_api(
)
+def reviewed_section(
+ provider: str,
+ api_key: str,
+ model: str,
+ max_tokens: int,
+ system_prompt: str,
+ agents_content: str,
+ content: str,
+ name: str,
+ label: str,
+ verbose: bool,
+ timeout: int,
+ usage: TokenUsage,
+) -> ReviewSection:
+ """Run one review call, retrying once if the response is not valid JSON."""
+ review_text, call_usage = call_api(
+ provider,
+ api_key,
+ model,
+ max_tokens,
+ system_prompt,
+ agents_content,
+ content,
+ name,
+ "",
+ verbose,
+ timeout,
+ )
+ usage.add(call_usage)
+ section = process_review_text(review_text, content, label)
+ if section.contract_violation:
+ print(
+ f"{label}: response was not valid JSON, retrying once...",
+ file=sys.stderr,
+ )
+ review_text, call_usage = call_api(
+ provider,
+ api_key,
+ model,
+ max_tokens,
+ system_prompt,
+ agents_content,
+ content,
+ name,
+ RETRY_NOTE,
+ verbose,
+ timeout,
+ )
+ usage.add(call_usage)
+ section = process_review_text(review_text, content, label)
+ return section
+
+
def get_last_message_id(patch_content: str) -> str | None:
"""Extract Message-ID from the last patch in an mbox."""
msg_ids = re.findall(
@@ -772,10 +1025,23 @@ def main() -> None:
Use --show-tokens (or -v/--verbose) to print a token usage summary
on stderr after the run. Off by default.
+Review Integrity:
+ The model must return structured JSON findings; text/markdown/html are
+ rendered locally from the verified findings. Each error/warning must
+ quote an exact line from the patch as evidence; findings whose evidence
+ is not present in the patch (e.g. review of a stale revision) are
+ reported as unverified and do not affect the error count. Every output
+ carries the `git patch-id --stable` of the reviewed content and a final
+ "X-AI-Review: ..." verdict line for CI to parse.
+
+Testing:
+ %(prog)s --replay saved-response.txt patch.patch # no API call
+
Exit Codes:
0 - Clean review (no errors or warnings)
1 - Operational failure (missing API key, file not found, etc.)
- 2 - Review found warnings (no errors)
+ 2 - Review found warnings, unverified findings, or the model violated
+ the output contract (no errors)
3 - Review found errors
""",
)
@@ -840,6 +1106,12 @@ def main() -> None:
metavar="SECONDS",
help="API request timeout in seconds (default: 300)",
)
+ parser.add_argument(
+ "--replay",
+ metavar="FILE",
+ help="Testing aid: process a saved model response instead of "
+ "calling the API (reviews the patch file as a single unit)",
+ )
# Date and release options
parser.add_argument(
@@ -930,9 +1202,9 @@ def main() -> None:
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:
+ # Get API key (not needed when replaying a saved response)
+ api_key = os.environ.get(config["env_var"], "")
+ if not api_key and not args.replay:
error(f"{config['env_var']} environment variable not set")
# Validate files
@@ -1000,9 +1272,11 @@ def main() -> None:
file=sys.stderr,
)
+ # Reviewed sections accumulate here in all modes
+ sections: list[ReviewSection] = []
+
# Handle --split-patches mode
- review_text = ""
- if args.split_patches:
+ if args.split_patches and not args.replay:
patches = split_mbox_patches(patch_content)
total_patches = len(patches)
@@ -1034,37 +1308,32 @@ def main() -> None:
)
# Review each patch separately
- all_reviews = []
for i, patch in enumerate(patches, patch_start or 1):
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,
+ sections.append(
+ reviewed_section(
+ args.provider,
+ api_key,
+ model,
+ args.tokens,
+ system_prompt,
+ agents_content,
+ patch,
+ f"{patch_name} ({patch_label})",
+ patch_label,
+ args.verbose,
+ args.timeout,
+ total_usage,
+ )
)
- total_usage.add(call_usage)
- all_reviews.append((patch_label, review_text))
-
- # Combine reviews
- review_text = format_combined_reviews(
- all_reviews, args.output_format, patch_name
- )
# Skip the normal API call
estimated_tokens = 0 # Bypass size check since we've already processed
- # Check if content is too large
- is_large = estimated_tokens > max_input_tokens
+ # Check if content is too large (replay skips API size limits)
+ is_large = estimated_tokens > max_input_tokens and not args.replay
if is_large:
print(
@@ -1102,33 +1371,28 @@ def main() -> None:
patch_content, _ = truncate_content(patch_content, max_input_tokens)
elif args.large_file == "chunk":
print("Processing in chunks...", file=sys.stderr)
- all_reviews = []
for chunk, chunk_num, total_chunks in chunk_content(
patch_content, max_input_tokens
):
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,
+ sections.append(
+ reviewed_section(
+ args.provider,
+ api_key,
+ model,
+ args.tokens,
+ system_prompt,
+ agents_content,
+ chunk,
+ f"{patch_name} ({chunk_label})",
+ chunk_label,
+ args.verbose,
+ args.timeout,
+ total_usage,
+ )
)
- total_usage.add(call_usage)
- all_reviews.append((chunk_label, review_text))
-
- # Combine chunk reviews
- review_text = format_combined_reviews(
- all_reviews, args.output_format, patch_name
- )
# Skip the normal single API call below
estimated_tokens = 0
@@ -1160,37 +1424,45 @@ def main() -> None:
print(f"From: {from_addr}", file=sys.stderr)
print("===============", file=sys.stderr)
- # 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 the whole file (unless already processed via chunks/split)
+ if args.replay:
+ replay_path = Path(args.replay)
+ if not replay_path.exists():
+ error(f"Replay file not found: {args.replay}")
+ review_text = replay_path.read_text(encoding="utf-8", errors="replace")
+ sections = [process_review_text(review_text, patch_content, patch_name)]
+ elif not sections:
+ sections.append(
+ reviewed_section(
+ args.provider,
+ api_key,
+ model,
+ args.tokens,
+ system_prompt,
+ agents_content,
+ patch_content,
+ patch_name,
+ patch_name,
+ args.verbose,
+ args.timeout,
+ total_usage,
+ )
)
- total_usage.add(call_usage)
- if not review_text:
+ if not sections:
error(f"No response received from {args.provider}")
+ # Verify identity of the reviewed content and derive the verdict
+ patch_ids = get_patch_ids(patch_content)
+ counts = section_counts(sections)
+ trailer = summary_trailer(counts, patch_ids)
+ review_body = render_sections(sections, args.output_format)
+
# Format output based on requested format
provider_name = config["name"]
+ patch_id_label = ",".join(pid[:16] for pid in patch_ids) or "unknown"
if args.output_format == "json":
- # For JSON, try to parse and add metadata
- try:
- review_data = json.loads(review_text)
- except json.JSONDecodeError:
- # If AI didn't return valid JSON, wrap the text
- review_data = {"raw_review": review_text}
-
usage_data = {
"api_calls": total_usage.api_calls,
"input_tokens": total_usage.input_tokens,
@@ -1205,19 +1477,29 @@ def main() -> None:
output_data = {
"metadata": {
"patch_file": patch_name,
+ "patch_ids": patch_ids,
"provider": args.provider,
"provider_name": provider_name,
"model": model,
"review_date": review_date,
"target_release": args.release,
"is_lts": is_lts_release(args.release) if args.release else False,
+ "counts": counts,
"token_usage": usage_data,
},
- "review": review_data,
+ "sections": [
+ {
+ "label": sec.label,
+ "contract_violation": sec.contract_violation,
+ "review": sec.data,
+ "unverified_findings": sec.unverified,
+ **({"raw_response": sec.raw} if sec.raw else {}),
+ }
+ for sec in sections
+ ],
}
output_text = json.dumps(output_data, indent=2)
elif args.output_format == "html":
- # Wrap HTML content with header
release_info = ""
if args.release:
lts_badge = " (LTS)" if is_lts_release(args.release) else ""
@@ -1226,8 +1508,9 @@ def main() -> None:
<!-- Reviewed using {provider_name} ({model}) on {review_date} -->
<div class="patch-review">
<h1>Patch Review: {patch_name}</h1>
-<p class="review-meta">Reviewed by {provider_name} ({model}) on {review_date}{release_info}</p>
-{review_text}
+<p class="review-meta">Reviewed by {provider_name} ({model}) on {review_date}{release_info}<br>Patch-Id: {patch_id_label}</p>
+{review_body}
+<p class="review-verdict">{escape(trailer)}</p>
</div>
"""
elif args.output_format == "markdown":
@@ -1238,8 +1521,11 @@ def main() -> None:
output_text = f"""# Patch Review: {patch_name}
*Reviewed by {provider_name} ({model}) on {review_date}*
+*Patch-Id: {patch_id_label}*
{release_info}
-{review_text}
+{review_body}
+
+{trailer}
"""
else: # text
release_info = ""
@@ -1248,8 +1534,10 @@ def main() -> None:
release_info = f"Target release: {args.release}{lts_badge}\n"
output_text = f"=== Patch Review: {patch_name} (via {provider_name}) ===\n"
output_text += f"Review date: {review_date}\n"
+ output_text += f"Patch-Id: {patch_id_label}\n"
output_text += release_info
- output_text += "\n" + review_text
+ output_text += "\n" + review_body
+ output_text += "\n\n" + trailer + "\n"
# Write output
if args.output:
@@ -1293,12 +1581,15 @@ def main() -> None:
email_body = f"""AI-generated review of {patch_name}
Reviewed using {provider_name} ({model}) on {review_date}
+Patch-Id: {patch_id_label}
{release_info}
This is an automated review. Please verify all suggestions.
---
-{review_text}
+{render_sections(sections, "text")}
+
+{trailer}
"""
if args.verbose:
@@ -1322,8 +1613,8 @@ def main() -> None:
print("", file=sys.stderr)
print(f"Review sent to: {', '.join(args.to_addrs)}", file=sys.stderr)
- # Exit with code based on review severity
- sys.exit(classify_review(review_text, args.output_format))
+ # Exit with code based on verified review severity
+ sys.exit(classify_review(counts))
if __name__ == "__main__":
--
2.53.0
^ permalink raw reply related
* [PATCH] crypto/virtio: cookies are allocated from mempool
From: Radu Nicolau @ 2026-07-03 14:49 UTC (permalink / raw)
To: dev; +Cc: maxime.coquelin, Radu Nicolau, jianjay.zhou, Fan Zhang
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(-)
diff --git a/drivers/crypto/virtio/virtio_cryptodev.c b/drivers/crypto/virtio/virtio_cryptodev.c
index 6f079f15f6..7cf94dc57c 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,30 +169,14 @@ 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_mempool_free(vq->mpool);
rte_free(vq);
return -ENOMEM;
}
--
2.52.0
^ permalink raw reply related
* [PATCH] test/suites: clean up per-test directories
From: Marat Khalili @ 2026-07-03 14:42 UTC (permalink / raw)
Cc: dev, thomas, stephen
After commit 5ef166838394 ("test: add file-prefix for all fast-tests on
Linux") each test within a suite has its own --file-prefix filled with
the test name to enable parallel execution. It brought a couple of
drawbacks though:
* Solution only applied to one test suite.
* Each working directory occupied ~25MB, with hundreds of tests this
easily overfilled /run filesystems of small VMs. Currently executing
fast-tests suite consumes 2.8GB of RAM, and adding more tests created
problems for github CI.
* While it enabled parallel execution of tests within a single build
directory, --file-prefix argument could no longer be specified
externally (caused duplicate argument error). Specifying custom
--file-prefix was necessary when multiple builds ran in the same
environment, so the corresponding workflow became broken.
Move --file-prefix logic into a separate shell script, preserving
existing argument and cleaning the work directory after the test.
Apply it uniformly to all tests.
Signed-off-by: Marat Khalili <marat.khalili@huawei.com>
---
app/test/suites/meson.build | 34 ++++----
app/test/suites/run_test_with_temp_prefix.sh | 82 ++++++++++++++++++++
2 files changed, 102 insertions(+), 14 deletions(-)
create mode 100755 app/test/suites/run_test_with_temp_prefix.sh
diff --git a/app/test/suites/meson.build b/app/test/suites/meson.build
index 786c459c24b3..3d75a8a66924 100644
--- a/app/test/suites/meson.build
+++ b/app/test/suites/meson.build
@@ -23,6 +23,16 @@ if not has_hugepage
endif
endif
+if is_linux
+ # use wrapper providing per-test work directories to allow parallel runs
+ test_args_prefix = [dpdk_test]
+ dpdk_test = find_program('run_test_with_temp_prefix.sh')
+ dpdk_test_as_arg = dpdk_test.full_path()
+else
+ test_args_prefix = []
+ dpdk_test_as_arg = dpdk_test
+endif
+
# process source files to determine the different unit test suites
# - fast_tests
# - perf_tests
@@ -42,6 +52,7 @@ foreach suite:test_suites
warning('Test "@0@" is not defined in any test suite'.format(t))
endif
test(t, dpdk_test,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds,
is_parallel: false)
@@ -49,6 +60,7 @@ foreach suite:test_suites
elif suite_name == 'stress-tests'
foreach t: suite_tests
test(t, dpdk_test,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds_stress,
is_parallel: false,
@@ -58,6 +70,7 @@ foreach suite:test_suites
# simple cases - tests without parameters or special handling
foreach t: suite_tests
test(t, dpdk_test,
+ args: test_args_prefix,
env: ['DPDK_TEST=' + t],
timeout: timeout_seconds,
is_parallel: false,
@@ -81,7 +94,7 @@ foreach suite:test_suites
nohuge = params[1] == 'NOHUGE_OK'
asan = params[2] == 'ASAN_OK'
- test_args = []
+ test_args = test_args_prefix
if nohuge
test_args += test_no_huge_args
elif not has_hugepage
@@ -94,25 +107,17 @@ foreach suite:test_suites
test_args += ['-d', dpdk_drivers_build_dir]
endif
- # use unique file-prefix to allow parallel runs
- file_prefix = []
- trace_prefix = []
- if is_linux
- file_prefix = ['--file-prefix=' + test_name.underscorify()]
- trace_prefix = [file_prefix[0] + '_with_traces']
- endif
-
test(test_name, dpdk_test,
- args : test_args + file_prefix,
+ args: test_args,
env: ['DPDK_TEST=' + test_name],
timeout : timeout_seconds_fast,
is_parallel : false,
suite : 'fast-tests')
if not is_windows and test_name == 'trace_autotest'
- trace_extra = ['--trace=.*',
- '--trace-dir=@0@'.format(meson.current_build_dir())]
+ test_args += ['--trace=.*']
+ test_args += ['--trace-dir=@0@'.format(meson.current_build_dir())]
test(test_name + '_with_traces', dpdk_test,
- args : test_args + trace_extra + trace_prefix,
+ args: test_args,
env: ['DPDK_TEST=' + test_name],
timeout : timeout_seconds_fast,
is_parallel : false,
@@ -124,7 +129,7 @@ endforeach
# standalone test for telemetry
if not is_windows and dpdk_conf.has('RTE_LIB_TELEMETRY')
- test_args = [dpdk_test]
+ test_args = [dpdk_test_as_arg] + test_args_prefix
test_args += test_no_huge_args
if get_option('default_library') == 'shared'
test_args += ['-d', dpdk_drivers_build_dir]
@@ -166,6 +171,7 @@ dump_test_names = [
]
foreach arg : dump_test_names
test(arg, dpdk_test,
+ args : test_args,
env : ['DPDK_TEST=' + arg],
timeout : timeout_seconds_fast,
is_parallel : false,
diff --git a/app/test/suites/run_test_with_temp_prefix.sh b/app/test/suites/run_test_with_temp_prefix.sh
new file mode 100755
index 000000000000..0c466ec25ea1
--- /dev/null
+++ b/app/test/suites/run_test_with_temp_prefix.sh
@@ -0,0 +1,82 @@
+#!/bin/bash
+# Run dpdk test with a temporary prefix and clean it up afterwards
+set -eEuo pipefail
+
+# Get value of the long option with name in the first argument, from the rest.
+get_arg_value() {
+ local arg_name="$1"
+ shift 1
+
+ local arg_value=''
+ local grab_next=false
+ local arg
+
+ for arg in "$@"; do
+ if $grab_next; then
+ arg_value="$arg"
+ grab_next=false
+ elif [[ "$arg" == "${arg_name}="* ]]; then
+ arg_value="${arg#*=}"
+ elif [[ "$arg" == "$arg_name" ]]; then
+ grab_next=true
+ fi
+ done
+
+ printf "%s\n" "$arg_value"
+}
+
+# Delete work directory passing through the return code
+delete_work_directory() {
+ local -r test_rc="$1"
+ [[ -z "${WORK_DIRECTORY:-}" ]] || rm -rf "$WORK_DIRECTORY"
+ return "$test_rc"
+}
+
+main() {
+ if [[ $# -eq 0 ]]; then
+ printf "Usage: %s <test_command> [arguments...]\n" "$0" >&2
+ printf "%s %s\n" \
+ "Runs a DPDK test with a temporary file prefix" \
+ "and cleans up the working directory." >&2
+ printf "Ensure the DPDK_TEST environment variable is set.\n" >&2
+ exit 1
+ fi
+ local test_command=("$@")
+
+ if [[ -z "${DPDK_TEST:-}" ]]; then
+ printf "Ensure the DPDK_TEST environment variable is set.\n" >&2
+ exit 1
+ fi
+ local -r dpdk_test="$DPDK_TEST"
+
+ # Make sure file prefix is determined and set in test args
+ local file_prefix="$(get_arg_value --file-prefix "${test_command[@]}")"
+ if [[ -z "$file_prefix" ]]; then
+ # If not yet specified, set --file-prefix to test name
+ file_prefix="$dpdk_test"
+ if [[ -n "$(get_arg_value --trace "${test_command[@]}")" ]]; then
+ # Some tests runs twice, with and without traces
+ file_prefix="${file_prefix}_with_traces"
+ fi
+ test_command+=("--file-prefix=$file_prefix")
+ fi
+
+ # Make sure runtime directory is determined and set in the environment
+ local runtime_directory="${RUNTIME_DIRECTORY:-}"
+ if [[ -z "$runtime_directory" ]]; then
+ # Follow the algorithm in eal_filesystem.c
+ if [[ "$EUID" -eq 0 ]]; then
+ runtime_directory='/var/run'
+ else
+ runtime_directory="${XDG_RUNTIME_DIR:-/tmp}"
+ fi
+ export RUNTIME_DIRECTORY="$runtime_directory"
+ fi
+
+ WORK_DIRECTORY="$runtime_directory/dpdk/$file_prefix"
+ trap 'delete_work_directory "$?"' EXIT
+
+ "${test_command[@]}"
+}
+
+main "$@"
--
2.43.0
^ permalink raw reply related
* Re: [v4] net/cksum: compute raw cksum for several segments
From: Stephen Hemminger @ 2026-07-03 14:41 UTC (permalink / raw)
To: Su Sai; +Cc: marat.khalili, dev
In-Reply-To: <20260618063242.99189-1-spiderdetective.ss@gmail.com>
On Thu, 18 Jun 2026 14:32:42 +0800
Su Sai <spiderdetective.ss@gmail.com> wrote:
> The rte_raw_cksum_mbuf function is used to compute
> the raw checksum of a packet.
> If the packet payload stored in multi mbuf, the function
> will goto the hard case. In hard case,
> the variable 'tmp' is a type of uint32_t,
> so rte_bswap16 will drop high 16 bit.
> Meanwhile, the variable 'sum' is a type of uint32_t,
> so 'sum += tmp' will drop the carry when overflow.
> Both drop will make cksum incorrect.
> This commit fixes the above bug.
>
> Signed-off-by: Su Sai <spiderdetective.ss@gmail.com>
> Acked-by: Marat Khalili <marat.khalili@huawei.com>
> ---
Still one error in the test.
Error — double-free in test_l4_cksum_multi_mbufs() (app/test/test_cksum.c)
The segments are chained into one packet (rte_pktmbuf_chain sets
m[0]->next = m[1], m[1]->next = m[2]), but cleanup then calls
rte_pktmbuf_free_bulk(m, segs_len) over the array.
rte_pktmbuf_free_bulk walks the ->next chain of each array element, so
index 0 frees the entire chain (m[0], m[1], m[2]), and indices 1 and 2
then free m[1]/m[2] a second time. This fires on the normal success
path every run, and on the fail path whenever ≥2 segments were chained
— a debug/ASAN build will abort on the sanity check, and a normal build
corrupts pool accounting. The array-of-independent-mbufs contract that
free_bulk expects doesn't hold here because the elements alias one
chain. Free the chain once through its head:
^ permalink raw reply
* Re: [PATCH] net/crc: add 4x folding loop for aarch64 NEON implementation
From: Stephen Hemminger @ 2026-07-03 14:31 UTC (permalink / raw)
To: Shreesh Adiga; +Cc: Wathsala Vithanage, dev
In-Reply-To: <20260616091158.731075-1-16567adigashreesh@gmail.com>
On Tue, 16 Jun 2026 14:41:58 +0530
Shreesh Adiga <16567adigashreesh@gmail.com> wrote:
> 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.
>
> Signed-off-by: Shreesh Adiga <16567adigashreesh@gmail.com>
> ---
Detailed AI review (not the CI one), spotted a correctness issue.
On the 4x folding loop patch:
The 4x-fold port matches the x86 SSE implementation and the constant
tables are correct, but the patch introduces one correctness bug.
Error: lib/net/net_crc_neon.c, "17 to 31 bytes" path uses the wrong
fold constant.
This patch repurposes rk1_rk2 as the fold-by-4 (512-bit) constant and
moves the fold-by-1 (128-bit) constant into the new rk3_rk4, matching
the SSE layout. The main paths were updated to select rk3_rk4 before
falling into partial_bytes, and the partial_bytes comment was correctly
updated to "k = rk3 & rk4".
The 17-to-31 byte branch was missed. It still does:
/* 17 to 31 bytes */
fold = vld1q_u64((const uint64_t *)data);
fold = veorq_u64(fold, temp);
n = 16;
k = params->rk1_rk2; /* now the fold-by-4 constant */
goto partial_bytes;
partial_bytes performs a single 128-bit fold and needs the fold-by-1
constant, but this path now feeds it rk1_rk2, which after the change
holds the 512-bit constant. CRC results are therefore wrong for every
input of length 17-31 bytes. This affects both crc32_eth and
crc16_ccitt, since they share this routine.
The fix is one line -- use rk3_rk4 here as the other paths do:
k = params->rk3_rk4;
Verification: I cross-compiled the routine (armv8-a+crypto) and ran it
under qemu against a scalar reflected CRC-32 reference for lengths
1-256. As submitted it mismatches at exactly lengths 17-31 (15
lengths); with the one-line change above, all lengths pass. The >=64,
>=32, ==16, and <16 paths are already correct.
One suggestion for v2, not required: the SSE version does not carry a
separate 17-31 branch at all -- it handles everything below 64 through
the single_fold_loop plus partial_bytes with rk3_rk4. Collapsing the
NEON path the same way would remove this class of bug rather than just
this instance.
^ permalink raw reply
* Re: [PATCH v2 0/9] Stability fixes for GVE
From: Stephen Hemminger @ 2026-07-03 14:27 UTC (permalink / raw)
To: Joshua Washington; +Cc: dev
In-Reply-To: <20260703131308.2507403-1-joshwash@google.com>
On Fri, 3 Jul 2026 06:12:57 -0700
Joshua Washington <joshwash@google.com> wrote:
> This patch series consists of mostly unrelated fixes in the GVE driver.
>
> Joshua Washington (9):
> net/gve: clear out shared memory region for stats report
> net/gve: delay adding mbuf head to software ring
> net/gve: copy data to QPL buffer when mbuf read does not
> net/gve: validate buf ID before processing Rx packet
> net/gve: set mbuf to null in software ring after use
> net/gve: free ctx mbuf if packet dropped after first segment
> net/gve: increase range of DMA memzone ids to 64 bits
> net/gve: don't reset ring size bounds to default on reset
> net/gve: restrict max ring size in GQ QPL to 2K
>
> drivers/net/gve/base/gve_adminq.c | 12 ++++++--
> drivers/net/gve/base/gve_osdep.h | 4 +--
> drivers/net/gve/gve_ethdev.c | 8 ++++--
> drivers/net/gve/gve_ethdev.h | 1 +
> drivers/net/gve/gve_rx.c | 3 ++
> drivers/net/gve/gve_rx_dqo.c | 6 ++++
> drivers/net/gve/gve_tx.c | 46 ++++++++++++++++++-------------
> 7 files changed, 53 insertions(+), 27 deletions(-)
>
> ---
> v2:
> * Remove unused definition
>
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
>
Better but fails on 32 bit build during post-merge testing
DPDK 26.07.0-rc2
User defined options
Cross files : /home/shemminger/DPDK/next-net/devtools/../config/x86/cross-32bit-debian.ini
buildtype : debugoptimized
default_library : shared
enable_deprecated_libs: *
examples : all
werror : true
Found ninja-1.13.2 at /usr/bin/ninja
ninja: Entering directory `./build-32b'
[937/2812] Compiling C object drivers/libtmp_rte_net_gve.a.p/net_gve_gve_tx.c.o
FAILED: [code=1] drivers/libtmp_rte_net_gve.a.p/net_gve_gve_tx.c.o
cc -Idrivers/libtmp_rte_net_gve.a.p -Idrivers -I../drivers -Idrivers/net/gve -I../drivers/net/gve -I../drivers/net/gve/base -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 -O2 -g -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 -Wno-pointer-to-int-cast -D_GNU_SOURCE -m32 -fPIC -march=native -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -Wno-address-of-packed-member -Wno-vla -DRTE_COMPONENT_CLASS=pmd_net -DRTE_COMPONENT_NAME=gve -DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.gve -MD -MQ drivers/libtmp_rte_net_gve.a.p/net_gve_gve_tx.c.o -MF drivers/libtmp_rte_net_gve.a.p/net_gve_gve_tx.c.o.d -o drivers/libtmp_rte_net_gve.a.p/net_gve_gve_tx.c.o -c ../drivers/net/gve/gve_tx.c
../drivers/net/gve/gve_tx.c: In function ‘gve_tx_burst_qpl’:
../drivers/net/gve/gve_tx.c:338:42: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
338 | qpl_write_addr = (void *)(txq->fifo_base + fifo_addr);
| ^
cc1: all warnings being treated as errors
^ permalink raw reply
* [PATCH v8] graph: add optional profiling stats
From: Morten Brørup @ 2026-07-03 14:22 UTC (permalink / raw)
To: dev, Jerin Jacob, Kiran Kumar K, Nithin Dabilpuram, Zhirun Yan,
Saeed Bishara
Cc: Morten Brørup
In-Reply-To: <20260619202047.2809165-1-mb@smartsharesystems.com>
Added graph node profiling stats, build time configurable by enabling
RTE_GRAPH_PROFILE in rte_config.h.
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v8:
* Added static const array as local variable, instead of indexing directly
into const array. (AI)
This also eliminates the space required between "} [idx];" weirdness.
* Added build time configurable RTE_GRAPH_PROFILE_BURST_SIZE to replace
the hardcoded burst size of 32. (AI)
v7:
* Use RTE_DIM() in histogram for loop.
* Added static_assert for histogram index values.
* Minor details to please checkpatch.
Although I disagree with requiring a space when indexing into
a constant array "(const type []){values} [idx];",
I have changed the code to comply.
v6:
* Consolidate the four histogram entries into one array. (Saeed Bishara)
* Sample at 32 objs instead of a half burst. (Saeed Bishara)
* Moved stats to different location in rte_node structure. (Jerin)
* Minor details to please checkpatch.
v5:
* Added stats for a half burst and a full burst.
v4:
* Added documentation. (AI)
* Added more comments. (AI)
* Improved dump. (AI)
* Debug shows both cycles/call and cycles/obj.
v3:
* Debug shows cycles/obj instead of cycles/call.
* Fixed missing --in-reply-to.
v2:
* Fixed indentation.
---
config/rte_config.h | 2 ++
doc/guides/prog_guide/graph_lib.rst | 3 ++
lib/graph/graph_debug.c | 51 +++++++++++++++++++++++++++++
lib/graph/node.c | 2 ++
lib/graph/rte_graph_worker_common.h | 29 ++++++++++++++--
5 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/config/rte_config.h b/config/rte_config.h
index 0447cdf2ad..7703a6325b 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -106,6 +106,8 @@
/* rte_graph defines */
#define RTE_GRAPH_BURST_SIZE 256
#define RTE_LIBRTE_GRAPH_STATS 1
+/* RTE_GRAPH_PROFILE is not set */
+#define RTE_GRAPH_PROFILE_BURST_SIZE 32
/****** driver defines ********/
diff --git a/doc/guides/prog_guide/graph_lib.rst b/doc/guides/prog_guide/graph_lib.rst
index 8dd49c19d2..4311c37cc2 100644
--- a/doc/guides/prog_guide/graph_lib.rst
+++ b/doc/guides/prog_guide/graph_lib.rst
@@ -49,6 +49,9 @@ Performance tuning parameters
RTE_GRAPH_BURST_SIZE config option.
The testing shows, on x86 and arm64 servers, The sweet spot is 256 burst
size. While on arm64 embedded SoCs, it is either 64 or 128.
+- Enable the ``RTE_GRAPH_PROFILE`` config option for more profiling details.
+ Set the ``RTE_GRAPH_PROFILE_BURST_SIZE`` config option to sample a specific
+ burst size.
- Disable node statistics (using ``RTE_LIBRTE_GRAPH_STATS`` config option)
if not needed.
diff --git a/lib/graph/graph_debug.c b/lib/graph/graph_debug.c
index e3b8cccdc1..32293dfb6d 100644
--- a/lib/graph/graph_debug.c
+++ b/lib/graph/graph_debug.c
@@ -93,6 +93,57 @@ rte_graph_obj_dump(FILE *f, struct rte_graph *g, bool all)
n->dispatch.total_sched_fail);
}
fprintf(f, " total_calls=%" PRId64 "\n", n->total_calls);
+ if (rte_graph_has_stats_feature())
+ fprintf(f, " total_cycles=%" PRIu64 ", avg cycles/call=%.1f\n",
+ n->total_cycles,
+ n->total_calls == 0 ? 0.0 :
+ (double)n->total_cycles / (double)n->total_calls);
+#ifdef RTE_GRAPH_PROFILE
+ int64_t calls_other = n->total_calls;
+ int64_t cycles_other = n->total_cycles;
+ int64_t objs_other = n->total_objs;
+ for (int idx = 0; idx < RTE_DIM(n->usage_stats) + 1; idx++) {
+ uint64_t calls;
+ uint64_t cycles;
+ double objs_per_call;
+ if (idx < RTE_DIM(n->usage_stats)) {
+ static const uint16_t profile_sample_sizes[] = {
+ 0, 1, RTE_GRAPH_PROFILE_BURST_SIZE,
+ RTE_GRAPH_BURST_SIZE};
+ static_assert(RTE_DIM(profile_sample_sizes) ==
+ RTE_DIM(n->usage_stats));
+ uint16_t idx_objs = profile_sample_sizes[idx];
+ fprintf(f, " objs[%u]\n", idx_objs);
+ calls = n->usage_stats[idx].calls;
+ cycles = n->usage_stats[idx].cycles;
+ objs_per_call = (double)idx_objs;
+ calls_other -= calls;
+ cycles_other -= cycles;
+ objs_other -= idx_objs * calls;
+ } else {
+ fprintf(f, " objs[other]\n");
+ if (calls_other > 0 && cycles_other > 0 && objs_other > 0) {
+ calls = calls_other;
+ cycles = cycles_other;
+ objs_per_call = (double)objs_other / (double)calls_other;
+ fprintf(f, " avg objs/call=%.1f\n", objs_per_call);
+ } else {
+ calls = 0;
+ cycles = 0;
+ objs_per_call = 0.0;
+ }
+ }
+ fprintf(f, " calls=%" PRIu64, calls);
+ if (calls != 0)
+ fprintf(f, ", cycles=%" PRIu64 ", avg cycles/call=%.1f",
+ cycles,
+ (double)cycles / (double)calls);
+ if (calls != 0 && objs_per_call != 0.0)
+ fprintf(f, ", avg cycles/obj=%.1f",
+ (double)cycles / (double)calls / objs_per_call);
+ fprintf(f, "\n");
+ }
+#endif
for (i = 0; i < n->nb_edges; i++)
fprintf(f, " edge[%d] <%s>\n", i,
n->nodes[i]->name);
diff --git a/lib/graph/node.c b/lib/graph/node.c
index 1fce3e6632..19b38881ae 100644
--- a/lib/graph/node.c
+++ b/lib/graph/node.c
@@ -110,10 +110,12 @@ __rte_node_register(const struct rte_node_register *reg)
rte_edge_t i;
size_t sz;
+#ifndef RTE_GRAPH_PROFILE
/* Limit Node specific metadata to one cacheline on 64B CL machine */
RTE_BUILD_BUG_ON((offsetof(struct rte_node, nodes) -
offsetof(struct rte_node, ctx)) !=
RTE_CACHE_LINE_MIN_SIZE);
+#endif
graph_spinlock_lock();
diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_worker_common.h
index 4ab53a533e..c40b63111f 100644
--- a/lib/graph/rte_graph_worker_common.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -121,6 +121,14 @@ struct __rte_cache_aligned rte_node {
rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
/** Fast path area cache line 2. */
+#ifdef RTE_GRAPH_PROFILE
+ /** Usage when this node processed 0, 1, 32 or a full burst of objects. */
+ struct __rte_cache_aligned {
+ uint64_t calls; /**< Calls done. */
+ uint64_t cycles; /**< Cycles spent. */
+ } usage_stats[4];
+ /** Fast path area cache line 3. */
+#endif
__extension__ struct __rte_cache_aligned {
#define RTE_NODE_CTX_SZ 16
union {
@@ -148,8 +156,10 @@ struct __rte_cache_aligned rte_node {
};
};
+#ifndef RTE_GRAPH_PROFILE
static_assert(offsetof(struct rte_node, nodes) - offsetof(struct rte_node, ctx)
== RTE_CACHE_LINE_MIN_SIZE, "rte_node fast path area must fit in 64 bytes");
+#endif
/**
* @internal
@@ -197,7 +207,7 @@ void __rte_node_stream_alloc_size(struct rte_graph *graph,
static __rte_always_inline void
__rte_node_process(struct rte_graph *graph, struct rte_node *node)
{
- uint64_t start;
+ uint64_t cycles;
uint16_t rc;
void **objs;
@@ -206,11 +216,24 @@ __rte_node_process(struct rte_graph *graph, struct rte_node *node)
rte_prefetch0(objs);
if (rte_graph_has_stats_feature()) {
- start = rte_rdtsc();
+ cycles = -rte_rdtsc();
rc = node->process(graph, node, objs, node->idx);
- node->total_cycles += rte_rdtsc() - start;
+ cycles += rte_rdtsc();
+ node->total_cycles += cycles;
node->total_calls++;
node->total_objs += rc;
+#ifdef RTE_GRAPH_PROFILE
+ if (rc <= 1) {
+ node->usage_stats[rc].calls++;
+ node->usage_stats[rc].cycles += cycles;
+ } else if (rc == RTE_GRAPH_PROFILE_BURST_SIZE) {
+ node->usage_stats[2].calls++;
+ node->usage_stats[2].cycles += cycles;
+ } else if (rc == RTE_GRAPH_BURST_SIZE) {
+ node->usage_stats[3].calls++;
+ node->usage_stats[3].cycles += cycles;
+ }
+#endif
} else {
node->process(graph, node, objs, node->idx);
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] test/bpf: remove validation tests from CI
From: Stephen Hemminger @ 2026-07-03 14:12 UTC (permalink / raw)
To: Marat Khalili; +Cc: Thomas Monjalon, dev@dpdk.org, Konstantin Ananyev
In-Reply-To: <2751fd1fa6d648a9b7f95797f13ef28b@huawei.com>
On Fri, 3 Jul 2026 08:48:33 +0000
Marat Khalili <marat.khalili@huawei.com> wrote:
> > The new tests for BPF validation are triggering a strange issue
> > in GitHub Action: the last tests are killed by signal 7 SIGBUS.
> >
> > The series fixing a lot of BPF issues was merged
> > but the related test has to be removed from the fast tests suite
> > which runs in some CI jobs.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>
> I think I know the issue, it's tmpfs exhaustion. After some recent changes each
> test consumes 25MB in the --file-prefix directory independently, easily driving
> small VMs out of space. My ~30 new tests were the last drop for the CI.
>
> I will try to prepare a fix today, of course please do everything that's
> needed for the CI stability meanwhile.
The BPF tests should be using unit_test_suite_runner instead of separate tests.
That would help.
^ permalink raw reply
* [PATCH v7] graph: add optional profiling stats
From: Morten Brørup @ 2026-07-03 13:53 UTC (permalink / raw)
To: dev, Jerin Jacob, Kiran Kumar K, Nithin Dabilpuram, Zhirun Yan,
Saeed Bishara
Cc: Morten Brørup
In-Reply-To: <20260619202047.2809165-1-mb@smartsharesystems.com>
graph: add optional profiling stats
Added graph node profiling stats, build time configurable by enabling
RTE_GRAPH_PROFILE in rte_config.h.
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
---
v7:
* Use RTE_DIM() in histogram for loop.
* Added static_assert for histogram index values.
* Minor details to please checkpatch.
Although I disagree with requiring a space when indexing into
a constant array "(const type []){values} [idx];",
I have changed the code to comply.
v6:
* Consolidate the four histogram entries into one array. (Saeed Bishara)
* Sample at 32 objs instead of a half burst. (Saeed Bishara)
* Moved stats to different location in rte_node structure. (Jerin)
* Minor details to please checkpatch.
v5:
* Added stats for a half burst and a full burst.
v4:
* Added documentation. (AI)
* Added more comments. (AI)
* Improved dump. (AI)
* Debug shows both cycles/call and cycles/obj.
v3:
* Debug shows cycles/obj instead of cycles/call.
* Fixed missing --in-reply-to.
v2:
* Fixed indentation.
---
config/rte_config.h | 1 +
doc/guides/prog_guide/graph_lib.rst | 1 +
lib/graph/graph_debug.c | 48 +++++++++++++++++++++++++++++
lib/graph/node.c | 2 ++
lib/graph/rte_graph_worker_common.h | 29 +++++++++++++++--
5 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/config/rte_config.h b/config/rte_config.h
index 0447cdf2ad..1942c1b1ec 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -106,6 +106,7 @@
/* rte_graph defines */
#define RTE_GRAPH_BURST_SIZE 256
#define RTE_LIBRTE_GRAPH_STATS 1
+/* RTE_GRAPH_PROFILE is not set */
/****** driver defines ********/
diff --git a/doc/guides/prog_guide/graph_lib.rst b/doc/guides/prog_guide/graph_lib.rst
index 8dd49c19d2..bc36042296 100644
--- a/doc/guides/prog_guide/graph_lib.rst
+++ b/doc/guides/prog_guide/graph_lib.rst
@@ -49,6 +49,7 @@ Performance tuning parameters
RTE_GRAPH_BURST_SIZE config option.
The testing shows, on x86 and arm64 servers, The sweet spot is 256 burst
size. While on arm64 embedded SoCs, it is either 64 or 128.
+- Enable the ``RTE_GRAPH_PROFILE`` config option for more profiling details.
- Disable node statistics (using ``RTE_LIBRTE_GRAPH_STATS`` config option)
if not needed.
diff --git a/lib/graph/graph_debug.c b/lib/graph/graph_debug.c
index e3b8cccdc1..427c6a6e77 100644
--- a/lib/graph/graph_debug.c
+++ b/lib/graph/graph_debug.c
@@ -93,6 +93,54 @@ rte_graph_obj_dump(FILE *f, struct rte_graph *g, bool all)
n->dispatch.total_sched_fail);
}
fprintf(f, " total_calls=%" PRId64 "\n", n->total_calls);
+ if (rte_graph_has_stats_feature())
+ fprintf(f, " total_cycles=%" PRIu64 ", avg cycles/call=%.1f\n",
+ n->total_cycles,
+ n->total_calls == 0 ? 0.0 :
+ (double)n->total_cycles / (double)n->total_calls);
+#ifdef RTE_GRAPH_PROFILE
+ int64_t calls_other = n->total_calls;
+ int64_t cycles_other = n->total_cycles;
+ int64_t objs_other = n->total_objs;
+ for (int idx = 0; idx < RTE_DIM(n->usage_stats) + 1; idx++) {
+ uint64_t calls;
+ uint64_t cycles;
+ double objs_per_call;
+ if (idx < RTE_DIM(n->usage_stats)) {
+ static_assert(RTE_DIM(n->usage_stats) == 4);
+ uint16_t idx_objs = (const uint16_t []){0, 1, 32,
+ RTE_GRAPH_BURST_SIZE} [idx];
+ fprintf(f, " objs[%u]\n", idx_objs);
+ calls = n->usage_stats[idx].calls;
+ cycles = n->usage_stats[idx].cycles;
+ objs_per_call = (double)idx_objs;
+ calls_other -= calls;
+ cycles_other -= cycles;
+ objs_other -= idx_objs * calls;
+ } else {
+ fprintf(f, " objs[other]\n");
+ if (calls_other > 0 && cycles_other > 0 && objs_other > 0) {
+ calls = calls_other;
+ cycles = cycles_other;
+ objs_per_call = (double)objs_other / (double)calls_other;
+ fprintf(f, " avg objs/call=%.1f\n", objs_per_call);
+ } else {
+ calls = 0;
+ cycles = 0;
+ objs_per_call = 0.0;
+ }
+ }
+ fprintf(f, " calls=%" PRIu64, calls);
+ if (calls != 0)
+ fprintf(f, ", cycles=%" PRIu64 ", avg cycles/call=%.1f",
+ cycles,
+ (double)cycles / (double)calls);
+ if (calls != 0 && objs_per_call != 0.0)
+ fprintf(f, ", avg cycles/obj=%.1f",
+ (double)cycles / (double)calls / objs_per_call);
+ fprintf(f, "\n");
+ }
+#endif
for (i = 0; i < n->nb_edges; i++)
fprintf(f, " edge[%d] <%s>\n", i,
n->nodes[i]->name);
diff --git a/lib/graph/node.c b/lib/graph/node.c
index 1fce3e6632..19b38881ae 100644
--- a/lib/graph/node.c
+++ b/lib/graph/node.c
@@ -110,10 +110,12 @@ __rte_node_register(const struct rte_node_register *reg)
rte_edge_t i;
size_t sz;
+#ifndef RTE_GRAPH_PROFILE
/* Limit Node specific metadata to one cacheline on 64B CL machine */
RTE_BUILD_BUG_ON((offsetof(struct rte_node, nodes) -
offsetof(struct rte_node, ctx)) !=
RTE_CACHE_LINE_MIN_SIZE);
+#endif
graph_spinlock_lock();
diff --git a/lib/graph/rte_graph_worker_common.h b/lib/graph/rte_graph_worker_common.h
index 4ab53a533e..bac0fc534e 100644
--- a/lib/graph/rte_graph_worker_common.h
+++ b/lib/graph/rte_graph_worker_common.h
@@ -121,6 +121,14 @@ struct __rte_cache_aligned rte_node {
rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
/** Fast path area cache line 2. */
+#ifdef RTE_GRAPH_PROFILE
+ /** Usage when this node processed 0, 1, 32 or a full burst of objects. */
+ struct __rte_cache_aligned {
+ uint64_t calls; /**< Calls done. */
+ uint64_t cycles; /**< Cycles spent. */
+ } usage_stats[4];
+ /** Fast path area cache line 3. */
+#endif
__extension__ struct __rte_cache_aligned {
#define RTE_NODE_CTX_SZ 16
union {
@@ -148,8 +156,10 @@ struct __rte_cache_aligned rte_node {
};
};
+#ifndef RTE_GRAPH_PROFILE
static_assert(offsetof(struct rte_node, nodes) - offsetof(struct rte_node, ctx)
== RTE_CACHE_LINE_MIN_SIZE, "rte_node fast path area must fit in 64 bytes");
+#endif
/**
* @internal
@@ -197,7 +207,7 @@ void __rte_node_stream_alloc_size(struct rte_graph *graph,
static __rte_always_inline void
__rte_node_process(struct rte_graph *graph, struct rte_node *node)
{
- uint64_t start;
+ uint64_t cycles;
uint16_t rc;
void **objs;
@@ -206,11 +216,24 @@ __rte_node_process(struct rte_graph *graph, struct rte_node *node)
rte_prefetch0(objs);
if (rte_graph_has_stats_feature()) {
- start = rte_rdtsc();
+ cycles = -rte_rdtsc();
rc = node->process(graph, node, objs, node->idx);
- node->total_cycles += rte_rdtsc() - start;
+ cycles += rte_rdtsc();
+ node->total_cycles += cycles;
node->total_calls++;
node->total_objs += rc;
+#ifdef RTE_GRAPH_PROFILE
+ if (rc <= 1) {
+ node->usage_stats[rc].calls++;
+ node->usage_stats[rc].cycles += cycles;
+ } else if (rc == 32) {
+ node->usage_stats[2].calls++;
+ node->usage_stats[2].cycles += cycles;
+ } else if (rc == RTE_GRAPH_BURST_SIZE) {
+ node->usage_stats[3].calls++;
+ node->usage_stats[3].cycles += cycles;
+ }
+#endif
} else {
node->process(graph, node, objs, node->idx);
}
--
2.43.0
^ 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