batman-adv crash when removing interface in certain configurations #680

belzebub40k · 2016-03-09T12:10:08Z

When I configure more then 2 VLANs via UCI for mesh-connections and add them to bat0 the network stack isn't working correctly any longer. Any invocation of ip, ifconfig, brctl, batctl etc will result in a stuck system.
If I configure just two mesh-VLANs the system is working fine. It is also possible to add more VLANs to bat0 via "vconfig eth0.x" and "batctl if add eth0.x".

The strange part is before the crash happens the kernel always prints "tried to remove device eth0 from br-client" but eth0 itself shouldn't be part of any bridge or whatever. I also don't see this message when I configure just two mesh-VLANs.

See attached files for backtrace and /etc/config/network.
If you need further information please let me know.

EDIT:

Tested with v2016.1.1 and master.
Problem occurs on two pieces of hardware.

dmesg.txt
network_config.txt

neocturne · 2016-03-09T12:29:37Z

Interesting, I'll try to reproduce this with other hardware.

Which batman-adv version are you using, compat 14 or 15?

belzebub40k · 2016-03-09T12:49:28Z

root@0216088335f8:/# batctl -v
batctl 2016.0 [batman-adv: 2016.0]

As far as I understand this refers to compat 15.

EDIT: I'll also try to reproduce it on a Raspberry Pi so we know if it is hardware related or not.

belzebub40k · 2016-03-10T08:57:30Z

It looks like this problem is some how related to br-client. If I remove br-client completely from /etc/config/network I'm able to configure 4 mesh-VLANs (didn't try more).

However this isn't really an option as this device is intended to be a backbone node with connects several Ubiquiti devices running stock firmware. We have a management VLAN (3) on each of them which is then bridged with bat0 via br-client. Without this we wouldn't be able to access them from remote.

neocturne · 2016-03-10T08:59:54Z

I've forwarded the issue to the batman-adv developers, they'll have a look.

belzebub40k · 2016-03-10T09:30:38Z

Thanks. Is there a ticket I can track and contribute to?

I just found some more details that may be helpful:
I can leave br-client enabled but need to remove bat0 from it. And again it wont harm the system if I add it manually once everything is up.

neocturne · 2016-03-11T16:41:39Z

I think there is no ticket yet, I'll open one.

I wasn't able to reproduce this issue on x86-vmware with the current Gluon master.

It might be worth noting that the crash happens when the VLAN interfaces are removed from bat0. netifd first sees eth0 as up, adding the 4 VLAN interfaces to bat0, one second later it is seen as down again; when eth0.102 is removed from bat0, something fails and the kernel instead tries to remove eth0 from br-client, which causes the crash as eth0 is not part of br-client.

belzebub40k · 2016-03-13T12:13:07Z

Can confirm that. The problem happens when the interface eth0 is going down. When I set everything up manually and then power cycle the switch it crashes with the same error message.

neocturne · 2016-03-18T12:15:01Z

https://www.open-mesh.org/issues/247

belzebub40k · 2016-03-18T13:37:29Z

Could this be the same problem as we have here?
http://patchwork.ozlabs.org/patch/587118/

belzebub40k · 2016-03-18T19:13:33Z

Ok tried the attached patch against master and it no longer crashes because of trying to remove eth0 from br-client. Now if fails because it tries to remove eth0 from local-node. Looks like I'm digging in the right direction.

dmesg.txt
patch.txt

BTW: I wasn't able to reproduce this behavior on other hardware like the Raspberry Pi.

EDIT:
It crashes only half the time with the message that it tries to remove eth0 from local-node. When it boots without a crash I wasn't able to force a crash (till now). For the moment the combination of this patch and removing the package next-node looks like a working setup.

neocturne · 2016-03-25T22:43:05Z

@belzebub40k, can you test the updated patch from http://lists.openwall.net/netdev/2016/03/25/103 ?

neocturne · 2016-03-25T22:44:17Z

Ah, it can also be found in patchwork: http://patchwork.ozlabs.org/patch/602124/

belzebub40k · 2016-03-28T15:25:14Z

Perfect until now it seems to work without problems. I will give it some more time for testing and keep you posted.

belzebub40k · 2016-04-06T16:23:53Z

