> -----Original Message-----
> From: Wang, Haiyue
> Sent: Sunday, November 3, 2019 10:34
> To: Slava Ovsiienko <viachesl...@mellanox.com>; Liu, Yu Y
> <yu.y....@intel.com>; Thomas Monjalon
> <tho...@monjalon.net>
> Cc: dev@dpdk.org; arybche...@solarflare.com; Yigit, Ferruh
> <ferruh.yi...@intel.com>;
> jerinjac...@gmail.com; Ye, Xiaolong <xiaolong...@intel.com>; Kinsella, Ray
> <ray.kinse...@intel.com>;
> Sun, Chenmin <chenmin....@intel.com>; Damjan Marion (damarion)
> <damar...@cisco.com>
> Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst mode
> information
>
> Hi Thomas, Slava,
>
> Please see the inline reply in one place.
>
> > -----Original Message-----
> > From: Slava Ovsiienko <viachesl...@mellanox.com>
> > Sent: Saturday, November 2, 2019 16:39
> > To: Liu, Yu Y <yu.y....@intel.com>; Wang, Haiyue <haiyue.w...@intel.com>;
> > Thomas Monjalon
> > <tho...@monjalon.net>
> > Cc: dev@dpdk.org; arybche...@solarflare.com; Yigit, Ferruh
> > <ferruh.yi...@intel.com>;
> > jerinjac...@gmail.com; Ye, Xiaolong <xiaolong...@intel.com>; Kinsella, Ray
> > <ray.kinse...@intel.com>;
> > Sun, Chenmin <chenmin....@intel.com>; Damjan Marion (damarion)
> > <damar...@cisco.com>
> > Subject: RE: [PATCH v1 3/3] ethdev: enhance the API for getting burst mode
> > information
> >
> > Hi
> > > -----Original Message-----
> > > From: Liu, Yu Y <yu.y....@intel.com>
> > > Sent: Saturday, November 2, 2019 8:56
> > > To: Wang, Haiyue <haiyue.w...@intel.com>; Thomas Monjalon
> > > <tho...@monjalon.net>
> > > Cc: dev@dpdk.org; arybche...@solarflare.com; Yigit, Ferruh
> > > <ferruh.yi...@intel.com>; jerinjac...@gmail.com; Ye, Xiaolong
> > > <xiaolong...@intel.com>; Kinsella, Ray <ray.kinse...@intel.com>; Sun,
> > > Chenmin <chenmin....@intel.com>; Slava Ovsiienko
> > > <viachesl...@mellanox.com>; Damjan Marion (damarion)
> > > <damar...@cisco.com>; Liu, Yu Y <yu.y....@intel.com>
> > > 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 community
> > > 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 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.
> >
> > 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.
> >
> > We could consider some requirements for the name suffices to distinguish
> > 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 burst
> > routines?
> > Is it going to share/preserve some resources (registers, etc.)? Is it
> > robust ?
> > Burst routines might not know whether vector extensions is used (they might
> > call
> > libraries, even rte_memcpy() can use vectors in implicit fashion).
> >
>
> 1.
>
> 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."
>
> 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 == ice_recv_scattered_pkts_vec_avx2)
> + len = snprintf(buf, sz, "AVX2 Vector Scattered Rx");
> + else if (dev->rx_pkt_burst == ice_recv_scattered_pkts_vec)
> + len = snprintf(buf, sz, "Vector Scattered Rx");
> + else if (dev->rx_pkt_burst == ice_recv_pkts_vec_avx2)
> + len = snprintf(buf, sz, "AVX2 Vector Rx");
> + else if (dev->rx_pkt_burst == ice_recv_pkts_vec)
> + len = 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 variety
> 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 known
> by
> the application.
>
> 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
> more,
> 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:
> "This 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_mode
> *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'
> firstly, assign
> 'RTE_ETH_BURST_VECTOR | RTE_ETH_BURST_SSE' to 'options' as needed, then
> finally,
> '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
> standardization, just
> want to reduce the duplicated name string things.
>
Also, one more thing about 'RTE_ETH_BURST_PER_QUEUE', I added new comment to
make it
be more understandable, how about this ?
/**< If the Rx/Tx queues have different burst mode description, then PMD return
* this bit, so the application can enumerate all queues burst mode description
* 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 burst
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 support
> > > it
> > > will be even better.
> > >
> > > Your support will be highly appreciated!
> > >
> > > Thanks & Regards,
> > > Yu Liu
> > >
> > > -----Original Message-----
> > > From: dev <dev-boun...@dpdk.org> On Behalf Of Wang, Haiyue
> > > Sent: Saturday, November 2, 2019 1:30 PM
> > > To: Thomas Monjalon <tho...@monjalon.net>
> > > Cc: dev@dpdk.org; arybche...@solarflare.com; Yigit, Ferruh
> > > <ferruh.yi...@intel.com>; jerinjac...@gmail.com; Ye, Xiaolong
> > > <xiaolong...@intel.com>; Kinsella, Ray <ray.kinse...@intel.com>; Sun,
> > > Chenmin <chenmin....@intel.com>; Slava Ovsiienko
> > > <viachesl...@mellanox.com>
> > > Subject: Re: [dpdk-dev] [PATCH v1 3/3] ethdev: enhance the API for getting
> > > burst mode information
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <tho...@monjalon.net>
> > > > Sent: Saturday, November 2, 2019 06:46
> > > > To: Wang, Haiyue <haiyue.w...@intel.com>
> > > > Cc: dev@dpdk.org; arybche...@solarflare.com; Yigit, Ferruh
> > > > <ferruh.yi...@intel.com>; jerinjac...@gmail.com; Ye, Xiaolong
> > > > <xiaolong...@intel.com>; Kinsella, Ray <ray.kinse...@intel.com>; Sun,
> > > > Chenmin <chenmin....@intel.com>; Slava Ovsiienko
> > > > <viachesl...@mellanox.com>
> > > > 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 prefer
> > > to do the
> > > best, that's why open source is amazing, thanks! ;-)
