mbox series

[RFC,v2,0/7] cfg80211/mac80211: support defining multiple radios per wiphy

Message ID cover.c104c0bb3a14f4ac26aee71f4979846f6ad87742.1717611760.git-series.nbd@nbd.name
Headers show
Series cfg80211/mac80211: support defining multiple radios per wiphy | expand

Message

Felix Fietkau June 5, 2024, 6:31 p.m. UTC 
The prerequisite for MLO support in cfg80211/mac80211 is that all the links
participating in MLO must be from the same wiphy/ieee80211_hw. To meet this
expectation, some drivers may need to group multiple discrete hardware each
acting as a link in MLO under single wiphy.

With this series, the bands and supported frequencies of each individual
radio are reported to user space. This allows user space to figure out the
limitations of what combination of channels can be used concurrently.

Each mac80211 channel context is assigned to a radio based on radio specific
frequency ranges and interface combinations.

This is loosely based on Karthikeyan Periyasamy's series
"[PATCH 00/13] wifi: Add multi physical hardware iface combination support"
with some differences:

 - a struct wiphy_radio is defined, which holds the frequency ranges and
   a full struct ieee80211_iface_combination array
 - a channel context is explicitly assigned to a radio when created
 - both global and per-radio interface combination limits are checked
   and enforced on channel context assignment

Changes since RFC:
 - replace static per-radio number of channels limit with full ifcomb
   checks
 - remove band bitmask in favor of only using freq ranges

Felix Fietkau (7):
  wifi: nl80211: split helper function from nl80211_put_iface_combinations
  wifi: cfg80211: add support for advertising multiple radios belonging to a wiphy
  wifi: cfg80211: extend interface combination check for multi-radio
  wifi: mac80211: add radio index to ieee80211_chanctx_conf
  wifi: mac80211: extend ifcomb check functions for multi-radio
  wifi: mac80211: move code in ieee80211_link_reserve_chanctx to a helper
  wifi: mac80211: add wiphy radio assignment and validation

 include/net/cfg80211.h       |  40 ++++++-
 include/net/mac80211.h       |   2 +-
 include/uapi/linux/nl80211.h |  48 ++++++++-
 net/mac80211/cfg.c           |   6 +-
 net/mac80211/chan.c          | 228 +++++++++++++++++++++++-------------
 net/mac80211/ibss.c          |   2 +-
 net/mac80211/ieee80211_i.h   |   4 +-
 net/mac80211/iface.c         |   2 +-
 net/mac80211/util.c          | 114 ++++++++++++------
 net/wireless/nl80211.c       | 190 +++++++++++++++++++++---------
 net/wireless/util.c          |  29 ++---
 11 files changed, 481 insertions(+), 184 deletions(-)

base-commit: 5bcd9a0a59953b5ff55ac226f903397319bf7f40

Comments

Karthikeyan Periyasamy June 6, 2024, 7:20 a.m. UTC | #1 
On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>   
>   	/*
>   	 * This is a bit strange, since the iteration used to rely only on
> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy *wiphy,
>   	 * cfg80211 already - the only thing not would appear to be any new
>   	 * interfaces (while being brought up) and channel/radar data.
>   	 */
> -	cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
> -				   &beacon_int_gcd, &beacon_int_different);
> +	if (!radio)
> +			cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
> +						   &beacon_int_gcd,
> +						   &beacon_int_different);
>   

Why its avoid for radio specific iface combination ?
Felix Fietkau June 6, 2024, 7:55 a.m. UTC | #2 
On 06.06.24 09:20, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>>   
>>   	/*
>>   	 * This is a bit strange, since the iteration used to rely only on
>> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy *wiphy,
>>   	 * cfg80211 already - the only thing not would appear to be any new
>>   	 * interfaces (while being brought up) and channel/radar data.
>>   	 */
>> -	cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>> -				   &beacon_int_gcd, &beacon_int_different);
>> +	if (!radio)
>> +			cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>> +						   &beacon_int_gcd,
>> +						   &beacon_int_different);
>>   
> 
> Why its avoid for radio specific iface combination ?

Because it iterates over all wdevs within cfg80211. I didn't think this 
was necessary, given that it already excludes MLO wdevs.

