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:

quoted

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