Re: [net-next,v5,00/12] add flow_rule infrastructure

Pablo Neira Ayuso
2018-12-13T19:54:44Z

On Thu, Dec 13, 2018 at 11:23:15AM -0800, Jakub Kicinski wrote:
> On Thu, 13 Dec 2018 12:06:39 +0200, Or Gerlitz wrote:
> > > Maybe having a side library that could take a ethtool/flower/nft flow
> > > and return common IR representation of that flow would be less painful?
> > > Drivers could then migrate at its own pace, for new functionality etc.
> > > Was that discussed before? I may have lost track of this discussion...
> >
> > Currently all drivers are ported to use the IR on the tc/flower offload path
> >
> > End of the day, seems that Jiri and Jakub are good with this and I didn't hear
> > any further rejections from other driver folks, so I guess my concerns
> > were basically addressed by them.
>
> 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.