Re: [PATCH nf-next,RFC 03/10] netfilter: bridge: kill NF_HOOK_THRESH() and state->tresh

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Oct 13, 2016 at 05:10:55PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Thu, Oct 13, 2016 at 02:25:45PM +0200, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Patch c5136b15ea36 ("netfilter: bridge: add and use br_nf_hook_thresh")
> > > > introduced br_nf_hook_thresh().
> > > > 
> > > > Replace NF_HOOK_THRESH() by br_nf_hook_thresh from
> > > > br_nf_forward_finish(), so we have no more callers for this macro.
> > > > 
> > > > As a result, state->thresh and explicit thresh parameter in the hook
> > > > state structure is not required anymore.
> > > > 
> > > > And we can get rid of fast forward code in nf_iterate() in the core path
> > > > that is only used by br_netfilter to search for the filter hook.
> > > 
> > > Note that you will need to move more parts of nf_hook_slow() into
> > > br_nf_hook_thresh(); the bridge netfilter does need to thresh feature
> > > that we have in nf_iterate().
> > 
> > br_nf_hook_thresh() is already skipping hooks before NF_BR_PRI_BRNF to
> > emulate thresh. What else is missing?
> 
> AFAICS you are removing the NF_BR_PRI_BRNF skipping in this patch,
> it relied on nf_hook_slow to do this (plus the state->thresh thing).

int br_nf_hook_thresh(unsigned int hook, struct net *net,
                      struct sock *sk, struct sk_buff *skb,
                      struct net_device *indev,
                      struct net_device *outdev,
                      int (*okfn)(struct net *, struct sock *,
                                  struct sk_buff *))
{
        struct nf_hook_entry *elem;
        struct nf_hook_state state;
        int ret;

        elem = rcu_dereference(net->nf.hooks[NFPROTO_BRIDGE][hook]);

        while (elem && (elem->ops.priority <= NF_BR_PRI_BRNF))
                elem = rcu_dereference(elem->next);

        ...

        nf_hook_state_init(&state, elem, hook, NFPROTO_BRIDGE, indev, ...

Hm, but this code (before actually calling nf_hook_slow) is skipping
the hook until we get to NF_BR_PRI_BRNF + 1.

Then hook state sets hook_entry to elem.

Am I missing anything?