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

[Aloka Dixit <alokad@codeaurora.org>]

On 2021-04-08 05:06, Johannes Berg wrote:
> On Wed, 2021-03-10 at 10:26 -0800, Aloka Dixit wrote:
>> From: John Crispin <john@phrozen.org>
>> 
>> Add a new helper ieee80211_set_multiple_bssid_options() takes 
>> propagating
>> the cfg80211 data down the stack.
>> 
>> The patch also makes sure that all members of the bss set will get 
>> closed
>> when either of them is shutdown.
> 
> s/either/any/
>> 
>>  static int ieee80211_del_iface(struct wiphy *wiphy, struct 
>> wireless_dev *wdev)
>>  {
>> +	struct ieee80211_sub_if_data *sdata;
>> +
>> +	sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
> 
> can be one line
> 

Okay

>> +	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();
> 
> You never tested this properly, this is wrong.
> 
> johannes

This was added for graceful shutdown of non-transmitting interfaces 
whenever the transmitting one is being brought down. But I see that 
dev_close() is happening twice now.

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.

Which option would be better,
(1) Add new function to mac80211_config_ops to return yes/no to whether 
cfg80211 can delete a particular interface when MBSSID is in use.
(2) Add a new pointer in struct wireless_dev to track the transmitting 
interface of the set. Then the yes/no decision can be taken in cfg80211 
itself.

Thanks.