From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 52A4A7CE5 for ; Tue, 19 Sep 2017 05:29:55 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Sep 2017 20:29:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,416,1500966000"; d="scan'208";a="1196558317" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by fmsmga001.fm.intel.com with ESMTP; 18 Sep 2017 20:29:53 -0700 Received: from pgsmsx110.gar.corp.intel.com (10.221.44.111) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 19 Sep 2017 11:29:52 +0800 Received: from pgsmsx103.gar.corp.intel.com ([169.254.2.203]) by PGSMSX110.gar.corp.intel.com ([169.254.13.249]) with mapi id 14.03.0319.002; Tue, 19 Sep 2017 11:29:52 +0800 From: "Zhao1, Wei" To: "Wu, Jingjing" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v3 1/3] net/i40e: fix clear xstats bug in vf port Thread-Index: AQHTMEd/bsdrkKoblkWtOB6R7NvynaK6/7QAgACNkcA= Date: Tue, 19 Sep 2017 03:29:52 +0000 Message-ID: References: <20170829022806.68101-1-wei.zhao1@intel.com> <1505715504-8797-1-git-send-email-wei.zhao1@intel.com> <9BB6961774997848B5B42BEC655768F810E76F10@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <9BB6961774997848B5B42BEC655768F810E76F10@SHSMSX103.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [172.30.20.205] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/3] net/i40e: fix clear xstats bug in vf port 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: , X-List-Received-Date: Tue, 19 Sep 2017 03:29:55 -0000 Hi, jinging > -----Original Message----- > From: Wu, Jingjing > Sent: Tuesday, September 19, 2017 10:59 AM > To: Zhao1, Wei ; dev@dpdk.org > Cc: Zhao1, Wei > Subject: RE: [dpdk-dev] [PATCH v3 1/3] net/i40e: fix clear xstats bug in = vf port >=20 >=20 >=20 > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Zhao > > Sent: Monday, September 18, 2017 2:18 PM > > To: dev@dpdk.org > > Cc: Zhao1, Wei > > Subject: [dpdk-dev] [PATCH v3 1/3] net/i40e: fix clear xstats bug in > > vf port > > > > There is a bug in vf clear xstats command, it do not record the > > statics data in offset struct member.So, vf need to keep record of > > xstats data from pf and update the statics according to offset. > > > > Fixes: da61cd0849766 ("i40evf: add extended stats") > > > > Signed-off-by: Wei Zhao > > > > --- > > > > Changes in v2: > > > > fix patch log check warning. > > > > --- > > > > changes in v3: > > > > remove nic_stats_display protect to a new patch > > --- > > drivers/net/i40e/i40e_ethdev_vf.c | 64 > > ++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 63 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c > > b/drivers/net/i40e/i40e_ethdev_vf.c > > index 38c3adc..806ff9e 100644 > > --- a/drivers/net/i40e/i40e_ethdev_vf.c > > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > > @@ -888,16 +888,74 @@ i40evf_update_stats(struct rte_eth_dev *dev, > > struct i40e_eth_stats **pstats) > > return 0; > > } > > > > +static void > > +i40evf_stat_update_48(uint64_t *offset, > > + uint64_t *stat) > > +{ > > + if (*stat >=3D *offset) > > + *stat =3D *stat - *offset; > > + else > > + *stat =3D (uint64_t)((*stat + > > + ((uint64_t)1 << I40E_48_BIT_WIDTH)) - *offset); > > + > > + *stat &=3D I40E_48_BIT_MASK; > > +} > > + > > +static void > > +i40evf_stat_update_32(uint64_t *offset, > > + uint64_t *stat) > > +{ > > + if (*stat >=3D *offset) > > + *stat =3D (uint64_t)(*stat - *offset); > > + else > > + *stat =3D (uint64_t)((*stat + > > + ((uint64_t)1 << I40E_32_BIT_WIDTH)) - *offset); } >=20 > The type of count is 64 bits. Is that correct to use 1 << I40E_32_BIT_WID= TH? No, this is because the register data is 32 bit width, although we store it= use 64 bit in memory. You can see another function i40e_update_vsi_stats(). >=20 > > + > > +static void > > +i40evf_update_vsi_stats(struct i40e_vsi *vsi, > > + struct i40e_eth_stats *nes) > > +{ > > + struct i40e_eth_stats *oes =3D &vsi->eth_stats_offset; > > + > > + i40evf_stat_update_48(&oes->rx_bytes, > > + &nes->rx_bytes); > > + i40evf_stat_update_48(&oes->rx_unicast, > > + &nes->rx_unicast); > > + i40evf_stat_update_48(&oes->rx_multicast, > > + &nes->rx_multicast); > > + i40evf_stat_update_48(&oes->rx_broadcast, > > + &nes->rx_broadcast); > > + i40evf_stat_update_32(&oes->rx_discards, > > + &nes->rx_discards); > > + i40evf_stat_update_32(&oes->rx_unknown_protocol, > > + &nes->rx_unknown_protocol); > > + i40evf_stat_update_48(&oes->tx_bytes, > > + &nes->tx_bytes); > > + i40evf_stat_update_48(&oes->tx_unicast, > > + &nes->tx_unicast); > > + i40evf_stat_update_48(&oes->tx_multicast, > > + &nes->tx_multicast); > > + i40evf_stat_update_48(&oes->tx_broadcast, > > + &nes->tx_broadcast); > > + i40evf_stat_update_32(&oes->tx_errors, &nes->tx_errors); > > + i40evf_stat_update_32(&oes->tx_discards, &nes->tx_discards); } > > + > > static int > > i40evf_get_statistics(struct rte_eth_dev *dev, struct rte_eth_stats *s= tats) > { > > int ret; > > struct i40e_eth_stats *pstats =3D NULL; > > + struct i40e_vf *vf =3D I40EVF_DEV_PRIVATE_TO_VF(dev->data- > > >dev_private); > > + struct i40e_vsi *vsi =3D &vf->vsi; > > > > ret =3D i40evf_update_stats(dev, &pstats); > > if (ret !=3D 0) > > return 0; > > > > + i40evf_update_vsi_stats(vsi, pstats); > > + >=20 > It looks like, with this change, the static gotten by user the incensemen= t from > the last query? > If so, I don't think it is our expected. >=20 This change only reset after clear command, so the data increase after the = reset command. > The names of functions are similar. Could you help to refine the code? > For example, merge i40evf_dev_stats_get and i40evf_get_statistics to be > one function. > Rename i40evf_update_stats like i40evf_query_stats, and chang > i40evf_update_vsi_stats To be i40evf_update_stats? I think it would be > clearer, what do you think? Ok, I wiil change the name in later version=20 >=20 > Thanks > Jingjing