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 7913CA0353; Mon, 4 Nov 2019 01:51:57 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C872E1BE8B; Mon, 4 Nov 2019 01:51:56 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id A085F1BDAC for ; Mon, 4 Nov 2019 01:51:55 +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 fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 03 Nov 2019 16:51:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,265,1569308400"; d="scan'208";a="212205662" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga001.fm.intel.com with ESMTP; 03 Nov 2019 16:51:54 -0800 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 3 Nov 2019 16:51:54 -0800 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by FMSMSX151.amr.corp.intel.com (10.18.125.4) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 3 Nov 2019 16:51:48 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by SHSMSX105.ccr.corp.intel.com ([169.254.11.225]) with mapi id 14.03.0439.000; Mon, 4 Nov 2019 08:51:46 +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+DJBHluQAAgad2ZiSAgAJqrBCAALLZgIAArVPQ Date: Mon, 4 Nov 2019 00:51:45 +0000 Message-ID: References: <20191031171139.105110-1-haiyue.wang@intel.com> <20693558.VL3dRorq05@xps> <65534969.fODW5VeCLa@xps> In-Reply-To: <65534969.fODW5VeCLa@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMmMzMTdhZDUtMDE2Mi00ZDYwLWI5M2EtNDY5ZGM4YTcxMjc4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiK096UmdkVEk2c3FXeVNcLytDUSs0eWhXNWQxTlYwaktLXC9sWUM4Vmo1Sm5RTDRHcFVYTjhhdlB3dnc3Tk13MjFkIn0= 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" > -----Original Message----- > From: Thomas Monjalon > Sent: Monday, November 4, 2019 06:21 > 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 > 03/11/2019 04:52, Wang, Haiyue: > > Hi Thomas, > > > > Reply the missed: > > > > From: Thomas Monjalon > > > > > > Thank you for trying to address comments done late. > > > > > > 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 pac= kets */ > > > > - RTE_ETH_BURST_BULK_ALLOC =3D (1 << 17), /**< Support mbuf bulk al= loc */ > > > > - RTE_ETH_BURST_SIMPLE =3D (1 << 18), > > > > - > > > > - RTE_ETH_BURST_PER_QUEUE =3D (1 << 19), /**< Support per queue bur= st */ > > > > -}; > > > > +#define RTE_ETH_BURST_SCALAR (1ULL << 0) > > > > +#define RTE_ETH_BURST_VECTOR (1ULL << 1) > > > > > > Only one bit is needed: if it is not vector, it is scalar. > > > I think you can remove the scalar bit. > > > > > > > 1. 'options' can be all zero, so we can't assume it is 'scalar', so let= 's the PMD > > set it explicitly. >=20 > So what means zero? neither scalar nor vector? >=20 No log output, since PMD doesn't want to say something. > > 2. As replied before, it seems a little redundant, but a bit-opaque def= inition > > 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 ... >=20 > I don't understand what is hard in parsing a bit. > But I'm probably not skilled enough. >=20 > Also I would be interested to understand what means scalar exactly here? > It is processing packets one by one? > And vector, does it mean four packets at once? > How can I differentiate a function which is processing packets two packet= s at once? >=20 About vector, What I know is : git log -p drivers/net/mlx5/mlx5_rxtx_vec_al= tivec.h For scalar, what I know is the function written by pure C syntax. > > > > +/**< bits[15:2] are reserved for each vector type */ > > > > > > Why 15:2 instead of 2:15? > > > > 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) > > > > > > 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. > > > > > > > +/**< Support per queue burst */ > > > > +#define RTE_ETH_BURST_PER_QUEUE (1ULL << 63) > > > > > > This comment is meaningless. > > > If this bit has a usage, please explain how to use this bit > > > in the comment. > > > > > > > Changes as: > > > > /**< If the Rx/Tx queues have different burst mode description, the bit= will be > > * set by PMD, then the application can enumerate to retrive burst desc= ription > > * for all PMD queues. > > */ >=20 > OK this is a better description, thanks. > Indeed this parameter looks to be a good idea. > If we would drop this bit-field, the per-queue bit could be > a parameter of the function. >=20 I've no objection to drop bit-field. If the experimental code can have some= thing good, that's fine enough.