On 13/08/2019 20:51, Pablo Neira Ayuso wrote: > On Mon, Aug 12, 2019 at 06:50:09PM +0100, Edward Cree wrote: >> Pablo, can you explain (because this commit message doesn't) why these per- >> driver lists are needed, and what the information/state is that has module >> (rather than, say, netdevice) scope? > The idea is to update drivers to support one flow_block per subsystem, > one for ethtool, one for tc, and so on. So far, existing drivers only > allow for binding one single flow_block to one of the existing > subsystems. So this limitation applies at driver level. That argues for per-driver _code_, not for per-driver _state_. For instance, each driver could (more logically) store this information in the netdev private data, rather than a static global. Or even, since each driver instance has a unique cb_ident = netdev_priv(net_dev), this doesn't need to be local to the driver at all and could just belong to the device owning the flow_block (which isn't necessarily the device doing the offload, per indirect blocks). 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. I'd also like to concur with what Jakub said on v2: "this series is really hard to follow... the number of things called some combination of block cb and list makes my head hurt :/". This really needs a design document explaining what all the bits are, how they fit together, and why they need to be like that.
