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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
