Re: [PATCH v4 0/2] few improvemnts for SORING lib

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 23 Apr 2026 10:16:23 +0100
Konstantin Ananyev <konstantin.ananyev@huawei.com> wrote:

> v3 -> v4
> - Remove too aggressive optimization (patch #1)
> - Fix AI review comments
> 
> 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 workers lcores per stage.
> 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 always finalize its own stage
>   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                  | 267 ++++++++++++++++++++++++
>  lib/ring/soring.c                      | 272 ++++++++++++++++++++++---
>  9 files changed, 680 insertions(+), 108 deletions(-)
>  create mode 100644 app/test/test_soring_peek_stress.c
> 

I don't use soring, but looks good to me.

One related observation, is that would be good if soring tests used unit_test_runner
instead of having its own sub test call chain open coded.


AI had some feedback.

Series looks good overall. One issue on patch 2/2:
The doc comments for rte_soring_enqueue_bulk_start() and rte_soring_enqueue_burst_start() in lib/ring/rte_soring.h say:
User has to call appropriate enqueue_elem_finish() to copy objects
into the queue and complete given enqueue operation.
There is no rte_soring_enqueue_elem_finish(). The text was copy-pasted from rte_ring_peek.h, which uses an elem naming convention that soring does not follow. soring's actual finish functions are rte_soring_enqueue_finish() (objects only) and rte_soring_enqueux_finish() (objects plus meta). Please update the references.
The dequeue start docs are fine - they reference dequeue_finish(), which does exist.
Minor nits (optional):

__dequeue_elems() has a double space in its parameter list:
"void *objs,  void *meta"
rte_soring_dequeue_finish() uses 'num' in the header but 'n' in the implementation.
test_soring_peek_stress.c always pairs start with finish, so it doesn't exercise the peek-specific use cases (inspect-then-abandon, partial finish). Not a blocker; coverage of those could come later.