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 6D455A034F; Sun, 3 Nov 2019 04:03:37 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C363C1E549; Sun, 3 Nov 2019 04:03:36 +0100 (CET) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 36B1E1E4EA for ; Sun, 3 Nov 2019 04:03:35 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Nov 2019 20:03:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,261,1569308400"; d="scan'208";a="195099123" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga008.jf.intel.com with ESMTP; 02 Nov 2019 20:03:33 -0700 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) 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:03:33 -0700 Received: from shsmsx105.ccr.corp.intel.com (10.239.4.158) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 2 Nov 2019 20:03:32 -0700 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; Sun, 3 Nov 2019 11:03:30 +0800 From: "Wang, Haiyue" To: 'Slava Ovsiienko' , "Liu, Yu Y" , 'Thomas Monjalon' CC: "'dev@dpdk.org'" , "'arybchenko@solarflare.com'" , "Yigit, Ferruh" , "'jerinjacobk@gmail.com'" , "Ye, Xiaolong" , "Kinsella, Ray" , "Sun, Chenmin" , "'Damjan Marion (damarion)'" Thread-Topic: [PATCH v1 3/3] ethdev: enhance the API for getting burst mode information Thread-Index: AQHVkA85NRUGBSr/iU+DJBHluQAAgad2ZiSAgAD2BdD//5LDgIAAHLKAgAGgxyCAABiuYA== Date: Sun, 3 Nov 2019 03:03:29 +0000 Message-ID: References: <20191031171139.105110-1-haiyue.wang@intel.com> <20191031171139.105110-3-haiyue.wang@intel.com> <20693558.VL3dRorq05@xps> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYmRlZWY5NTctNDg0Mi00YWU4LTlmOTEtZTU0NDIxNzRmZDU5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiYnpZRXhvTGc5MWxKMGpLaWJYTEFSVHRoTkdjYk5Qcm5ZZzF0cWd1SFJIRTI0VXNvdEM0dHlrOW9ETUFpc05zSSJ9 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: Wang, Haiyue > Sent: Sunday, November 3, 2019 10:34 > To: Slava Ovsiienko ; Liu, Yu Y ; Thomas Monjalon > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh ; > jerinjacobk@gmail.com; Ye, Xiaolong ; Kinsella, Ra= y ; > Sun, Chenmin ; Damjan Marion (damarion) > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst mod= e information >=20 > Hi Thomas, Slava, >=20 > Please see the inline reply in one place. >=20 > > -----Original Message----- > > From: Slava Ovsiienko > > Sent: Saturday, November 2, 2019 16:39 > > To: Liu, Yu Y ; Wang, Haiyue ; Thomas Monjalon > > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh ; > > jerinjacobk@gmail.com; Ye, Xiaolong ; Kinsella, = Ray ; > > Sun, Chenmin ; Damjan Marion (damarion) > > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst m= ode information > > > > Hi > > > -----Original Message----- > > > From: Liu, Yu Y > > > Sent: Saturday, November 2, 2019 8:56 > > > To: Wang, Haiyue ; Thomas Monjalon > > > > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh > > > ; jerinjacobk@gmail.com; Ye, Xiaolong > > > ; Kinsella, Ray ; Sun, > > > Chenmin ; Slava Ovsiienko > > > ; Damjan Marion (damarion) > > > ; Liu, Yu Y > > > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst= mode > > > information > > > > > > Add Damjan from FD.io for awareness... > > > > > > Hi Thomas, > > > > > > Long time no see. Sorry I use outlook which is not friendly to commun= ity > > > email. > > > > > > >Anyway I will propose to replace this API in the next release. > > > Will your plan be affected by API/ABI stable plan? > > > BTW, if you propose new change in next release, it will make DPDK > > > consumer(FD.io) to change again. > > > So even if it is not affected to the API/ABI stable plan, do we still= have time > > > to get a solution for everyone in DPDK 19.11 with your > > > contribution/acceleration? > > > > > > > I suspect a real hidden issue in Intel CPUs that you try to mitigat= e. > > > Please be rest assured it is not the case. > > > This request is just from one FD.io project internal bug " tx/rx burs= t function > > > is shown as nil" reported by Chenmin. > > > > Why just the presenting string with function name (possible with suffix= ) is not enough? > > I would like to see this API (strings approach) in mlx5 either, droppi= ng the entire feature > > does not look nice, as for me. > > > > We could consider some requirements for the name suffices to distinguis= h whether > > function uses vector instructions and which ones if any. > > > > > My understanding is DPDK behavior was taken as bug for someone in FD.= io > > > project and potentially will mislead other DPDK consumer. > > > > Why does FD.io code want to know which vector extension is used by burs= t routines? > > Is it going to share/preserve some resources (registers, etc.)? Is it r= obust ? > > Burst routines might not know whether vector extensions is used (they m= ight call > > libraries, even rte_memcpy() can use vectors in implicit fashion). > > >=20 > 1. >=20 > The original issue description is: > "VPP uses dladdr() to translate a function address to name, however, some= tx/rx functions > in DPDK are invisible for dladdr(), which is because they are defined as= static." >=20 > 2. >=20 > So the RFC design is: one function, one description, like: > https://patchwork.dpdk.org/patch/57644/ >=20 > +#ifdef RTE_ARCH_X86 > + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec_avx2) > + len =3D snprintf(buf, sz, "AVX2 Vector Scattered Rx"); > + else if (dev->rx_pkt_burst =3D=3D ice_recv_scattered_pkts_vec) > + len =3D snprintf(buf, sz, "Vector Scattered Rx"); > + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec_avx2) > + len =3D snprintf(buf, sz, "AVX2 Vector Rx"); > + else if (dev->rx_pkt_burst =3D=3D ice_recv_pkts_vec) > + len =3D snprintf(buf, sz, "Vector Rx"); > +#endif >=20 > 3. Since the main issue is as Damjan replied in another thread: > "people are reporting lower performance caused by DPDK deciding for va= riety > of reasons to switch from vector PMD to scalar one." >=20 > And Ferruh replied also: > "As I understand this is to let applications to give informed decisio= n based > on what vectorization is used in the driver, currently this is not k= nown by > the application. >=20 > And as previously replied, the main target of the API is to define t= he vector > path, not all optimizations, so the number is limited." >=20 > So we enhanced it with bit, example detail is (Yes, we defined a lit= more, > so we removed it in this patch): > https://patchwork.dpdk.org/patch/61196/ >=20 > 4. And thanks Jerin's suggestion, I think his word can be more accurate: = "This would > help to reuse some of the flags to name conversion logic across all PM= Ds" for the > reason we try to use bit to reduce some string format effort, it will = be handled > by the API internally "burst_mode_options_append(struct rte_eth_burst_= mode *mode)". > Now the new API will return the string finally: >=20 > #define RTE_ETH_BURST_MODE_ALT_OPT_SIZE 1024 > struct rte_eth_burst_mode { > uint64_t options; >=20 > /**< Each PMD can fill specific burst mode information into this, and > * ethdev APIs will append the 'options' string format at its end. > */ > char alternate_options[RTE_ETH_BURST_MODE_ALT_OPT_SIZE]; > }; >=20 > So MLX PMD can add 'full_empw', 'mtsc_empw' etc into 'alternate_options' = firstly, assign > 'RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE' to 'options' as needed, then f= inally, > 'alternate_options' will be "full_empw, Vector SSE". >=20 > Intel PMD can just assign "options", then finally, 'alternate_options' wi= ll be "Vector SSE". >=20 > How about the design idea ? Again, this 'options' is not to do standardiz= ation, just > want to reduce the duplicated name string things. >=20 Also, one more thing about 'RTE_ETH_BURST_PER_QUEUE', I added new comment t= o make it be more understandable, how about this ? /**< If the Rx/Tx queues have different burst mode description, then PMD re= turn * this bit, so the application can enumerate all queues burst mode descrip= tion * as needed. */ #define RTE_ETH_BURST_PER_QUEUE (1ULL << 63) so that, application can know more about PMD's burst information: rte_eth_rx_burst_mode_get(port_id, 0, &mode); /* Use queue Id #0 to get b= urst mode firstly*/ if (mode.options & RTE_ETH_BURST_PER_QUEUE) loop other queues. > > With best regards, Slava > > > > > Haiyue is working with Chenmin to address the issue and with your sup= port it > > > will be even better. > > > > > > Your support will be highly appreciated! > > > > > > Thanks & Regards, > > > Yu Liu > > > > > > -----Original Message----- > > > From: dev On Behalf Of Wang, Haiyue > > > Sent: Saturday, November 2, 2019 1:30 PM > > > To: Thomas Monjalon > > > Cc: dev@dpdk.org; arybchenko@solarflare.com; Yigit, Ferruh > > > ; jerinjacobk@gmail.com; Ye, Xiaolong > > > ; Kinsella, Ray ; Sun, > > > Chenmin ; Slava Ovsiienko > > > > > > Subject: Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for ge= tting > > > burst mode information > > > > > > > -----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, Ray ; Su= n, > > > > Chenmin ; Slava Ovsiienko > > > > > > > > Subject: Re: [PATCH v1 3/3] ethdev: enhance the API for getting bur= st > > > > mode information > > > > > > > > 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 > > > > > > > > > > > +#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 vecto= r > > > > 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 mitigat= e. > > > > No need to reply to this comment. > > > > Anyway I will propose to replace this API in the next release. > > > > > > Never mind, if this design is truly ugly, drop it all now. I also pre= fer to do the > > > best, that's why open source is amazing, thanks! ;-)