* [B.A.T.M.A.N.] Regression in VLAN sysfs patches
@ 2013-08-15 13:13 Simon Wunderlich
2013-08-15 17:45 ` [B.A.T.M.A.N.] [PATCH] batman-adv: print some info for vlan debugging Antonio Quartulli
0 siblings, 1 reply; 7+ messages in thread
From: Simon Wunderlich @ 2013-08-15 13:13 UTC (permalink / raw)
To: Antonio Quartulli, b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 6164 bytes --]
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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCH] batman-adv: print some info for vlan debugging
2013-08-15 13:13 [B.A.T.M.A.N.] Regression in VLAN sysfs patches Simon Wunderlich
@ 2013-08-15 17:45 ` Antonio Quartulli
2013-08-15 19:32 ` [B.A.T.M.A.N.] [PATCH] batman-adv: check if a softif_vlan already exists Antonio Quartulli
0 siblings, 1 reply; 7+ messages in thread
From: Antonio Quartulli @ 2013-08-15 17:45 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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);
--
1.8.1.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCH] batman-adv: check if a softif_vlan already exists
2013-08-15 17:45 ` [B.A.T.M.A.N.] [PATCH] batman-adv: print some info for vlan debugging Antonio Quartulli
@ 2013-08-15 19:32 ` Antonio Quartulli
2013-08-15 21:05 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
0 siblings, 1 reply; 7+ messages in thread
From: Antonio Quartulli @ 2013-08-15 19:32 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [B.A.T.M.A.N.] [PATCHv2] batman-adv: check if a softif_vlan already exists
2013-08-15 19:32 ` [B.A.T.M.A.N.] [PATCH] batman-adv: check if a softif_vlan already exists Antonio Quartulli
@ 2013-08-15 21:05 ` Antonio Quartulli
2013-08-19 20:20 ` Simon Wunderlich
0 siblings, 1 reply; 7+ messages in thread
From: Antonio Quartulli @ 2013-08-15 21:05 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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;
--
1.8.1.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: check if a softif_vlan already exists
2013-08-15 21:05 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
@ 2013-08-19 20:20 ` Simon Wunderlich
2013-08-20 6:39 ` Antonio Quartulli
0 siblings, 1 reply; 7+ messages in thread
From: Simon Wunderlich @ 2013-08-19 20:20 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 937 bytes --]
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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: check if a softif_vlan already exists
2013-08-19 20:20 ` Simon Wunderlich
@ 2013-08-20 6:39 ` Antonio Quartulli
2013-08-20 8:13 ` Marek Lindner
0 siblings, 1 reply; 7+ messages in thread
From: Antonio Quartulli @ 2013-08-20 6:39 UTC (permalink / raw)
To: The list for a Better Approach To Mobile Ad-hoc Networking
[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]
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
--
Antonio Quartulli
..each of us alone is worth nothing..
Ernesto "Che" Guevara
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: check if a softif_vlan already exists
2013-08-20 6:39 ` Antonio Quartulli
@ 2013-08-20 8:13 ` Marek Lindner
0 siblings, 0 replies; 7+ messages in thread
From: Marek Lindner @ 2013-08-20 8:13 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Antonio Quartulli
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-08-20 8:13 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-15 13:13 [B.A.T.M.A.N.] Regression in VLAN sysfs patches Simon Wunderlich
2013-08-15 17:45 ` [B.A.T.M.A.N.] [PATCH] batman-adv: print some info for vlan debugging Antonio Quartulli
2013-08-15 19:32 ` [B.A.T.M.A.N.] [PATCH] batman-adv: check if a softif_vlan already exists Antonio Quartulli
2013-08-15 21:05 ` [B.A.T.M.A.N.] [PATCHv2] " Antonio Quartulli
2013-08-19 20:20 ` Simon Wunderlich
2013-08-20 6:39 ` Antonio Quartulli
2013-08-20 8:13 ` Marek Lindner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.