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

From: Jerin Jacob <jerinjacobk@gmail.com>
To: Thomas Monjalon <thomas@monjalon.net>
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
Date: Fri, 25 Oct 2019 21:32:58 +0530	[thread overview]
Message-ID: <CALBAE1MMo5oQPR9z5b1DszPuNe4UHjXM7B8PsJojJzJp6VFt-g@mail.gmail.com> (raw)
In-Reply-To: <1811898.7XjjD7ZjLQ@xps>

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 */
};