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 0317EA04C0; Fri, 25 Sep 2020 10:59:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D6CAE1E535; Fri, 25 Sep 2020 10:59:04 +0200 (CEST) Received: from huawei.com (szxga07-in.huawei.com [45.249.212.35]) by dpdk.org (Postfix) with ESMTP id 56E471E4CA for ; Fri, 25 Sep 2020 10:59:03 +0200 (CEST) Received: from DGGEMS410-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id 3F8828C359B6A108F474; Fri, 25 Sep 2020 16:59:02 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS410-HUB.china.huawei.com (10.3.19.210) with Microsoft SMTP Server id 14.3.487.0; Fri, 25 Sep 2020 16:58:52 +0800 To: Ferruh Yigit , CC: , , "Jerin Jacob Kollanukkaran" , Andrew Rybchenko , Thomas Monjalon , "Wenzhuo Lu" , Beilei Xing , "Bernard Iremonger" , lihuisong References: <1598961165-20832-2-git-send-email-humin29@huawei.com> <1599219135-53194-1-git-send-email-humin29@huawei.com> <1599219135-53194-2-git-send-email-humin29@huawei.com> <5038a8f5-18f4-2553-edb1-c51863e2bc07@huawei.com> From: "Min Hu (Connor)" Message-ID: <2442b2bc-4b94-089f-270f-087b56a9bb42@huawei.com> Date: Fri, 25 Sep 2020 16:58:52 +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: 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] [PATCH V2 1/4] ethdev: fix compiling errors for per-queue statistics 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 Yigit, Your method is very practical. Thanks! But we found that testpmd fails to start with "--rx-queue-stats-mapping" and "--tx-queue-stats-mapping". We've found the cause and modified it. These modifications are being tested. Currently, the following modifications are related to framework APIs. 1)struct 'rte_eth_dcb_tc_queue_mapping' 2)fix 'rte_eth_dev_set_rx/tx_queue_stats_mapping' function to resolve the problem that DPDK project fails to build when 'RTE_ETHDEV_QUEUE_STAT_CNTRS' > 256. According to my current analysis, these modifications for queue stats mapping in testpmd do not involve the modification of framework APIs. Therefore, I think we should give priority to focusing on the current changes to the DPDK framework APIs. After all, DPDK-20.11 is approaching. What do you think? 在 2020/9/23 17:18, Ferruh Yigit 写道: > On 9/23/2020 3:31 AM, Min Hu (Connor) wrote: >> >> >> 在 2020/9/5 2:31, Ferruh Yigit 写道: >>> On 9/4/2020 12:32 PM, Min Hu (Connor) wrote: >>>> From: Huisong Li >>>> >>>> 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: stable@dpdk.org >>>> >>>> Signed-off-by: Huisong Li >>>> Signed-off-by: Min Hu (Connor) >>>> Reviewed-by: Wei Hu (Xavier) >>>> Reviewed-by: Dongdong Liu >>> >>> 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. > > I will check v3, but as far as know stats mapping exist because of > limitations of some NICs, that there are N queues but M stats registers > where N > M. So need to map some queues to stat registers, and there is > N:1 relation there, so multiple queue stats can be represented in single > stat register. > > But for rest of the NICs it should be possible to display the queue > stats independent from the mapping. > > >> 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]$ > > Above seems bash error, using '' can help for it, like > --rx-queue-stats-mapping='(0,2,2)' > .