From: Ben Greear <[email protected]>
Always tell driver to apply bss color settings if beacon indicates
the bss coloring has been set.
And only enable bss coloring if beacon indicates bss color setting
is valid and also enabled.
Signed-off-by: Ben Greear <[email protected]>
---
net/mac80211/cfg.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 327682995c92..aaa62c05428c 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1314,7 +1314,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK);
changed |= BSS_CHANGED_HE_OBSS_PD;
- if (params->beacon.he_bss_color.enabled)
+ if (params->beacon.he_bss_color_valid)
changed |= BSS_CHANGED_HE_BSS_COLOR;
}
@@ -1494,6 +1494,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
int err;
struct ieee80211_bss_conf *link_conf;
u64 changed = 0;
+ bool color_en;
lockdep_assert_wiphy(wiphy);
@@ -1530,9 +1531,9 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
if (err < 0)
return err;
- if (beacon->he_bss_color_valid &&
- beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
- link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
+ color_en = beacon->he_bss_color.enabled && beacon->he_bss_color_valid;
+ if (color_en != link_conf->he_bss_color.enabled) {
+ link_conf->he_bss_color.enabled = color_en;
changed |= BSS_CHANGED_HE_BSS_COLOR;
}
--
2.42.0
On 3/7/2024 10:10 AM, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> Always tell driver to apply bss color settings if beacon indicates
> the bss coloring has been set.
why? please describe the problem you are fixing
>
> And only enable bss coloring if beacon indicates bss color setting
> is valid and also enabled.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> net/mac80211/cfg.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 327682995c92..aaa62c05428c 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1314,7 +1314,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK);
> changed |= BSS_CHANGED_HE_OBSS_PD;
>
> - if (params->beacon.he_bss_color.enabled)
> + if (params->beacon.he_bss_color_valid)
> changed |= BSS_CHANGED_HE_BSS_COLOR;
> }
>
> @@ -1494,6 +1494,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
> int err;
> struct ieee80211_bss_conf *link_conf;
> u64 changed = 0;
> + bool color_en;
>
> lockdep_assert_wiphy(wiphy);
>
> @@ -1530,9 +1531,9 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
> if (err < 0)
> return err;
>
> - if (beacon->he_bss_color_valid &&
> - beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
> - link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
> + color_en = beacon->he_bss_color.enabled && beacon->he_bss_color_valid;
> + if (color_en != link_conf->he_bss_color.enabled) {
> + link_conf->he_bss_color.enabled = color_en;
> changed |= BSS_CHANGED_HE_BSS_COLOR;
> }
>
On 3/8/24 09:40, Jeff Johnson wrote:
> On 3/7/2024 10:10 AM, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> Always tell driver to apply bss color settings if beacon indicates
>> the bss coloring has been set.
>
> why? please describe the problem you are fixing
To ensure driver is set to known state. This does not fix any known bug,
but seems appropriate, and I noticed driver wasn't being set to
disabled when BSS color was disabled while debugging some other issues
(the driver in question defaulted to disabled).
>> And only enable bss coloring if beacon indicates bss color setting
>> is valid and also enabled.
This seems more correct behaviour to me. It does not fix any known
problem.
Thanks,
Ben
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> net/mac80211/cfg.c | 9 +++++----
>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> index 327682995c92..aaa62c05428c 100644
>> --- a/net/mac80211/cfg.c
>> +++ b/net/mac80211/cfg.c
>> @@ -1314,7 +1314,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
>> IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK);
>> changed |= BSS_CHANGED_HE_OBSS_PD;
>>
>> - if (params->beacon.he_bss_color.enabled)
>> + if (params->beacon.he_bss_color_valid)
>> changed |= BSS_CHANGED_HE_BSS_COLOR;
>> }
>>
>> @@ -1494,6 +1494,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
>> int err;
>> struct ieee80211_bss_conf *link_conf;
>> u64 changed = 0;
>> + bool color_en;
>>
>> lockdep_assert_wiphy(wiphy);
>>
>> @@ -1530,9 +1531,9 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
>> if (err < 0)
>> return err;
>>
>> - if (beacon->he_bss_color_valid &&
>> - beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
>> - link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
>> + color_en = beacon->he_bss_color.enabled && beacon->he_bss_color_valid;
>> + if (color_en != link_conf->he_bss_color.enabled) {
>> + link_conf->he_bss_color.enabled = color_en;
>> changed |= BSS_CHANGED_HE_BSS_COLOR;
>> }
>>
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
> From: Ben Greear <[email protected]>
>
> Always tell driver to apply bss color settings if beacon indicates
> the bss coloring has been set.
>
> And only enable bss coloring if beacon indicates bss color setting
> is valid and also enabled.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> net/mac80211/cfg.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 327682995c92..aaa62c05428c 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1314,7 +1314,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
> IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK);
> changed |= BSS_CHANGED_HE_OBSS_PD;
>
> - if (params->beacon.he_bss_color.enabled)
> + if (params->beacon.he_bss_color_valid)
> changed |= BSS_CHANGED_HE_BSS_COLOR;
This change seems correct to me since he_bss_color_valid is set to true when userspace
(e.g. hostapd) provides the bss color info.
> }
>
> @@ -1494,6 +1494,7 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
> int err;
> struct ieee80211_bss_conf *link_conf;
> u64 changed = 0;
> + bool color_en;
>
> lockdep_assert_wiphy(wiphy);
>
> @@ -1530,9 +1531,9 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
> if (err < 0)
> return err;
>
> - if (beacon->he_bss_color_valid &&
> - beacon->he_bss_color.enabled != link_conf->he_bss_color.enabled) {
> - link_conf->he_bss_color.enabled = beacon->he_bss_color.enabled;
> + color_en = beacon->he_bss_color.enabled && beacon->he_bss_color_valid;
> + if (color_en != link_conf->he_bss_color.enabled) {
> + link_conf->he_bss_color.enabled = color_en;
> changed |= BSS_CHANGED_HE_BSS_COLOR;
> }
Here both approaches seem fine to me (but I do not have a strong opinion).
The one proposed by Ben disables by default bss coloring if color is not
valid.
Regards,
Lorenzo
>
> --
> 2.42.0
>
>