diff mbox series

[RFC,v3,4/8] wifi: mac80211: add support for DFS with multiple radios

Message ID 7cc3d89225f365c85b363874725cfbc77c9c1db5.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
DFS can be supported with multi-channel combinations, as long as each DFS
capable radio only supports one channel.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

Karthikeyan Periyasamy June 7, 2024, 4:25 a.m. UTC | #1
On 6/6/2024 11:37 PM, Felix Fietkau wrote:
> DFS can be supported with multi-channel combinations, as long as each DFS
> capable radio only supports one channel.
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>   1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index 40fbf397ce74..e9c4cf611e94 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c

...

>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>   {
>   	struct ieee80211_local *local = hw_to_local(hw);
> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>   			if (comb->num_different_channels > 1)
>   				return -EINVAL;
>   		}
> -	} else {
> -		/* DFS is not supported with multi-channel combinations yet */
> -		for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
> -			const struct ieee80211_iface_combination *comb;
> -
> -			comb = &local->hw.wiphy->iface_combinations[i];
> +	} else if (hw->wiphy->n_radio) {
> +		for (i = 0; i < hw->wiphy->n_radio; i++) {
> +			const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>   
> -			if (comb->radar_detect_widths &&
> -			    comb->num_different_channels > 1)
> +			if (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
> +							  radio->n_iface_combinations))
>   				return -EINVAL;
>   		}
> +	} else {
> +		if (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
> +						  hw->wiphy->n_iface_combinations))
> +			return -EINVAL;
>   	}
>   
>   	/* Only HW csum features are currently compatible with mac80211 */

Are we omitting the "wiphy->iface_combinations" if the radio specific 
iface combination advertised ?

If so, it looks like unused "wiphy->iface_combinations" for radio 
specific combination advertised.
Felix Fietkau June 7, 2024, 4:35 a.m. UTC | #2
On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>> DFS can be supported with multi-channel combinations, as long as each DFS
>> capable radio only supports one channel.
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>   1 file changed, 24 insertions(+), 8 deletions(-)
>> 
>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>> index 40fbf397ce74..e9c4cf611e94 100644
>> --- a/net/mac80211/main.c
>> +++ b/net/mac80211/main.c
> 
> ...
> 
>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>   {
>>   	struct ieee80211_local *local = hw_to_local(hw);
>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>>   			if (comb->num_different_channels > 1)
>>   				return -EINVAL;
>>   		}
>> -	} else {
>> -		/* DFS is not supported with multi-channel combinations yet */
>> -		for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>> -			const struct ieee80211_iface_combination *comb;
>> -
>> -			comb = &local->hw.wiphy->iface_combinations[i];
>> +	} else if (hw->wiphy->n_radio) {
>> +		for (i = 0; i < hw->wiphy->n_radio; i++) {
>> +			const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>   
>> -			if (comb->radar_detect_widths &&
>> -			    comb->num_different_channels > 1)
>> +			if (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>> +							  radio->n_iface_combinations))
>>   				return -EINVAL;
>>   		}
>> +	} else {
>> +		if (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>> +						  hw->wiphy->n_iface_combinations))
>> +			return -EINVAL;
>>   	}
>>   
>>   	/* Only HW csum features are currently compatible with mac80211 */
> 
> Are we omitting the "wiphy->iface_combinations" if the radio specific
> iface combination advertised ?
> 
> If so, it looks like unused "wiphy->iface_combinations" for radio
> specific combination advertised.

This patch series assumes that you have both wiphy->iface_combinations 
and radio->iface_combinations set. wiphy->iface_combinations applies to 
the full wiphy, whereas radio->iface_combinations only applies to vifs 
assigned to the radio.

