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 F3E40A04C3; Mon, 28 Sep 2020 15:47:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 161171D6BE; Mon, 28 Sep 2020 15:47:18 +0200 (CEST) Received: from huawei.com (szxga07-in.huawei.com [45.249.212.35]) by dpdk.org (Postfix) with ESMTP id 3F3431BCAC; Mon, 28 Sep 2020 15:47:15 +0200 (CEST) Received: from DGGEMS405-HUB.china.huawei.com (unknown [172.30.72.60]) by Forcepoint Email with ESMTP id BCC33F03264B4D70BB62; Mon, 28 Sep 2020 21:47:12 +0800 (CST) Received: from [10.67.103.128] (10.67.103.128) by DGGEMS405-HUB.china.huawei.com (10.3.19.205) with Microsoft SMTP Server id 14.3.487.0; Mon, 28 Sep 2020 21:47:11 +0800 To: Thomas Monjalon , Ferruh Yigit CC: "techboard@dpdk.org" , , , "jerinj@marvell.com" , "Ray Kinsella" , References: <1598845317-55956-1-git-send-email-humin29@huawei.com> <1601176596-29900-2-git-send-email-humin29@huawei.com> <32785804.XpyAPG8jY8@thomas> From: "Min Hu (Connor)" Message-ID: <1f51b8e9-de81-9a11-788e-2d16ea3822f3@huawei.com> Date: Mon, 28 Sep 2020 21:47:11 +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: <32785804.XpyAPG8jY8@thomas> Content-Type: text/plain; charset="gbk"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.128] X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [dpdk-techboard] [PATCH V5 1/2] dpdk: resolve compiling errors for per-queue stats 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" ÔÚ 2020/9/28 17:16, Thomas Monjalon дµÀ: > 28/09/2020 10:59, Ferruh Yigit: >> On 9/27/2020 4:16 AM, 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 drivers and librte_ethdev >>> during compiling dpdk project. But it is possible and permitted 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. > > The explanation is too much complex and misleading. > You mean you cannot increase RTE_ETHDEV_QUEUE_STAT_CNTRS > above 256 because it is an 8-bit type? > Hi, Thomas, you may misunderstand it. The goal of this patch is that resolving some errors or warnings when compiling code with RTE_ETHDEV_QUEUE_STAT_CNTRS > 256. The reson is some vlaue(uint8_t) will never catch over RTE_ETHDEV_QUEUE_STAT_CNTRS, as uint8_t ranges from 0-255. e.g., static void nic_stats_display(uint16_t port_id) { uint8_t i; for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS; i++) { ... } ... } RTE_ETHDEV_QUEUE_STAT_CNTRS can be increased to more than 256, which is reasonable in principle. Because the number of queue statistics is controlled by it. And mapping queue to queue statistics counter is also controlled by it. So these "*_queue_stats_mapping" API also trigger errors or warning. > [...] >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> int rte_eth_dev_set_tx_queue_stats_mapping(uint16_t port_id, >>> - uint16_t tx_queue_id, uint8_t stat_idx); >>> + uint16_t tx_queue_id, uint16_t stat_idx); > [...] >>> int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id, >>> uint16_t rx_queue_id, >>> - uint8_t stat_idx); >>> + uint16_t stat_idx); > [...] >> cc'ed tech-board, >> >> The patch breaks the ethdev ABI without a deprecation notice from previous >> release(s). >> >> It is mainly a fix to the port_id storage type, which we have updated from >> uint8_t to uint16_t in past but some seems remained for >> 'rte_eth_dev_set_tx_queue_stats_mapping()' & >> 'rte_eth_dev_set_rx_queue_stats_mapping()' APIs. > > No, it is not related to the port id, but the number of limited stats. > >> Since the ethdev library already heavily breaks the ABI this release, I am for >> getting this fix, instead of waiting the fix for one more year. > > If stats can be managed for more than 256 queues, I think it means > it is not limited. In this case, we probably don't need the API > *_queue_stats_mapping which was invented for a limitation of ixgbe. > First, my modification for "stat_idx" data type is resonable and it does not cause some problems. Both queue id and stat_idx have same type, which is also reasoable in usage. Then, "*_queue_stats_mapping" these API is not only invented for ixgbe. other net drivers also implements the API, like enic, igc, octeontx2 etc. > The problem is probably somewhere else (in testpmd), > that's why I am against this patch. > > > . >