On Mon, 6 May 2019 08:16:31 +0200, Jiri Pirko wrote: > Sun, May 05, 2019 at 07:34:32PM CEST, jakub.kicin...@netronome.com wrote: > >On Sat, 4 May 2019 15:16:54 +0200, Jiri Pirko wrote: > >> Sat, May 04, 2019 at 01:46:25PM CEST, jakub.kicin...@netronome.com wrote: > >> >From: Pieter Jansen van Vuuren <pieter.jansenvanvuu...@netronome.com> > >> > > >> >Some actions like the police action are stateful and could share state > >> >between devices. This is incompatible with offloading to multiple devices > >> >and drivers might want to test for shared blocks when offloading. > >> >Store a pointer to the tcf_block structure in the tc_cls_common_offload > >> >structure to allow drivers to determine when offloads apply to a shared > >> >block. > >> > >> I don't this this is good idea. If your driver supports shared blocks, > >> you should register the callback accordingly. See: > >> mlxsw_sp_setup_tc_block_flower_bind() where tcf_block_cb_lookup() and > >> __tcf_block_cb_register() are used to achieve that. > > > >Right, in some ways. Unfortunately we don't support shared blocks > >fully, i.e. we register multiple callbacks and get the rules > >replicated. It's a FW limitation, but I don't think we have shared > >blocks on the roadmap, since rule storage is not an issue for our HW. > > > >But even if we did support sharing blocks, we'd have to teach TC that > >some rules can only be offloaded if there is only a single callback > >registered, right? In case the block is shared between different ASICs. > > I don't see why sharing block between different ASICs is a problem. The > sharing implementation is totally up to the driver. It can duplicate the > rules even within one ASIC. According to that, it registers one or more > callbacks.
If we want to replicate software semantics for act_police all ports sharing the port should count against the same rate limit. This is pretty much impossible unless the rule is offloaded to a single ASIC and the ASIC/FW supports proper block/action sharing. > In this patchset, you use the block only to see if it is shared or not. > When TC calls the driver to bind, it provides the block struct: > ndo_setup_tc > type == TC_SETUP_BLOCK > f->command == TC_BLOCK_BIND > You can check for sharing there and remember it for the future check in > filter insertion. > > I would like to avoid passing block pointer during filter insertion. It > is misleading and I'm pretty sure it would lead to misuse by drivers. > > I see that Dave already applied this patchset. Could you please send > follow-up removing the block pointer from filter offload struct? Makes sense, we'll follow up shortly!
