[PATCH] netfilter: turn NF_HOOK into an inline function

[PATCH] netfilter: turn NF_HOOK into an inline function

[Arnd Bergmann]

A recent change to the dst_output handling caused a new warning
when the call to NF_HOOK() is the only used of a local variable
passed as 'dev', and CONFIG_NETFILTER is disabled:

net/ipv6/ip6_output.c: In function 'ip6_output':
net/ipv6/ip6_output.c:135:21: warning: unused variable 'dev' [-Wunused-variable]

The reason for this is that the NF_HOOK macro in this case does
not reference the variable at all. To avoid that warning now
and in the future, this changes the macro into an equivalent
inline function, which tells the compiler that the variable is
passed correctly but still unused.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: ede2059dbaf9 ("dst: Pass net into dst->output")

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index edb3dc32f1da..1ff5c3f82820 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -347,8 +347,23 @@ nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, u_int8_t family)
 }
 
 #else /* !CONFIG_NETFILTER */
-#define NF_HOOK(pf, hook, net, sk, skb, indev, outdev, okfn) (okfn)(net, sk, skb)
-#define NF_HOOK_COND(pf, hook, net, sk, skb, indev, outdev, okfn, cond) (okfn)(net, sk, skb)
+static inline int
+NF_HOOK_COND(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
+	     struct sk_buff *skb, struct net_device *in, struct net_device *out,
+	     int (*okfn)(struct net *, struct sock *, struct sk_buff *),
+	     bool cond)
+{
+	return okfn(net, sk, skb);
+}
+
+static inline int
+NF_HOOK(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, struct sk_buff *skb,
+	struct net_device *in, struct net_device *out,
+	int (*okfn)(struct net *, struct sock *, struct sk_buff *))
+{
+	return okfn(net, sk, skb);
+}
+
 static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
 			  struct sock *sk, struct sk_buff *skb,
 			  struct net_device *indev, struct net_device *outdev,

Re: [PATCH] netfilter: turn NF_HOOK into an inline function

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]

Hi Arnd,

