diff mbox series

[RFC,v3,6/8] wifi: mac80211: extend ifcomb check functions for multi-radio

Message ID bc603fc671010bb720e75881ef0e22d81ec6e2eb.1717696995.git-series.nbd@nbd.name
State Superseded
Headers show
Series cfg80211/mac80211: support defining multiple radios per wiphy | expand

Commit Message

Felix Fietkau June 6, 2024, 6:07 p.m. UTC
Add support for counting global and per-radio max/current number of
channels, as well as checking radio-specific interface combinations.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/cfg.c         |   7 +-
 net/mac80211/chan.c        |  17 +++--
 net/mac80211/ibss.c        |   2 +-
 net/mac80211/ieee80211_i.h |   6 +-
 net/mac80211/iface.c       |   2 +-
 net/mac80211/util.c        | 131 +++++++++++++++++++++++++++-----------
 6 files changed, 116 insertions(+), 49 deletions(-)

Comments

Karthikeyan Periyasamy June 7, 2024, 4:45 a.m. UTC | #1
On 6/6/2024 11:37 PM, Felix Fietkau wrote:
> Add support for counting global and per-radio max/current number of
> channels, as well as checking radio-specific interface combinations.
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/cfg.c         |   7 +-
>   net/mac80211/chan.c        |  17 +++--
>   net/mac80211/ibss.c        |   2 +-
>   net/mac80211/ieee80211_i.h |   6 +-
>   net/mac80211/iface.c       |   2 +-
>   net/mac80211/util.c        | 131 +++++++++++++++++++++++++++-----------
>   6 files changed, 116 insertions(+), 49 deletions(-)
> 

...

>   
> +static u32
> +__ieee80211_get_radio_mask(struct ieee80211_sub_if_data *sdata)
> +{
> +	struct ieee80211_local *local = sdata->local;
> +	struct ieee80211_chanctx_conf *conf;
> +	struct ieee80211_link_data *link;
> +	u32 mask = 0;
> +
> +	for_each_sdata_link(local, link) {
> +		conf = rcu_dereference(link->conf->chanctx_conf);
> +		if (!conf || conf->radio_idx < 0)
> +			continue;
> +
> +		mask |= BIT(conf->radio_idx);
> +	}
> +
> +	return mask;
> +}
> +

I believe __ieee80211_get_radio_mask(sdata) should return the radio mask
used by this sdata right ?

if so, then you should not use "for_each_sdata_link(local, link)" 
because it iterate for all the sdata in the given local and give the 
radio mask. So always return all the radio (bitmap mask) used by the 
wiphy currently.

You can use either of below one

for_each_vif_active_link()

  or

for (link_id = 0; link_id < ARRAY_SIZE(sdata->link); link_id++)


> +int ieee80211_get_radio_mask(struct wiphy *wiphy, struct net_device *dev,
> +			     u32 *mask)
> +{
> +	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +
> +	*mask = __ieee80211_get_radio_mask(sdata);
> +
> +	return 0;
> +}
> +
> +static bool
> +ieee80211_sdata_uses_radio(struct ieee80211_sub_if_data *sdata, int radio_idx)
> +{
> +	if (radio_idx < 0)
> +		return true;
> +
> +	return __ieee80211_get_radio_mask(sdata) & BIT(radio_idx);

same here __ieee80211_get_radio_mask(sdata) usage

> +}
> +
Felix Fietkau June 7, 2024, 4:49 a.m. UTC | #2
On 07.06.24 06:45, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>> Add support for counting global and per-radio max/current number of
>> channels, as well as checking radio-specific interface combinations.
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>>   net/mac80211/cfg.c         |   7 +-
>>   net/mac80211/chan.c        |  17 +++--
>>   net/mac80211/ibss.c        |   2 +-
>>   net/mac80211/ieee80211_i.h |   6 +-
>>   net/mac80211/iface.c       |   2 +-
>>   net/mac80211/util.c        | 131 +++++++++++++++++++++++++++-----------
>>   6 files changed, 116 insertions(+), 49 deletions(-)
>> 
> 
> ...
> 
>>   
>> +static u32
>> +__ieee80211_get_radio_mask(struct ieee80211_sub_if_data *sdata)
>> +{
>> +	struct ieee80211_local *local = sdata->local;
>> +	struct ieee80211_chanctx_conf *conf;
>> +	struct ieee80211_link_data *link;
>> +	u32 mask = 0;
>> +
>> +	for_each_sdata_link(local, link) {
>> +		conf = rcu_dereference(link->conf->chanctx_conf);
>> +		if (!conf || conf->radio_idx < 0)
>> +			continue;
>> +
>> +		mask |= BIT(conf->radio_idx);
>> +	}
>> +
>> +	return mask;
>> +}
>> +
> 
> I believe __ieee80211_get_radio_mask(sdata) should return the radio mask
> used by this sdata right ?
> 
> if so, then you should not use "for_each_sdata_link(local, link)"
> because it iterate for all the sdata in the given local and give the
> radio mask. So always return all the radio (bitmap mask) used by the
> wiphy currently.
> 
> You can use either of below one
> 
> for_each_vif_active_link()
> 
>    or
> 
> for (link_id = 0; link_id < ARRAY_SIZE(sdata->link); link_id++)

