2013-03-30 09:37:17

by Fengguang Wu

[permalink] [raw]
Subject: WARNING: at net/batman-adv/hard-interface.c:92 batadv_is_on_batman_iface()

Greetings,

I got the below WARNING in net-next/master (head commit f498354793d5)
and the first bad commit is

commit c54419321455631079c7d6e60bc732dd0c5914c5
Author: Pravin B Shelar <[email protected]>
Date: Mon Mar 25 14:49:35 2013 +0000

GRE: Refactor GRE tunneling code.

Following patch refactors GRE code into ip tunneling code and GRE
specific code. Common tunneling code is moved to ip_tunnel module.
ip_tunnel module is written as generic library which can be used
by different tunneling implementations.

ip_tunnel module contains following components:
- packet xmit and rcv generic code. xmit flow looks like
(gre_xmit/ipip_xmit)->ip_tunnel_xmit->ip_local_out.
- hash table of all devices.
- lookup for tunnel devices.
- control plane operations like device create, destroy, ioctl, netlink
operations code.
- registration for tunneling modules, like gre, ipip etc.
- define single pcpu_tstats dev->tstats.
- struct tnl_ptk_info added to pass parsed tunnel packet parameters.

ipip.h header is renamed to ip_tunnel.h

Signed-off-by: Pravin B Shelar <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

[ 27.507156] libceph: loaded (mon/osd proto 15/24)
[ 27.515409] ------------[ cut here ]------------
[ 27.518917] WARNING: at /c/kernel-tests/src/stable/net/batman-adv/hard-interface.c:92 batadv_is_on_batman_iface+0x5c/0x7a()
[ 27.521935] Hardware name: Bochs
[ 27.525356] Cannot find parent device
[ 27.528296] Modules linked in:
[ 27.529923] Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc4-00896-g03ba910 #794
[ 27.531747] Call Trace:
[ 27.534683] [<ffffffff81098ab4>] warn_slowpath_common+0x83/0x9e
[ 27.536338] [<ffffffff81098b72>] warn_slowpath_fmt+0x46/0x48
[ 27.537872] [<ffffffff828e6402>] ? rcu_read_unlock+0x1c/0x2d
[ 27.539441] [<ffffffff810ca498>] ? local_clock+0x19/0x52
[ 27.541023] [<ffffffff82c0e796>] batadv_is_on_batman_iface+0x5c/0x7a
[ 27.542803] [<ffffffff82c0efe2>] batadv_hard_if_event+0x8f/0x285
[ 27.544551] [<ffffffff828e4ff0>] register_netdevice_notifier+0x71/0x17e
[ 27.550617] [<ffffffff8420a030>] ? batadv_iv_init+0x3f/0x3f
[ 27.552248] [<ffffffff8420a114>] batadv_init+0xe4/0x104
[ 27.553770] [<ffffffff810020be>] do_one_initcall+0x7f/0x13d
[ 27.555386] [<ffffffff84177eb0>] kernel_init_freeable+0x141/0x1d0
[ 27.557177] [<ffffffff84177734>] ? do_early_param+0x8c/0x8c
[ 27.558813] [<ffffffff82c1f146>] ? rest_init+0xda/0xda
[ 27.560348] [<ffffffff82c1f154>] kernel_init+0xe/0xdb
[ 27.561813] [<ffffffff82c694ac>] ret_from_fork+0x7c/0xb0
[ 27.563356] [<ffffffff82c1f146>] ? rest_init+0xda/0xda
[ 27.564974] ---[ end trace f78f9f0651ffcb0b ]---

