Re: [dpdk-dev] [PATCH] ethdev: fix queue mapping documentation

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 6/29/2018 10:44 AM, Jerin Jacob wrote:
> The RTE_MAX_ETHPORT_QUEUE_STATS_MAPS does not exists, change
> to the correct definition(RTE_ETHDEV_QUEUE_STAT_CNTRS)
> 
> Fixes: 5de201df8927 ("ethdev: add stats per queue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 36e3984ea..375ea24ce 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -2144,7 +2144,7 @@ void rte_eth_xstats_reset(uint16_t port_id);
>   * @param stat_idx
>   *   The per-queue packet statistics functionality number that the transmit
>   *   queue is to be assigned.
> - *   The value must be in the range [0, RTE_MAX_ETHPORT_QUEUE_STATS_MAPS - 1].
> + *   The value must be in the range [0, RTE_ETHDEV_QUEUE_STAT_CNTRS - 1].

Yes RTE_MAX_ETHPORT_QUEUE_STATS_MAPS doesn't exits and comment is wrong, but
RTE_ETHDEV_QUEUE_STAT_CNTRS also slightly not correct.

I think how testpmd uses it increase the confusion.

In ixgbe there is no stats registers per queue, 128 queues are represented by 16
register set. stat_idx here is the index of that 16 registers. You map queue to
stats requester to get queue stats.

Also there is RTE_ETHDEV_QUEUE_STAT_CNTRS config in the ethdev API, which is the
hardcoded size of the queue stats, its default value is 16. This limits number
of the queues we can get stats from but saves allocated space. (Why not dynamic?)

You can increase the RTE_ETHDEV_QUEUE_STAT_CNTRS to the max supported number of
queue and ethdev code will be all valid. But "stat_idx" can't go beyond 16 (for
ixgbe) because it is hardware limitation and it may change from hw to hw.

Also technically it should be possible to reduce RTE_ETHDEV_QUEUE_STAT_CNTRS to
a low number, like 2, but in ixgbe map two queues into stat registers 14 & 15
and display those two set as queue stat 0 and 1. It seems current implementation
prevents this and forces the queues mapped should be less than
RTE_ETHDEV_QUEUE_STAT_CNTRS. Overall it seems there is a mixed used of
RTE_ETHDEV_QUEUE_STAT_CNTRS and stats queue index values, I assume because both
are same values.

I suggest updating it as:
"
The value must be in the range:
 [0 - MIN(HW Stat Registers Size, RTE_ETHDEV_QUEUE_STAT_CNTRS) - 1]
"

>   * @return
>   *   Zero if successful. Non-zero otherwise.
>   */
> @@ -2164,7 +2164,7 @@ int rte_eth_dev_set_tx_queue_stats_mapping(uint16_t port_id,
>   * @param stat_idx
>   *   The per-queue packet statistics functionality number that the receive
>   *   queue is to be assigned.
> - *   The value must be in the range [0, RTE_MAX_ETHPORT_QUEUE_STATS_MAPS - 1].
> + *   The value must be in the range [0, RTE_ETHDEV_QUEUE_STAT_CNTRS - 1].
>   * @return
>   *   Zero if successful. Non-zero otherwise.
>   */
>