- Felix
Karthikeyan Periyasamy June 7, 2024, 4:54 a.m. UTC | #3
On 6/7/2024 10:05 AM, Felix Fietkau wrote:
> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>> DFS can be supported with multi-channel combinations, as long as each 
>>> DFS
>>> capable radio only supports one channel.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>> ---
>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>> index 40fbf397ce74..e9c4cf611e94 100644
>>> --- a/net/mac80211/main.c
>>> +++ b/net/mac80211/main.c
>>
>> ...
>>
>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>   {
>>>       struct ieee80211_local *local = hw_to_local(hw);
>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw 
>>> *hw)
>>>               if (comb->num_different_channels > 1)
>>>                   return -EINVAL;
>>>           }
>>> -    } else {
>>> -        /* DFS is not supported with multi-channel combinations yet */
>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>>> -            const struct ieee80211_iface_combination *comb;
>>> -
>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>> +    } else if (hw->wiphy->n_radio) {
>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>> -            if (comb->radar_detect_widths &&
>>> -                comb->num_different_channels > 1)
>>> +            if 
>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>> +                              radio->n_iface_combinations))
>>>                   return -EINVAL;
>>>           }
>>> +    } else {
>>> +        if 
>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>> +                          hw->wiphy->n_iface_combinations))
>>> +            return -EINVAL;
>>>       }
>>>       /* Only HW csum features are currently compatible with mac80211 */
>>
>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>> iface combination advertised ?
>>
>> If so, it looks like unused "wiphy->iface_combinations" for radio
>> specific combination advertised.
> 
> This patch series assumes that you have both wiphy->iface_combinations 
> and radio->iface_combinations set. wiphy->iface_combinations applies to 
> the full wiphy, whereas radio->iface_combinations only applies to vifs 
> assigned to the radio.


If radio->iface_combinations is set then always vifs assigned to the 
radio. so wiphy->iface_combinations get avoid for all the use cases.

Ultimately either of one combination only get used by this proposal.

or I am missing something here ?


> 
> - Felix
Felix Fietkau June 7, 2024, 5:03 a.m. UTC | #4
On 07.06.24 06:54, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/7/2024 10:05 AM, Felix Fietkau wrote:
>> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>> DFS can be supported with multi-channel combinations, as long as each 
>>>> DFS
>>>> capable radio only supports one channel.
>>>>
>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>> ---
>>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>>> index 40fbf397ce74..e9c4cf611e94 100644
>>>> --- a/net/mac80211/main.c
>>>> +++ b/net/mac80211/main.c
>>>
>>> ...
>>>
>>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>>   {
>>>>       struct ieee80211_local *local = hw_to_local(hw);
>>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw 
>>>> *hw)
>>>>               if (comb->num_different_channels > 1)
>>>>                   return -EINVAL;
>>>>           }
>>>> -    } else {
>>>> -        /* DFS is not supported with multi-channel combinations yet */
>>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>>>> -            const struct ieee80211_iface_combination *comb;
>>>> -
>>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>>> +    } else if (hw->wiphy->n_radio) {
>>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>>> -            if (comb->radar_detect_widths &&
>>>> -                comb->num_different_channels > 1)
>>>> +            if 
>>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>>> +                              radio->n_iface_combinations))
>>>>                   return -EINVAL;
>>>>           }
>>>> +    } else {
>>>> +        if 
>>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>>> +                          hw->wiphy->n_iface_combinations))
>>>> +            return -EINVAL;
>>>>       }
>>>>       /* Only HW csum features are currently compatible with mac80211 */
>>>
>>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>>> iface combination advertised ?
>>>
>>> If so, it looks like unused "wiphy->iface_combinations" for radio
>>> specific combination advertised.
>> 
>> This patch series assumes that you have both wiphy->iface_combinations 
>> and radio->iface_combinations set. wiphy->iface_combinations applies to 
>> the full wiphy, whereas radio->iface_combinations only applies to vifs 
>> assigned to the radio.
> 
> 
> If radio->iface_combinations is set then always vifs assigned to the
> radio. so wiphy->iface_combinations get avoid for all the use cases.
> 
> Ultimately either of one combination only get used by this proposal.
> 
> or I am missing something here ?

The functions that perform interface combination checks are called both 
with -1 as radio_idx (meaning per-wiphy), as well as with the assigned 
radio id. That way, both kinds of combinations/limits are checked and 
enforced.

