Re: [PATCH v2 1/2] app/testpmd: assign share group dynamically

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 24 Mar 2026 17:56:56 +0100
Dariusz Sosnowski <dsosnowski@nvidia.com> wrote:

> Testpmd exposes "--rxq-share=[N]" parameter which controls
> sharing Rx queues. Before this patch logic was that either:
> 
> - all queues were assigned to the same share group
>   (when N was not passed),
> - or ports were grouped in subsets of N ports,
>   each subset got different share group index.
> 
> 2nd option did not work well with dynamic representor probing,
> where new representors would be assigned to new share group.
> 
> This patch changes the logic in testpmd to dynamically
> assign share group index. Each unique switch and Rx domain
> will get different share group.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---

AI review found that the option definition still expects arg.


Warning: Option definition still uses OPTIONAL_ARG but argument is now ignored.
The --rxq-share parameter definition at line 354 in
parameters.c still uses OPTIONAL_ARG(TESTPMD_OPT_RXQ_SHARE) (which maps
to optional_argument in getopt), but the parsing now unconditionally
sets rxq_share = 1 and ignores optarg. If a user passes --rxq-share=5,
the value is silently discarded. The option definition should be
changed to NO_ARG(TESTPMD_OPT_RXQ_SHARE) to match the new behavior, and
the documentation now correctly documents it as a bare flag.

Warning: share_group_slots entries are never cleared on port removal.
The share_group_slots[] array grows as new (domain_id, rx_domain) pairs
are seen, but entries are never removed when ports are hot-unplugged or
closed. Over many hotplug cycles with distinct domain IDs, slots could
fill up. In practice this is bounded by RTE_MAX_ETHPORTS (default 32)
so it is unlikely to be a real problem, but an RTE_ASSERT will fire if
it does overflow — worth a comment noting this limitation.

Info: Documentation grammar nit.
In run_app.rst, "This engine does Rx only and update stream statistics accordingly" — "update" should be "updates".