- Felix
Karthikeyan Periyasamy June 6, 2024, 8:56 a.m. UTC | #3 
On 6/6/2024 1:25 PM, Felix Fietkau wrote:
> On 06.06.24 09:20, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>>>       /*
>>>        * This is a bit strange, since the iteration used to rely only on
>>> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy 
>>> *wiphy,
>>>        * cfg80211 already - the only thing not would appear to be any 
>>> new
>>>        * interfaces (while being brought up) and channel/radar data.
>>>        */
>>> -    cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>> -                   &beacon_int_gcd, &beacon_int_different);
>>> +    if (!radio)
>>> +            cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>> +                           &beacon_int_gcd,
>>> +                           &beacon_int_different);
>>
>> Why its avoid for radio specific iface combination ?
> 
> Because it iterates over all wdevs within cfg80211. I didn't think this 
> was necessary, given that it already excludes MLO wdevs.
> 

Dont tie the radio specific iface advertisement with MLO.

Usually the existing code consider "params->new_beacon_int" the MLO 
scenario also.
Felix Fietkau June 6, 2024, 8:58 a.m. UTC | #4 
On 06.06.24 10:56, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 1:25 PM, Felix Fietkau wrote:
>> On 06.06.24 09:20, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>>>>       /*
>>>>        * This is a bit strange, since the iteration used to rely only on
>>>> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy 
>>>> *wiphy,
>>>>        * cfg80211 already - the only thing not would appear to be any 
>>>> new
>>>>        * interfaces (while being brought up) and channel/radar data.
>>>>        */
>>>> -    cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>> -                   &beacon_int_gcd, &beacon_int_different);
>>>> +    if (!radio)
>>>> +            cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>> +                           &beacon_int_gcd,
>>>> +                           &beacon_int_different);
>>>
>>> Why its avoid for radio specific iface combination ?
>> 
>> Because it iterates over all wdevs within cfg80211. I didn't think this 
>> was necessary, given that it already excludes MLO wdevs.
>> 
> 
> Dont tie the radio specific iface advertisement with MLO.
> 
> Usually the existing code consider "params->new_beacon_int" the MLO
> scenario also.

For your hardware, do beacon intervals need to be matched/aligned per 
radio or globally?

- Felix
Karthikeyan Periyasamy June 6, 2024, 9:52 a.m. UTC | #5 
On 6/6/2024 2:28 PM, Felix Fietkau wrote:
> On 06.06.24 10:56, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/6/2024 1:25 PM, Felix Fietkau wrote:
>>> On 06.06.24 09:20, Karthikeyan Periyasamy wrote:
>>>>
>>>>
>>>> On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>>>>>       /*
>>>>>        * This is a bit strange, since the iteration used to rely 
>>>>> only on
>>>>> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy 
>>>>> *wiphy,
>>>>>        * cfg80211 already - the only thing not would appear to be 
>>>>> any new
>>>>>        * interfaces (while being brought up) and channel/radar data.
>>>>>        */
>>>>> -    cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>>> -                   &beacon_int_gcd, &beacon_int_different);
>>>>> +    if (!radio)
>>>>> +            cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>>> +                           &beacon_int_gcd,
>>>>> +                           &beacon_int_different);
>>>>
>>>> Why its avoid for radio specific iface combination ?
>>>
>>> Because it iterates over all wdevs within cfg80211. I didn't think 
>>> this was necessary, given that it already excludes MLO wdevs.
>>>
>>
>> Dont tie the radio specific iface advertisement with MLO.
>>
>> Usually the existing code consider "params->new_beacon_int" the MLO
>> scenario also.
> 
> For your hardware, do beacon intervals need to be matched/aligned per 
> radio or globally?
> 

Our hardware supports radio aligned beacon interval.

Currently, ath12k use use same beacon interval configuration all radio 
iface combination.

Even in radio specific iface combination, we should check the beacon 
interval for the non MLO VAPs.

