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

[Shahaf Shuler <shahafs@mellanox.com>]

Tuesday, September 19, 2017 12:09 AM, Thomas Monjalon:
> >
> > It would be simpler, however am not sure invalid port is the only error to
> consider. Another possible error can be the PMD is not supporting this
> function.
> > On that case returning 0 is not good enough. The application cannot know
> why the offload is not set, is it because it is set wrong or the PMD just don't
> support this API (which leads me to my next point).
> 
> We can skip error reporting on "get" functions and rely on "set" functions to
> return error if offload API is not supported or for other miscellaneous errors.

It will complex the set function then. 
Instead of using the return value to understand all offloads were set, the application will need to provide another pointer for the function to understand which offloads were actually set. 

I understand this is nice to use the return value of the get without the need of temporary variable, it will save some lines in the code.
But I think that good error reporting to make the application crystal clear why the offloads on get are 0 wins. 


> > I can commit for mlx5 and mlx4 PMDs. I know other PMD maintainers plan
> to convert their PMDs as well. If we take this approach we must make sure
> they all move.
> 
> We can try to get an agreement from more vendors at Dublin summit.
> If not, we can wait more than one release cycle for late support.

Yes we can discuss on it in Dublin.  Still I want to emphasize my concern:
There is no point in moving PMD to the new API if there is no application to use it . Besides of user applications this refers also to testpmd and other examples on dpdk tree (which I plan to convert on 18.02).  
PMD maintainers may object to this conversion if their PMD still uses the old offloads APIs.

So can we have guarantee from Thomas/Ferruh that this series, as long it is technically OK, will be accepted? Will we force all that object to change their PMDs?

If not, I think this is bad approach to put the API floating around ethdev with no PMD to implement it.



> 
> > There is another possible API (API3):
> > 1. keep the per-port, per-queue configuration.
> > 2. add the get function for better error reporting and visibility for
> application.
> > 3. keep the current on the flight vlan offloading configuration as an
> exception. In case there will be a need to configure more offloads on the
> flight we can move to API2.
> >
> > With API1 I am obviously OK.
> > I agree API2 is more generic.
> > API3 is a nice compromise, if we don't want to force all PMD to convert.
> 
> The question is: do we want to choose a compromise while breaking this
> API?

Maybe compromise is not the right word.

We are striving for the generic API2 which has all the full functionality and generalize API1 by supporting on the fly configuration as well.
Maybe for user applications there is no such use-case. How many application decides on the flight to suddenly change the crc strip or the scatter setting ? 
Moreover, how many PMDs will actually support such on the flight configuration?
How easy will it be for application to work with the API for PMD which don't support on the flight configuration? They will need to try, and if fail stop the port and try again - in that sense there no much benefit for API2. 

Currently we have only the vlan offloads which can be set on the flight and maybe it is more than enough, I don't know, am not familiar with enough application to be sure. 

API3 propose to wait with this approach till we will have a proper use case from users.