Sadly it does work properly. It crashes with the same error as reported initially when the counterpart is power cycled. However disconnecting it physically doesn't cause a crash.

Today I got my BananaPro and I tried unpatched (except those to profiles.mk to build the image) v2016.1.3 on it and it doesn't crash which is very strange as it is mostly a BananaPi M1 with a WiFi chip. <-- NOT TRUE

~~I'm at a point where I consider faulty hardware. I try to get my hands on an other hardware revision of the M1. The one I have is a clone from sinovoip and not the original from Lemaker.~~

neocturne · 2016-04-06T16:48:57Z

I think the issue is that the kernel code is completely b0rked and may fail in various spectacular ways.

Andrew Collins posted this patch as RFC in March: http://patchwork.ozlabs.org/patch/603101/ It has apparently fallen through the cracks and never applied. It solves a refcnt problem (thanks Nik for pointing out this patch) with stacked devices that involves macvlan on a bridge, a bond into the bridge, and the bridge and macvlan are enslaved to a vrf: +--------+ | myvrf | +--------+ | | | +---------+ | | macvlan | | +---------+ | | +----------+ | bridge | +----------+ | +--------+ | bond0 | +--------+ | +--------+ | swp3 | +--------+ Deleting bond0 hangs waiting for bond0 to become free. The splat in dmesg is: [ 206.485340] ------------[ cut here ]------------ [ 206.486052] WARNING: CPU: 0 PID: 746 at /home/dsa/kernel-3.git/net/core/dev.c:6772 rollback_registered_many+0x28a/0x2da [ 206.487563] Modules linked in: macvlan bonding bridge stp llc vrf [ 206.488946] CPU: 0 PID: 746 Comm: ifdown Not tainted 4.8.0-rc7+ torvalds#144 [ 206.489768] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014 [ 206.490973] 0000000000000000 ffff88003b3a3ca8 ffffffff81287ad3 0000000000000000 [ 206.491939] 0000000000000000 ffff88003b3a3ce8 ffffffff8104f19d 00001a743b3a3b58 [ 206.492914] ffff88003c93e000 ffff88003b3a3d48 ffff88003c877400 ffff88003b3a3d48 [ 206.493873] Call Trace: [ 206.494184] [<ffffffff81287ad3>] dump_stack+0x81/0xb6 [ 206.494805] [<ffffffff8104f19d>] __warn+0xc5/0xe0 [ 206.495382] [<ffffffff8104f265>] warn_slowpath_null+0x18/0x1a [ 206.496094] [<ffffffff81412703>] rollback_registered_many+0x28a/0x2da [ 206.496888] [<ffffffff8141277c>] rollback_registered+0x29/0x36 [ 206.497596] [<ffffffff81412950>] unregister_netdevice_queue+0x6a/0xa6 [ 206.498383] [<ffffffffa004ff10>] bonding_store_bonds+0x132/0x186 [bonding] [ 206.499209] [<ffffffff8133baff>] class_attr_store+0x1e/0x20 [ 206.499893] [<ffffffff811c21ce>] sysfs_kf_write+0x44/0x4b [ 206.500558] [<ffffffff811c14cb>] kernfs_fop_write+0x113/0x15d [ 206.501269] [<ffffffff8116216b>] __vfs_write+0x21/0xa0 [ 206.501897] [<ffffffff810833cf>] ? percpu_down_read+0x4e/0x7a [ 206.502595] [<ffffffff81164f25>] ? __sb_start_write+0x5a/0xab [ 206.503302] [<ffffffff81164f25>] ? __sb_start_write+0x5a/0xab [ 206.504015] [<ffffffff81162d7f>] vfs_write+0xa2/0xc6 [ 206.504637] [<ffffffff8116397a>] SyS_write+0x4b/0x79 [ 206.505251] [<ffffffff8150833c>] entry_SYSCALL_64_fastpath+0x1f/0xbd [ 206.506054] ---[ end trace a578aa9ea7e7176a ]--- [ 206.512017] PF_BRIDGE: RTM_SETLINK with unknown ifindex [ 216.765567] unregister_netdevice: waiting for bond0 to become free. Usage count = 1 The splate is from this line in rollback_registered_many(): WARN_ON(netdev_has_any_upper_dev(dev)); --- This is the original commit message from Andrew: This is an RFC patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. This is more to generate discussion than anything else. I don't particularly like this approach, I'm hoping someone has a better idea. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Patch is from Andrew Collins <acollins@cradlepoint.com>, but did not have a formal sign-off. The patch applies to top of tree, so no change made on my part I just pulled it off the web. From: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com>

