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 79E26A04E6; Fri, 30 Oct 2020 21:54:31 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CA541AA1F; Fri, 30 Oct 2020 21:54:29 +0100 (CET) Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id E57B39B02 for ; Fri, 30 Oct 2020 21:54:27 +0100 (CET) IronPort-SDR: Zw+l6j8acpZ4JxjBlTPd45IAWW/mDO/3j+P+wdvwk4rgg/N1t4PBpGxC3a+UmMzCmHmVi1HqKf oFFOdYSbKRIg== X-IronPort-AV: E=McAfee;i="6000,8403,9790"; a="156438684" X-IronPort-AV: E=Sophos;i="5.77,434,1596524400"; d="scan'208";a="156438684" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Oct 2020 13:54:25 -0700 IronPort-SDR: yzRumbqGqt2EM79toFvjnO0QIsh+2ia/hjw10yUwmajb39Oaw1x8EBsoKvpNe3m8Ov6S9Gimoy DTx2PxEaX7gQ== X-IronPort-AV: E=Sophos;i="5.77,434,1596524400"; d="scan'208";a="537196304" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.218.82]) ([10.213.218.82]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Oct 2020 13:54:24 -0700 To: "Min Hu (Connor)" , dev@dpdk.org Cc: bruce.richardson@intel.com, thomas.monjalon@6wind.com, lihuisong@huawei.com References: <1603182389-10087-1-git-send-email-humin29@huawei.com> <1603182389-10087-2-git-send-email-humin29@huawei.com> From: Ferruh Yigit Message-ID: Date: Fri, 30 Oct 2020 20:54:20 +0000 MIME-Version: 1.0 In-Reply-To: <1603182389-10087-2-git-send-email-humin29@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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 <...>