| Message ID | 8fc2f117346fcb4ed11bb20cdf9cb1f88bcf64b4.1717611760.git-series.nbd@nbd.name (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | cfg80211/mac80211: support defining multiple radios per wiphy | expand |
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 ?
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
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.
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
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.
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
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 27355e08ae52..c1439ac975d6 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -1595,6 +1595,7 @@ struct cfg80211_color_change_settings { * * Used to pass interface combination parameters * + * @radio: when set, check radio specific interface combinations. * @num_different_channels: the number of different channels we want * to use for verification * @radar_detect: a bitmap where each bit corresponds to a channel @@ -1608,6 +1609,7 @@ struct cfg80211_color_change_settings { * the verification */ struct iface_combination_params { + const struct wiphy_radio *radio; int num_different_channels; u8 radar_detect; int iftype_num[NUM_NL80211_IFTYPES]; diff --git a/net/wireless/util.c b/net/wireless/util.c index 2bde8a354631..fc3e8fbb4cc2 100644 --- a/net/wireless/util.c +++ b/net/wireless/util.c @@ -2309,9 +2309,6 @@ static void cfg80211_calculate_bi_data(struct wiphy *wiphy, u32 new_beacon_int, { struct wireless_dev *wdev; - *beacon_int_gcd = 0; - *beacon_int_different = false; - list_for_each_entry(wdev, &wiphy->wdev_list, list) { int wdev_bi; @@ -2366,13 +2363,15 @@ int cfg80211_iter_combinations(struct wiphy *wiphy, void *data), void *data) { + const struct wiphy_radio *radio = params->radio; + const struct ieee80211_iface_combination *c; const struct ieee80211_regdomain *regdom; enum nl80211_dfs_regions region = 0; - int i, j, iftype; + int i, j, n, iftype; int num_interfaces = 0; u32 used_iftypes = 0; - u32 beacon_int_gcd; - bool beacon_int_different; + u32 beacon_int_gcd = 0; + bool beacon_int_different = false; /* * 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); if (params->radar_detect) { rcu_read_lock(); @@ -2402,13 +2403,17 @@ int cfg80211_iter_combinations(struct wiphy *wiphy, used_iftypes |= BIT(iftype); } - for (i = 0; i < wiphy->n_iface_combinations; i++) { - const struct ieee80211_iface_combination *c; + if (radio) { + c = radio->iface_combinations; + n = radio->n_iface_combinations; + } else { + c = wiphy->iface_combinations; + n = wiphy->n_iface_combinations; + } + for (i = 0; i < n; i++, c++) { struct ieee80211_iface_limit *limits; u32 all_iftypes = 0; - c = &wiphy->iface_combinations[i]; - if (num_interfaces > c->max_interfaces) continue; if (params->num_different_channels > c->num_different_channels)
Add a field in struct iface_combination_params to check per-radio interface combinations instead of per-wiphy ones. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- include/net/cfg80211.h | 2 ++ net/wireless/util.c | 29 +++++++++++++++++------------ 2 files changed, 19 insertions(+), 12 deletions(-)