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 45CAF2647; Sun, 13 Aug 2017 02:05:57 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Aug 2017 17:05:56 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,365,1498546800"; d="scan'208";a="138948929" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga005.fm.intel.com with ESMTP; 12 Aug 2017 17:05:56 -0700 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sat, 12 Aug 2017 17:05:56 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sat, 12 Aug 2017 17:05:56 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.236]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.128]) with mapi id 14.03.0319.002; Sun, 13 Aug 2017 08:05:54 +0800 From: "Zhang, Qi Z" To: Kevin Traynor , "Wu, Jingjing" CC: "dev@dpdk.org" , "stable@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH] net/i40e: fix flow control watermark mismatch Thread-Index: AQHTEYSYxfg5tvGYBUqFZDd3uBTrA6J9Ld4AgAQ2hjA= Date: Sun, 13 Aug 2017 00:05:53 +0000 Message-ID: <039ED4275CED7440929022BC67E70611530CE060@SHSMSX103.ccr.corp.intel.com> References: <20170810104807.59436-1-qi.z.zhang@intel.com> In-Reply-To: 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: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/i40e: fix flow control watermark mismatch X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 13 Aug 2017 00:05:59 -0000 Hi Kevin: > -----Original Message----- > From: Kevin Traynor [mailto:ktraynor@redhat.com] > Sent: Thursday, August 10, 2017 11:16 PM > To: Zhang, Qi Z ; Wu, Jingjing > Cc: dev@dpdk.org; stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix flow control watermark > mismatch >=20 > On 08/10/2017 11:48 AM, Qi Zhang wrote: > > Flow control watermark is not read out correctly, that may cause an > > application who not intend to change watermark but does change it with > > a rte_eth_dev_flow_ctrl_set call right after > > rte_eth_dev_flow_ctrl_get. > > > > Fixes: f53577f06925 ("i40e: support flow control") > > Cc: stable@dpdk.org > > > > Signed-off-by: Qi Zhang > > --- > > drivers/net/i40e/i40e_ethdev.c | 19 +++++++++++++------ > > 1 file changed, 13 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/i40e/i40e_ethdev.c > > b/drivers/net/i40e/i40e_ethdev.c index 71cb7d3..8b008c4 100644 > > --- a/drivers/net/i40e/i40e_ethdev.c > > +++ b/drivers/net/i40e/i40e_ethdev.c > > @@ -86,12 +86,6 @@ > > /* Flow control default timer */ > > #define I40E_DEFAULT_PAUSE_TIME 0xFFFFU > > > > -/* Flow control default high water */ -#define > > I40E_DEFAULT_HIGH_WATER (0x1C40/1024) > > - > > -/* Flow control default low water */ > > -#define I40E_DEFAULT_LOW_WATER (0x1A40/1024) > > - > > /* Flow control enable fwd bit */ > > #define I40E_PRTMAC_FWD_CTRL 0x00000001 > > > > @@ -101,6 +95,12 @@ > > /* Kilobytes shift */ > > #define I40E_KILOSHIFT 10 > > > > +/* Flow control default high water */ #define > I40E_DEFAULT_HIGH_WATER > > +(0xF2000 >> I40E_KILOSHIFT) > > + > > +/* Flow control default low water */ > > +#define I40E_DEFAULT_LOW_WATER (0xF2000 >> I40E_KILOSHIFT) > > + >=20 > I'm not sure these are needed anymore. Would seem if you want a correct > value prior to flow_ctl_get() being called, the registers should also be = read > during init. Is there a need for the PMD to have the correct value prior = to > flow_ctrl_get() ? I didn't see one. You are right, the value here does not take effect based on current impleme= ntation, I just correct it to align with datasheet. I think it's not necessary to remove it here, it may be used in future. The idea fix is: during init, the watermark is set with default value, so i= t is not necessary to read out from hw register during flow_ctl_get,=20 But due to I40E_GLRPB_GHW limitation, it is shared by different ports on th= e same device, it is possible the value is changed on another port, but loc= al variable not sync, so we have to read out register every flow_ctl_get. Regards Qi >=20 > > /* Receive Average Packet Size in Byte*/ #define > > I40E_PACKET_AVERAGE_SIZE 128 > > > > @@ -3199,6 +3199,13 @@ i40e_flow_ctrl_get(struct rte_eth_dev *dev, > struct rte_eth_fc_conf *fc_conf) > > struct i40e_pf *pf =3D > I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); > > > > fc_conf->pause_time =3D pf->fc_conf.pause_time; > > + > > + /* read out from register, in case they are modified by other port */ > > + pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS] =3D > > + I40E_READ_REG(hw, I40E_GLRPB_GHW) >> I40E_KILOSHIFT; > > + pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS] =3D > > + I40E_READ_REG(hw, I40E_GLRPB_GLW) >> I40E_KILOSHIFT; > > + > > fc_conf->high_water =3D > pf->fc_conf.high_water[I40E_MAX_TRAFFIC_CLASS]; > > fc_conf->low_water =3D > pf->fc_conf.low_water[I40E_MAX_TRAFFIC_CLASS]; > > > >