git bisect start 03ba9107c92bdc2eec1d14ea19d5a00209969d87 v3.8 --
git bisect good cb715a836642e0ec69350670d1c2f800f3e2d2e4 # 10 2013-03-29 14:42:04 Merge branch 'x86-cpu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good b67bfe0d42cac56c512dd5da4b1b347a23f4b70a # 10 2013-03-29 17:23:34 hlist: drop the node parameter from iterators
git bisect good 59d8e5eb2bd5593d8220db0e25206cdfc42e83ea # 10 2013-03-29 20:04:53 Merge branch 'akpm' (fixes from Andrew)
git bisect good 71420228feb826b365b3fef4de01e83c00e9151d # 10 2013-03-29 22:46:41 Merge remote-tracking branch 'origin/drm-intel-fixes' into drm-intel-nightly
git bisect good 5470b462c3f0c6fa980c320968a165bd0f34ca8d # 10 2013-03-30 00:08:49 Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next into for-davem
git bisect bad 38502af77e07b5d6650b9ff99a0b482d86366592 # 0 2013-03-30 00:16:41 tuntap: set transport header before passing it to kernel
git bisect good 0465277f6b3fd0535428ae935644ac30ce903de0 # 10 2013-03-30 01:46:40 ipv4: provide addr and netconf dump consistency info
git bisect good 21168245031062212c0b805d0bd466ee6dd4a16f # 10 2013-03-30 04:28:14 dsa: factor freeing of dsa_platform_data
git bisect bad cb6bf35502d53364d15737295bc64f804c4587ce # 0 2013-03-30 05:47:34 firewire net, ipv6: IPv6 over Firewire (RFC3146) support.
git bisect bad 206aaafcd279e2cb836d772282517540c6cb3814 # 0 2013-03-30 05:55:36 VXLAN: Use IP Tunnels tunnel ENC encap API
git bisect good 25c7704d8bdcf6746dab4397953df51759924b37 # 10 2013-03-30 07:21:49 ipv4: Fix ip-header identification for gso packets.
git bisect bad c54419321455631079c7d6e60bc732dd0c5914c5 # 0 2013-03-30 08:42:46 GRE: Refactor GRE tunneling code.
git bisect good eaac5f3d3ad33547b299935e6db0cfc7be9a576a # 10 2013-03-30 10:11:30 net: Print functions in /proc/net/ptype without the offset.
git bisect good eaac5f3d3ad33547b299935e6db0cfc7be9a576a # 30 2013-03-30 14:14:09 net: Print functions in /proc/net/ptype without the offset.
git bisect bad 03ba9107c92bdc2eec1d14ea19d5a00209969d87 # 0 2013-03-30 15:34:09 Merge remote-tracking branch 'net-next/master' into devel-inn-x86_64-2013-03-27-06-54

Thanks,
Fengguang


Attachments:
(No filename) (4.97 kB)
dmesg-kvm-inn-39711-2013-03-27-09-08-30-3.9.0-rc4-00896-g03ba910-794 (113.62 kB)
03ba9107c92bdc2eec1d14ea19d5a00209969d87-bisect.log (14.35 kB)
.config-bisect (126.11 kB)
Download all attachments

2013-03-30 11:54:14

by Antonio Quartulli

[permalink] [raw]
Subject: Re: WARNING: at net/batman-adv/hard-interface.c:92 batadv_is_on_batman_iface()

Hello all,

On Sat, Mar 30, 2013 at 05:36:35PM +0800, Fengguang Wu wrote:
> Greetings,
>
> I got the below WARNING in net-next/master (head commit f498354793d5)
> and the first bad commit is
>
> commit c54419321455631079c7d6e60bc732dd0c5914c5
> Author: Pravin B Shelar <[email protected]>
> Date: Mon Mar 25 14:49:35 2013 +0000
>
> GRE: Refactor GRE tunneling code.
>
> Following patch refactors GRE code into ip tunneling code and GRE
> specific code. Common tunneling code is moved to ip_tunnel module.
> ip_tunnel module is written as generic library which can be used
> by different tunneling implementations.
>
> ip_tunnel module contains following components:
> - packet xmit and rcv generic code. xmit flow looks like
> (gre_xmit/ipip_xmit)->ip_tunnel_xmit->ip_local_out.
> - hash table of all devices.
> - lookup for tunnel devices.
> - control plane operations like device create, destroy, ioctl, netlink
> operations code.
> - registration for tunneling modules, like gre, ipip etc.
> - define single pcpu_tstats dev->tstats.
> - struct tnl_ptk_info added to pass parsed tunnel packet parameters.
>
> ipip.h header is renamed to ip_tunnel.h
>
> Signed-off-by: Pravin B Shelar <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
> [ 27.507156] libceph: loaded (mon/osd proto 15/24)
> [ 27.515409] ------------[ cut here ]------------
> [ 27.518917] WARNING: at /c/kernel-tests/src/stable/net/batman-adv/hard-interface.c:92 batadv_is_on_batman_iface+0x5c/0x7a()
> [ 27.521935] Hardware name: Bochs
> [ 27.525356] Cannot find parent device
> [ 27.528296] Modules linked in:
> [ 27.529923] Pid: 1, comm: swapper/0 Not tainted 3.9.0-rc4-00896-g03ba910 #794
> [ 27.531747] Call Trace:
> [ 27.534683] [<ffffffff81098ab4>] warn_slowpath_common+0x83/0x9e
> [ 27.536338] [<ffffffff81098b72>] warn_slowpath_fmt+0x46/0x48
> [ 27.537872] [<ffffffff828e6402>] ? rcu_read_unlock+0x1c/0x2d
> [ 27.539441] [<ffffffff810ca498>] ? local_clock+0x19/0x52
> [ 27.541023] [<ffffffff82c0e796>] batadv_is_on_batman_iface+0x5c/0x7a
> [ 27.542803] [<ffffffff82c0efe2>] batadv_hard_if_event+0x8f/0x285
> [ 27.544551] [<ffffffff828e4ff0>] register_netdevice_notifier+0x71/0x17e
> [ 27.550617] [<ffffffff8420a030>] ? batadv_iv_init+0x3f/0x3f
> [ 27.552248] [<ffffffff8420a114>] batadv_init+0xe4/0x104
> [ 27.553770] [<ffffffff810020be>] do_one_initcall+0x7f/0x13d
> [ 27.555386] [<ffffffff84177eb0>] kernel_init_freeable+0x141/0x1d0
> [ 27.557177] [<ffffffff84177734>] ? do_early_param+0x8c/0x8c
> [ 27.558813] [<ffffffff82c1f146>] ? rest_init+0xda/0xda
> [ 27.560348] [<ffffffff82c1f154>] kernel_init+0xe/0xdb
> [ 27.561813] [<ffffffff82c694ac>] ret_from_fork+0x7c/0xb0
> [ 27.563356] [<ffffffff82c1f146>] ? rest_init+0xda/0xda
> [ 27.564974] ---[ end trace f78f9f0651ffcb0b ]---
>

