19/09/2017 09:33, Shahaf Shuler: > 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 think we must forbid setting offloads partially. If one setting is not possible, nothing should be done. I don't understand why it would complicate the "set" function. Anyway, we must report errors in "set" function. One more question is: do we want to return a mask of "accepted" offload by getting the mask as a pointer? > 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. No strong opinion about error return in "get" function. It is probably reasonnable to distinguish offload value 0 and "get" function not implemented. > > > 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. If, as a community, we decide that configuring offloads on the fly is not a requirement, OK to not plan it in the API. If we decide to not do it now, we could change again the API later. Opinions?