- Felix
Karthikeyan Periyasamy June 7, 2024, 6:45 a.m. UTC | #5
On 6/7/2024 10:33 AM, Felix Fietkau wrote:
> On 07.06.24 06:54, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/7/2024 10:05 AM, Felix Fietkau wrote:
>>> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>>>
>>>>
>>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>>> DFS can be supported with multi-channel combinations, as long as 
>>>>> each DFS
>>>>> capable radio only supports one channel.
>>>>>
>>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>>> ---
>>>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>>>> index 40fbf397ce74..e9c4cf611e94 100644
>>>>> --- a/net/mac80211/main.c
>>>>> +++ b/net/mac80211/main.c
>>>>
>>>> ...
>>>>
>>>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>>>   {
>>>>>       struct ieee80211_local *local = hw_to_local(hw);
>>>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct 
>>>>> ieee80211_hw *hw)
>>>>>               if (comb->num_different_channels > 1)
>>>>>                   return -EINVAL;
>>>>>           }
>>>>> -    } else {
>>>>> -        /* DFS is not supported with multi-channel combinations 
>>>>> yet */
>>>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>>>>> -            const struct ieee80211_iface_combination *comb;
>>>>> -
>>>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>>>> +    } else if (hw->wiphy->n_radio) {
>>>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>>>> -            if (comb->radar_detect_widths &&
>>>>> -                comb->num_different_channels > 1)
>>>>> +            if 
>>>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>>>> +                              radio->n_iface_combinations))
>>>>>                   return -EINVAL;
>>>>>           }
>>>>> +    } else {
>>>>> +        if 
>>>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>>>> +                          hw->wiphy->n_iface_combinations))
>>>>> +            return -EINVAL;
>>>>>       }
>>>>>       /* Only HW csum features are currently compatible with 
>>>>> mac80211 */
>>>>
>>>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>>>> iface combination advertised ?
>>>>
>>>> If so, it looks like unused "wiphy->iface_combinations" for radio
>>>> specific combination advertised.
>>>
>>> This patch series assumes that you have both 
>>> wiphy->iface_combinations and radio->iface_combinations set. 
>>> wiphy->iface_combinations applies to the full wiphy, whereas 
>>> radio->iface_combinations only applies to vifs assigned to the radio.
>>
>>
>> If radio->iface_combinations is set then always vifs assigned to the
>> radio. so wiphy->iface_combinations get avoid for all the use cases.
>>
>> Ultimately either of one combination only get used by this proposal.
>>
>> or I am missing something here ?
> 
> The functions that perform interface combination checks are called both 
> with -1 as radio_idx (meaning per-wiphy), as well as with the assigned 
> radio id. That way, both kinds of combinations/limits are checked and 
> enforced.
> 
In the radio specific iface advertisement, global iface combination 
represent the union or intersection of all radio iface combination ?
Felix Fietkau June 7, 2024, 8:16 a.m. UTC | #6
On 07.06.24 08:45, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/7/2024 10:33 AM, Felix Fietkau wrote:
>> On 07.06.24 06:54, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/7/2024 10:05 AM, Felix Fietkau wrote:
>>>> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>>>>
>>>>>
>>>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>>>> DFS can be supported with multi-channel combinations, as long as 
>>>>>> each DFS
>>>>>> capable radio only supports one channel.
>>>>>>
>>>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>>>> ---
>>>>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>>>>> index 40fbf397ce74..e9c4cf611e94 100644
>>>>>> --- a/net/mac80211/main.c
>>>>>> +++ b/net/mac80211/main.c
>>>>>
>>>>> ...
>>>>>
>>>>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>>>>   {
>>>>>>       struct ieee80211_local *local = hw_to_local(hw);
>>>>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct 
>>>>>> ieee80211_hw *hw)
>>>>>>               if (comb->num_different_channels > 1)
>>>>>>                   return -EINVAL;
>>>>>>           }
>>>>>> -    } else {
>>>>>> -        /* DFS is not supported with multi-channel combinations 
>>>>>> yet */
>>>>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>>>>>> -            const struct ieee80211_iface_combination *comb;
>>>>>> -
>>>>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>>>>> +    } else if (hw->wiphy->n_radio) {
>>>>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>>>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>>>>> -            if (comb->radar_detect_widths &&
>>>>>> -                comb->num_different_channels > 1)
>>>>>> +            if 
>>>>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>>>>> +                              radio->n_iface_combinations))
>>>>>>                   return -EINVAL;
>>>>>>           }
>>>>>> +    } else {
>>>>>> +        if 
>>>>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>>>>> +                          hw->wiphy->n_iface_combinations))
>>>>>> +            return -EINVAL;
>>>>>>       }
>>>>>>       /* Only HW csum features are currently compatible with 
>>>>>> mac80211 */
>>>>>
>>>>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>>>>> iface combination advertised ?
>>>>>
>>>>> If so, it looks like unused "wiphy->iface_combinations" for radio
>>>>> specific combination advertised.
>>>>
>>>> This patch series assumes that you have both 
>>>> wiphy->iface_combinations and radio->iface_combinations set. 
>>>> wiphy->iface_combinations applies to the full wiphy, whereas 
>>>> radio->iface_combinations only applies to vifs assigned to the radio.
>>>
>>>
>>> If radio->iface_combinations is set then always vifs assigned to the
>>> radio. so wiphy->iface_combinations get avoid for all the use cases.
>>>
>>> Ultimately either of one combination only get used by this proposal.
>>>
>>> or I am missing something here ?
>> 
>> The functions that perform interface combination checks are called both 
>> with -1 as radio_idx (meaning per-wiphy), as well as with the assigned 
>> radio id. That way, both kinds of combinations/limits are checked and 
>> enforced.
>> 
> In the radio specific iface advertisement, global iface combination
> represent the union or intersection of all radio iface combination ?