The reason why batman-adv it raising this warning is because this call is
returning NULL:

dev_get_by_index(&init_net, net_dev->iflink);

net_dev is an interface that has been registered now and batman-adv is trying to
analyse it to decide if it is a potential candidate for its virtual device or
not.

To the best of my knowledge, if the function above is returning NULL, it means
that the iflink attribute contains a broken value.

Am I wrong or iflink should never contain 0? If there is no parent device it
should contain the same value of ifindex. Right?


Regards,

--
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara


Attachments:
(No filename) (3.54 kB)
(No filename) (836.00 B)
Download all attachments

2013-03-30 17:31:50

by Antonio Quartulli

[permalink] [raw]
Subject: [PATCH] ip_gre: don't overwrite iflink during net_dev init

iflink is currently set to 0 in __gre_tunnel_init(). This
function is invoked in gre_tap_init() and
ipgre_tunnel_init() which are both used to initialise the
ndo_init field of the respective net_device_ops structs
(ipgre.. and gre_tap..) used by GRE interfaces.

However, in netdevice_register() iflink is first set to -1,
then ndo_init is invoked and then iflink is assigned to a
proper value if and only if it still was -1.

Assigning 0 to iflink in ndo_init is therefore first
preventing netdev_register() to correctly assign it a proper
value and then breaking iflink at all since 0 has not
correct meaning.

Fix this by removing the iflink assignment in
__gre_tunnel_init().

Introduced by c54419321455631079c7d6e60bc732dd0c5914c5
("GRE: Refactor GRE tunneling code.")

Reported-by: Fengguang Wu <[email protected]>
Cc: Pravin B Shelar <[email protected]>
Cc: "David S. Miller" <[email protected]>
Signed-off-by: Antonio Quartulli <[email protected]>
---
net/ipv4/ip_gre.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index ad662e9..e5dfd28 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -660,7 +660,6 @@ static void __gre_tunnel_init(struct net_device *dev)

dev->needed_headroom = LL_MAX_HEADER + sizeof(struct iphdr) + 4;
dev->mtu = ETH_DATA_LEN - sizeof(struct iphdr) - 4;
- dev->iflink = 0;

dev->features |= NETIF_F_NETNS_LOCAL | GRE_FEATURES;
dev->hw_features |= GRE_FEATURES;
--
1.8.1.5

2013-03-30 21:29:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] ip_gre: don't overwrite iflink during net_dev init

From: Antonio Quartulli <[email protected]>
Date: Sat, 30 Mar 2013 18:07:37 +0100

> iflink is currently set to 0 in __gre_tunnel_init(). This
> function is invoked in gre_tap_init() and
> ipgre_tunnel_init() which are both used to initialise the
> ndo_init field of the respective net_device_ops structs
> (ipgre.. and gre_tap..) used by GRE interfaces.
>
> However, in netdevice_register() iflink is first set to -1,
> then ndo_init is invoked and then iflink is assigned to a
> proper value if and only if it still was -1.
>
> Assigning 0 to iflink in ndo_init is therefore first
> preventing netdev_register() to correctly assign it a proper
> value and then breaking iflink at all since 0 has not
> correct meaning.
>
> Fix this by removing the iflink assignment in
> __gre_tunnel_init().
>
> Introduced by c54419321455631079c7d6e60bc732dd0c5914c5
> ("GRE: Refactor GRE tunneling code.")
>
> Reported-by: Fengguang Wu <[email protected]>
> Signed-off-by: Antonio Quartulli <[email protected]>

Please always indicate, in the subject line inside of the [] brackets,
what tree the patch is targetted at.

I can use "git describe" on the guilty commit, but why take a chance?

Applied to net-next, thanks.