From: "Nicolas de Pesloüan" <nicolas.2p.debian@gmail.com>
To: Stephen Hemminger <shemminger@vyatta.com>
Cc: David Miller <davem@davemloft.net>,
Jay Vosburgh <fubar@us.ibm.com>,
Andy Gospodarek <andy@greyhouse.net>,
netdev@vger.kernel.org
Subject: Re: [PATCH] bonding: fix error handling if slave is busy (v2)
Date: Sun, 01 Jan 2012 01:28:50 +0100 [thread overview]
Message-ID: <4EFFA8C2.9040708@gmail.com> (raw)
In-Reply-To: <20111231161322.40b16d69@nehalam.linuxnetplumber.net>
Le 01/01/2012 01:13, Stephen Hemminger a écrit :
> On Sun, 01 Jan 2012 01:09:50 +0100
> Nicolas de Pesloüan<nicolas.2p.debian@gmail.com> wrote:
>
>> Le 01/01/2012 00:26, Stephen Hemminger a écrit :
>>> If slave device already has a receive handler registered, then the
>>> error unwind of bonding device enslave function is broken.
>>>
>>> The following will leave a pointer to freed memory in the slave
>>> device list, causing a later kernel panic.
>>> # modprobe dummy
>>> # ip li add dummy0-1 link dummy0 type macvlan
>>> # modprobe bonding
>>> # echo +dummy0>/sys/class/net/bond0/bonding/slaves
>>>
>>> The fix is to detach the slave (which removes it from the list)
>>> in the unwind path.
>>>
>>> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
>>
>> Thanks Stephen.
>>
>> Reviewed-by: Nicolas de Pesloüan<nicolas.2p.debian@free.fr>
>
> The locking in bond driver is a tangled web.
>
> Would be cleaner to get rid of bond->lock altogether.
> Slave add/delete should be protected by RTNL, and the lookup should
> be converted to RCU. The problem is that bonding driver implements
> own form of circular list to handle round-robin etc.
Bonding has become an incredibly complex thing, due to the large number of corner cases it needs to
handle. And the locking system in probably part of the problem.
Unfortunately, I'm far from a Linux locking specialist, so I cannot comment on this... I just
noticed that searching for RTNL in Documentations yields no result... :-(
Nicolas.
next prev parent reply other threads:[~2012-01-01 0:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-30 22:40 [PATCH] bonding: fix error handling if slave is busy Stephen Hemminger
2011-12-31 16:11 ` Nicolas de Pesloüan
2011-12-31 23:26 ` [PATCH] bonding: fix error handling if slave is busy (v2) Stephen Hemminger
2012-01-01 0:09 ` Nicolas de Pesloüan
2012-01-01 0:13 ` Stephen Hemminger
2012-01-01 0:28 ` Nicolas de Pesloüan [this message]
2012-01-03 17:49 ` David Miller
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=4EFFA8C2.9040708@gmail.com \
--to=nicolas.2p.debian@gmail.com \
--cc=andy@greyhouse.net \
--cc=davem@davemloft.net \
--cc=fubar@us.ibm.com \
--cc=netdev@vger.kernel.org \
--cc=shemminger@vyatta.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).