The global interface combination should be a union of all radio 
interface combinations.
You can also use them to apply extra limits, e.g. if you have a limit on 
the per-wiphy number of interfaces (regardless of the assigned radio), 
you use the global interface combination to apply it.

- Felix
Karthikeyan Periyasamy June 7, 2024, 8:54 a.m. UTC | #7
On 6/7/2024 1:46 PM, Felix Fietkau wrote:
> On 07.06.24 08:45, Karthikeyan Periyasamy wrote:
>>
>>
>> On 6/7/2024 10:33 AM, Felix Fietkau wrote:
>>> On 07.06.24 06:54, Karthikeyan Periyasamy wrote:
>>>>
>>>>
>>>> On 6/7/2024 10:05 AM, Felix Fietkau wrote:
>>>>> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>>>>>
>>>>>>
>>>>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>>>>> DFS can be supported with multi-channel combinations, as long as 
>>>>>>> each DFS
>>>>>>> capable radio only supports one channel.
>>>>>>>
>>>>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>>>>> ---
>>>>>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>>>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>>>>>> index 40fbf397ce74..e9c4cf611e94 100644
>>>>>>> --- a/net/mac80211/main.c
>>>>>>> +++ b/net/mac80211/main.c
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>>>>>   {
>>>>>>>       struct ieee80211_local *local = hw_to_local(hw);
>>>>>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct 
>>>>>>> ieee80211_hw *hw)
>>>>>>>               if (comb->num_different_channels > 1)
>>>>>>>                   return -EINVAL;
>>>>>>>           }
>>>>>>> -    } else {
>>>>>>> -        /* DFS is not supported with multi-channel combinations 
>>>>>>> yet */
>>>>>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; 
>>>>>>> i++) {
>>>>>>> -            const struct ieee80211_iface_combination *comb;
>>>>>>> -
>>>>>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>>>>>> +    } else if (hw->wiphy->n_radio) {
>>>>>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>>>>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>>>>>> -            if (comb->radar_detect_widths &&
>>>>>>> -                comb->num_different_channels > 1)
>>>>>>> +            if 
>>>>>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>>>>>> +                              radio->n_iface_combinations))
>>>>>>>                   return -EINVAL;
>>>>>>>           }
>>>>>>> +    } else {
>>>>>>> +        if 
>>>>>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>>>>>> +                          hw->wiphy->n_iface_combinations))
>>>>>>> +            return -EINVAL;
>>>>>>>       }
>>>>>>>       /* Only HW csum features are currently compatible with 
>>>>>>> mac80211 */
>>>>>>
>>>>>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>>>>>> iface combination advertised ?
>>>>>>
>>>>>> If so, it looks like unused "wiphy->iface_combinations" for radio
>>>>>> specific combination advertised.
>>>>>
>>>>> This patch series assumes that you have both 
>>>>> wiphy->iface_combinations and radio->iface_combinations set. 
>>>>> wiphy->iface_combinations applies to the full wiphy, whereas 
>>>>> radio->iface_combinations only applies to vifs assigned to the radio.
>>>>
>>>>
>>>> If radio->iface_combinations is set then always vifs assigned to the
>>>> radio. so wiphy->iface_combinations get avoid for all the use cases.
>>>>
>>>> Ultimately either of one combination only get used by this proposal.
>>>>
>>>> or I am missing something here ?
>>>
>>> The functions that perform interface combination checks are called 
>>> both with -1 as radio_idx (meaning per-wiphy), as well as with the 
>>> assigned radio id. That way, both kinds of combinations/limits are 
>>> checked and enforced.
>>>
>> In the radio specific iface advertisement, global iface combination
>> represent the union or intersection of all radio iface combination ?
> 
> The global interface combination should be a union of all radio 
> interface combinations.
> You can also use them to apply extra limits, e.g. if you have a limit on 
> the per-wiphy number of interfaces (regardless of the assigned radio), 
> you use the global interface combination to apply it.
> 
If the global combination follow union representation, the non-ML client 
takes wrong/invalid perception against the global advertisement.

