On 2/8/24 7:28 AM, Lorenzo Bianconi wrote:
On Tue, 2024-01-30 at 10:08 -0800, greearb@xxxxxxxxxxxxxxx wrote:
From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
Old code would not set it to disabled, just assumed that driver
would default to disabled. Change this to explicitly request
bss color be flushed on initial driver configuration.
Arguably, the current behaviour is in line with the documentation of
BSS_CHANGED_HE_BSS_COLOR ... but I would tend to agree that
enabling/disabling coloring should be covered by it as well. Lorenzo?
And I think the beacon-change logic was slightly wrong, so adjust
that as well.
That's not a great thing for a commit message to say ...
Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
---
net/mac80211/cfg.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 1c7fb0959cfd..1a6c6c764cbc 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1331,8 +1331,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)
- changed |= BSS_CHANGED_HE_BSS_COLOR;
+ changed |= BSS_CHANGED_HE_BSS_COLOR;
I think we should use beacon->he_bss_color_valid here instead of
params->beacon.he_bss_color.enabled, agree?
Either way, the value changed from un-set/un-known to some value, so why not just
mark it changed and flush to the driver?
Thanks,
Ben
}
if (params->he_cap) {
@@ -1512,6 +1511,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);
@@ -1549,9 +1549,9 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
return err;
changed |= 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;
}
this seems fine to me. >
Regards,
Lorenzo
Not sure how this isn't updating the color itself, Lorenzo?
johannes
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
