On Thu, 20 Dec 2018 01:03:13 +0100, Pablo Neira Ayuso wrote:

On Wed, Dec 19, 2018 at 11:57:03AM -0800, Jakub Kicinski wrote:

quoted

quoted

Anyway, the problem that this patchset addresses _already exists_ with
ethtool_rxnfc and cls_flower in place, it would be good to fix it now.  

That's not the point of contention.  

What is your alternative plan to unify driver codebase for
ethtool_rx_flow_spec and tc/cls_flower to offload ACL from the ingress
path?

The point of contention for me is the "reuse of ndo_setup_tc" :)  
I haven't played with your IR, but it looks fairly close to the flower
dissector based thing, so it's probably fine

quoted

Did you consider things like tunnels and bonds?  There are a lot of
problems which have been solved for TC offloads, if you create a
separate subsystem offload you'll have to repeat all that work.  

Did I repeat any work to unify ethtool_rx_flow_spec and tc/cls_flower?
No :-). I spinned over the existing work and delivered an incremental
update.

Ethtool exists, so you don't have to teach the driver about it.  nft
offload does not, yet, exist in upstream drivers AFAIK.  Besides, if
ethtool n-tuples were rich and supported complex use cases we wouldn't
have TC flower offloads, so it's not apples to apples ;)

quoted

I'm not suggesting we replace netfilter with TC.  I'm suggesting we
replace nf_flow_offload table with something that fits into TC.

You're not going to offload entire netfilter.  You want to offload
simplistic flows through the nf_flow_table.  What I'm saying, is add a
equivalent of that table into TC.  Make user space "link" netfilter to
that.  

This patchset is not related to nf_flow_table infrastructure.

Let's not let the elephant back into the room, please :)

From what I'm reading, you assume we can use nf_flow_table everywhere,
which is probably true if you assume people use your NICs. However,
there is a class of hardware where CPU is extremely smallish to cope
with flows in software, where dataplane is almost entirely offloaded
to hardware. In that scenario, nf_flow_table cannot be used.

I'm confused, could you rephrase?  How does you work help such devices?
How is tc not suitable for them?