From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7BE38A04E6; Wed, 18 Nov 2020 04:39:45 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5FEBD592C; Wed, 18 Nov 2020 04:39:44 +0100 (CET) Received: from szxga05-in.huawei.com (szxga05-in.huawei.com [45.249.212.191]) by dpdk.org (Postfix) with ESMTP id 85E854C90 for ; Wed, 18 Nov 2020 04:39:42 +0100 (CET) Received: from DGGEMS414-HUB.china.huawei.com (unknown [172.30.72.60]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4CbT7y3CzPzhbn5; Wed, 18 Nov 2020 11:39:26 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS414-HUB.china.huawei.com (10.3.19.214) with Microsoft SMTP Server id 14.3.487.0; Wed, 18 Nov 2020 11:39:32 +0800 To: Ferruh Yigit , CC: , , References: <1603182389-10087-1-git-send-email-humin29@huawei.com> <1603182389-10087-2-git-send-email-humin29@huawei.com> <34c8c7ac-a799-8624-b530-3c136fd16811@huawei.com> <3f67aa47-7bec-d31c-8162-e800c9800e8a@huawei.com> <531504ce-0547-48c2-4e34-c2ed6cb12e57@intel.com> From: "Min Hu (Connor)" Message-ID: <4ee8c31f-070b-c706-2394-5808a5f7e3a2@huawei.com> Date: Wed, 18 Nov 2020 11:39:32 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <531504ce-0547-48c2-4e34-c2ed6cb12e57@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [RFC V2 1/2] app/testpmd: fix queue stats mapping configuration X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" HI， Ferruh, Ok，I will send V3 patches. thanks. 在 2020/11/12 17:52, Ferruh Yigit 写道: > On 11/12/2020 2:28 AM, Min Hu (Connor) wrote: >> Hi Ferruh, any suggestions? >> >> >> 在 2020/11/3 14:30, Min Hu (Connor) 写道: >>> Hi Ferruh, >>> >>> I agree with your proposal. But if we remove record structures, we will >>> not be able to query the current queue stats mapping configuration. Or >>> we can provide a query API for the PMD driver that uses the >>> set_queue_stats_mapping API, and driver records these mapping >>> information from user. >>> >>> What do you think? >>> > > Sorry for delay, > > Yes that information will be lost, but since the queue stats mapping is > not commonly used, I think it can be OK to loose it. > > As you said another option is to add a new ethdev API to get queue stats > mapping, but because of same reason not sure about adding it. We can add > it later if there is a request for it. > >>> >>> 在 2020/10/31 4:54, Ferruh Yigit 写道: >>>> On 10/20/2020 9:26 AM, Min Hu (Connor) wrote: >>>>> From: Huisong Li >>>>> >>>>> 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 >>>> >>>> <...> >>>> >>>> . > > .