From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 4E3571BBA5 for ; Wed, 27 Jun 2018 15:38:42 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Jun 2018 06:38:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,279,1526367600"; d="scan'208";a="211594296" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 27 Jun 2018 06:38:40 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 27 Jun 2018 06:38:38 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.51]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.116]) with mapi id 14.03.0319.002; Wed, 27 Jun 2018 21:38:37 +0800 From: "Zhang, Qi Z" To: Olivier Matz , "Zhao1, Wei" CC: "dev@dpdk.org" , "Lu, Wenzhuo" Thread-Topic: [RFC] net/ixgbe: fix Tx descriptor status api Thread-Index: AQHUDI/N6CJBq1nb7EuS2EcP2UBJnaRxPYAAgAB3hACAAmbyUA== Date: Wed, 27 Jun 2018 13:38:36 +0000 Message-ID: <039ED4275CED7440929022BC67E706115323F421@SHSMSX103.ccr.corp.intel.com> References: <20180625142057.6296-1-olivier.matz@6wind.com> <20180626084608.c4kfsc5ccnfwypgw@platinum> In-Reply-To: <20180626084608.c4kfsc5ccnfwypgw@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzk2NTQ4N2YtNTkwMS00MWRhLWI5ZjMtNDhlY2RmZDAxOTJlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoia0dVeGlScnMrcmZVXC9pRlFGdHEyNG9yRnU4MUpzY3BQSDBTaUpveVFJak5nN3kwVDVNbklKTUU5bE1HS3FESUwifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 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-dev] [RFC] net/ixgbe: fix Tx descriptor status api 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: Wed, 27 Jun 2018 13:38:44 -0000 Hi Oliver > -----Original Message----- > From: Olivier Matz [mailto:olivier.matz@6wind.com] > Sent: Tuesday, June 26, 2018 4:46 PM > To: Zhao1, Wei > Cc: Zhang, Qi Z ; dev@dpdk.org; Lu, Wenzhuo > > Subject: Re: [RFC] net/ixgbe: fix Tx descriptor status api >=20 > Hi Wei, >=20 > On Tue, Jun 26, 2018 at 01:38:22AM +0000, Zhao1, Wei wrote: > > Hi, Olivier Matz > > > > Will you commit fix patch for i40e and ixgbe and em? >=20 > If you think the patch are relevant, yes :) >=20 > Here is a pre-version (last 5 patches): > http://git.droids-corp.org/?p=3Ddpdk.git;a=3Dshortlog;h=3Drefs/heads/tx-d= esc >=20 > It still need to fix checkpatch issues, few more tests, and rebase on nex= t-net. >=20 > > And the code " dd =3D (desc / txq->tx_rs_thresh + 1) * txq->tx_rs_thres= h - 1;" > > Is only proper for tx function ixgbe_xmit_pkts_simple and > ixgbe_xmit_pkts_vec (). > > But not proper for ixgbe_xmit_pkts (), the RS bit set rule is different= from all > these two. >=20 > Can you please give more detail please? I think the problem in ixgbe_xmit_pkts is, we cannot guarantee tx_rs_thresh= *n -1 will always get RS bit. Because the next tx_rs_thresh cycle counted start from the last segment of = the packet that cross the boundary, it could be any value.=20 So probably we should return -ENOSUP if pkt_tx_burst =3D ixgbe_xmit_pkts, = or we need to change the method that write RS in ixgbe_xmit_pkts. Btw, yes, we should look before the tail but not after, you fix is correct. Thanks Qi =20 >=20 > Note this code, maybe you are talking about this? >=20 > + /* In full featured mode, RS bit is only set in the last descript= or */ > + /* of a multisegments packet */ > + if (!((txq->offloads =3D=3D 0) && > + (txq->tx_rs_thresh >=3D RTE_PMD_IXGBE_TX_MAX_BURST))) > + dd =3D txq->sw_ring[dd].last_id; >=20 > Maybe there is something better to test? >=20 > Just to ensure we are on the same line, here are some more infos. >=20 > =3D=3D=3D >=20 > - sw advances the tail pointer > - hw advances the head pointer > - the software populates the ring with full buffers to be sent by > the hw > - head points to the in-progress descriptor. > - sw writes new descriptors at tail > - head =3D=3D tail means that the transmit queue is empty > - when the hw has processed a descriptor, it sets the DD bit if > the descriptor has the RS (report status) bit. > - the driver never reads the head (needs a pci transaction), instead it m= onitors > the DD bit of a descriptor that has the RS bit >=20 > txq->tx_tail: sw value for tail register > txq->tx_free_thresh: free buffers if count(free descs) < this value > txq->tx_rs_thresh: RS bit is set every rs_thresh descriptor > txq->tx_next_dd: next desc to scan for DD bit > txq->tx_next_rs: next desc to set RS bit > txq->last_desc_cleaned: last descriptor that have been cleaned > txq->nb_tx_free: number of free descriptors >=20 > Example: >=20 > |----------------------------------------------------------------| > | D R R R > | > | ............xxxxxxxxxxxxxxxxxxxxxxxxx | > | <- descs not sent yet -> | > | ............xxxxxxxxxxxxxxxxxxxxxxxxx | > |----------------------------------------------------------------| > ^last_desc_cleaned=3D8 ^next_rs=3D47 > ^next_dd=3D15 ^sw_tail=3D45 > ^hw_head=3D20 >=20 > <---- nb_used ---------> >=20 > The hardware is currently processing the descriptor 20 'R' means the > descriptor has the RS bit 'D' means the descriptor has the DD + RS bits '= x' are > packets in txq (not sent) '.' are packet already sent but not freed by sw >=20 > In this example, we have rs_thres=3D8. On next call to ixgbe_tx_free_bufs= (), > some buffers will be freed. >=20 > =3D=3D=3D >=20 > Let's call ixgbe_dev_tx_descriptor_status(10): >=20 >=20 > - original version: >=20 > desc =3D 45 + 10 =3D 55 > desc =3D ((55 + 8 - 1) / 8) * 8 =3D (62 / 8) * 8 =3D 56 >=20 > wrong because it goes in the wrong direction, and because > 56 does not have the RS bit >=20 > - after your patch: >=20 > desc =3D 45 + 10 =3D 55 > desc =3D (((55 / 8) + 1) * 8) - 1 =3D (7 * 8) - 1 =3D 55 >=20 > wrong because it goes in the wrong direction >=20 > - after my patch >=20 > desc =3D 45 - 10 - 1 =3D 34 > desc =3D (((34 / 8) + 1) * 8) - 1 =3D (5 * 8) - 1 =3D 39 >=20 > looks correct >=20 >=20 >=20 > Regards, > Olivier