This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net>

neocturne · 2016-10-20T12:06:54Z

As there is an upstream fix now, I'll try backporting it to the Gluon kernels.

poldy79 · 2016-10-20T21:43:33Z

As we might see this on our gateways, do you have a link to the upstream ticket?

T-X · 2016-10-20T22:46:51Z

Two upstream fixes related to interface removal:

batman-adv: don't access unregistered net_device object (present since v2015.2)
batman-adv: Modify mesh_iface outside sysfs context (master branch only, no release with it yet, not queued for stable kernels?)
- History:
  - batman-adv: Fix hardif remove/add race
  - batman-adv: postpone sysfs removal when unregistering
- Then both reverted:
  - Revert "batman-adv: Fix hardif remove/add race"
  - batman-adv: Revert "postpone sysfs removal when unregistering"

Still pending is this ticket with an according patchset to fix this which still needs a review:

(no claim that this list is complete - @neoraider, if you noticed any other patches feel free to add :-) )

EDIT: Ah, and just saw the patch by Andrew Collins which @NeoRaider refered to above. @poldy79 and @plumpudding, this might be the patch you are most urgently looking for as it matches the kernel traces in #910 and here exactly. Though @NeoRaider seems to have an updated, newer version of this in mind?

EDIT2: This looks like the accepted patch @NeoRaider seems to be refering to:

So far it seems to be part of Linux 4.9-rc1 only but is queued for stable kernels. The patchwork ticket states that it is not completely fixed yet though and that another patch will follow.

belzebub40k · 2016-10-26T20:56:48Z

@NeoRaider I did a dirty backport of Andrew Collins kernel patch and it looks much better. However now it seems like there is a race condition caused by eth0 getting up and down quickly. This sounds exactly like the problem addressed with the batman-adv patches mentioned by @T-X . I'll try with batman-adv master instead of 2016.2.

belzebub40k · 2016-10-27T18:05:02Z

Did not figure out how to change to batman-adv master thus just applied batman-adv: Modify mesh_iface outside sysfs context as a patch. That seams to fix the race condition mostly. At least it does not crash the kernel when uplugging and re-plugging eth0. But I still see messages which indicate that the race condition isn't fixed fully.

batman_adv: (null): no_rebroadcast: Changing from: disabled to: enabled
NOHZ: local_softirq_pending 08

The batman_adv (null) messages appear during every restart of eth0. local_softirq_pending happend just one time and I wasn't able to reproduce it a second time by now.

console log boot and restart of the interface triggered by unplugging or rebooting the switch

console.txt

applied patches

[ Upstream commit 9340903 ] This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ Upstream commit 9340903 ] This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ Upstream commit 93409033ae653f1c9a949202fb537ab095b2092f ] This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ Upstream commit 9340903 ] This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ Upstream commit 93409033ae653f1c9a949202fb537ab095b2092f ] This is a respin of a patch to fix a relatively easily reproducible kernel panic related to the all_adj_list handling for netdevs in recent kernels. The following sequence of commands will reproduce the issue: ip link add link eth0 name eth0.100 type vlan id 100 ip link add link eth0 name eth0.200 type vlan id 200 ip link add name testbr type bridge ip link set eth0.100 master testbr ip link set eth0.200 master testbr ip link add link testbr mac0 type macvlan ip link delete dev testbr This creates an upper/lower tree of (excuse the poor ASCII art): /---eth0.100-eth0 mac0-testbr- \---eth0.200-eth0 When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one reference to eth0 is added, so this results in a panic. This change adds reference count propagation so things are handled properly. Matthias Schiffer reported a similar crash in batman-adv: freifunk-gluon/gluon#680 https://www.open-mesh.org/issues/247 which this patch also seems to resolve. Signed-off-by: Andrew Collins <acollins@cradlepoint.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>