From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Hutchings Subject: Re: [PATCH V2] vlan:return error when real dev is enslaved Date: Tue, 15 Nov 2011 17:34:31 +0000 Message-ID: <1321378471.2744.5.camel@bwh-desktop> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Patrick McHardy , "David S. Miller" , "open list:VLAN (802.1Q)" , open list To: Weiping Pan Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Tue, 2011-11-15 at 20:44 +0800, Weiping Pan wrote: > Qinhuibin reported a kernel panic when he do some operation about vla= n. > https://lkml.org/lkml/2011/11/6/218 >=20 > The operation is as below: > ifconfig eth2 up > modprobe bonding > modprobe 8021q > ifconfig bond0 up > ifenslave bond0 eth2 > vconfig add eth2 3300 > vconfig add bond0 33 > vconfig rem eth2.3300 > > the panic stack is as below: > [] panic_event+0x49/0x70 [ipmi_msghandler] > [] notifier_call_chain+0x37/0x70 > [] panic+0xa2/0x195 > [] oops_end+0xd8/0x140 > [] no_context+0xf7/0x280 > [] __bad_area_nosemaphore+0x175/0x250 > [] page_fault+0x28/0x30 > [] igb_vlan_rx_kill_vid+0x4d/0x100 [igb] > [] bond_vlan_rx_kill_vid+0x9f/0x290 [bonding] > [] unregister_vlan_dev+0x136/0x180 [8021q] > [] vlan_ioctl_handler+0x170/0x3f0 [8021q] > [] sock_ioctl+0x21f/0x280 > [] vfs_ioctl+0x2f/0xb0 > [] do_vfs_ioctl+0x3cb/0x5a0 > [] sys_ioctl+0xa1/0xb0 > [] system_call_fastpath+0x16/0x1b > [<00007f108a2b8bd7>] 0x7f108a2b8bd7 > And the nic is as below: > [root@localhost ~]# ethtool -i eth2 > driver: igb > version: 3.0.6-k2 > firmware-version: 1.2-1 > bus-info: 0000:04:00.0 > kernel version=EF=BC=9A > 2.6.32.12-0.7 also happen in 2.6.32-131 >=20 > For kernel 2.6.32, the reason of this bug is that when we do "vconfig= add bond0 33", > adapter->vlgrp is overwritten in igb_vlan_rx_register. So when we do = "vconfig rem > eth2.3300", it can't find the correct vlgrp. >=20 > And this bug is avoided by vlan cleanup patchset from Jiri Pirko > , especially commit b2cb09b1a772(igb: do vlan clea= nup). Since this won't be applied to mainline first, you should send it directly to stable@vger.kernel.org as well as to netdev. > But it is not a correct operation to creat a vlan interface on eth2 > when it have been enslaved by bond0, so this patch is to return error > when the real dev is already enslaved. >=20 > Changelog: > V2: use pr_err instead of pr_info >=20 > Signed-off-by: Weiping Pan > --- > net/8021q/vlan.c | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) >=20 > diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c > index 5471628..7ce50ba 100644 > --- a/net/8021q/vlan.c > +++ b/net/8021q/vlan.c > @@ -148,6 +148,11 @@ int vlan_check_real_dev(struct net_device *real_= dev, u16 vlan_id) > const char *name =3D real_dev->name; > const struct net_device_ops *ops =3D real_dev->netdev_ops; > =20 > + if (real_dev->flags & IFF_SLAVE) { > + pr_err("Error, %s was already enslaved\n", name); > + return -EOPNOTSUPP; I think the appropriate error code is EBUSY. The operation is supporte= d (probably - we haven't checked for VLAN_CHALLENGED yet) but the device is otherwise occupied. Ben. > + } > + > if (real_dev->features & NETIF_F_VLAN_CHALLENGED) { > pr_info("VLANs not supported on %s\n", name); > return -EOPNOTSUPP; --=20 Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.