Search Linux Wireless

Re: [PATCH v9 2/4] mac80211: add multiple bssid support to interface handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2021-04-19 04:28, Johannes Berg wrote:
Hi,

> > +	if (sdata && sdata->vif.type == NL80211_IFTYPE_AP) {
> > +		if (sdata->vif.multiple_bssid.flags &
> > IEEE80211_VIF_MBSS_TRANSMITTING) {
> > +			struct ieee80211_sub_if_data *child;
> > +
> > +			rcu_read_lock();
> > +			list_for_each_entry_rcu(child, &sdata->local->interfaces, list)
> > +				if (child->vif.multiple_bssid.parent == &sdata->vif)
> > +					dev_close(child->wdev.netdev);
> > +			rcu_read_unlock();

This was added for graceful shutdown of non-transmitting interfaces
whenever the transmitting one is being brought down. 


I know, I asked you to.

But I see that
dev_close() is happening twice now.


That wouldn't be an issue.

The issue is that dev_close() needs to be able to sleep, and it even
contains a might_sleep(), so you can't do it under the RCU protection
you used here.

Inclining towards removing this and just return error to application if it tries to remove transmitting before all non-transmitting are deleted.
However, currently the "parent" pointer to indicate the transmitting
interface is maintained in mac80211, nothing in cfg80211.

That seems kinda awkward, considering e.g. if hostapd crashes and then a new instance has to clean up, it might not really have much knowledge of
the order in which it should be doing that.

I think it's better if you just fix the locking here?

johannes

Hi Johannes,
Thanks for the response, I need more help.

Is rcu_read_lock is not allowed around dev_close() because it might block or *ANY* lock? Because both functions with new dev_close() themselves are called with locks held, (1) ieee80211_do_stop() happens inside wiphy_lock(sdata->local->hw.wiphy)
(2) ieee80211_del_iface() happens inside mutex_lock(&rdev->wiphy.mtx).
Should these be unlocked temporarily too before calling dev_close()?
Didn't find any instance of wiphy.mtx lock/unlock in mac80211.

Also, in cfg.c, list_for_each_entry(sdata, &local->interfaces, list) is called with two different murexes: (1) local->iflist_mtx
(2) local->mtx

Which is the correct one for this purpose among above two and rcu_read_lock()?
Once that decided, would following be sufficient -
    lock()
    list_for_each_entry(sdata, &local->interfaces, list) {
        get_child_pointer
        unlock()
        dev_close()
        lock()
    }
    unlock

Thanks.



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux