Re: [patch net-next v2 01/12] flow_offload: Introduce offload of HW stats type

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Mar 03, 2020 at 09:06:48PM +0000, Edward Cree wrote:
> On 03/03/2020 20:27, Pablo Neira Ayuso wrote:
> > On Tue, Mar 03, 2020 at 06:55:54PM +0000, Edward Cree wrote:
> >> On 02/03/2020 22:49, Jakub Kicinski wrote:
> >>> On Mon, 2 Mar 2020 22:46:59 +0100 Pablo Neira Ayuso wrote:
> >>>> On Mon, Mar 02, 2020 at 12:18:52PM -0800, Jakub Kicinski wrote:
> >>>>> On Mon, 2 Mar 2020 20:24:37 +0100 Pablo Neira Ayuso wrote:  
> >>>>>> It looks to me that you want to restrict the API to tc for no good
> >>>>>> _technical_ reason.  
> >> The technical reason is that having two ways to do things where one would
> >>  suffice means more code to be written, tested, debugged.  So if you want
> >>  to add this you need to convince us that the existing way (a) doesn't
> >>  meet your needs and (b) can't be extended to cover them.
> > One single unified way to express the hardware offload for _every_
> > supported frontend is the way to go. The flow_offload API provides a
> > framework to model all hardware offloads for each existing front-end.
> >
> > I understand your motivation might be a specific front-end of your
> > choice, that's fair enough.
> I think we've misunderstood each other (90% my fault).
> 
> When you wrote "restrict the API to tc" I read that as "restrict growth of
>  the API for flow offloading" (which I *do* want); I've now re-parsed and
>  believe you meant it as "limit the API so that only tc may use it" (which
>  is not my desire at all).
> 
> Thus, when I spoke of "two ways to do things" I meant that _within_ the
>  (unified) flow_offload API there should be a single approach to stats
>  (the counters attached to actions), to which levels above and below it
>  impedance-match as necessary (e.g. by merging netfilter count actions
>  onto the following action as Jakub described) rather than bundling
>  two interfaces (tc-style counters and separate counter actions)
>  into one API (which would mean that drivers would all need to write
>  code to handle both kinds, at no gain of expressiveness).

It's not that natural to express counters like you prefer for
netfilter, but fair enough, we'll carry on that extra burden of
merging counters to actions.

Sometimes decisions just need a second round: I will expect broken
endianness in drivers because of the 32-bit word choice for the
payload mangling API. But that's a different story.

Noone to blame, there is still experimentation going on in this API.

> I was *not* referring to tc and netfilter
>  as the "two different ways", but  I can see why you read it that
>  way.

I don't follow, sorry.

> I hope that makes sense now.

Sure, thank you.