[Top] [All Lists]

Re: [PATCH 2.6.12-rc2] bonding: partially back out dev_set_mac_address

To: "David S. Miller" <davem@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2.6.12-rc2] bonding: partially back out dev_set_mac_address
From: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date: Mon, 25 Apr 2005 22:41:36 +1000
Cc: fubar@xxxxxxxxxx, netdev@xxxxxxxxxxx, jgarzik@xxxxxxxxx
In-reply-to: <20050424185149.278ffb93.davem@xxxxxxxxxxxxx>
References: <20050408221629.GA21125@xxxxxxxxxxxxxxxxxxx> <200504082356.j38Ntr7k010144@xxxxxxxxxxxxxxxxxxxxxx> <20050409002137.GA21726@xxxxxxxxxxxxxxxxxxx> <20050409003108.GA21962@xxxxxxxxxxxxxxxxxxx> <20050424185149.278ffb93.davem@xxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6+20040907i
On Sun, Apr 24, 2005 at 06:51:49PM -0700, David S. Miller wrote:
> > So if you want a quick and dirty fix, why not make bonding call
> > dev_set_mac_address from a work queue?
> I'd say we still have at least two weeks until 2.6.12 final goes out.
> So if you want to work on the proper long-term fix now, by all means
> please do so.

Well I had a go at it but I realised that this isn't something that
can be fixed properly in two weeks.  The work queue hack can
probably be done in that time frame but it'll be darn ugly.

I found another reason why we need to defer the dev_set_mac_address to
process context: We need to hold the RTNL while it's taking place.
Otherwise someone else could come along and change the slave's MAC
address at the same time.  Because device drivers tend to rely on the
RTNL to guarantee non-reentrancy, this is likely to break.

I also managed to find a little bug along the way though so let's quash
it :) bond_alb_set_mac_address did not acquire bond->lock before
operating on the slaves.  As it can be done at any time by the user,
this could interfere with the timers.

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

Visit Openswan at
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page:
PGP Key:

Attachment: p
Description: Text document

<Prev in Thread] Current Thread [Next in Thread>