Received-SPF: pass (google.com: domain of linux-wireless+bounces-4501-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <69b95f70-d43f-4ed5-9aa6-5a3f498700bf@candelatech.com>
Date: Fri, 8 Mar 2024 09:44:04 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] wifi: mac80211: Improve bss-color configuration.
Content-Language: en-MW
To: Jeff Johnson <quic_jjohnson@quicinc.com>, linux-wireless@vger.kernel.org
References: <20240307181039.3219840-1-greearb@candelatech.com>
 <7ab9885b-5fb6-4686-b29b-d287e569293b@quicinc.com>
From: Ben Greear <greearb@candelatech.com>
Organization: Candela Technologies
In-Reply-To: <7ab9885b-5fb6-4686-b29b-d287e569293b@quicinc.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 3/8/24 09:40, Jeff Johnson wrote:
> On 3/7/2024 10:10 AM, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> 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 <greearb@candelatech.com>
>> ---
>>   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 <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com