From: Ding Tianhong <dingtianhong@huawei.com>
To: Ben Hutchings <ben@decadent.org.uk>,
John Fastabend <john.r.fastabend@intel.com>
Cc: Patrick McHardy <kaber@trash.net>,
"David S. Miller" <davem@davemloft.net>,
Netdev <netdev@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net RESEND] vlan: don't allow to add VLAN on VLAN device
Date: Wed, 5 Mar 2014 09:31:14 +0800 [thread overview]
Message-ID: <53167E62.4020601@huawei.com> (raw)
In-Reply-To: <1393978247.16256.27.camel@deadeye.wl.decadent.org.uk>
On 2014/3/5 8:10, Ben Hutchings wrote:
> On Thu, 2014-02-27 at 19:45 -0800, John Fastabend wrote:
>> On 2/27/2014 6:43 PM, Ding Tianhong wrote:
>>> I run these steps:
>>>
>>> modprobe 8021q
>>> vconfig add eth2 20
>>> vconfig add eth2.20 20
>>> ifconfig eth2 xx.xx.xx.xx
>>>
>>> then the Call Trace happened:
>>>
>>
>> [...]
>>
>>> ========================================================================
>>>
>>> The reason is that if add vlan on vlan dev, the vlan dev will create vlan_info,
>>> then the notification will let the real dev to run dev_set_rx_mode() and hold
>>> netif_addr_lock, and then the real dev will call ndo_set_rx_mode(), if the real
>>> dev is vlan dev, the ndo_set_rx_mode() will hold netif_addr_lock again, so deadlock
>>> happened.
>>>
>>> Don't allow to add vlan on vlan dev to fix this problem.
>>>
>>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>>> ---
>>
>> I'm not sure we can just disable stacked vlans. There might be something
>> using them today and they have worked in the past. Lets try to find a
>> better fix.
>
> I don't think there's any deadlock possible here. We try to acquire the
> addr_list_lock for eth2.20, then the addr_list_lock for eth2. We never
> try to acquire them in the opposite order. The fix would involve
> telling lockdep about lock ordering between stacked net_devices (I have
> no idea how that's done).
>
> Ben.
>
Yep, it is a warning when the lockdep is open, I review the code again, and the deadlock would not happen,
just the same class of locks twice, so I think it is not a bugfix, just like a optimization.
Regards
Ding
prev parent reply other threads:[~2014-03-05 1:31 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-28 2:43 [PATCH net RESEND] vlan: don't allow to add VLAN on VLAN device Ding Tianhong
2014-02-28 3:45 ` John Fastabend
2014-02-28 5:26 ` Ding Tianhong
2014-02-28 5:41 ` Florian Fainelli
2014-02-28 6:34 ` Ding Tianhong
2014-03-05 0:10 ` Ben Hutchings
2014-03-05 1:31 ` Ding Tianhong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53167E62.4020601@huawei.com \
--to=dingtianhong@huawei.com \
--cc=ben@decadent.org.uk \
--cc=davem@davemloft.net \
--cc=john.r.fastabend@intel.com \
--cc=kaber@trash.net \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.