From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1845EA0352; Sun, 3 Nov 2019 04:52:37 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 604B61DFE3; Sun, 3 Nov 2019 04:52:36 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 5DE2F1DFE2 for ; Sun, 3 Nov 2019 04:52:34 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Nov 2019 20:52:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,261,1569308400"; d="scan'208";a="212068133" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga001.fm.intel.com with ESMTP; 02 Nov 2019 20:52:33 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 2 Nov 2019 20:52:30 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 2 Nov 2019 20:52:30 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.149]) with mapi id 14.03.0439.000; Sun, 3 Nov 2019 11:52:28 +0800 From: "Wang, Haiyue" To: Thomas Monjalon CC: "dev@dpdk.org" , "arybchenko@solarflare.com" , "Yigit, Ferruh" , "jerinjacobk@gmail.com" , "Ye, Xiaolong" , "Kinsella, Ray" , "Sun, Chenmin" , Slava Ovsiienko Thread-Topic: [PATCH v1 3/3] ethdev: enhance the API for getting burst mode information Thread-Index: AQHVkA85NRUGBSr/iU+DJBHluQAAgad2ZiSAgAJqrBA= Date: Sun, 3 Nov 2019 03:52:27 +0000 Message-ID: References: <20191031171139.105110-1-haiyue.wang@intel.com> <20191031171139.105110-3-haiyue.wang@intel.com> <20693558.VL3dRorq05@xps> In-Reply-To: <20693558.VL3dRorq05@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTAxNjA5NmMtMjdiYy00ZmNhLWFmOGMtMjhhNDkwNjQ2ZGQ3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiN3dwTVdkYjZRcHVvN3lTeDlmQk1PTEVXZ2NleTFseVpYalBKNTRLZ3NFSnA4M0kzc0tKc1dJbitERzVrYVRcL0UifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 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] [PATCH v1 3/3] ethdev: enhance the API for getting burst mode information 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Thomas, Reply the missed: > -----Original Message----- > From: Thomas Monjalon > Sent: Saturday, November 2, 2019 06:46 > To: Wang, Haiyue > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh ; > jerinjacobk@gmail.com; Ye, Xiaolong ; Kinsella, Ra= y ; > Sun, Chenmin ; Slava Ovsiienko > Subject: Re: [PATCH v1 3/3] ethdev: enhance the API for getting burst mod= e information >=20 > Thank you for trying to address comments done late. >=20 > 31/10/2019 18:11, Haiyue Wang: > > --- a/lib/librte_ethdev/rte_ethdev.h > > +++ b/lib/librte_ethdev/rte_ethdev.h > > -enum rte_eth_burst_mode_option { > > - RTE_ETH_BURST_SCALAR =3D (1 << 0), > > - RTE_ETH_BURST_VECTOR =3D (1 << 1), > > - > > - /**< bits[15:2] are reserved for each vector type */ > > - RTE_ETH_BURST_ALTIVEC =3D (1 << 2), > > - RTE_ETH_BURST_NEON =3D (1 << 3), > > - RTE_ETH_BURST_SSE =3D (1 << 4), > > - RTE_ETH_BURST_AVX2 =3D (1 << 5), > > - RTE_ETH_BURST_AVX512 =3D (1 << 6), > > - > > - RTE_ETH_BURST_SCATTERED =3D (1 << 16), /**< Support scattered packets= */ > > - RTE_ETH_BURST_BULK_ALLOC =3D (1 << 17), /**< Support mbuf bulk alloc = */ > > - RTE_ETH_BURST_SIMPLE =3D (1 << 18), > > - > > - RTE_ETH_BURST_PER_QUEUE =3D (1 << 19), /**< Support per queue burst *= / > > -}; > > +#define RTE_ETH_BURST_SCALAR (1ULL << 0) > > +#define RTE_ETH_BURST_VECTOR (1ULL << 1) >=20 > Only one bit is needed: if it is not vector, it is scalar. > I think you can remove the scalar bit. >=20 1. 'options' can be all zero, so we can't assume it is 'scalar', so let's t= he PMD set it explicitly. 2. As replied before, it seems a little redundant, but a bit-opaque definit= ion will make string format easily, like a direct 'for' loop: ---------------------------------- static const struct { uint64_t option; const char *name; } rte_burst_option_names[] =3D { { RTE_ETH_BURST_SCALAR, "Scalar" }, { RTE_ETH_BURST_VECTOR, "Vector" }, { RTE_ETH_BURST_ALTIVEC, "AltiVec" }, { RTE_ETH_BURST_NEON, "Neon" }, ---------------------------------- And in fact, only one vector type will be returned, but if use the bit save= definition like register, there will be something like 'RTE_ETH_BURST_VERTOR_TYPE_MASK= ', this will make PMD and string format harder ... > > +/**< bits[15:2] are reserved for each vector type */ >=20 > Why 15:2 instead of 2:15? >=20 Changed as: /**< bits 2 ~ 15 are reserved for each vector type */ > > +#define RTE_ETH_BURST_ALTIVEC (1ULL << 2) > > +#define RTE_ETH_BURST_NEON (1ULL << 3) > > +#define RTE_ETH_BURST_SSE (1ULL << 4) > > +#define RTE_ETH_BURST_AVX2 (1ULL << 5) > > +#define RTE_ETH_BURST_AVX512 (1ULL << 6) >=20 > Of course, I still believe that giving a special treatment > to vector instructions is wrong. > You did not justify why it needs to be defined in bits > instead of string. I am not asking again because anyway you > don't really reply. I think you are executing an order you received > and I don't want to blame you more. > I suspect a real hidden issue in Intel CPUs that you try to mitigate. > No need to reply to this comment. > Anyway I will propose to replace this API in the next release. >=20 > > +/**< Support per queue burst */ > > +#define RTE_ETH_BURST_PER_QUEUE (1ULL << 63) >=20 > This comment is meaningless. > If this bit has a usage, please explain how to use this bit > in the comment. >=20 Changes as: /**< If the Rx/Tx queues have different burst mode description, the bit wil= l be * set by PMD, then the application can enumerate to retrive burst descript= ion * for all PMD queues. */