Ex:

Global iface = 14 ( Radio iface: 2G = 4, 5G = 4, 6G = 6 ).

2G non-client read the global configuration and understand it can able 
to create 14 interfaces. But in reality it allowed to create max 4 
interface only.

we have to follow intersection or minimal set, no ?
Felix Fietkau June 7, 2024, 9 a.m. UTC | #8
On 07.06.24 10:54, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/7/2024 1:46 PM, Felix Fietkau wrote:
>> On 07.06.24 08:45, Karthikeyan Periyasamy wrote:
>>>
>>>
>>> On 6/7/2024 10:33 AM, Felix Fietkau wrote:
>>>> On 07.06.24 06:54, Karthikeyan Periyasamy wrote:
>>>>>
>>>>>
>>>>> On 6/7/2024 10:05 AM, Felix Fietkau wrote:
>>>>>> On 07.06.24 06:25, Karthikeyan Periyasamy wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>>>>>>>> DFS can be supported with multi-channel combinations, as long as 
>>>>>>>> each DFS
>>>>>>>> capable radio only supports one channel.
>>>>>>>>
>>>>>>>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>>>>>>>> ---
>>>>>>>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>>>>>>>   1 file changed, 24 insertions(+), 8 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>>>>>>>> index 40fbf397ce74..e9c4cf611e94 100644
>>>>>>>> --- a/net/mac80211/main.c
>>>>>>>> +++ b/net/mac80211/main.c
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>>>>>>>   {
>>>>>>>>       struct ieee80211_local *local = hw_to_local(hw);
>>>>>>>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct 
>>>>>>>> ieee80211_hw *hw)
>>>>>>>>               if (comb->num_different_channels > 1)
>>>>>>>>                   return -EINVAL;
>>>>>>>>           }
>>>>>>>> -    } else {
>>>>>>>> -        /* DFS is not supported with multi-channel combinations 
>>>>>>>> yet */
>>>>>>>> -        for (i = 0; i < local->hw.wiphy->n_iface_combinations; 
>>>>>>>> i++) {
>>>>>>>> -            const struct ieee80211_iface_combination *comb;
>>>>>>>> -
>>>>>>>> -            comb = &local->hw.wiphy->iface_combinations[i];
>>>>>>>> +    } else if (hw->wiphy->n_radio) {
>>>>>>>> +        for (i = 0; i < hw->wiphy->n_radio; i++) {
>>>>>>>> +            const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>>>>>>> -            if (comb->radar_detect_widths &&
>>>>>>>> -                comb->num_different_channels > 1)
>>>>>>>> +            if 
>>>>>>>> (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>>>>>>>> +                              radio->n_iface_combinations))
>>>>>>>>                   return -EINVAL;
>>>>>>>>           }
>>>>>>>> +    } else {
>>>>>>>> +        if 
>>>>>>>> (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
>>>>>>>> +                          hw->wiphy->n_iface_combinations))
>>>>>>>> +            return -EINVAL;
>>>>>>>>       }
>>>>>>>>       /* Only HW csum features are currently compatible with 
>>>>>>>> mac80211 */
>>>>>>>
>>>>>>> Are we omitting the "wiphy->iface_combinations" if the radio specific
>>>>>>> iface combination advertised ?
>>>>>>>
>>>>>>> If so, it looks like unused "wiphy->iface_combinations" for radio
>>>>>>> specific combination advertised.
>>>>>>
>>>>>> This patch series assumes that you have both 
>>>>>> wiphy->iface_combinations and radio->iface_combinations set. 
>>>>>> wiphy->iface_combinations applies to the full wiphy, whereas 
>>>>>> radio->iface_combinations only applies to vifs assigned to the radio.
>>>>>
>>>>>
>>>>> If radio->iface_combinations is set then always vifs assigned to the
>>>>> radio. so wiphy->iface_combinations get avoid for all the use cases.
>>>>>
>>>>> Ultimately either of one combination only get used by this proposal.
>>>>>
>>>>> or I am missing something here ?
>>>>
>>>> The functions that perform interface combination checks are called 
>>>> both with -1 as radio_idx (meaning per-wiphy), as well as with the 
>>>> assigned radio id. That way, both kinds of combinations/limits are 
>>>> checked and enforced.
>>>>
>>> In the radio specific iface advertisement, global iface combination
>>> represent the union or intersection of all radio iface combination ?
>> 
>> The global interface combination should be a union of all radio 
>> interface combinations.
>> You can also use them to apply extra limits, e.g. if you have a limit on 
>> the per-wiphy number of interfaces (regardless of the assigned radio), 
>> you use the global interface combination to apply it.
>> 
> If the global combination follow union representation, the non-ML client
> takes wrong/invalid perception against the global advertisement.
> 
> Ex:
> 
> Global iface = 14 ( Radio iface: 2G = 4, 5G = 4, 6G = 6 ).
> 
> 2G non-client read the global configuration and understand it can able
> to create 14 interfaces. But in reality it allowed to create max 4
> interface only.
> 
> we have to follow intersection or minimal set, no ?

