Re: [dpdk-dev] [PATCH 4/4] ethdev: add helpers to move to the new offloads API

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Sep 18, 2017 at 01:32:29PM +0200, Thomas Monjalon wrote:
> 18/09/2017 13:11, Ananyev, Konstantin:
> > From: Richardson, Bruce
> > > On Mon, Sep 18, 2017 at 11:57:03AM +0100, Ananyev, Konstantin wrote:
> > > > From: Richardson, Bruce
> > > > > On Thu, Sep 14, 2017 at 10:02:26AM +0200, Thomas Monjalon wrote:
> > > > > > 13/09/2017 23:42, Ananyev, Konstantin:
> > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > 13/09/2017 14:56, Ananyev, Konstantin:
> > > > > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > > > > > Konstantin, I would like your opinion about the proposal below.
> > > > > > > > It is about making on the fly configuration more generic.
> > > > > > > > You say it is possible to configure VLAN on the fly,
> > > > > > > > and I think we should make it possible for other offload features.
> > > > > > >
> > > > > > > It would be a good thing, but I don't think it is possible for all offloads.
> > > > > > > For some of them you still have to stop the queue(port) first.
> [...]
> [technical details skipped]
> [...]
> > > > > > > If so, then it seems reasonable to me.
> > > > > >
> > > > > > Good, thank you
> > > > > >
> > > > > >
> > > > > Sorry I'm a bit late to the review, but the above suggestion of separate
> > > > > APIs for enabling offloads, seems much better than passing in the flags
> > > > > in structures to the existing calls. From what I see all later revisions
> > > > > of this patchset still use the existing flags parameter to setup calls
> > > > > method.
> > > > >
> > > > > Some advantages that I see of the separate APIs:
> > > > > * allows some settings to be set before start, and others afterwards,
> > > > >   with an appropriate return value if dynamic config not supported.
> > > > > * we can get fine grained error reporting from these - the set calls can
> > > > >   all return the mask indicating what offloads could not be applied -
> > > > >   zero means all ok, 1 means a problem with that setting. This may be
> > > > >   easier for the app to use than feature discovery in some cases.
> > > > > * for those PMDs which support configuration at a per-queue level, it
> > > > >   can allow the user to specify the per-port settings as a default, and
> > > > >   then override that value at the queue level, if you just want one queue
> > > > >   different from the rest.
> > > >
> > > > I think we all in favor to have a separate API here.
> > > > Though from the discussion we had at latest TB, I am not sure it is doable
> > > > in 17.11 timeframe.
> > > 
> > > Ok, so does that imply no change in this release, and that the existing
> > > set is to be ignored?
> > 
> > No, my understanding the current plan is to go forward with Shahaf patches,
> > and then apply another one (new set/get API) on top of them.
> 
> Yes, it is what we agreed (hope to see it in minutes).
> If someone can do these new patches in 17.11 timeframe, it's great!
> Bruce, do you want to make it a try?

If I have the chance, I can try, but given how short time is and that
userspace is on next week, I very much doubt I'll even get it started.

/Bruce