[auto build test ERROR on next-20151009 -- if it's inappropriate base, please ignore]

config: i386-randconfig-i0-201540 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net/decnet/dn_route.c: In function 'dn_forward':
>> net/decnet/dn_route.c:823:32: error: 'dev' undeclared (first use in this function)
             &init_net, NULL, skb, dev, skb->dev,
                                   ^
   net/decnet/dn_route.c:823:32: note: each undeclared identifier is reported only once for each function it appears in

vim +/dev +823 net/decnet/dn_route.c

^1da177e Linus Torvalds    2005-04-16  807  	 */
^1da177e Linus Torvalds    2005-04-16  808  	if (++cb->hops > 30)
^1da177e Linus Torvalds    2005-04-16  809  		goto drop;
^1da177e Linus Torvalds    2005-04-16  810  
d8d1f30b Changli Gao       2010-06-10  811  	skb->dev = rt->dst.dev;
^1da177e Linus Torvalds    2005-04-16  812  
^1da177e Linus Torvalds    2005-04-16  813  	/*
^1da177e Linus Torvalds    2005-04-16  814  	 * If packet goes out same interface it came in on, then set
^1da177e Linus Torvalds    2005-04-16  815  	 * the Intra-Ethernet bit. This has no effect for short
^1da177e Linus Torvalds    2005-04-16  816  	 * packets, so we don't need to test for them here.
^1da177e Linus Torvalds    2005-04-16  817  	 */
^1da177e Linus Torvalds    2005-04-16  818  	cb->rt_flags &= ~DN_RT_F_IE;
^1da177e Linus Torvalds    2005-04-16  819  	if (rt->rt_flags & RTCF_DOREDIRECT)
^1da177e Linus Torvalds    2005-04-16  820  		cb->rt_flags |= DN_RT_F_IE;
^1da177e Linus Torvalds    2005-04-16  821  
29a26a56 Eric W. Biederman 2015-09-15  822  	return NF_HOOK(NFPROTO_DECNET, NF_DN_FORWARD,
29a26a56 Eric W. Biederman 2015-09-15 @823  		       &init_net, NULL, skb, dev, skb->dev,
8f40b161 David S. Miller   2011-07-17  824  		       dn_to_neigh_output);
^1da177e Linus Torvalds    2005-04-16  825  
^1da177e Linus Torvalds    2005-04-16  826  drop:
^1da177e Linus Torvalds    2005-04-16  827  	kfree_skb(skb);
^1da177e Linus Torvalds    2005-04-16  828  	return NET_RX_DROP;
^1da177e Linus Torvalds    2005-04-16  829  }
^1da177e Linus Torvalds    2005-04-16  830  
^1da177e Linus Torvalds    2005-04-16  831  /*

:::::: The code at line 823 was first introduced by commit
:::::: 29a26a56803855a79dbd028cd61abee56237d6e5 netfilter: Pass struct net into the netfilter hooks

:::::: TO: Eric W. Biederman <ebiederm@xmission.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 20113 bytes --]

Re: [PATCH] netfilter: turn NF_HOOK into an inline function

[Arnd Bergmann]

On Friday 09 October 2015 21:03:57 kbuild test robot wrote:
> ^1da177e Linus Torvalds    2005-04-16  818      cb->rt_flags &= ~DN_RT_F_IE;
> ^1da177e Linus Torvalds    2005-04-16  819      if (rt->rt_flags & RTCF_DOREDIRECT)
> ^1da177e Linus Torvalds    2005-04-16  820              cb->rt_flags |= DN_RT_F_IE;
> ^1da177e Linus Torvalds    2005-04-16  821  
> 29a26a56 Eric W. Biederman 2015-09-15  822      return NF_HOOK(NFPROTO_DECNET, NF_DN_FORWARD,
> 29a26a56 Eric W. Biederman 2015-09-15 @823                     &init_net, NULL, skb, dev, skb->dev,
> 8f40b161 David S. Miller   2011-07-17  824                     dn_to_neigh_output);
> ^1da177e Linus Torvalds    2005-04-16  825  
> ^1da177e Linus Torvalds    2005-04-16  826  drop:
> 

Ah, right. The 'dev' variable here is declared as

#ifdef CONFIG_NETFILTER
        struct net_device *dev = skb->dev;
#endif

Apparently because the code produced the same warning as the ipv6 code.

Removing the #ifdef here would make that code nicer and let us use
my patch. Alternatively we could put the same #ifdef into IPV6 and
not use the inline function.

Any opinions?

	Arnd

Re: [PATCH] netfilter: turn NF_HOOK into an inline function

[Eric W. Biederman]

Arnd Bergmann <arnd@arndb.de> writes:

> On Friday 09 October 2015 21:03:57 kbuild test robot wrote:
>> ^1da177e Linus Torvalds    2005-04-16  818      cb->rt_flags &= ~DN_RT_F_IE;
>> ^1da177e Linus Torvalds    2005-04-16  819      if (rt->rt_flags & RTCF_DOREDIRECT)
>> ^1da177e Linus Torvalds    2005-04-16  820              cb->rt_flags |= DN_RT_F_IE;
>> ^1da177e Linus Torvalds    2005-04-16  821  
>> 29a26a56 Eric W. Biederman 2015-09-15  822      return NF_HOOK(NFPROTO_DECNET, NF_DN_FORWARD,
>> 29a26a56 Eric W. Biederman 2015-09-15 @823                     &init_net, NULL, skb, dev, skb->dev,
>> 8f40b161 David S. Miller   2011-07-17  824                     dn_to_neigh_output);
>> ^1da177e Linus Torvalds    2005-04-16  825  
>> ^1da177e Linus Torvalds    2005-04-16  826  drop:
>> 
>
> Ah, right. The 'dev' variable here is declared as
>
> #ifdef CONFIG_NETFILTER
>         struct net_device *dev = skb->dev;
> #endif
>
> Apparently because the code produced the same warning as the ipv6 code.
>
> Removing the #ifdef here would make that code nicer and let us use
> my patch. Alternatively we could put the same #ifdef into IPV6 and
> not use the inline function.

Compilers are good at removing unused variabes (SSA should guarantee
this will always happen), and #ifdefs sucks.  I vote for your inline.
Especially as the case that is actively used and tested
(CONFIG_NETFILTER) has these two functions as inline functions.

Eric

Re: [PATCH] netfilter: turn NF_HOOK into an inline function

[Eric W. Biederman]

Arnd Bergmann <arnd@arndb.de> writes:

> A recent change to the dst_output handling caused a new warning
> when the call to NF_HOOK() is the only used of a local variable
> passed as 'dev', and CONFIG_NETFILTER is disabled:
>
> net/ipv6/ip6_output.c: In function 'ip6_output':
> net/ipv6/ip6_output.c:135:21: warning: unused variable 'dev' [-Wunused-variable]
>
> The reason for this is that the NF_HOOK macro in this case does
> not reference the variable at all. To avoid that warning now
> and in the future, this changes the macro into an equivalent
> inline function, which tells the compiler that the variable is
> passed correctly but still unused.

For clarification the actual change that trigger this is I passed in net
instead of computing net as net = dev_net(dev).  Which was the second
use of the dev variable.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: ede2059dbaf9 ("dst: Pass net into dst->output")
>
> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> index edb3dc32f1da..1ff5c3f82820 100644
> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -347,8 +347,23 @@ nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, u_int8_t family)
>  }
>  
>  #else /* !CONFIG_NETFILTER */
> -#define NF_HOOK(pf, hook, net, sk, skb, indev, outdev, okfn) (okfn)(net, sk, skb)
> -#define NF_HOOK_COND(pf, hook, net, sk, skb, indev, outdev, okfn, cond) (okfn)(net, sk, skb)
> +static inline int
> +NF_HOOK_COND(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
> +	     struct sk_buff *skb, struct net_device *in, struct net_device *out,
> +	     int (*okfn)(struct net *, struct sock *, struct sk_buff *),
> +	     bool cond)
> +{
> +	return okfn(net, sk, skb);
> +}
> +
> +static inline int
> +NF_HOOK(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, struct sk_buff *skb,
> +	struct net_device *in, struct net_device *out,
> +	int (*okfn)(struct net *, struct sock *, struct sk_buff *))
> +{
> +	return okfn(net, sk, skb);
> +}
> +
>  static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
>  			  struct sock *sk, struct sk_buff *skb,
>  			  struct net_device *indev, struct net_device *outdev,

