[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <46FFC512.9060101@trash.net>
Date: Sun, 30 Sep 2007 17:47:30 +0200
From: Patrick McHardy <kaber@...sh.net>
To: Herbert Xu <herbert@...dor.apana.org.au>
CC: "Eric W. Biederman" <ebiederm@...ssion.com>, davem@...emloft.net,
netdev@...r.kernel.org, oliver@...kum.name,
linux-usb-devel@...ts.sourceforge.net
Subject: Re: [PATCH] rtnl: Simplify ASSERT_RTNL
Herbert Xu wrote:
> On Sat, Sep 29, 2007 at 05:32:41PM +0200, Patrick McHardy wrote:
>
>>For unicast addresses its not strictly necessary since they may
>>only be changed under the RTNL anyway. The reason why it takes
>>the tx_lock is for consistency with multicast address handling,
>>which can't rely on the RTNL since IPv6 changes them from
>>BH context. The idea was that the ->set_rx_mode function should
>>handle both secondary unicast and multicast addresses for
>>simplicity.
>
>
> In any case, coming back to the original question, the RTNL
> assertion is simply wrong in this case because if we're being
> called from IPv6 then the RTNL won't even be held.
>
> So I think we need to
>
> 1) Move the assert into dev_set_promiscuity.
> 2) Take the TX lock in dev_set_promiscuity.
In the IPv6 case we're only changing the multicast list,
so we're never calling into __dev_set_promiscuity.
I actually even added a comment about this :)
/* Unicast addresses changes may only happen under the rtnl,
* therefore calling __dev_set_promiscuity here is safe.
*/
I would prefer to keep the ASSERT_RTNL in __dev_set_promiscuity
since it also covers the __dev_set_rx_mode path. How about
adding an ASSERT_RTNL_ATOMIC without the might_sleep or simply
open coding it?
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists