On 16/08/2019 02:04, Pablo Neira Ayuso wrote: > On Wed, Aug 14, 2019 at 05:17:20PM +0100, Edward Cree wrote: >> TBH I'm still not clear why you need a flow_block per subsystem, rather than >> just having multiple subsystems feed their offload requests through the same >> flow_block but with different enum tc_setup_type or enum tc_fl_command or >> some other indication that this is "netfilter" rather than "tc" asking for a >> tc_cls_flower_offload. > In tc, the flow_block is set up by when the ingress qdisc is > registered. The usual scenario for most drivers is to have one single > flow_block per registered ingress qdisc, this makes a 1:1 mapping > between ingress qdisc and flow_block. > > Still, you can register two or more ingress qdiscs to make them share > the same policy via 'tc block'. In that case all those qdiscs use one > single flow_block. This makes a N:1 mapping between these qdisc > ingress and the flow_block. This policy applies to all ingress qdiscs > that are part of the same tc block. By 'tc block', I'm refering to the > tcf_block structure. > > In netfilter, there are ingress basechains that are registered per > device. Each basechain gets a flow_block by when the basechain is > registered. Shared blocks as in tcf_block are not yet supported, but > it should not be hard to extend it to make this work. > > To reuse the same flow_block as entry point for all subsystems as your > propose - assuming offloads for two or more subsystems are in place - > then all of them would need to have the same block sharing > configuration, which might not be the case, ie. tc ingress might have > a eth0 and eth1 use the same policy via flow_block, while netfilter > might have one basechain for eth0 and another for eth1 (no policy > sharing). Thank you, that's very helpful. >> This really needs a design document explaining what all the bits are, how >> they fit together, and why they need to be like that. > I did not design this flow_block abstraction, this concept was already > in place under a different name and extend it so the ethtool/netfilter > subsystems to avoid driver code duplication for offloads. It's more the new implementation that you've created as part of this extension that I was asking about, although I agree that the abstraction that already existed is in need of documentation too.
