netfilter: turn NF_HOOK into an inline function

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")


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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,
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html