On 04/06/2018 06:51 AM, Jose Abreu wrote: > Hi Florian, > > On 05-04-2018 16:50, Florian Fainelli wrote: >> >> On 04/05/2018 03:47 AM, Jose Abreu wrote: >>> Hi All, >>> >>> I would like to know your opinion regarding adding support for >>> driver private ioctl's in ethtool. >>> >>> Background: Synopsys Ethernet IP's have a certain number of >>> features which can be reconfigured at runtime. Giving you two >>> examples: One of the most recent one is the safety features, >>> which can be enabled/disabled and forced at runtime. Another one >>> is a Flexible RX Parser which can route specific packets to >>> specific RX DMA channels. Given that these are features specific >>> to our IP's it would not be useful to add an uniform API for this >>> because the users would only be one or two drivers ... >> Parsing of packets and directing the matched packets to specific >> queues/channels can be done through ethtool rxnfc API, tc/cls_flower as >> well, so you should really check whether those APIs don't already allow >> you to do what you want. > > Hmm, but in our case this is directly done by HW, we just have to > program a kind of a table which will route automatically the > packets. Does this API support this?
I was sort of expecting you to look at the ethtool rxnfc API to see if it is suitable given your hardware, but if this is indeed a table programming, then yes, this is what it is designed for. You might want to consider using the newer, albeit more complex tc/cls_flower if that works for your use case. > >> >> ethtool already supports a concept of private flags, not ioctl() though >> which allows you to toggle boolean values for instance (or technically >> up to how many bits a "flag" is used to represent) is that enough or do >> you need to turn on/off the feature as well as pass configuration >> parameters? > > Some of them I can just turn on/off but the remaining need > configuration and sometimes the configuration is extensive (like > in the case of RX Parser when we have to pass the routing table). > >> >>> This new feature would change the help usage for ethtool so that >>> each driver private option would be shown, and then each driver >>> specific file would have a structure with all the available >>> options. Finally, each driver would have to handle the private >>> IOCTL's. >>> >>> We already have this working locally and now I would like to know >>> your opinion about upstreaming this ... Do you think this can be >>> useful for anyone else? Or should we change direction to use, for >>> example, debugfs/configfs? >> In general, even if there is only one driver implementing a particular >> feature, the approach chosen is to come up with an API that is as >> generic as possible. Even if there is a single user of that API in tree, >> having something that was thought to be generic is better than allowing >> uncontrolled private ioctl() implementations. > > I understand your point of view but this seems like an overkill > to the -net subsystem because its specific to our IP, or are you > just mentioning a new ethtool entry? i.e. adding a new #define to > the list, plus -net handling ... It depends on the feature, it can be a new set of defines just like it can be a completely new ethtool command number with custom data structures between user and kernel space. -- Florian
