From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9FE65A0A0B; Mon, 1 Feb 2021 17:21:15 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15DD716065A; Mon, 1 Feb 2021 17:21:15 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 4CD1F40693 for ; Mon, 1 Feb 2021 17:21:13 +0100 (CET) IronPort-SDR: uL35qROfvQvj9aOZZHb9VSJTozMkr1kYLntF/4hvjUdaYMKwJJhdDKZHErUjVO0cqq/r8V7j4T AJfwu071DXxg== X-IronPort-AV: E=McAfee;i="6000,8403,9882"; a="180787615" X-IronPort-AV: E=Sophos;i="5.79,392,1602572400"; d="scan'208";a="180787615" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2021 08:21:09 -0800 IronPort-SDR: uOysYqAYX6SkwOXHCIxl2BvQ77VVBKtuoy0EA4+RCEmi9emsCsN0SZWm+Iq2UNaiOsyiC+7mSO OUjOrQj/RpTw== X-IronPort-AV: E=Sophos;i="5.79,392,1602572400"; d="scan'208";a="390982163" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.210.23]) ([10.213.210.23]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2021 08:21:08 -0800 To: Ido Goshen Cc: "dev@dpdk.org" References: <20210201083012.28544-1-ido@cgstowernetworks.com> <732a3fe2-1a64-1861-4360-2df996e2b5f3@intel.com> From: Ferruh Yigit Message-ID: <78f296a2-aa02-f1f3-8fad-a9bbd126dca0@intel.com> Date: Mon, 1 Feb 2021 16:21:03 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] net/pcap: imissed stats support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 2/1/2021 2:02 PM, Ido Goshen wrote: > > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Monday, 1 February 2021 13:49 >> To: Ido Goshen >> Cc: dev@dpdk.org >> Subject: Re: [PATCH v2] net/pcap: imissed stats support >> >> On 2/1/2021 8:30 AM, Ido Goshen wrote: >>> Signed-off-by: Ido Goshen >>> --- >>> v2: >>> * sum all queues (rx_missed_total += fix) >>> * null pcap protection >>> * inter stop/start persistancy (counter won't reset on stop) >>> >>> drivers/net/pcap/rte_eth_pcap.c | 28 >> ++++++++++++++++++++++++++++ >>> 1 file changed, 28 insertions(+) >>> >>> diff --git a/drivers/net/pcap/rte_eth_pcap.c >>> b/drivers/net/pcap/rte_eth_pcap.c index a32b1f3f3..18c59d61c 100644 >>> --- a/drivers/net/pcap/rte_eth_pcap.c >>> +++ b/drivers/net/pcap/rte_eth_pcap.c >>> @@ -58,6 +58,8 @@ struct queue_stat { >>> volatile unsigned long pkts; >>> volatile unsigned long bytes; >>> volatile unsigned long err_pkts; >>> + volatile unsigned long missed_reset; >>> + volatile unsigned long missed_mnemonic; >> >> Can you please put some comments why 'missed_mnemonic' is required? >> > ok > >> <...> >> >>> @@ -695,6 +715,10 @@ eth_stats_get(struct rte_eth_dev *dev, struct >> rte_eth_stats *stats) >>> stats->q_ibytes[i] = internal->rx_queue[i].rx_stat.bytes; >>> rx_packets_total += stats->q_ipackets[i]; >>> rx_bytes_total += stats->q_ibytes[i]; >>> + unsigned long rx_missed = eth_pcap_stats_missed_get(dev, >> i) + >>> + internal- >>> rx_queue[i].rx_stat.missed_mnemonic - >>> + internal->rx_queue[i].rx_stat.missed_reset; >> >> >> Instead of including the 'missed_mnemonic' to the regular calculation, what >> do you think to save the 'imissed' value to 'missed_mnemonic' in 'port_stop' >> and load it back in the 'eth_dev_start'? >> This balanced usage can simplify the code I think. > > Not sure I get the request, isn't it what the patch already doing? > Value is already stored in 'eth_dev_stop' and added back when needed. > What do you mean by load it back on 'eth_dev_start' - where to load it to? > Please explain further > Please ignore above comment, it was wrong, the intention was to not complicate the stats function with 'missed_mnemonic' & 'missed_reset' details, for that what do you think to move the 'missed_mnemonic' & 'missed_reset' usage into the 'eth_pcap_stats_missed_get()'? This also fixes imissed stats after multiple 'port_stop'. Also a 'eth_pcap_stats_missed_reset()' can be added for same reason.