Right, I copied the wrong code :)
Will fix, thanks.

- Felix
Karthikeyan Periyasamy June 7, 2024, 9:22 a.m. UTC | #3
On 6/6/2024 11:37 PM, Felix Fietkau wrote:

...

>   
> +static u32
> +__ieee80211_get_radio_mask(struct ieee80211_sub_if_data *sdata)
> +{
> +	struct ieee80211_local *local = sdata->local;
> +	struct ieee80211_chanctx_conf *conf;
> +	struct ieee80211_link_data *link;
> +	u32 mask = 0;
> +
> +	for_each_sdata_link(local, link) {
> +		conf = rcu_dereference(link->conf->chanctx_conf);
> +		if (!conf || conf->radio_idx < 0)
> +			continue;
> +
> +		mask |= BIT(conf->radio_idx);
> +	}
> +
> +	return mask;
> +}
> +
> +int ieee80211_get_radio_mask(struct wiphy *wiphy, struct net_device *dev,
> +			     u32 *mask)
> +{
> +	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +
> +	*mask = __ieee80211_get_radio_mask(sdata);
> +
> +	return 0;
> +}
> +
> +static bool
> +ieee80211_sdata_uses_radio(struct ieee80211_sub_if_data *sdata, int radio_idx)
> +{
> +	if (radio_idx < 0)
> +		return true;
> +
> +	return __ieee80211_get_radio_mask(sdata) & BIT(radio_idx);
> +}
> +
> +static void
> +ieee80211_fill_ifcomb_params(struct ieee80211_local *local,
> +			     struct iface_combination_params *params,
> +			     const struct cfg80211_chan_def *chandef,
> +			     struct ieee80211_sub_if_data *sdata,
> +			     int radio_idx)
> +{
> +	struct ieee80211_sub_if_data *sdata_iter;
> +	struct ieee80211_chanctx *ctx;
> +
> +	list_for_each_entry(ctx, &local->chanctx_list, list) {
> +		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
> +			continue;
> +
> +		if (radio_idx >= 0 && ctx->conf.radio_idx != radio_idx)
> +			continue;
> +
> +		if (chandef &&
> +		    cfg80211_chandef_compatible(chandef, &ctx->conf.def))
> +			continue;
> +
> +		params->num_different_channels++;
> +
> +		params->radar_detect |=
> +			ieee80211_chanctx_radar_detect(local, ctx);
> +	}
> +
> +	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> +		struct wireless_dev *wdev_iter;
> +
> +		wdev_iter = &sdata_iter->wdev;
> +
> +		if (sdata_iter == sdata ||
> +		    !ieee80211_sdata_running(sdata_iter) ||
> +		    cfg80211_iftype_allowed(local->hw.wiphy,
> +					    wdev_iter->iftype, 0, 1))
> +			continue;
> +
> +		if (!ieee80211_sdata_uses_radio(sdata_iter, radio_idx))
> +			continue;
> +
> +		params->iftype_num[wdev_iter->iftype]++;
> +	}
> +}
> +
>   int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>   				 const struct cfg80211_chan_def *chandef,
>   				 enum ieee80211_chanctx_mode chanmode,
> -				 u8 radar_detect)
> +				 u8 radar_detect, int radio_idx)
>   {
> +	bool shared = chanmode == IEEE80211_CHANCTX_SHARED;
>   	struct ieee80211_local *local = sdata->local;
> -	struct ieee80211_sub_if_data *sdata_iter;
>   	enum nl80211_iftype iftype = sdata->wdev.iftype;
>   	struct ieee80211_chanctx *ctx;
>   	int total = 1;
> @@ -3977,6 +4060,8 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>   	if (iftype != NL80211_IFTYPE_UNSPECIFIED)
>   		params.iftype_num[iftype] = 1;
>   
> +	ieee80211_fill_ifcomb_params(local, &params, shared ? chandef : NULL,
> +				     sdata, radio_idx);
>   	list_for_each_entry(ctx, &local->chanctx_list, list) {
>   		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
>   			continue;
> @@ -3986,28 +4071,9 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
>   			params.num_different_channels++;
>   			continue;
>   		}
> -		if (chandef && chanmode == IEEE80211_CHANCTX_SHARED &&
> -		    cfg80211_chandef_compatible(chandef,
> -						&ctx->conf.def))
> -			continue;
>   		params.num_different_channels++;
>   	}
>   
> -	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> -		struct wireless_dev *wdev_iter;
> -
> -		wdev_iter = &sdata_iter->wdev;
> -
> -		if (sdata_iter == sdata ||
> -		    !ieee80211_sdata_running(sdata_iter) ||
> -		    cfg80211_iftype_allowed(local->hw.wiphy,
> -					    wdev_iter->iftype, 0, 1))
> -			continue;
> -
> -		params.iftype_num[wdev_iter->iftype]++;
> -		total++;
> -	}
> -
>   	if (total == 1 && !params.radar_detect)
>   		return 0;