Maybe I misunderstood your question earlier. I meant that the driver can 
set the global limit as a sort of union of all per-radio limits.
For each individual radio, the intersection of the global limit (taking 
into account other radios as well) and the per-radio limit (considering 
only per-radio vifs) applies.

- Felix
Karthikeyan Periyasamy June 12, 2024, 2:23 p.m. UTC | #9
On 6/6/2024 11:37 PM, Felix Fietkau wrote:
> DFS can be supported with multi-channel combinations, as long as each DFS
> capable radio only supports one channel.
> 
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
> ---
>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>   1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index 40fbf397ce74..e9c4cf611e94 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -1084,6 +1084,21 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
>   	return 0;
>   }
>   
> +static bool
> +ieee80211_ifcomb_check_radar(const struct ieee80211_iface_combination *comb,
> +			     int n_comb)
> +{
> +	int i;
> +
> +	/* DFS is not supported with multi-channel combinations yet */
> +	for (i = 0; i < n_comb; i++, comb++)
> +		if (comb->radar_detect_widths &&
> +		    comb->num_different_channels > 1)
> +			return false;
> +
> +	return true;
> +}
> +
>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>   {
>   	struct ieee80211_local *local = hw_to_local(hw);
> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>   			if (comb->num_different_channels > 1)
>   				return -EINVAL;
>   		}
> -	} else {
> -		/* DFS is not supported with multi-channel combinations yet */
> -		for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
> -			const struct ieee80211_iface_combination *comb;
> -
> -			comb = &local->hw.wiphy->iface_combinations[i];
> +	} else if (hw->wiphy->n_radio) {
> +		for (i = 0; i < hw->wiphy->n_radio; i++) {
> +			const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>   
> -			if (comb->radar_detect_widths &&
> -			    comb->num_different_channels > 1)
> +			if (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
> +							  radio->n_iface_combinations))
>   				return -EINVAL;
>   		}

When driver advertise per radio iface combination, you are not checking 
the global iface combination. using the uncheck global iface combination 
lead to unknown issues.


