Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information

["Wang, Haiyue" <haiyue.wang@intel.com>]

> -----Original Message-----
> From: Slava Ovsiienko [mailto:viacheslavo@mellanox.com]
> Sent: Saturday, October 26, 2019 14:41
> To: Thomas Monjalon <thomas@monjalon.net>; Jerin Jacob <jerinjacobk@gmail.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; dpdk-dev
> <dev@dpdk.org>; Ye, Xiaolong <xiaolong.ye@intel.com>; Kinsella, Ray <ray.kinsella@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Sun, Chenmin <chenmin.sun@intel.com>; Andrew
> Rybchenko <arybchenko@solarflare.com>; Stephen Hemminger <stephen@networkplumber.org>; David Marchand
> <david.marchand@redhat.com>; Jerin Jacob <jerinj@marvell.com>
> Subject: RE: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Saturday, October 26, 2019 1:27
> > To: Jerin Jacob <jerinjacobk@gmail.com>
> > Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Haiyue Wang
> > <haiyue.wang@intel.com>; dpdk-dev <dev@dpdk.org>;
> > xiaolong.ye@intel.com; ray.kinsella@intel.com; Bernard Iremonger
> > <bernard.iremonger@intel.com>; chenmin.sun@intel.com; Andrew
> > Rybchenko <arybchenko@solarflare.com>; Slava Ovsiienko
> > <viacheslavo@mellanox.com>; Stephen Hemminger
> > <stephen@networkplumber.org>; David Marchand
> > <david.marchand@redhat.com>; Jerin Jacob <jerinj@marvell.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst
> > mode information
> >
> > 25/10/2019 18:02, Jerin Jacob:
> > > On Fri, Oct 25, 2019 at 9:15 PM Thomas Monjalon
> > <thomas@monjalon.net> wrote:
> > > > 25/10/2019 16:08, Ferruh Yigit:
> > > > > On 10/25/2019 10:36 AM, Thomas Monjalon wrote:
> > > > > > 15/10/2019 09:51, Haiyue Wang:
> > > > > >> Some PMDs have more than one RX/TX burst paths, add the ethdev
> > > > > >> API that allows an application to retrieve the mode information
> > > > > >> about Rx/Tx packet burst such as Scalar or Vector, and Vector
> > > > > >> technology like AVX2.
> > > > > >
> > > > > > I missed this patch. I and Andrew, maintainers of ethdev, were not
> > CC'ed.
> > > > > > Ferruh, I would expect to be Cc'ed and/or get a notification before
> > merging.
> > > > >
> > > > > It has been discussed in the mail list and went through multiple
> > > > > discussions, patch is out since the August, +1 to cc all
> > > > > maintainers I missed that part, but when the patch is reviewed and
> > there is no objection, why block the merge?
> > > >
> > > > I'm not saying blocking the merge.
> > > > My bad is that I missed the patch and I am asking for help with a
> > > > notification in this case. Same for Andrew I guess.
> > > > Note: it is merged in master and I am looking to improve this feature.
> > >
> > > > > >> +/**
> > > > > >> + * Ethernet device RX/TX queue packet burst mode information
> > structure.
> > > > > >> + * Used to retrieve information about packet burst mode setting.
> > > > > >> + */
> > > > > >> +struct rte_eth_burst_mode {
> > > > > >> +  uint64_t options;
> > > > > >> +};
> > > > > >
> > > > > > Why a struct for an integer?
> > > > >
> > > > > Again by a request from me, to not need to break the API if we
> > > > > need to add more thing in the future.
> > > >
> > > > I would replace it with a string. This is the most flexible API.
> > >
> > > IMO, Probably, best of both worlds make a good option here, as Haiyue
> > > suggested if we have an additional dev_specific[1] in structure.
> > > and when a pass to the application, let common code make final string
> > > as (options flags to string + dev_specific)
> > >
> > > options flag can be zero if PMD does not have any generic flags nor
> > > interested in such a scheme.
> > > Generic flags will help at least to have some common code.
> > >
> > > [1]
> > > struct rte_eth_burst_mode {
> > >         uint64_t options;
> > >         char dev_specific[128]; /* PMD has specific burst mode
> > > information */ };
> >
> > I really don't see how we can have generic flags.
> > The flags which are proposed are just matching the functions implemented in
> > Intel PMDs.
> > And this is a complicate solution.
> > Why not just returning a name for the selected Rx/Tx mode?
> >
> +1
> Just an example, mlx5 PMD now has  approx. 60 tx_burst routines and most of these ones
> are not related to vectorized variations (but to tx offloads). Returning  the name would be more
> flexible and informative.
> 
> It is very common case to ask customer about which tx routine is engaged with particular settings,
> now we have to ask to build the debug version and present the log.
> 

Yes, this API is from that VPP can't get the static function by default compile options.
Current design is from CPU's view like vectorized, mbuf bulk allocation. We think it can
be bit data. Name is not good for this, you have to strcat them for different combinations.

But for hardware specific, name is a good one. We didn't design this API for this at
first. ;-)



> With best regards, Slava