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 53109A04B1; Wed, 23 Sep 2020 11:18:09 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2BB1E1C208; Wed, 23 Sep 2020 11:18:09 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 55EAD1C1E1 for ; Wed, 23 Sep 2020 11:18:08 +0200 (CEST) IronPort-SDR: wXK6BF3bBj8vNpaa85V1g3akGfaanjWdYVEdKmMHnTK2LVt5Har6P2DPeVZTefRXyqD9RSwtKr ah2tSw3zfuAA== X-IronPort-AV: E=McAfee;i="6000,8403,9752"; a="222413219" X-IronPort-AV: E=Sophos;i="5.77,293,1596524400"; d="scan'208";a="222413219" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Sep 2020 02:18:07 -0700 IronPort-SDR: yTx4lm20aUZBE3HshRM5W7EEVehz5aefhEPbMKZtY9EzrCIWQmJSBSdzLssgaXKt6lHbk5b3Pi 7xuoD4KAfhnw== X-IronPort-AV: E=Sophos;i="5.77,293,1596524400"; d="scan'208";a="305301454" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.218.147]) ([10.213.218.147]) by orsmga003-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Sep 2020 02:18:04 -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 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: Ferruh Yigit Message-ID: Date: Wed, 23 Sep 2020 10:18:01 +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: <5038a8f5-18f4-2553-edb1-c51863e2bc07@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/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)'