[PATCH v2] netfilter: turn NF_HOOK into an inline function

[Arnd Bergmann]

A recent change to the dst_output handling caused a new warning
when the call to NF_HOOK() is the only used of a local variable
passed as 'dev', and CONFIG_NETFILTER is disabled:

net/ipv6/ip6_output.c: In function 'ip6_output':
net/ipv6/ip6_output.c:135:21: warning: unused variable 'dev' [-Wunused-variable]

The reason for this is that the NF_HOOK macro in this case does
not reference the variable at all, and the call to dev_net(dev)
got removed from the ip6_output function. To avoid that warning now
and in the future, this changes the macro into an equivalent
inline function, which tells the compiler that the variable is
passed correctly but still unused.

The dn_forward function apparently had the same problem in
the past and added a local workaround that no longer works
with the inline function. In order to avoid a regression, we
have to also remove the #ifdef from decnet in the same patch.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: ede2059dbaf9 ("dst: Pass net into dst->output")
---
v2: folded the #ifdef removal and reworded based on Eric's feedback.

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index edb3dc32f1da..1ff5c3f82820 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -347,8 +347,23 @@ nf_nat_decode_session(struct sk_buff *skb, struct flowi *fl, u_int8_t family)
 }
 
 #else /* !CONFIG_NETFILTER */
-#define NF_HOOK(pf, hook, net, sk, skb, indev, outdev, okfn) (okfn)(net, sk, skb)
-#define NF_HOOK_COND(pf, hook, net, sk, skb, indev, outdev, okfn, cond) (okfn)(net, sk, skb)
+static inline int
+NF_HOOK_COND(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
+	     struct sk_buff *skb, struct net_device *in, struct net_device *out,
+	     int (*okfn)(struct net *, struct sock *, struct sk_buff *),
+	     bool cond)
+{
+	return okfn(net, sk, skb);
+}
+
+static inline int
+NF_HOOK(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk, struct sk_buff *skb,
+	struct net_device *in, struct net_device *out,
+	int (*okfn)(struct net *, struct sock *, struct sk_buff *))
+{
+	return okfn(net, sk, skb);
+}
+
 static inline int nf_hook(u_int8_t pf, unsigned int hook, struct net *net,
 			  struct sock *sk, struct sk_buff *skb,
 			  struct net_device *indev, struct net_device *outdev,
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 27fce283117b..607a14f20d88 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -789,9 +789,7 @@ static int dn_forward(struct sk_buff *skb)
 	struct dn_dev *dn_db = rcu_dereference(dst->dev->dn_ptr);
 	struct dn_route *rt;
 	int header_len;
-#ifdef CONFIG_NETFILTER
 	struct net_device *dev = skb->dev;
-#endif
 
 	if (skb->pkt_type != PACKET_HOST)
 		goto drop;

Re: [PATCH v2] netfilter: turn NF_HOOK into an inline function

[Pablo Neira Ayuso]

On Fri, Oct 09, 2015 at 08:45:42PM +0200, Arnd Bergmann wrote:
> A recent change to the dst_output handling caused a new warning
> when the call to NF_HOOK() is the only used of a local variable
> passed as 'dev', and CONFIG_NETFILTER is disabled:
> 
> net/ipv6/ip6_output.c: In function 'ip6_output':
> net/ipv6/ip6_output.c:135:21: warning: unused variable 'dev' [-Wunused-variable]
> 
> The reason for this is that the NF_HOOK macro in this case does
> not reference the variable at all, and the call to dev_net(dev)
> got removed from the ip6_output function. To avoid that warning now
> and in the future, this changes the macro into an equivalent
> inline function, which tells the compiler that the variable is
> passed correctly but still unused.
> 
> The dn_forward function apparently had the same problem in
> the past and added a local workaround that no longer works
> with the inline function. In order to avoid a regression, we
> have to also remove the #ifdef from decnet in the same patch.

Applied, thanks.