> +	} else {
> +		if (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
> +						  hw->wiphy->n_iface_combinations))
> +			return -EINVAL;
>   	}
>   
>   	/* Only HW csum features are currently compatible with mac80211 */
Felix Fietkau June 12, 2024, 2:29 p.m. UTC | #10
On 12.06.24 16:23, Karthikeyan Periyasamy wrote:
> 
> 
> On 6/6/2024 11:37 PM, Felix Fietkau wrote:
>> DFS can be supported with multi-channel combinations, as long as each DFS
>> capable radio only supports one channel.
>> 
>> Signed-off-by: Felix Fietkau <nbd@nbd.name>
>> ---
>>   net/mac80211/main.c | 32 ++++++++++++++++++++++++--------
>>   1 file changed, 24 insertions(+), 8 deletions(-)
>> 
>> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
>> index 40fbf397ce74..e9c4cf611e94 100644
>> --- a/net/mac80211/main.c
>> +++ b/net/mac80211/main.c
>> @@ -1084,6 +1084,21 @@ static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
>>   	return 0;
>>   }
>>   
>> +static bool
>> +ieee80211_ifcomb_check_radar(const struct ieee80211_iface_combination *comb,
>> +			     int n_comb)
>> +{
>> +	int i;
>> +
>> +	/* DFS is not supported with multi-channel combinations yet */
>> +	for (i = 0; i < n_comb; i++, comb++)
>> +		if (comb->radar_detect_widths &&
>> +		    comb->num_different_channels > 1)
>> +			return false;
>> +
>> +	return true;
>> +}
>> +
>>   int ieee80211_register_hw(struct ieee80211_hw *hw)
>>   {
>>   	struct ieee80211_local *local = hw_to_local(hw);
>> @@ -1173,17 +1188,18 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
>>   			if (comb->num_different_channels > 1)
>>   				return -EINVAL;
>>   		}
>> -	} else {
>> -		/* DFS is not supported with multi-channel combinations yet */
>> -		for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
>> -			const struct ieee80211_iface_combination *comb;
>> -
>> -			comb = &local->hw.wiphy->iface_combinations[i];
>> +	} else if (hw->wiphy->n_radio) {
>> +		for (i = 0; i < hw->wiphy->n_radio; i++) {
>> +			const struct wiphy_radio *radio = &hw->wiphy->radio[i];
>>   
>> -			if (comb->radar_detect_widths &&
>> -			    comb->num_different_channels > 1)
>> +			if (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
>> +							  radio->n_iface_combinations))
>>   				return -EINVAL;
>>   		}
> 
> When driver advertise per radio iface combination, you are not checking
> the global iface combination. using the uncheck global iface combination
> lead to unknown issues.

The only constraint being checked here is num_different_channel > 1 XOR 
comb->radar_detect_widths != 0.
The global wiphy needs both being set, since it applies the sum of all 
radios. For each individual radio, the constraint is enforced, so there 
should not be any unknown issues.

- Felix
diff mbox series

Patch

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 40fbf397ce74..e9c4cf611e94 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1084,6 +1084,21 @@  static int ieee80211_init_cipher_suites(struct ieee80211_local *local)
 	return 0;
 }
 
+static bool
+ieee80211_ifcomb_check_radar(const struct ieee80211_iface_combination *comb,
+			     int n_comb)
+{
+	int i;
+
+	/* DFS is not supported with multi-channel combinations yet */
+	for (i = 0; i < n_comb; i++, comb++)
+		if (comb->radar_detect_widths &&
+		    comb->num_different_channels > 1)
+			return false;
+
+	return true;
+}
+
 int ieee80211_register_hw(struct ieee80211_hw *hw)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
@@ -1173,17 +1188,18 @@  int ieee80211_register_hw(struct ieee80211_hw *hw)
 			if (comb->num_different_channels > 1)
 				return -EINVAL;
 		}
-	} else {
-		/* DFS is not supported with multi-channel combinations yet */
-		for (i = 0; i < local->hw.wiphy->n_iface_combinations; i++) {
-			const struct ieee80211_iface_combination *comb;
-
-			comb = &local->hw.wiphy->iface_combinations[i];
+	} else if (hw->wiphy->n_radio) {
+		for (i = 0; i < hw->wiphy->n_radio; i++) {
+			const struct wiphy_radio *radio = &hw->wiphy->radio[i];
 
-			if (comb->radar_detect_widths &&
-			    comb->num_different_channels > 1)
+			if (!ieee80211_ifcomb_check_radar(radio->iface_combinations,
+							  radio->n_iface_combinations))
 				return -EINVAL;
 		}
+	} else {
+		if (!ieee80211_ifcomb_check_radar(hw->wiphy->iface_combinations,
+						  hw->wiphy->n_iface_combinations))
+			return -EINVAL;
 	}
 
 	/* Only HW csum features are currently compatible with mac80211 */