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 D0B5FA04C0; Fri, 25 Sep 2020 11:36:37 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 56D3C1E88E; Fri, 25 Sep 2020 11:36:36 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id B47D51DD3F for ; Fri, 25 Sep 2020 11:36:33 +0200 (CEST) IronPort-SDR: a7ayptn91Igq3VYDYBW+9m750uHH8EY4AT/XFkzoS+6qCHjOORJj1Aq+jVEUPDmy/IIOCFmV4C 2spqzKj4T8Zg== X-IronPort-AV: E=McAfee;i="6000,8403,9754"; a="141510335" X-IronPort-AV: E=Sophos;i="5.77,301,1596524400"; d="scan'208";a="141510335" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2020 02:36:32 -0700 IronPort-SDR: YE0YD88QqkC5roiBmkIvh9z76UOWUcnb0nPbTiItX/V0V0lxCqtEaiFyO/x4kciI78F7k25x7A Gbgm4bs7Hjgw== X-IronPort-AV: E=Sophos;i="5.77,301,1596524400"; d="scan'208";a="487399791" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.10.211]) ([10.252.10.211]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2020 02:36:30 -0700 To: "Min Hu (Connor)" , dev@dpdk.org Cc: stephen@networkplumber.org, bruce.richardson@intel.com, 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> <2442b2bc-4b94-089f-270f-087b56a9bb42@huawei.com> From: Ferruh Yigit Message-ID: <43477c7f-2d0d-2242-659d-74a6bca1e9b1@intel.com> Date: Fri, 25 Sep 2020 10:36:26 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: <2442b2bc-4b94-089f-270f-087b56a9bb42@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" On 9/25/2020 9:58 AM, Min Hu (Connor) wrote: > 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? > Makes sense, agree to continue with this patchset and queue stats mapping related changes can be done later. > > 在 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)' >> .