Re: [RFC] ethtool: Support for driver private ioctl's
Hi Florian, On 07-04-2018 20:58, Florian Fainelli wrote: > > 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. > I took a quick look at rxrnfc API and it doesn't seem to match entirely my requirements. The feature I want to introduce is called Flexible RX Parser and will let me route specific packets to specific DMA channels number. This is different from rxrnfc API because, and I far as I understand, the API was designed to add rules to packet types whilst in my case I can add a rule to *any* of the packet content (within the first 256 bytes of packet, at max). So technically I can route packets based on destination/ source mac address, packet type, lenght, protocol, source/destination IP , So, I guess cls_flower it will be ... I'm slowing looking at some code and docs but it would be great if you could pin point me to some HW that has similar behavior ? Thanks and Best Regards, Jose Miguel Abreu
Re: [RFC] ethtool: Support for driver private ioctl's
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
Re: [RFC] ethtool: Support for driver private ioctl's
Hi Andrew, On 06-04-2018 15:47, Andrew Lunn wrote: > On Fri, Apr 06, 2018 at 02:51:15PM +0100, 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. > Hi Jose > > Is there a reason somebody would decide to use the Ethernet in > 'unsafe' mode? Cannot you just turn it on by default? Yes, its already on by default. I was just trying to give an example of an user-reconfigurable feature, maybe it was not the best one :) Thanks and Best Regards, Jose Miguel Abreu > >Andrew
Re: [RFC] ethtool: Support for driver private ioctl's
On Fri, Apr 06, 2018 at 02:51:15PM +0100, 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. Hi Jose Is there a reason somebody would decide to use the Ethernet in 'unsafe' mode? Cannot you just turn it on by default? Andrew
Re: [RFC] ethtool: Support for driver private ioctl's
Hi Michal, On 06-04-2018 10:07, Michal Kubecek wrote: > On Thu, Apr 05, 2018 at 08:50:49AM -0700, Florian Fainelli wrote: >> On 04/05/2018 03:47 AM, Jose Abreu wrote: >>> 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. >> >> 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? > Perhaps introducing "driver/device specific tunables" (i.e. something > like tunables or PHY tunables but specific to a particular device) could > be a way. But it could get out of control quickly and users wouldn't be > happy if they had to set the same (or almost the same) parameter under > five different names for five NIC vendors. Yeah, that wouldn't be good but I think this should be a responsibility to developer: To see if there is an existing API/ethtool entry before implementing the "tunable". I think a big concern, for me at least, is that ethtool already has a lot of options and introducing even more would lead the user to confusion ... Thanks and Best Regards, Jose Miguel Abreu > > Michal Kubecek
Re: [RFC] ethtool: Support for driver private ioctl's
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? > > 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 ... Thanks and Best Regards, Jose Miguel Abreu
Re: [RFC] ethtool: Support for driver private ioctl's
On Thu, Apr 05, 2018 at 08:50:49AM -0700, Florian Fainelli wrote: > On 04/05/2018 03:47 AM, Jose Abreu wrote: > > 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. > > 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? Perhaps introducing "driver/device specific tunables" (i.e. something like tunables or PHY tunables but specific to a particular device) could be a way. But it could get out of control quickly and users wouldn't be happy if they had to set the same (or almost the same) parameter under five different names for five NIC vendors. Michal Kubecek
Re: [RFC] ethtool: Support for driver private ioctl's
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. 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? > > 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. -- Florian
[RFC] ethtool: Support for driver private ioctl's
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 ... 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? Thanks and Best Regards, Jose Miguel Abreu
