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

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 24 Mar 2026 13:37:08 +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 feedback:

The logic is sound — assign_share_group() maps each unique (domain_id,
rx_domain) pair to a share group index via a simple linear scan of a
fixed-size slot table. No correctness bugs found. Two warnings:

Warning: share_group_slots[] entries are never freed on port removal.
With dynamic representor probing and hot-unplug cycles, the slot array
(sized RTE_MAX_ETHPORTS, default 32) could fill up. The RTE_ASSERT will
fire in debug builds, but in release builds the behavior would be
reading an uninitialized slot. Consider adding cleanup when a port is
removed, or document the limitation.

Warning: share_group_slots[] should be declared static. It's a
file-scope global without the rte_ prefix and only accessed within
testpmd.c. Making it static gives it proper scoping.