[PATCH v3 0/2] few improvemnts for SORING lib
Stephen Hemminger
stephen at networkplumber.org
Sat Apr 18 05:28:39 CEST 2026
On Fri, 17 Apr 2026 22:23:56 +0100
Konstantin Ananyev <konstantin.ananyev at huawei.com> wrote:
> v2 -> v3
> - fix MSVC complaints
>
> v1 -> v2
> - fix formal API comments (doxygen complaints)
> - add section to release notes
>
> First patch aims to improve enqueue/dequeue performance, specially
> for the cases with multiple stage workers lcores.
> Second one introduces 'Peek API' similar to what we have for
> conventional rte_ring. Also it adds new test-cases for this new API.
>
> Konstantin Ananyev (2):
> ring: make soring to finalize its own stage only
> ring: introduce peek API for soring
>
> app/test/meson.build | 1 +
> app/test/test_soring_mt_stress.c | 74 +++++++
> app/test/test_soring_peek_stress.c | 75 +++++++
> app/test/test_soring_stress.c | 3 +
> app/test/test_soring_stress.h | 1 +
> app/test/test_soring_stress_impl.h | 87 +-------
> doc/guides/rel_notes/release_26_07.rst | 8 +
> lib/ring/rte_soring.h | 264 ++++++++++++++++++++++
> lib/ring/soring.c | 289 ++++++++++++++++++++-----
> 9 files changed, 669 insertions(+), 133 deletions(-)
> create mode 100644 app/test/test_soring_peek_stress.c
>
Lots of little typos caught by AI.
Based on review of both patches against the upstream DPDK tree, here is a summary of findings suitable for posting to the list. I recommend you edit/trim before sending since this is a draft.
Subject: Re: [PATCH v3 0/2] few improvemnts for SORING lib
General: "improvemnts" in the cover letter subject is a typo (improvements).
Same type of typo recurs in the patches themselves (see below); worth
a quick pass on the prose.
[PATCH v3 1/2] ring: make soring to finalize its own stage only
Warning:
The file-level comment block at the top of lib/ring/soring.c is only
partially updated. The hunk rewrites the paragraph starting with
"Note that 'finalize()' for given stage...", but leaves the earlier
paragraph describing release() behavior intact:
"After that, it checks does old head value equals to current tail
value? If yes, then it performs 'finalize()' operation, otherwise
'release()' just returns (without spinning on stage tail value).
As updated state[] is shared by all threads, some other thread can
do 'finalize()' for given stage.
That allows 'release()' to avoid excessive waits on the tail value."
After this patch, release() unconditionally calls
__rte_soring_stage_finalize() (no tail==pos check), and acquire()/
dequeue() no longer call finalize() at all. The stale paragraph should
be updated to match, otherwise readers will be misled about both the
release() fast path and the "shared state[] lets another thread
finalize" claim.
Info:
"Accorging" in the commit message body -> "According".
[PATCH v3 2/2] ring: introduce peek API for soring
Warnings:
Doxygen description/signature mismatch. Several of the new doxygen
blocks are copy-paste from the "enqueux"/"dequeux" variants and
describe a metadata parameter that the declared function does not
take:
- rte_soring_enqueue_finish: described as "Complete to enqueue
several objects plus metadata" but takes no meta argument.
- rte_soring_dequeue_bulk_start: "Start to dequeue several objects
plus metadata" - takes no meta.
- rte_soring_dequeue_burst_start: same.
Also, the @param objs blocks for the dequeue_bulk_start /
dequeue_burst_start / dequeux_bulk_start / dequeux_burst_start
variants say "Size of objects to enqueue" - that should be "to
dequeue".
Missing experimental warning in doxygen for rte_soring_dequeue_finish.
Every other new function in this patch carries
@warning
@b EXPERIMENTAL: this API may change without prior notice.
rte_soring_dequeue_finish is declared __rte_experimental but its
doxygen block omits this block. Please add for consistency.
Missing RTE_ASSERT in soring_dequeue_finish(). soring_enqueue_start(),
soring_enqueue_finish() and soring_dequeue_start() all begin with
RTE_ASSERT(r != NULL && r->nb_stage > 0);
soring_dequeue_finish() does not. Inconsistent with the rest of the
datapath; please add.
Info:
Doxygen typos:
- "Start to enqueue excact number of objects" -> "exact"
(rte_soring_enqueue_bulk_start description).
- "please refer to corresping rte_ring peek API" -> "corresponding"
(SORING Peek API header comment at the top of the new block).
Double space in new helper signature:
__dequeue_elems(const struct rte_soring *r, void *objs, void *meta,
^^
__enqueue_elems directly above it does not have this. Minor.
Stray trailing blank line inside the initializer in
app/test/test_soring_peek_stress.c ("static const struct test_case
tests[] = {\n\n\t{"). Cosmetic.
SPDX header in the new file test_soring_peek_stress.c reads
"Copyright(c) 2024 Huawei" but the file is being introduced in 2026.
(Copyright formatting is normally left to checkpatch, flagging for
awareness only.)
Nothing in either patch rose to the level of a correctness bug in my
review - no leaks, no UAF, no missing error paths, no races beyond what
the existing soring already accepts. The dependence on patch 1/2 for the
refactor into __enqueue_elems/__dequeue_elems helpers in patch 2/2 is
fine given normal series-apply order.
More information about the dev
mailing list