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 80AFAA034F; Sun, 3 Nov 2019 03:34:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AC4681E53C; Sun, 3 Nov 2019 03:34:13 +0100 (CET) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id DDD6B1E4EA for ; Sun, 3 Nov 2019 03:34:11 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 02 Nov 2019 19:34:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,261,1569308400"; d="scan'208";a="191392550" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga007.jf.intel.com with ESMTP; 02 Nov 2019 19:34:10 -0700 Received: from fmsmsx161.amr.corp.intel.com (10.18.125.9) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 2 Nov 2019 19:34:09 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX161.amr.corp.intel.com (10.18.125.9) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sat, 2 Nov 2019 19:34:09 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.2]) with mapi id 14.03.0439.000; Sun, 3 Nov 2019 10:34:07 +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//5LDgIAAHLKAgAGgxyA= Date: Sun, 3 Nov 2019 02:34:06 +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" Hi Thomas, Slava, Please see the inline reply in one place. > -----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, 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 > > -----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 m= ode > > information > > > > Add Damjan from FD.io for awareness... > > > > Hi Thomas, > > > > Long time no see. Sorry I use outlook which is not friendly to communit= y > > 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 h= ave 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 mitigate. > > Please be rest assured it is not the case. > > This request is just from one FD.io project internal bug " tx/rx burst = function > > is shown as nil" reported by Chenmin. >=20 > 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, dropping= the entire feature > does not look nice, as for me. >=20 > We could consider some requirements for the name suffices to distinguish = whether > function uses vector instructions and which ones if any. >=20 > > My understanding is DPDK behavior was taken as bug for someone in FD.io > > project and potentially will mislead other DPDK consumer. >=20 > Why does FD.io code want to know which vector extension is used by burst = routines? > Is it going to share/preserve some resources (registers, etc.)? Is it rob= ust ? > Burst routines might not know whether vector extensions is used (they mig= ht call > libraries, even rte_memcpy() can use vectors in implicit fashion). >=20 1. The original issue description is: "VPP uses dladdr() to translate a function address to name, however, some t= x/rx functions in DPDK are invisible for dladdr(), which is because they are defined as s= tatic." 2. So the RFC design is: one function, one description, like: https://patchwork.dpdk.org/patch/57644/ +#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 3. Since the main issue is as Damjan replied in another thread: "people are reporting lower performance caused by DPDK deciding for vari= ety of reasons to switch from vector PMD to scalar one." And Ferruh replied also: "As I understand this is to let applications to give informed decision = based on what vectorization is used in the driver, currently this is not kno= wn by the application. =20 And as previously replied, the main target of the API is to define the= vector path, not all optimizations, so the number is limited." So we enhanced it with bit, example detail is (Yes, we defined a lit m= ore, so we removed it in this patch): https://patchwork.dpdk.org/patch/61196/ 4. And thanks Jerin's suggestion, I think his word can be more accurate: "T= his would help to reuse some of the flags to name conversion logic across all PMDs= " 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_mo= de *mode)". Now the new API will return the string finally: #define RTE_ETH_BURST_MODE_ALT_OPT_SIZE 1024 struct rte_eth_burst_mode { uint64_t options; /**< 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]; }; So MLX PMD can add 'full_empw', 'mtsc_empw' etc into 'alternate_options' fi= rstly, assign 'RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE' to 'options' as needed, then fin= ally, 'alternate_options' will be "full_empw, Vector SSE". Intel PMD can just assign "options", then finally, 'alternate_options' will= be "Vector SSE". How about the design idea ? Again, this 'options' is not to do standardizat= ion, just want to reduce the duplicated name string things. > With best regards, Slava >=20 > > Haiyue is working with Chenmin to address the issue and with your suppo= rt 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 gett= ing > > 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 ; Sun, > > > Chenmin ; Slava Ovsiienko > > > > > > Subject: Re: [PATCH v1 3/3] ethdev: enhance the API for getting burst > > > 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 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. > > > > Never mind, if this design is truly ugly, drop it all now. I also prefe= r to do the > > best, that's why open source is amazing, thanks! ;-)