DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Min Hu (Connor)" <humin29@huawei.com>, dev@dpdk.org
Cc: bruce.richardson@intel.com, thomas.monjalon@6wind.com,
	lihuisong@huawei.com
Subject: Re: [dpdk-dev] [RFC V2 1/2] app/testpmd: fix queue stats mapping configuration
Date: Fri, 30 Oct 2020 20:54:20 +0000
Message-ID: <f8f572d5-7bbf-d7a9-67a8-77ba4da81f0a@intel.com> (raw)
In-Reply-To: <1603182389-10087-2-git-send-email-humin29@huawei.com>

On 10/20/2020 9:26 AM, Min Hu (Connor) wrote:
> From: Huisong Li <lihuisong@huawei.com>
> 
> Currently, the queue stats mapping has the following problems:
> 1) Many PMD drivers don't support queue stats mapping. But there is no
> failure message after executing the command "set stat_qmap rx 0 2 2".
> 2) Once queue mapping is set, unrelated and unmapped queues are also
> displayed.
> 3) There is no need to keep cache line alignment for
> 'struct queue_stats_mappings'.
> 4) The mapping arrays, 'tx_queue_stats_mappings_array' &
> 'rx_queue_stats_mappings_array' are global and their sizes are based on
> fixed max port and queue size assumptions.
> 5) The configuration result does not take effect or can not be queried
> in real time.
> 
> Therefore, we have made the following adjustments:
> 1) If PMD supports queue stats mapping, configure to driver in real time
> after executing the command "set stat_qmap rx/tx ...". If not,
> the command can not be accepted.
> 2) Only display queues that mapping done by adding a new 'active' field
>   in queue_stats_mappings struct.
> 3) Remove cache alignment for 'struct queue_stats_mappings'.
> 4) Add a new port_stats_mappings struct in rte_port.
> The struct contains number of rx/txq stats mapping, rx/tx
> queue_stats_mapping_enabled flag, and rx/tx queue_stats_mapping array.
> Size of queue_stats_mapping_array is set to "RTE_ETHDEV_QUEUE_STAT_CNTRS"
>   to ensure that the same number of queues can be set for each port.
> 

Hi Connor,

I think above adjustment are good, but after the decision to use xstats for the 
queue stats, what do you think about more simplification,

1)
What testpmd does is, records the queue stats mapping commands and registers 
them later on port start & forwarding start.
What happens if recording and registering completely removed?
When "set stat_qmap .." issued, it just call the ethdev APIs to do the mapping 
in device.
This lets us removing record structures, "struct port_stats_mappings p_stats_map"
Also can remove 'map_port_queue_stats_mapping_registers()' and its sub functions.

2)
Also lets remove "tx-queue-stats-mapping" & "rx-queue-stats-mapping" parameters, 
which enables removing 'parse_queue_stats_mapping_config()' function too

3)
Another problem is to display the queue stats, in 'fwd_stats_display()' & 
'nic_stats_display()', there is a check if the queue stats mapping enable or not 
('rx_queue_stats_mapping_enabled' & 'tx_queue_stats_mapping_enabled'),
I think displaying queue stats and queue stat mapping should be separate, why 
not drop checks for queue stats mapping and display queue stats for 'nb_rxq' & 
'nb_txq' queues?

Does above make sense?


Majority of the drivers doesn't require queue stat mapping to get the queue 
stats, lets don't pollute main usage with this requirement.


> Fixes: 4dccdc789bf4b ("app/testpmd: simplify handling of stats mappings error")
> Fixes: 013af9b6b64f6 ("app/testpmd: various updates")
> Fixes: ed30d9b691b21 ("app/testpmd: add stats per queue")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>

<...>


  reply	other threads:[~2020-10-30 20:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-20  8:26 [dpdk-dev] [RFC V2 0/2] fix queue stats mapping Min Hu (Connor)
2020-10-20  8:26 ` [dpdk-dev] [RFC V2 1/2] app/testpmd: fix queue stats mapping configuration Min Hu (Connor)
2020-10-30 20:54   ` Ferruh Yigit [this message]
2020-11-03  6:30     ` Min Hu (Connor)
2020-11-12  2:28       ` Min Hu (Connor)
2020-11-12  9:52         ` Ferruh Yigit
2020-11-18  3:39           ` Min Hu (Connor)
2020-11-20 11:50           ` [dpdk-dev] [RFC V4] " Min Hu (Connor)
2020-11-20 17:26             ` Ferruh Yigit
2020-11-20 23:21               ` Stephen Hemminger
2020-11-20 23:33                 ` Ferruh Yigit
2020-11-21  4:29                   ` Stephen Hemminger
2020-11-23  7:22                     ` Min Hu (Connor)
2020-11-23  9:51                     ` Ferruh Yigit
2020-11-30  8:29                       ` Min Hu (Connor)
2020-12-02 10:44                         ` Ferruh Yigit
2020-12-02 12:48                           ` [dpdk-dev] [PATCH V1] " Min Hu (Connor)
2020-12-08 15:48                             ` Ferruh Yigit
2020-12-07  1:28                           ` [dpdk-dev] [RFC V4] " Min Hu (Connor)
2020-10-20  8:26 ` [dpdk-dev] [RFC V2 2/2] app/testpmd: fix starting failed with queue-stats-mapping Min Hu (Connor)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f8f572d5-7bbf-d7a9-67a8-77ba4da81f0a@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=humin29@huawei.com \
    --cc=lihuisong@huawei.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git