On 3/22/2016 6:20 PM, Richardson, Bruce wrote: > On Tue, Mar 22, 2016 at 05:50:28AM +0000, Qiu, Michael wrote: >> On 3/21/2016 11:27 PM, Kyle Larose wrote: >>> On Mon, Mar 21, 2016 at 10:52 AM, Bruce Richardson >>> <bruce.richardson at intel.com> wrote: >>>> On Sun, Mar 20, 2016 at 08:18:57PM +0100, Thomas Monjalon wrote: >>>>> 2016-03-20 14:17, Zhang, Helin: >>>>>> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] >>>>>>> 2016-03-18 10:16, Stephen Hemminger: >>>>>>>> Right now, all those offload features are pretty much unusable in a >>>>>>>> real product without lots and lots of extra codes and huge bug >>>>>>>> surface. It bothers me enough that I would recommend removing much of >>>>>>>> the >>>>>>> filter/offload/ptype stuff from DPDK! >>>>>>> >>>>>>> One of the biggest challenge is to think about a good filtering API. >>>>>>> The offloading has some interaction with the mbuf struct. >>>>>>> >>>>>>> I would like to suggest rewriting ethdev API by keeping it as is for >>>>>>> some time for >>>>>>> compatibility while creating a new one. What about the prefix >>>>>>> dpdk_netdev_ to >>>>>>> progressively replace rte_eth_dev? >>>>>> I totally agree with to add new and generic APIs for user applications. >>>>>> But I don't >>>>>> think we need to remove all current APIs. Generic APIs may not support >>>>>> all advanced >>>>>> hardware features, while specific APIs can. Why not support all? One >>>>>> generic APIs for >>>>>> common users, and others APIs for advanced users. >>>>> Yes we cannot access to every features of a device through generic API. >>>>> Until now we were trying to add an ethdev API for every features even if >>>>> it >>>>> is used by only one driver. >>>>> I think we should allow a direct access to the driver by the applications >>>>> and >>>>> work on generic API only for common features. >>>> Definite +1. >>>> I think that we need to start pushing driver-specific functionality to get >>>> exposed >>>> via a driver's header files. That allow users who want to extract the max >>>> functionality from a particular NIC to do so via those APIs calls, while >>>> not >>>> polluting the generic ethdev layer. >>>> >>> What sort of requirements on ABI/API compatibility would this place on >>> the drivers? I would hope that it would be treated like any other >>> public API within DPDK. I don't think this would be too onerous, but >>> it would require that the drivers be designed to deal with it. (I.e. >>> don't just expose any old internal driver function). >> Why not to implement one simple API with variable arguments, just like >> syscall ioctl() does. And drivers implement it's specific hardware >> features with a feature bit param, and other needed variable arguments. >> >> Thanks, >> Michael > A very much dislike that idea. > * It makes the code much harder to read as you have to closely examine all the > parameters to work out what a function call is actually meant to do.
It's not a big deal, if we have a document. > * It makes it much harder to see that you have an implicit dependency on a > specific device. Having to include a driver specific header file e.g. > i40e.h, > and call a function named e.g. i40e_do_magic_stuff(), makes it pretty > explicit > that you have a dependency on i40e-based hardware Software does not want to bind to specific hardware I think, what about the transportability? > * It prevents the compiler from doing type-checking on parameters and > informing > you of little inconsistencies. Maybe, we could do self-check for the parameters I think. > > For all these reasons, I prefer the device-specific functions option. However, > at the same time, we also need to ensure we have a reasonable set of generic > APIs so that the cases where users are forced to drop down to the lower-level > device-specific primitives are reduced. For software, it do not care which hardware it is, it only cares about what ability you have. Thanks, Michael > Regards, > /Bruce > >>>> On the other hand, I don't like the idea of dpdk_netdev. I think we can >>>> work >>>> within the existing rte_eth_dev framework. >>>> >>>> /Bruce >>>> >>