* [PATCH] mac80211: Ensure bss-coloring is always configured @ 2024-01-30 18:08 greearb 2024-01-31 11:53 ` Johannes Berg 0 siblings, 1 reply; 7+ messages in thread From: greearb @ 2024-01-30 18:08 UTC (permalink / raw) To: linux-wireless; +Cc: Ben Greear From: Ben Greear <greearb@candelatech.com> 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. And I think the beacon-change logic was slightly wrong, so adjust that as well. Signed-off-by: Ben Greear <greearb@candelatech.com> --- 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; } 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; } -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-01-30 18:08 [PATCH] mac80211: Ensure bss-coloring is always configured greearb @ 2024-01-31 11:53 ` Johannes Berg 2024-02-08 15:28 ` Lorenzo Bianconi 0 siblings, 1 reply; 7+ messages in thread From: Johannes Berg @ 2024-01-31 11:53 UTC (permalink / raw) To: greearb, linux-wireless; +Cc: Lorenzo Bianconi On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: > From: Ben Greear <greearb@candelatech.com> > > 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@candelatech.com> > --- > 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; > } > > 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; > } > Not sure how this isn't updating the color itself, Lorenzo? johannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-01-31 11:53 ` Johannes Berg @ 2024-02-08 15:28 ` Lorenzo Bianconi 2024-02-08 15:41 ` Ben Greear 0 siblings, 1 reply; 7+ messages in thread From: Lorenzo Bianconi @ 2024-02-08 15:28 UTC (permalink / raw) To: Johannes Berg; +Cc: greearb, linux-wireless [-- Attachment #1: Type: text/plain, Size: 2529 bytes --] > On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: > > From: Ben Greear <greearb@candelatech.com> > > > > 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@candelatech.com> > > --- > > 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? > > } > > > > 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 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-02-08 15:28 ` Lorenzo Bianconi @ 2024-02-08 15:41 ` Ben Greear 2024-02-08 15:55 ` Lorenzo Bianconi 0 siblings, 1 reply; 7+ messages in thread From: Ben Greear @ 2024-02-08 15:41 UTC (permalink / raw) To: Lorenzo Bianconi, Johannes Berg; +Cc: linux-wireless On 2/8/24 7:28 AM, Lorenzo Bianconi wrote: >> On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: >>> From: Ben Greear <greearb@candelatech.com> >>> >>> 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@candelatech.com> >>> --- >>> 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@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-02-08 15:41 ` Ben Greear @ 2024-02-08 15:55 ` Lorenzo Bianconi 2024-02-08 16:25 ` Ben Greear 0 siblings, 1 reply; 7+ messages in thread From: Lorenzo Bianconi @ 2024-02-08 15:55 UTC (permalink / raw) To: Ben Greear; +Cc: Johannes Berg, linux-wireless [-- Attachment #1: Type: text/plain, Size: 3510 bytes --] > On 2/8/24 7:28 AM, Lorenzo Bianconi wrote: > > > On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: > > > > From: Ben Greear <greearb@candelatech.com> > > > > > > > > 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@candelatech.com> > > > > --- > > > > 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? IIUC what you mean here, if bss color changes from an un-set/un-known to a configured (valid) value, beacon->he_bss_color_valid will be true (he_bss_color_valid is used to indicate userspace provided a proper color for beacons). What is the difference? The other way around ("undo" some leftover color from a previous run), it seems a driver/fw bug, and it must be fixed there. Don't you think? Regards, Lorenzo > > 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@candelatech.com> > Candela Technologies Inc http://www.candelatech.com [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-02-08 15:55 ` Lorenzo Bianconi @ 2024-02-08 16:25 ` Ben Greear 2024-02-08 16:44 ` Lorenzo Bianconi 0 siblings, 1 reply; 7+ messages in thread From: Ben Greear @ 2024-02-08 16:25 UTC (permalink / raw) To: Lorenzo Bianconi; +Cc: Johannes Berg, linux-wireless On 2/8/24 07:55, Lorenzo Bianconi wrote: >> On 2/8/24 7:28 AM, Lorenzo Bianconi wrote: >>>> On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: >>>>> From: Ben Greear <greearb@candelatech.com> >>>>> >>>>> 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@candelatech.com> >>>>> --- >>>>> 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? > > IIUC what you mean here, if bss color changes from an un-set/un-known to a > configured (valid) value, beacon->he_bss_color_valid will be true > (he_bss_color_valid is used to indicate userspace provided a proper color for > beacons). What is the difference? > The other way around ("undo" some leftover color from a previous run), it > seems a driver/fw bug, and it must be fixed there. Don't you think? Well, no. I think the stack should set to known state, thus my original patch to do so. But, not worth arguing about as it seems to have no functional difference at this point. Thanks, Ben > > Regards, > Lorenzo > >> >> 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@candelatech.com> >> Candela Technologies Inc http://www.candelatech.com -- Ben Greear <greearb@candelatech.com> Candela Technologies Inc http://www.candelatech.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mac80211: Ensure bss-coloring is always configured 2024-02-08 16:25 ` Ben Greear @ 2024-02-08 16:44 ` Lorenzo Bianconi 0 siblings, 0 replies; 7+ messages in thread From: Lorenzo Bianconi @ 2024-02-08 16:44 UTC (permalink / raw) To: Ben Greear; +Cc: Johannes Berg, linux-wireless [-- Attachment #1: Type: text/plain, Size: 4344 bytes --] > On 2/8/24 07:55, Lorenzo Bianconi wrote: > > > On 2/8/24 7:28 AM, Lorenzo Bianconi wrote: > > > > > On Tue, 2024-01-30 at 10:08 -0800, greearb@candelatech.com wrote: > > > > > > From: Ben Greear <greearb@candelatech.com> > > > > > > > > > > > > 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@candelatech.com> > > > > > > --- > > > > > > 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? > > > > IIUC what you mean here, if bss color changes from an un-set/un-known to a > > configured (valid) value, beacon->he_bss_color_valid will be true > > (he_bss_color_valid is used to indicate userspace provided a proper color for > > beacons). What is the difference? > > The other way around ("undo" some leftover color from a previous run), it > > seems a driver/fw bug, and it must be fixed there. Don't you think? > > Well, no. I think the stack should set to known state, thus my original > patch to do so. > > But, not worth arguing about as it seems to have no functional difference > at this point. I think the second part is fine and should be applied. Regards, Lorenzo > > Thanks, > Ben > > > > > Regards, > > Lorenzo > > > > > > > > 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@candelatech.com> > > > Candela Technologies Inc http://www.candelatech.com > > -- > Ben Greear <greearb@candelatech.com> > Candela Technologies Inc http://www.candelatech.com > > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-08 16:44 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-30 18:08 [PATCH] mac80211: Ensure bss-coloring is always configured greearb 2024-01-31 11:53 ` Johannes Berg 2024-02-08 15:28 ` Lorenzo Bianconi 2024-02-08 15:41 ` Ben Greear 2024-02-08 15:55 ` Lorenzo Bianconi 2024-02-08 16:25 ` Ben Greear 2024-02-08 16:44 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox