在 2020/9/5 2:31, Ferruh Yigit 写道:
On 9/4/2020 12:32 PM, Min Hu (Connor) wrote:
From: Huisong Li <lihuis...@huawei.com>

Currently, only statistics of rx/tx queues with queue_id less than
RTE_ETHDEV_QUEUE_STAT_CNTRS can be displayed. If there is a certain
application scenario that it needs to use 256 or more than 256 queues
and display all statistics of rx/tx queue. At this moment, we have to
change the macro to be equaled to the queue number.

However, modifying the macro to be greater than 256 will trigger
many errors and warnings from test-pmd, PMD driver and librte_ethdev
during compiling dpdk project. But it is possible and permited that
rx/tx queue number is greater than 256 and all statistics of rx/tx
queue need to be displayed. In addition, the data type of rx/tx queue
number in rte_eth_dev_configure API is 'uint16_t'. So It is unreasonable
to use the 'uint8_t' type for variables that control which per-queue
statistics can be displayed.

Fixes: ed30d9b691b2 ("app/testpmd: add stats per queue")
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Fixes: abf7275bbaa2 ("ixgbe: move to drivers/net/")
Fixes: e6defdfddc3b ("net/igc: enable statistics")
Fixes: 2265e4b4e84b ("net/octeontx2: add basic stats operation")
Fixes: 6c3169a3dc04 ("virtio: move to drivers/net/")
Cc: sta...@dpdk.org

Signed-off-by: Huisong Li <lihuis...@huawei.com>
Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
Reviewed-by: Dongdong Liu <liudongdo...@huawei.com>

The patch mostly looks good and it enables build with
'RTE_ETHDEV_QUEUE_STAT_CNTRS' > 256.
Only I put a comment for a testpmd change to enable the "set stat_qmap" command
for map value > 256.


BUT there are many things to fix in the queue stats mapping, since you are
already on it can you help on a few things on testpmd related to it, if you have
time for it?

1) Getting queue stats shouldn't require stats mapping, it should be controlled
separately. Many PMDs doesn't require/do the stats mapping but they still can
collect the per queue stats, which can be displayed independent from mapping.

2) Even you map only one queue, while displaying stats it will display
'RTE_ETHDEV_QUEUE_STAT_CNTRS' queues, and when that number is high it makes hard
to see the actual interested values.
If there is no mapping, it should display min(number_of_queues,
RTE_ETHDEV_QUEUE_STAT_CNTRS).
If there is mapping it should display queues that mapping done, this may require
adding a new 'active' field to 'struct queue_stats_mappings'.

3) Why 'struct queue_stats_mappings' is cache aligned, is it really needed?

4) The mapping arrays, 'tx_queue_stats_mappings_array' &
'rx_queue_stats_mappings_array' are global and their size is based on fixed max
port and queue size assumptions, can those mapping array be done per port and
'RTE_ETHDEV_QUEUE_STAT_CNTRS' size per port?

It does seem to be unreasonable. We also try do it, and found that
it is hard to control per queue stats and queue stats mapping, separately.
For details, see next V3 patch.
We're working on debugging this stats_mapping setting. During this process,
we have a problem that starting testpmd with --rx/tx-queue-stats-mapping
parameter fails. log as follows:
[root]$ ./testpmd -l 1,3,4,8,12 -n 4 -w 0000:04:00.0 --file-prefix=lee --log-level=7
 -- -i --rxq=6 --txq=6 --burst=64 --rxd=2048 --txd=2048 --nb-cores=4
 --rx-queue-stats-mapping=(0,2,2) --tx-queue-stats-mapping=(0,3,3)

-bash: syntax error near unexpected token `('
[root]$

thanks.

Thanks,
ferruh


.

Reply via email to