netdev
[Top] [All Lists]

Re: [RFC, PATCH 2/5]: netfilter+ipsec - output hooks

To: Patrick McHardy <kaber@xxxxxxxxx>
Subject: Re: [RFC, PATCH 2/5]: netfilter+ipsec - output hooks
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Fri, 19 Mar 2004 21:59:39 +1100
Cc: "David S. Miller" <davem@xxxxxxxxxx>, netdev@xxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4059CEEC.7070009@xxxxxxxxx>
References: <20040308110331.GA20719@xxxxxxxxxxxxxxxxxxx> <404C874D.4000907@xxxxxxxxx> <20040308115858.75cdddca.davem@xxxxxxxxxx> <4059CEEC.7070009@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.5.1+cvs20040105i
On Thu, Mar 18, 2004 at 05:31:40PM +0100, Patrick McHardy wrote:
> This patch adds new output-hooks. Packets with dst->xfrm != NULL
> traverse the POST_ROUTING hook before dst_output is called. The
> transformers mark the packets in the control buffer with a new flag
> IPSKB_XFRM_TRANSFORMED, these packets then traverse the LOCAL_OUT
> hook when they hit ip_output.

Thank you very much for your patches.  This is definitely the biggest
show stopper with the current IPsec stack.

I've just got a minor point about this one:

> @@ -119,12 +119,14 @@
>  /* This is gross, but inline doesn't cut it for avoiding the function
>     call in fast path: gcc doesn't inline (needs value tracking?). --RR */
>  #ifdef CONFIG_NETFILTER_DEBUG
> -#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)                  \
> - nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN)
> +#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)               
> \
> +(!(cond)                                                                     
> \
> + ? (okfn)(skb)                                                               
> \
> + : nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN))

Any reason why this is written with a negated cond? I get confused by
double negations :)

>  #define NF_HOOK_THRESH nf_hook_slow
>  #else
> -#define NF_HOOK(pf, hook, skb, indev, outdev, okfn)                  \
> -(list_empty(&nf_hooks[(pf)][(hook)])                                 \
> +#define NF_HOOK_COND(pf, hook, skb, indev, outdev, okfn, cond)               
> \
> +(!(cond) || list_empty(&nf_hooks[(pf)][(hook)])                              
> \

Ditto, what about

((cond) && !list_empty(&nf_hooks[(pf)][(hook)) \
 ? nf_hook_slow((pf), (hook), (skb), (indev), (outdev), (okfn), INT_MIN) \
 : (okfn)(skb))
-- 
Debian GNU/Linux 3.0 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

<Prev in Thread] Current Thread [Next in Thread>