so dont avoid the beacon interval check.
Felix Fietkau June 6, 2024, 9:57 a.m. UTC | #6 
On 06.06.24 11:52, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 2:28 PM, Felix Fietkau wrote:
>> On 06.06.24 10:56, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/6/2024 1:25 PM, Felix Fietkau wrote:
>>>> On 06.06.24 09:20, Karthikeyan Periyasamy wrote:
>>>>>
>>>>>
>>>>> On 6/6/2024 12:01 AM, Felix Fietkau wrote:
>>>>>>       /*
>>>>>>        * This is a bit strange, since the iteration used to rely 
>>>>>> only on
>>>>>> @@ -2384,8 +2383,10 @@ int cfg80211_iter_combinations(struct wiphy 
>>>>>> *wiphy,
>>>>>>        * cfg80211 already - the only thing not would appear to be 
>>>>>> any new
>>>>>>        * interfaces (while being brought up) and channel/radar data.
>>>>>>        */
>>>>>> -    cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>>>> -                   &beacon_int_gcd, &beacon_int_different);
>>>>>> +    if (!radio)
>>>>>> +            cfg80211_calculate_bi_data(wiphy, params->new_beacon_int,
>>>>>> +                           &beacon_int_gcd,
>>>>>> +                           &beacon_int_different);
>>>>>
>>>>> Why its avoid for radio specific iface combination ?
>>>>
>>>> Because it iterates over all wdevs within cfg80211. I didn't think 
>>>> this was necessary, given that it already excludes MLO wdevs.
>>>>
>>>
>>> Dont tie the radio specific iface advertisement with MLO.
>>>
>>> Usually the existing code consider "params->new_beacon_int" the MLO
>>> scenario also.
>> 
>> For your hardware, do beacon intervals need to be matched/aligned per 
>> radio or globally?
>> 
> 
> Our hardware supports radio aligned beacon interval.
> 
> Currently, ath12k use use same beacon interval configuration all radio
> iface combination.
> 
> Even in radio specific iface combination, we should check the beacon
> interval for the non MLO VAPs.
> 
> so dont avoid the beacon interval check.

Okay, I'll look into making this work.

- Felix
Aditya Kumar Singh June 6, 2024, 10 a.m. UTC | #7 
On 6/6/24 00:01, Felix Fietkau wrote:
> The prerequisite for MLO support in cfg80211/mac80211 is that all the links
> participating in MLO must be from the same wiphy/ieee80211_hw. To meet this
> expectation, some drivers may need to group multiple discrete hardware each
> acting as a link in MLO under single wiphy.
> 
> With this series, the bands and supported frequencies of each individual
> radio are reported to user space. This allows user space to figure out the
> limitations of what combination of channels can be used concurrently.
> 
> Each mac80211 channel context is assigned to a radio based on radio specific
> frequency ranges and interface combinations.
> 
> This is loosely based on Karthikeyan Periyasamy's series
> "[PATCH 00/13] wifi: Add multi physical hardware iface combination support"
> with some differences:
> 
>   - a struct wiphy_radio is defined, which holds the frequency ranges and
>     a full struct ieee80211_iface_combination array
>   - a channel context is explicitly assigned to a radio when created
>   - both global and per-radio interface combination limits are checked
>     and enforced on channel context assignment
> 
> Changes since RFC:
>   - replace static per-radio number of channels limit with full ifcomb
>     checks
>   - remove band bitmask in favor of only using freq ranges

What about handling 2 GHz + 5 GHz issue we discussed in v1 related to 
radar detection width and num chan ctx? Is that taken care?
Felix Fietkau June 6, 2024, 10:10 a.m. UTC | #8 
On 06.06.24 12:00, Aditya Kumar Singh wrote:
> On 6/6/24 00:01, Felix Fietkau wrote:
>> The prerequisite for MLO support in cfg80211/mac80211 is that all the links
>> participating in MLO must be from the same wiphy/ieee80211_hw. To meet this
>> expectation, some drivers may need to group multiple discrete hardware each
>> acting as a link in MLO under single wiphy.
>> 
>> With this series, the bands and supported frequencies of each individual
>> radio are reported to user space. This allows user space to figure out the
>> limitations of what combination of channels can be used concurrently.
>> 
>> Each mac80211 channel context is assigned to a radio based on radio specific
>> frequency ranges and interface combinations.
>> 
>> This is loosely based on Karthikeyan Periyasamy's series
>> "[PATCH 00/13] wifi: Add multi physical hardware iface combination support"
>> with some differences:
>> 
>>   - a struct wiphy_radio is defined, which holds the frequency ranges and
>>     a full struct ieee80211_iface_combination array
>>   - a channel context is explicitly assigned to a radio when created
>>   - both global and per-radio interface combination limits are checked
>>     and enforced on channel context assignment
>> 
>> Changes since RFC:
>>   - replace static per-radio number of channels limit with full ifcomb
>>     checks
>>   - remove band bitmask in favor of only using freq ranges
> 
> What about handling 2 GHz + 5 GHz issue we discussed in v1 related to
> radar detection width and num chan ctx? Is that taken care?

Seems that I missed that one. I will take care of it in the next version.

Thanks,

- Felix