On Thu, Dec 13, 2018 at 08:54:39PM +0100, Pablo Neira Ayuso wrote: > On Thu, Dec 13, 2018 at 11:23:15AM -0800, Jakub Kicinski wrote: > > I think having the drivers call the IR translation could be a good > > compromise instead of having flower always pass down converted flows. > > > > tc flower -> flower offload object -> setup_tc -> driver -> > > -> flow_offload_from_flower() -> driver -> driver's common handling > > > > This patchset already does that for ethtool: > > > > ethtool -> ethtool flow -> ethtool_op -> driver -> > > -> ethtool_rx_flow_rule_create() -> driver -> driver's common handling > > > > It feels like a bit of a waste to let the driver patches go, but > > perhaps it's a good way to move forward? > > I think Michal Kubecek mentioned that it would be good if, moving > forward, core/ethtool.c calls ethtool_rx_flow_rule_create(), so this is: > > ethtool -> ethtool flow -> ethtool_rx_flow_rule_create() -> > -> ethtool_op -> driver -> driver's common handling > > In that case, ethtool would match what flower is doing. > > We can probably offer the flow_rule representation as parameter via > ethtool_op indirection, so there is a single ethtool_rx_flow_rule_create() > call. Still, this new infrastructure is used by two ethtool drivers, > so probably it is too early for this consolidation work.
The main reason why I would prefer the translation in the generic ethtool code is that once we have netlink interface for ethtool, current model would require us to translate netlink messages to the data structures for current ethtool_ops handlers (which are essentially the same as those used by ethtool ioctl interface) only to translate them once more in the NIC handler to the common representation. So in the long term it seems better to let ethtool_ops use your representation of Rx flow rules and translate both the ioctl structures and netlink messages into it in the generic code so that NIC drivers would only work with flow_rule. But at the moment, this would mean a lot of work for not so much gain as it would mean adjusting all NIC drivers supporting Rx flow rule handling via ethtool. Also, it would look strange if the new ethtool_ops handlers passed extack which the only caller wouldn't use. Michal Kubecek
