netfilter: turn NF_HOOK into an inline function
Subsystems (3)
PATCH 1/1 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.
include/linux/netfilter.h +17 -2
@@ -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,
kbuild test robot
build botconfig: i386-randconfig-i0-201540 (attached as .config) reproduce:
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 <[email protected]> :::::: CC: David S. Miller <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Quoted 10 lines · 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
Quoted 16 lines · Arnd Bergmann
Quoted 10 lines · 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;
Quoted 7 lines
#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
Quoted 4 lines · Arnd Bergmann
A recent change to the
dst_outputhandling caused a new warning when the call toNF_HOOK()is the only used of a local variable passed as 'dev', andCONFIG_NETFILTERis disabled:
Quoted 8 lines
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_HOOKmacro 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.
Quoted 33 lines
Signed-off-by Arnd Bergmann <[email protected]>Fixes ede2059dbaf9 ("dst: Pass net into dst->output")include/linux/netfilter.h +17
-2@@ -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 1/1 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, 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.
---
v2: folded the #ifdef removal and reworded based on Eric's feedback.
include/linux/netfilter.h +17 -2
@@ -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,
net/decnet/dn_route.c +0 -2
@@ -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;
Quoted 14 lines · Arnd Bergmann
A recent change to the
dst_outputhandling caused a new warning when the call toNF_HOOK()is the only used of a local variable passed as 'dev', andCONFIG_NETFILTERis 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_HOOKmacro in this case does not reference the variable at all, and the call todev_net(dev)got removed from theip6_outputfunction. 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.
Quoted 4 lines
The
dn_forwardfunction 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.