ieee80211_check_combinations() missed to update "params.radio", no ?
Karthikeyan Periyasamy June 7, 2024, 10:07 a.m. UTC | #4
On 6/6/2024 11:37 PM, Felix Fietkau wrote:
> Add support for counting global and per-radio max/current number of
> channels, as well as checking radio-specific interface combinations.
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/cfg.c         |   7 +-
>   net/mac80211/chan.c        |  17 +++--
>   net/mac80211/ibss.c        |   2 +-
>   net/mac80211/ieee80211_i.h |   6 +-
>   net/mac80211/iface.c       |   2 +-
>   net/mac80211/util.c        | 131 +++++++++++++++++++++++++++-----------
>   6 files changed, 116 insertions(+), 49 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 62119e957cd8..950b7b72f0b8 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -263,7 +263,7 @@ static int ieee80211_start_p2p_device(struct wiphy *wiphy,
>   
>   	lockdep_assert_wiphy(sdata->local->hw.wiphy);
>   
> -	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
> +	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>   	if (ret < 0)
>   		return ret;
>   
> @@ -285,7 +285,7 @@ static int ieee80211_start_nan(struct wiphy *wiphy,
>   
>   	lockdep_assert_wiphy(sdata->local->hw.wiphy);
>   
> -	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
> +	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>   	if (ret < 0)
>   		return ret;
>   
> @@ -4001,7 +4001,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
>   		goto out;
>   
>   	/* if reservation is invalid then this will fail */
> -	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
> +	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0, -1);

Once we reach the global limit, all the -1 passing caller get fail 
becuase the iface check param (existing and new) is validated against 
the global limit. since global limt as a sort of union of all per-radio 
limits.

Ex:
Global iface = 6 (Radio iface 2GHz:4, 5GHz:4, 6GHz:6)


So far 6 iface created (Radio iface 2GHz:2, 5GHz:3, 6GHz:1)

In this case, new iface creation get fail because caller uses 
ieee80211_check_combinations() with -1 as radio idx, so it checked 
against global limit
Felix Fietkau June 7, 2024, 10:22 a.m. UTC | #5
On 07.06.24 12:07, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>> Add support for counting global and per-radio max/current number of
>> channels, as well as checking radio-specific interface combinations.
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>>   net/mac80211/cfg.c         |   7 +-
>>   net/mac80211/chan.c        |  17 +++--
>>   net/mac80211/ibss.c        |   2 +-
>>   net/mac80211/ieee80211_i.h |   6 +-
>>   net/mac80211/iface.c       |   2 +-
>>   net/mac80211/util.c        | 131 +++++++++++++++++++++++++++-----------
>>   6 files changed, 116 insertions(+), 49 deletions(-)
>> 
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> index 62119e957cd8..950b7b72f0b8 100644
>> --- a/net/mac80211/cfg.c
>> +++ b/net/mac80211/cfg.c
>> @@ -263,7 +263,7 @@ static int ieee80211_start_p2p_device(struct wiphy *wiphy,
>>   
>>   	lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>   
>> -	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>> +	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>   	if (ret < 0)
>>   		return ret;
>>   
>> @@ -285,7 +285,7 @@ static int ieee80211_start_nan(struct wiphy *wiphy,
>>   
>>   	lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>   
>> -	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>> +	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>   	if (ret < 0)
>>   		return ret;
>>   
>> @@ -4001,7 +4001,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
>>   		goto out;
>>   
>>   	/* if reservation is invalid then this will fail */
>> -	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
>> +	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0, -1);
> 
> Once we reach the global limit, all the -1 passing caller get fail
> becuase the iface check param (existing and new) is validated against
> the global limit. since global limt as a sort of union of all per-radio
> limits.
> 
> Ex:
> Global iface = 6 (Radio iface 2GHz:4, 5GHz:4, 6GHz:6)
> 
> 
> So far 6 iface created (Radio iface 2GHz:2, 5GHz:3, 6GHz:1)
> 
> In this case, new iface creation get fail because caller uses
> ieee80211_check_combinations() with -1 as radio idx, so it checked
> against global limit

Use the sum of the number of interfaces from each radio instead of the 
maximum.

- Felix
Karthikeyan Periyasamy June 7, 2024, 10:53 a.m. UTC | #6
On 6/7/2024 3:52 PM, Felix Fietkau wrote:
> On 07.06.24 12:07, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>> Add support for counting global and per-radio max/current number of
>>> channels, as well as checking radio-specific interface combinations.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>> ---
>>>   net/mac80211/cfg.c         |   7 +-
>>>   net/mac80211/chan.c        |  17 +++--
>>>   net/mac80211/ibss.c        |   2 +-
>>>   net/mac80211/ieee80211_i.h |   6 +-
>>>   net/mac80211/iface.c       |   2 +-
>>>   net/mac80211/util.c        | 131 
>>> +++++++++++++++++++++++++++-----------
>>>   6 files changed, 116 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>>> index 62119e957cd8..950b7b72f0b8 100644
>>> --- a/net/mac80211/cfg.c
>>> +++ b/net/mac80211/cfg.c
>>> @@ -263,7 +263,7 @@ static int ieee80211_start_p2p_device(struct 
>>> wiphy *wiphy,
>>>       lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>> -    ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>>> +    ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>>       if (ret < 0)
>>>           return ret;
>>> @@ -285,7 +285,7 @@ static int ieee80211_start_nan(struct wiphy *wiphy,
>>>       lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>> -    ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>>> +    ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>>       if (ret < 0)
>>>           return ret;
>>> @@ -4001,7 +4001,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, 
>>> struct net_device *dev,
>>>           goto out;
>>>       /* if reservation is invalid then this will fail */
>>> -    err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
>>> +    err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 
>>> 0, -1);
>>
>> Once we reach the global limit, all the -1 passing caller get fail
>> becuase the iface check param (existing and new) is validated against
>> the global limit. since global limt as a sort of union of all per-radio
>> limits.
>>
>> Ex:
>> Global iface = 6 (Radio iface 2GHz:4, 5GHz:4, 6GHz:6)
>>
>>
>> So far 6 iface created (Radio iface 2GHz:2, 5GHz:3, 6GHz:1)
>>
>> In this case, new iface creation get fail because caller uses
>> ieee80211_check_combinations() with -1 as radio idx, so it checked
>> against global limit
> 
> Use the sum of the number of interfaces from each radio instead of the 
> maximum.

Oh, then legacy user have misconception of the global interfaces 
advertised and try to fail for the allowed limits.
Felix Fietkau June 7, 2024, 11:04 a.m. UTC | #7
On 07.06.24 12:53, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/7/2024 3:52 PM, Felix Fietkau wrote:
>> On 07.06.24 12:07, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>> Add support for counting global and per-radio max/current number of
>>>> channels, as well as checking radio-specific interface combinations.
>>>>
>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>> ---
>>>>   net/mac80211/cfg.c         |   7 +-
>>>>   net/mac80211/chan.c        |  17 +++--
>>>>   net/mac80211/ibss.c        |   2 +-
>>>>   net/mac80211/ieee80211_i.h |   6 +-
>>>>   net/mac80211/iface.c       |   2 +-
>>>>   net/mac80211/util.c        | 131 
>>>> +++++++++++++++++++++++++++-----------
>>>>   6 files changed, 116 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>>>> index 62119e957cd8..950b7b72f0b8 100644
>>>> --- a/net/mac80211/cfg.c
>>>> +++ b/net/mac80211/cfg.c
>>>> @@ -263,7 +263,7 @@ static int ieee80211_start_p2p_device(struct 
>>>> wiphy *wiphy,
>>>>       lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>>> -    ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>>>> +    ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>>>       if (ret < 0)
>>>>           return ret;
>>>> @@ -285,7 +285,7 @@ static int ieee80211_start_nan(struct wiphy *wiphy,
>>>>       lockdep_assert_wiphy(sdata->local->hw.wiphy);
>>>> -    ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
>>>> +    ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
>>>>       if (ret < 0)
>>>>           return ret;
>>>> @@ -4001,7 +4001,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, 
>>>> struct net_device *dev,
>>>>           goto out;
>>>>       /* if reservation is invalid then this will fail */
>>>> -    err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
>>>> +    err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 
>>>> 0, -1);
>>>
>>> Once we reach the global limit, all the -1 passing caller get fail
>>> becuase the iface check param (existing and new) is validated against
>>> the global limit. since global limt as a sort of union of all per-radio
>>> limits.
>>>
>>> Ex:
>>> Global iface = 6 (Radio iface 2GHz:4, 5GHz:4, 6GHz:6)
>>>
>>>
>>> So far 6 iface created (Radio iface 2GHz:2, 5GHz:3, 6GHz:1)
>>>
>>> In this case, new iface creation get fail because caller uses
>>> ieee80211_check_combinations() with -1 as radio idx, so it checked
>>> against global limit
>> 
>> Use the sum of the number of interfaces from each radio instead of the 
>> maximum.
> 
> Oh, then legacy user have misconception of the global interfaces
> advertised and try to fail for the allowed limits.

Sure, but that might be an issue either way until user space is updated 
and users start looking at the per-radio ifcomb data.
The global data is simply not enough to describe the details of the 
radio split.

- Felix
Johannes Berg June 12, 2024, 12:05 p.m. UTC | #8
On Fri, 2024-06-07 at 13:04 +0200, Felix Fietkau wrote:
> > > 
> > > Use the sum of the number of interfaces from each radio instead of the 
> > > maximum.
> > 
> > Oh, then legacy user have misconception of the global interfaces
> > advertised and try to fail for the allowed limits.
> 
> Sure, but that might be an issue either way until user space is updated 
> and users start looking at the per-radio ifcomb data.

I'm kind of with Karthikeyan here - this could be understood as a
regression, since you're now telling userspace something you can't
actually do.

> The global data is simply not enough to describe the details of the 
> radio split.

Obviously, but that doesn't mean the global data as advertised in the
existing attributes must be *wrong*. It could be a subset, and the
superset data is only available to new implementations.

johannes
Felix Fietkau June 12, 2024, 12:21 p.m. UTC | #9
On 12.06.24 14:05, Johannes Berg wrote:
> On Fri, 2024-06-07 at 13:04 +0200, Felix Fietkau wrote:
>> > > 
>> > > Use the sum of the number of interfaces from each radio instead of the 
>> > > maximum.
>> > 
>> > Oh, then legacy user have misconception of the global interfaces
>> > advertised and try to fail for the allowed limits.
>> 
>> Sure, but that might be an issue either way until user space is updated 
>> and users start looking at the per-radio ifcomb data.
> 
> I'm kind of with Karthikeyan here - this could be understood as a
> regression, since you're now telling userspace something you can't
> actually do.

Well, we can actually do it, just with some extra restrictions - i.e. 
the interfaces we create need to be spread across radios to match the 
per-radio limits.

>> The global data is simply not enough to describe the details of the 
>> radio split.
> 
> Obviously, but that doesn't mean the global data as advertised in the
> existing attributes must be *wrong*. It could be a subset, and the
> superset data is only available to new implementations.
So you'd prefer something like picking one radio and advertising its 
limits instead?

- Felix
diff mbox series

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 62119e957cd8..950b7b72f0b8 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -263,7 +263,7 @@  static int ieee80211_start_p2p_device(struct wiphy *wiphy,
 
 	lockdep_assert_wiphy(sdata->local->hw.wiphy);
 
-	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
+	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
 	if (ret < 0)
 		return ret;
 
@@ -285,7 +285,7 @@  static int ieee80211_start_nan(struct wiphy *wiphy,
 
 	lockdep_assert_wiphy(sdata->local->hw.wiphy);
 
-	ret = ieee80211_check_combinations(sdata, NULL, 0, 0);
+	ret = ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
 	if (ret < 0)
 		return ret;
 
@@ -4001,7 +4001,7 @@  __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 		goto out;
 
 	/* if reservation is invalid then this will fail */
-	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
+	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0, -1);
 	if (err) {
 		ieee80211_link_unreserve_chanctx(link_data);
 		goto out;
@@ -5199,4 +5199,5 @@  const struct cfg80211_ops mac80211_config_ops = {
 	.del_link_station = ieee80211_del_link_station,
 	.set_hw_timestamp = ieee80211_set_hw_timestamp,
 	.set_ttlm = ieee80211_set_ttlm,
+	.get_radio_mask = ieee80211_get_radio_mask,
 };
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 574ae961a7cf..0e899c07bc2b 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -47,24 +47,29 @@  int ieee80211_chanctx_refcount(struct ieee80211_local *local,
 	       ieee80211_chanctx_num_reserved(local, ctx);
 }
 
-static int ieee80211_num_chanctx(struct ieee80211_local *local)
+static int ieee80211_num_chanctx(struct ieee80211_local *local, int radio_idx)
 {
 	struct ieee80211_chanctx *ctx;
 	int num = 0;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	list_for_each_entry(ctx, &local->chanctx_list, list)
+	list_for_each_entry(ctx, &local->chanctx_list, list) {
+		if (radio_idx >= 0 && ctx->conf.radio_idx != radio_idx)
+			continue;
 		num++;
+	}
 
 	return num;
 }
 
-static bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local)
+static bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local,
+					     int radio_idx)
 {
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	return ieee80211_num_chanctx(local) < ieee80211_max_num_channels(local);
+	return ieee80211_num_chanctx(local, radio_idx) <
+	       ieee80211_max_num_channels(local, radio_idx);
 }
 
 static struct ieee80211_chanctx *
@@ -1072,7 +1077,7 @@  int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
 
 	new_ctx = ieee80211_find_reservation_chanctx(local, chanreq, mode);
 	if (!new_ctx) {
-		if (ieee80211_can_create_new_chanctx(local)) {
+		if (ieee80211_can_create_new_chanctx(local, -1)) {
 			new_ctx = ieee80211_new_chanctx(local, chanreq, mode,
 							false);
 			if (IS_ERR(new_ctx))
@@ -1767,7 +1772,7 @@  int _ieee80211_link_use_channel(struct ieee80211_link_data *link,
 	link->radar_required = ret;
 
 	ret = ieee80211_check_combinations(sdata, &chanreq->oper, mode,
-					   radar_detect_width);
+					   radar_detect_width, -1);
 	if (ret < 0)
 		goto out;
 
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index bf338f3d4dd3..522e964bb186 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -1745,7 +1745,7 @@  int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 		IEEE80211_CHANCTX_SHARED : IEEE80211_CHANCTX_EXCLUSIVE;
 
 	ret = ieee80211_check_combinations(sdata, &params->chandef, chanmode,
-					   radar_detect_width);
+					   radar_detect_width, -1);
 	if (ret < 0)
 		return ret;
 
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3fd7b1adbfab..33c783432383 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2038,6 +2038,8 @@  static inline bool ieee80211_sdata_running(struct ieee80211_sub_if_data *sdata)
 {
 	return test_bit(SDATA_STATE_RUNNING, &sdata->state);
 }
+int ieee80211_get_radio_mask(struct wiphy *wiphy, struct net_device *dev,
+			     u32 *mask);
 
 /* link handling */
 void ieee80211_link_setup(struct ieee80211_link_data *link);
@@ -2620,8 +2622,8 @@  void ieee80211_recalc_dtim(struct ieee80211_local *local,
 int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 				 const struct cfg80211_chan_def *chandef,
 				 enum ieee80211_chanctx_mode chanmode,
-				 u8 radar_detect);
-int ieee80211_max_num_channels(struct ieee80211_local *local);
+				 u8 radar_detect, int radio_idx);
+int ieee80211_max_num_channels(struct ieee80211_local *local, int radio_idx);
 void ieee80211_recalc_chanctx_chantype(struct ieee80211_local *local,
 				       struct ieee80211_chanctx *ctx);
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index d1a49ee4a194..5dc85f9409cd 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -397,7 +397,7 @@  static int ieee80211_check_concurrent_iface(struct ieee80211_sub_if_data *sdata,
 		}
 	}
 
-	return ieee80211_check_combinations(sdata, NULL, 0, 0);
+	return ieee80211_check_combinations(sdata, NULL, 0, 0, -1);
 }
 
 static int ieee80211_check_queues(struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 927f752a0209..9a7b70b1553c 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3928,13 +3928,96 @@  static u8 ieee80211_chanctx_radar_detect(struct ieee80211_local *local,
 	return radar_detect;
 }
 
+static u32
+__ieee80211_get_radio_mask(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_chanctx_conf *conf;
+	struct ieee80211_link_data *link;
+	u32 mask = 0;
+
+	for_each_sdata_link(local, link) {
+		conf = rcu_dereference(link->conf->chanctx_conf);
+		if (!conf || conf->radio_idx < 0)
+			continue;
+
+		mask |= BIT(conf->radio_idx);
+	}
+
+	return mask;
+}
+
+int ieee80211_get_radio_mask(struct wiphy *wiphy, struct net_device *dev,
+			     u32 *mask)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+
+	*mask = __ieee80211_get_radio_mask(sdata);
+
+	return 0;
+}
+
+static bool
+ieee80211_sdata_uses_radio(struct ieee80211_sub_if_data *sdata, int radio_idx)
+{
+	if (radio_idx < 0)
+		return true;
+
+	return __ieee80211_get_radio_mask(sdata) & BIT(radio_idx);
+}
+
+static void
+ieee80211_fill_ifcomb_params(struct ieee80211_local *local,
+			     struct iface_combination_params *params,
+			     const struct cfg80211_chan_def *chandef,
+			     struct ieee80211_sub_if_data *sdata,
+			     int radio_idx)
+{
+	struct ieee80211_sub_if_data *sdata_iter;
+	struct ieee80211_chanctx *ctx;
+
+	list_for_each_entry(ctx, &local->chanctx_list, list) {
+		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
+			continue;
+
+		if (radio_idx >= 0 && ctx->conf.radio_idx != radio_idx)
+			continue;
+
+		if (chandef &&
+		    cfg80211_chandef_compatible(chandef, &ctx->conf.def))
+			continue;
+
+		params->num_different_channels++;
+
+		params->radar_detect |=
+			ieee80211_chanctx_radar_detect(local, ctx);
+	}
+
+	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
+		struct wireless_dev *wdev_iter;
+
+		wdev_iter = &sdata_iter->wdev;
+
+		if (sdata_iter == sdata ||
+		    !ieee80211_sdata_running(sdata_iter) ||
+		    cfg80211_iftype_allowed(local->hw.wiphy,
+					    wdev_iter->iftype, 0, 1))
+			continue;
+
+		if (!ieee80211_sdata_uses_radio(sdata_iter, radio_idx))
+			continue;
+
+		params->iftype_num[wdev_iter->iftype]++;
+	}
+}
+
 int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 				 const struct cfg80211_chan_def *chandef,
 				 enum ieee80211_chanctx_mode chanmode,
-				 u8 radar_detect)
+				 u8 radar_detect, int radio_idx)
 {
+	bool shared = chanmode == IEEE80211_CHANCTX_SHARED;
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_sub_if_data *sdata_iter;
 	enum nl80211_iftype iftype = sdata->wdev.iftype;
 	struct ieee80211_chanctx *ctx;
 	int total = 1;
@@ -3977,6 +4060,8 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 	if (iftype != NL80211_IFTYPE_UNSPECIFIED)
 		params.iftype_num[iftype] = 1;
 
+	ieee80211_fill_ifcomb_params(local, &params, shared ? chandef : NULL,
+				     sdata, radio_idx);
 	list_for_each_entry(ctx, &local->chanctx_list, list) {
 		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
 			continue;
@@ -3986,28 +4071,9 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 			params.num_different_channels++;
 			continue;
 		}
-		if (chandef && chanmode == IEEE80211_CHANCTX_SHARED &&
-		    cfg80211_chandef_compatible(chandef,
-						&ctx->conf.def))
-			continue;
 		params.num_different_channels++;
 	}
 
