Hello,
it seems there is an regression in the VLAN sysfs patches. I've tracked the issue a little bit, it seems to be somewhere between:
6f27447 Antonio Quartulli batman-adv: fix vlan compat code (broken)
... (few more patches i can't test because vlan compat is broken)
cc14598 Linus Lüssing batman-adv: (style) fix for switched vid-ifiindex parameter order (works)
The regression is probably in some of Antonios VLAN/sysfs patches.
It can be triggered by calling:
ifconfig bat0 down ; ifconfig bat0 up
Which will lead to a WARNING [1]. This can be reproduced every time I call it. Note that I've not explicitly configured a VLAN on bat0, this is automatically created/added. I'm using a fairly old OpenWRT revision (29637) on kernel 2.6.39.4 here in virtual machines.
Cheers, Simon
[1]
[ 39.317111] 8021q: adding VLAN 0 to HW filter on device bat0 [ 39.322208] ------------[ cut here ]------------ [ 39.324213] WARNING: at fs/sysfs/dir.c:455 sysfs_add_one+0x8b/0xb0() [ 39.325378] sysfs: cannot create duplicate filename '/devices/virtual/net/bat0/mesh/vlan0' [ 39.326955] Modules linked in: virtio_net batman_adv ebt_snat ebt_dnat ebt_arpreply ebt_ip ebt_arp ebt_redirect ebt_mark ebt_vlan ebt_stp ebt_pkttype ebt_mark_m ebt_limit ebt_among ebt_802_3 ebtable_nat ebtable_filter ebtable_broute nf_nat_irc nf_conntrack_irc nf_nat_ftp nf_conntrack_ftp xt_string xt_layer7 ipt_MASQUERADE iptable_nat nf_nat xt_CT xt_conntrack xt_NOTRACK iptable_raw xt_state nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack ipt_REJECT xt_TCPMSS ipt_LOG xt_comment xt_multiport xt_mac xt_limit iptable_mangle iptable_filter ts_fsm ts_bm ts_kmp libcrc32c ipv6 virtio_rng crc32c crypto_hash crypto_algapi [ 39.340649] Pid: 1166, comm: ifconfig Not tainted 2.6.39.4 #180 [ 39.342140] Call Trace: [ 39.342686] [<c10d557b>] ? sysfs_add_one+0x8b/0xb0 [ 39.343899] [<c10284af>] warn_slowpath_common+0x5f/0x80 [ 39.345480] [<c10d557b>] ? sysfs_add_one+0x8b/0xb0 [ 39.347153] [<c102854e>] warn_slowpath_fmt+0x2e/0x30 [ 39.348504] [<c10d557b>] sysfs_add_one+0x8b/0xb0 [ 39.349761] [<c10d55fb>] create_dir+0x5b/0x90 [ 39.351197] [<c10d56ee>] sysfs_create_dir+0x8e/0xa0 [ 39.352558] [<c11386c5>] kobject_add_internal+0xc5/0x1d0 [ 39.353945] [<c1138ad5>] kobject_add+0x75/0x90 [ 39.355177] [<c1138c86>] kobject_create_and_add+0x36/0x70 [ 39.356550] [<c4b8293e>] batadv_sysfs_add_vlan+0x62/0x216 [batman_adv] [ 39.358314] [<c108ce09>] ? __kmalloc+0x119/0x130 [ 39.359554] [<c4b8051d>] batadv_softif_create_vlan+0xb6/0xa25 [batman_adv] [ 39.361571] [<c4b806b9>] batadv_softif_create_vlan+0x252/0xa25 [batman_adv] [ 39.363355] [<c4b80672>] batadv_softif_create_vlan+0x20b/0xa25 [batman_adv] [ 39.365227] [<c127f925>] vlan_device_event+0xc5/0x510 [ 39.366578] [<c126f39c>] ? packet_notifier+0x19c/0x1b0 [ 39.368010] [<c126f200>] ? packet_dev_mc+0xd0/0xd0 [ 39.369239] [<c10425a0>] notifier_call_chain+0x30/0x60 [ 39.370600] [<c104266a>] raw_notifier_call_chain+0x1a/0x20 [ 39.372089] [<c120d04e>] call_netdevice_notifiers+0x4e/0x60 [ 39.373468] [<c121b855>] ? rtmsg_ifinfo+0xb5/0xe0 [ 39.374713] [<c121055e>] __dev_notify_flags+0x2e/0x70 [ 39.376240] [<c12105e3>] dev_change_flags+0x43/0x60 [ 39.377803] [<c1258665>] devinet_ioctl+0x285/0x670 [ 39.378985] [<c1210ec5>] ? dev_ioctl+0x665/0x6c0 [ 39.379871] [<c1210be5>] ? dev_ioctl+0x385/0x6c0 [ 39.380797] [<c12597f2>] inet_ioctl+0x72/0xa0 [ 39.381689] [<c11fea88>] sock_ioctl+0x218/0x250 [ 39.382603] [<c11fe870>] ? sock_fasync+0x80/0x80 [ 39.383550] [<c109e09e>] do_vfs_ioctl+0x4ee/0x530 [ 39.384488] [<c108df28>] ? fd_install+0x48/0x60 [ 39.385606] [<c11ff738>] ? sys_socket+0x48/0x70 [ 39.386665] [<c120054a>] ? sys_socketcall+0x6a/0x270 [ 39.387666] [<c109e119>] sys_ioctl+0x39/0x60 [ 39.388541] [<c128d325>] syscall_call+0x7/0xb [ 39.389417] ---[ end trace c59b34f09c69e69f ]--- [ 39.390323] kobject_add_internal failed for vlan0 with -EEXIST, don't try to register things with the same name in the same directory. [ 39.392757] Pid: 1166, comm: ifconfig Tainted: G W 2.6.39.4 #180 [ 39.394234] Call Trace: [ 39.395176] [<c128a7fc>] ? printk+0x18/0x1c [ 39.396080] [<c11387a8>] kobject_add_internal+0x1a8/0x1d0 [ 39.397123] [<c1138ad5>] kobject_add+0x75/0x90 [ 39.398037] [<c1138c86>] kobject_create_and_add+0x36/0x70 [ 39.399241] [<c4b8293e>] batadv_sysfs_add_vlan+0x62/0x216 [batman_adv] [ 39.400455] [<c108ce09>] ? __kmalloc+0x119/0x130 [ 39.401382] [<c4b8051d>] batadv_softif_create_vlan+0xb6/0xa25 [batman_adv] [ 39.402634] [<c4b806b9>] batadv_softif_create_vlan+0x252/0xa25 [batman_adv] [ 39.404345] [<c4b80672>] batadv_softif_create_vlan+0x20b/0xa25 [batman_adv] [ 39.405675] [<c127f925>] vlan_device_event+0xc5/0x510 [ 39.406671] [<c126f39c>] ? packet_notifier+0x19c/0x1b0 [ 39.407821] [<c126f200>] ? packet_dev_mc+0xd0/0xd0 [ 39.408836] [<c10425a0>] notifier_call_chain+0x30/0x60 [ 39.410132] [<c104266a>] raw_notifier_call_chain+0x1a/0x20 [ 39.411492] [<c120d04e>] call_netdevice_notifiers+0x4e/0x60 [ 39.412554] [<c121b855>] ? rtmsg_ifinfo+0xb5/0xe0 [ 39.413485] [<c121055e>] __dev_notify_flags+0x2e/0x70 [ 39.414473] [<c12105e3>] dev_change_flags+0x43/0x60 [ 39.415752] [<c1258665>] devinet_ioctl+0x285/0x670 [ 39.416716] [<c1210ec5>] ? dev_ioctl+0x665/0x6c0 [ 39.417651] [<c1210be5>] ? dev_ioctl+0x385/0x6c0 [ 39.418691] [<c12597f2>] inet_ioctl+0x72/0xa0 [ 39.419606] [<c11fea88>] sock_ioctl+0x218/0x250 [ 39.420642] [<c11fe870>] ? sock_fasync+0x80/0x80 [ 39.421544] [<c109e09e>] do_vfs_ioctl+0x4ee/0x530 [ 39.422456] [<c108df28>] ? fd_install+0x48/0x60 [ 39.423348] [<c11ff738>] ? sys_socket+0x48/0x70 [ 39.424641] [<c120054a>] ? sys_socketcall+0x6a/0x270 [ 39.426055] [<c109e119>] sys_ioctl+0x39/0x60 [ 39.426982] [<c128d325>] syscall_call+0x7/0xb [ 39.427944] kobject_create_and_add: kobject_add error: -17 [ 39.428999] batman_adv: bat0: Can't add sysfs directory: bat0/vlan0
This patch will print some more information when doing ifconfig bat0 up & down
please report the output.
Signed-off-by: Antonio Quartulli ordex@autistici.org --- soft-interface.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/soft-interface.c b/soft-interface.c index d018c49..7876476 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -525,6 +525,9 @@ static int batadv_interface_add_vid(struct net_device *dev, __be16 proto,
vid |= BATADV_VLAN_HAS_TAG;
+ printk("batadv_add_vid(): creating vlan %s %d\n", dev->name, + BATADV_PRINT_VID(vid)); + return batadv_softif_create_vlan(bat_priv, vid); }
@@ -551,9 +554,14 @@ static int batadv_interface_kill_vid(struct net_device *dev, __be16 proto, if (proto != htons(ETH_P_8021Q)) return -EINVAL;
+ printk("batadv_kill_vid(): %s %d\n", dev->name, + BATADV_PRINT_VID(vid | BATADV_VLAN_HAS_TAG)); + vlan = batadv_softif_vlan_get(bat_priv, vid | BATADV_VLAN_HAS_TAG); - if (!vlan) + if (!vlan) { + printk("batadv_kill_vid(): couldn't find vlan to kill\n"); return -ENOENT; + }
batadv_softif_destroy_vlan(bat_priv, vlan);
From: Antonio Quartulli antonio@open-mesh.com
Before creating a new softif_vlan it is better to check if that does already exist. If so batman-adv should refuse to create a new structure otherwise this would lead to an inconsistent state.
Normally this is not a problem because the operating system will prevent from creating the same vlan twice, but some ancient kernels exhibited an improper behaviour that led to a bug.
Introduced by 952cebb57518ec18dfdebfcb2b85539f58f20779 ("batman-adv: add per VLAN interface attribute framework")
Reported-by: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de Signed-off-by: Antonio Quartulli antonio@open-mesh.com --- soft-interface.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/soft-interface.c b/soft-interface.c index d018c49..2459967 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -450,6 +450,10 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid) struct batadv_softif_vlan *vlan; int err;
+ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (vlan) + return -EEXIST; + vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC); if (!vlan) return -ENOMEM;
From: Antonio Quartulli antonio@open-mesh.com
Before creating a new softif_vlan it is better to check if that does already exist. If so batman-adv should refuse to create a new structure otherwise this would lead to an inconsistent state.
Normally this is not a problem because the operating system will prevent from creating the same vlan twice, but some ancient kernels exhibited an improper behaviour that led to a bug.
Introduced by 952cebb57518ec18dfdebfcb2b85539f58f20779 ("batman-adv: add per VLAN interface attribute framework")
Reported-by: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de Signed-off-by: Antonio Quartulli antonio@open-mesh.com ---
however I don't know if returning -EEXIST will make ifconfig happy..it may be that we simply have to return 0 and silently ignore the problem.. let's see :)
cheers,
changes from v1: - fixed refcounting
soft-interface.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/soft-interface.c b/soft-interface.c index d018c49..bafc811 100644 --- a/soft-interface.c +++ b/soft-interface.c @@ -450,6 +450,12 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid) struct batadv_softif_vlan *vlan; int err;
+ vlan = batadv_softif_vlan_get(bat_priv, vid); + if (vlan) { + batadv_softif_vlan_free_ref(vlan); + return -EEXIST; + } + vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC); if (!vlan) return -ENOMEM;
Hey Antonio,
Thanks, that solves the problem on my ancient kernel! :)
I don't see a problem with -EEXIST (at least there is no error whatsoever), so I think we can leave it at that.
Tested-by: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
Just one thing ...
On Thu, Aug 15, 2013 at 11:05:55PM +0200, Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Before creating a new softif_vlan it is better to check if that does already exist. If so batman-adv should refuse to create a new structure otherwise this would lead to an inconsistent state.
Normally this is not a problem because the operating system will prevent from creating the same vlan twice, but some ancient kernels exhibited an improper behaviour that led to a bug.
You might want to skip that when sending upstream? They might not care about older kernels. Duno. :)
Thanks! Simon
On Mon, Aug 19, 2013 at 10:20:28PM +0200, Simon Wunderlich wrote:
Hey Antonio,
Thanks, that solves the problem on my ancient kernel! :)
I don't see a problem with -EEXIST (at least there is no error whatsoever), so I think we can leave it at that.
Tested-by: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
Just one thing ...
On Thu, Aug 15, 2013 at 11:05:55PM +0200, Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Before creating a new softif_vlan it is better to check if that does already exist. If so batman-adv should refuse to create a new structure otherwise this would lead to an inconsistent state.
Normally this is not a problem because the operating system will prevent from creating the same vlan twice, but some ancient kernels exhibited an improper behaviour that led to a bug.
You might want to skip that when sending upstream? They might not care about older kernels. Duno. :)
This patch is only for us, because it is going to be squashed with a previous one before going to David. So I'd leave the commit message as it is.
Cheers,
Thanks! Simon
On Tuesday, August 20, 2013 14:39:06 Antonio Quartulli wrote:
Tested-by: Simon Wunderlich simon.wunderlich@s2003.tu-chemnitz.de
Just one thing ...
On Thu, Aug 15, 2013 at 11:05:55PM +0200, Antonio Quartulli wrote:
From: Antonio Quartulli antonio@open-mesh.com
Before creating a new softif_vlan it is better to check if that does already exist. If so batman-adv should refuse to create a new structure otherwise this would lead to an inconsistent state.
Normally this is not a problem because the operating system will prevent from creating the same vlan twice, but some ancient kernels exhibited an improper behaviour that led to a bug.
You might want to skip that when sending upstream? They might not care about older kernels. Duno. :)
This patch is only for us, because it is going to be squashed with a previous one before going to David. So I'd leave the commit message as it is.
Applied in revision 00f2151.
Thanks, Marek
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Marek Lindner -
Simon Wunderlich