On Mon, 28 May 2018 13:48:28 +0300, Or Gerlitz wrote:
> On Fri, May 25, 2018 at 5:25 AM, Jakub Kicinski wrote:
> > This series from John adds the ability to replay filter offload requests
> > when new offload callback is being registered on a TC block. This is most
> > likely to take place for shared blocks today, when a block which already
> > has rules is bound to another interface. Prior to this patch set if any
> > of the rules were offloaded the block bind would fail.
>
> Can you elaborate a little further here? is this something that you are
> planning
> to use for the uplink LAG use-case? AFAIU if we apply share-block to nfp as
> things are prior to this patch, it would work, so there's a case where
> it doesn't and this is now handled with the series?
Just looking at things as they stand today, no bond/forward looking
plans - nfp "supports" shared blocks by registering multiple callbacks
to the block. There are two problems:
(a) one can't install a second callback if some rules are already
offloaded because of:
/* At this point, playback of previous block cb calls is not supported,
* so forbid to register to block which already has some offloaded
* filters present.
*/
if (tcf_block_offload_in_use(block))
return ERR_PTR(-EOPNOTSUPP);
in __tcf_block_cb_register(), so block sharing has to be set up
before any rules are added.
(b) when block is unshared filters are not removed today and driver
would have to sweep its rule table, as John notes. It's not a big
deal but this series fixes it nicely in the core, too.
Looking forward there are two things we can use shared blocks for: we
can try to teach user space to share ingress blocks on all legs of bonds
instead of trying to propagate the rules from the bond down in the
kernel, which is more tricky to get right. We will need reliable
replay for that, because we want new links to be able to join and leave
the bond when rules are already present.
Second use case, which is more far fetched, is trying to discover and
register callbacks for blocks of tunnel devices directly, and avoid the
egdev infrastructure...
We should discuss the above further, but regardless, I think this
patchset is quite a nice addition on it's own. Would you agree?
