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 EDF8BA0C46; Mon, 27 Sep 2021 18:00:23 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8CDCD410DC; Mon, 27 Sep 2021 18:00:23 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 5A1B440E3C for ; Mon, 27 Sep 2021 18:00:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1632758421; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rIX+ErhaSx3GUYjXBEUVwUDX9dsZqL5A3VZ7QAwld8w=; b=aKq3XV+7Zb/ZbmVft8FJXvOEQvn/ApCItY57TsUFuY9SQlDOogqfWtdiN1OAFxfQv8NLXv uBTcQugoN8Se5CbDYhAl8rHcWvLnocnsPy2GqV0fLio42RLYW9R0koPNsiDRiXPT6FEOFG LzafWshhVcALsEJ9afI/A5e2n3eu+uE= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-150-rFnan0zFOwaZCM3v-AA2vQ-1; Mon, 27 Sep 2021 12:00:20 -0400 X-MC-Unique: rFnan0zFOwaZCM3v-AA2vQ-1 Received: by mail-wm1-f69.google.com with SMTP id z137-20020a1c7e8f000000b0030cd1800d86so429077wmc.2 for ; Mon, 27 Sep 2021 09:00:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:subject:in-reply-to :content-transfer-encoding; bh=rIX+ErhaSx3GUYjXBEUVwUDX9dsZqL5A3VZ7QAwld8w=; b=UV+IRDgPq1IYB1wztxQZxFHC+mh07p9Fe6LdPdwBTD2J9eqZKYb2XUPP71anHVdnzy i6jk/q5ZI6cc2ynFQkmWVb6Z4WsqYcTXXqjKvH8deqQndJ8XnqhaANK3oksGOsBEY/ZT 3ZTuUR5ox35ZPo+gJTsQ2vzZ4vnrlmGIErE+OcpS06g/1LRZQ3J6ov1hTxdX9ZJB45qU 97g12hC/zOOQu8Q+WmxBfDKS4d458jxKGMU9qssqLzc1Jwmw347b1XumarU8HfTAmgAr InEHVVl5OBKYXA3heP75t6wVCclnV8k7v4/ZZQXYazU/gkflGuuI1CAXemqz4whRaorY 1gCg== X-Gm-Message-State: AOAM532MIDB7alZ54BpS89bCBZxTCOg8/lDX8BeVE4lNHnqRW6eTFv5a YKOOfz+scclFVORpxfrSwAd4EvWJswJi/c4M1tgUckai6liuBXaJi7iyDoNmT2xt4IZEy3GL267 hbPU= X-Received: by 2002:adf:fb89:: with SMTP id a9mr743186wrr.164.1632758419193; Mon, 27 Sep 2021 09:00:19 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwg8Hhg8X0YGwHwgNrUHvtIl7RL1Q+oZAH4UP651ggEmA1EuKW+44tqQQYR5xX0qdn1lE+Vzw== X-Received: by 2002:adf:fb89:: with SMTP id a9mr743141wrr.164.1632758418956; Mon, 27 Sep 2021 09:00:18 -0700 (PDT) Received: from [192.168.0.36] ([78.18.26.217]) by smtp.gmail.com with ESMTPSA id n14sm6128100wms.0.2021.09.27.09.00.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 Sep 2021 09:00:18 -0700 (PDT) Message-ID: <863055cd-d436-6bdd-e6fe-2aa4d57db19a@redhat.com> Date: Mon, 27 Sep 2021 17:00:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: Alvin Zhang , beilei.xing@intel.com, junfeng.guo@intel.com Cc: dev@dpdk.org, stable@dpdk.org, "Zhang, Qi Z" , Ferruh Yigit References: <20210926075757.15116-1-alvinx.zhang@intel.com> From: Kevin Traynor In-Reply-To: <20210926075757.15116-1-alvinx.zhang@intel.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ktraynor@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix Rx packet statistics 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 26/09/2021 08:57, Alvin Zhang wrote: > Some packets are discarded by the NIC because they are larger than > the MTU, these packets should be counted as "RX error" instead of > "RX packet". > > The register 'GL_RXERR1' can count above discarded packets. > This patch adds reading and calculation of the 'GL_RXERR1' counter > when reporting DPDK statistics. > > Fixes: f4a91c38b4ad ("i40e: add extended stats") > Cc: stable@dpdk.org > > Signed-off-by: Alvin Zhang > --- > drivers/net/i40e/i40e_ethdev.c | 16 +++++++++++++--- > drivers/net/i40e/i40e_ethdev.h | 10 ++++++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) > It's a bit hard to understand the code for someone not familiar with the i40e stats. I think it needs careful review from i40e maintainers. A few questions below, Did you test this with testpmd? Can you show an example of a test where these packets are now correctly accounted for? I see there is also an RXERR2 register that catches other errors, does it need to be considered as well? > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c > index 7a2a828..30a2cdf 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -532,7 +532,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf, > /* store statistics names and its offset in stats structure */ > struct rte_i40e_xstats_name_off { > char name[RTE_ETH_XSTATS_NAME_SIZE]; > - unsigned offset; > + int offset; It is unusual to see you changing an offset to an int. You are expecting negative offsets? > }; > > static const struct rte_i40e_xstats_name_off rte_i40e_stats_strings[] = { > @@ -542,6 +542,8 @@ struct rte_i40e_xstats_name_off { > {"rx_dropped_packets", offsetof(struct i40e_eth_stats, rx_discards)}, > {"rx_unknown_protocol_packets", offsetof(struct i40e_eth_stats, > rx_unknown_protocol)}, > + {"rx_err1", offsetof(struct i40e_pf, rx_err1) - > + offsetof(struct i40e_pf, stats)}, rx_err1 is correct by datasheet but meaningless to a user. Suggest to find a more descriptive name, or document what it is, or tell the user to reference the datasheet. > {"tx_unicast_packets", offsetof(struct i40e_eth_stats, tx_unicast)}, > {"tx_multicast_packets", offsetof(struct i40e_eth_stats, tx_multicast)}, > {"tx_broadcast_packets", offsetof(struct i40e_eth_stats, tx_broadcast)}, > @@ -3238,6 +3240,10 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) > pf->offset_loaded, > &os->eth.rx_unknown_protocol, > &ns->eth.rx_unknown_protocol); > + i40e_stat_update_48(hw, I40E_GL_RXERR1_H(hw->pf_id + I40E_MAX_VF), > + I40E_GL_RXERR1_L(hw->pf_id + I40E_MAX_VF), > + pf->offset_loaded, &pf->rx_err1_offset, > + &pf->rx_err1); > i40e_stat_update_48_in_64(hw, I40E_GLPRT_GOTCH(hw->port), > I40E_GLPRT_GOTCL(hw->port), > pf->offset_loaded, &os->eth.tx_bytes, > @@ -3437,7 +3443,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast + > pf->main_vsi->eth_stats.rx_multicast + > pf->main_vsi->eth_stats.rx_broadcast - > - pf->main_vsi->eth_stats.rx_discards; > + pf->main_vsi->eth_stats.rx_discards - > + pf->rx_err1; > stats->opackets = ns->eth.tx_unicast + > ns->eth.tx_multicast + > ns->eth.tx_broadcast; > @@ -3451,7 +3458,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) > pf->main_vsi->eth_stats.rx_discards; > stats->ierrors = ns->crc_errors + > ns->rx_length_errors + ns->rx_undersize + > - ns->rx_oversize + ns->rx_fragments + ns->rx_jabber; > + ns->rx_oversize + ns->rx_fragments + ns->rx_jabber + > + pf->rx_err1; > > if (pf->vfs) { > for (i = 0; i < pf->vf_num; i++) { > @@ -6232,6 +6240,8 @@ struct i40e_vsi * > memset(&pf->stats_offset, 0, sizeof(struct i40e_hw_port_stats)); > memset(&pf->internal_stats, 0, sizeof(struct i40e_eth_stats)); > memset(&pf->internal_stats_offset, 0, sizeof(struct i40e_eth_stats)); > + pf->rx_err1 = 0; > + pf->rx_err1_offset = 0; > > ret = i40e_pf_get_switch_config(pf); > if (ret != I40E_SUCCESS) { > diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h > index cd6deab..846c8d4 100644 > --- a/drivers/net/i40e/i40e_ethdev.h > +++ b/drivers/net/i40e/i40e_ethdev.h > @@ -19,6 +19,13 @@ > #include "base/i40e_type.h" > #include "base/virtchnl.h" > > +#define I40E_GL_RXERR1_H(_i) (0x00318004 + ((_i) * 8)) > +/** > + * _i=0...143, > + * counters 0-127 are for the 128 VFs, > + * counters 128-143 are for the 16 PFs > + */ > + > #define I40E_VLAN_TAG_SIZE 4 > > #define I40E_AQ_LEN 32 > @@ -1134,6 +1141,9 @@ struct i40e_pf { > > struct i40e_hw_port_stats stats_offset; > struct i40e_hw_port_stats stats; > + u64 rx_err1; /* rxerr1 */ > + u64 rx_err1_offset; > + > /* internal packet statistics, it should be excluded from the total */ > struct i40e_eth_stats internal_stats_offset; > struct i40e_eth_stats internal_stats; >