On Tue, Sep 02, 2014 at 03:41:52PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 02, 2014 at 02:28:14PM +0100, Patrick McHardy wrote:
> > > > Same thing applies to all other objects that don't have a unique identifier
> > > > chosen by the kernel.
> > >
> > > All other objects are always notified in order from the commit path,
> > > so they seem fine to me.
> >
> > You're right, this only seems to affect sets.
>
> Yes, but only the sets that are released through
> nf_tables_unbind_set().
>
> > > > > The anonymous sets are problematic, we need to notify this from the
> > > > > commit path too to ensure the right ordering. I was trying to avoid
> > > > > some specific notify() interface in expr->ops but it seems we need it
> > > > > for nft_lookup.c.
> > > > >
> > > > > Can you think of a better solution?
> > > >
> > > > No, unless we can come up with a way that's synchronous.
> > >
> > > I would really like not to go back to the two nearly consecutive
> > > synchronize_rcu() calls, it's slow. I've been thinking on replacing
> > > the current check in the packet path by static keys, but I didn't
> > > manage to find the way yet.
> >
> > Which check exactly are you referring to?
>
> >From the nft_do_chain:
>
> list_for_each_entry_continue_rcu(rule, &chain->rules, list) {
>
> /* This rule is not active, skip. */
> if (unlikely(rule->genmask & (1 << gencursor)))
> continue;
>
> Plus the trick that uses the synchronize_rcu() from the commit path to
> make sure all packets have left the previous generation to deactivate
> the rule before we start removing them from the list.
This won't work. Static keys can't be dynamically allocated, which would
be necessary for this to work. This about it, the code that needs to be
patches is global, but the activeness is a property of a rule.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