-	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
-		struct wireless_dev *wdev_iter;
-
-		wdev_iter = &sdata_iter->wdev;
-
-		if (sdata_iter == sdata ||
-		    !ieee80211_sdata_running(sdata_iter) ||
-		    cfg80211_iftype_allowed(local->hw.wiphy,
-					    wdev_iter->iftype, 0, 1))
-			continue;
-
-		params.iftype_num[wdev_iter->iftype]++;
-		total++;
-	}
-
 	if (total == 1 && !params.radar_detect)
 		return 0;
 
@@ -4024,28 +4090,21 @@  ieee80211_iter_max_chans(const struct ieee80211_iface_combination *c,
 					  c->num_different_channels);
 }
 
-int ieee80211_max_num_channels(struct ieee80211_local *local)
+int ieee80211_max_num_channels(struct ieee80211_local *local, int radio_idx)
 {
-	struct ieee80211_sub_if_data *sdata;
-	struct ieee80211_chanctx *ctx;
+	struct wiphy *wiphy = local->hw.wiphy;
 	u32 max_num_different_channels = 1;
 	int err;
 	struct iface_combination_params params = {0};
 
-	lockdep_assert_wiphy(local->hw.wiphy);
-
-	list_for_each_entry(ctx, &local->chanctx_list, list) {
-		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
-			continue;
-
-		params.num_different_channels++;
+	if (radio_idx >= wiphy->n_radio)
+		return -EINVAL;
+	else if (radio_idx >= 0)
+		params.radio = &wiphy->radio[radio_idx];
 
-		params.radar_detect |=
-			ieee80211_chanctx_radar_detect(local, ctx);
-	}
+	lockdep_assert_wiphy(local->hw.wiphy);
 
-	list_for_each_entry_rcu(sdata, &local->interfaces, list)
-		params.iftype_num[sdata->wdev.iftype]++;
+	ieee80211_fill_ifcomb_params(local, &params, NULL, NULL, radio_idx);
 
 	err = cfg80211_iter_combinations(local->hw.wiphy, &params,
 					 ieee80211_iter_max_chans,