diff mbox series

[1/2] wifi: iwlwifi: Fix eMLSR band comparison.

Message ID 20240828202630.2151365-1-greearb@candelatech.com
State New
Headers show
Series [1/2] wifi: iwlwifi: Fix eMLSR band comparison. | expand

Commit Message

Ben Greear Aug. 28, 2024, 8:26 p.m. UTC
From: Ben Greear <greearb@candelatech.com>

Do not make assumptions about what band 'a' or 'b' are on.
And add new reason code for when eMLSR is disabled due to
band.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/link.c | 13 ++++++++++---
 drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 ++
 2 files changed, 12 insertions(+), 3 deletions(-)

Comments

Miri Korenblit Nov. 21, 2024, 8:11 p.m. UTC | #1
> -----Original Message-----
> From: greearb@candelatech.com <greearb@candelatech.com>
> Sent: Wednesday, 28 August 2024 23:26
> To: linux-wireless@vger.kernel.org
> Cc: Ben Greear <greearb@candelatech.com>
> Subject: [PATCH 1/2] wifi: iwlwifi: Fix eMLSR band comparison.
> 
> From: Ben Greear <greearb@candelatech.com>
> 
> Do not make assumptions about what band 'a' or 'b' are on.
> And add new reason code for when eMLSR is disabled due to band.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>  drivers/net/wireless/intel/iwlwifi/mvm/link.c | 13 ++++++++++---
> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 ++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/link.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
> index bb3de07bc6be..f3fb37fec8a8 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/link.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
> @@ -16,6 +16,7 @@
>  	HOW(EXIT_LOW_RSSI)		\
>  	HOW(EXIT_COEX)			\
>  	HOW(EXIT_BANDWIDTH)		\
> +	HOW(EXIT_BAND)			\
>  	HOW(EXIT_CSA)			\
>  	HOW(EXIT_LINK_USAGE)
> 
> @@ -750,11 +751,17 @@ bool iwl_mvm_mld_valid_link_pair(struct
> ieee80211_vif *vif,
>  	    iwl_mvm_esr_disallowed_with_link(mvm, vif, b, false))
>  		return false;
> 
> -	if (a->chandef->width != b->chandef->width ||
> -	    !(a->chandef->chan->band == NL80211_BAND_6GHZ &&
> -	      b->chandef->chan->band == NL80211_BAND_5GHZ))
> +	if (a->chandef->width != b->chandef->width)
>  		ret |= IWL_MVM_ESR_EXIT_BANDWIDTH;
> 
> +	/* Only supports 5g and 6g bands at the moment */
> +	if (((a->chandef->chan->band != NL80211_BAND_6GHZ) &&
> +	     (a->chandef->chan->band != NL80211_BAND_5GHZ)) ||
> +	    ((b->chandef->chan->band != NL80211_BAND_6GHZ) &&
> +	     (b->chandef->chan->band != NL80211_BAND_5GHZ)) ||
> +	    (a->chandef->chan->band == b->chandef->chan->band))
> +		ret |= IWL_MVM_ESR_EXIT_BAND;
> +
>  	if (ret) {
>  		IWL_DEBUG_INFO(mvm,
>  			       "Links %d and %d are not a valid pair for EMLSR, a-
> >chwidth: %d  b: %d band-a: %d  band-b: %d\n", diff --git
> a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> index ac4e135ed91b..22bec9ca46bb 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
> @@ -368,6 +368,7 @@ struct iwl_mvm_vif_link_info {
>   *	preventing the enablement of EMLSR
>   * @IWL_MVM_ESR_EXIT_CSA: CSA happened, so exit EMLSR
>   * @IWL_MVM_ESR_EXIT_LINK_USAGE: Exit EMLSR due to low tpt on secondary
> link
> + * @IWL_MVM_ESR_EXIT_BAND: Exit EMLSR due to incompatible Bands
>   */
>  enum iwl_mvm_esr_state {
>  	IWL_MVM_ESR_BLOCKED_PREVENTION	= 0x1,
> @@ -382,6 +383,7 @@ enum iwl_mvm_esr_state {
>  	IWL_MVM_ESR_EXIT_BANDWIDTH	= 0x80000,
>  	IWL_MVM_ESR_EXIT_CSA		= 0x100000,
>  	IWL_MVM_ESR_EXIT_LINK_USAGE	= 0x200000,
> +	IWL_MVM_ESR_EXIT_BAND		= 0x400000,
>  };
> 
>  #define IWL_MVM_BLOCK_ESR_REASONS 0xffff
> --
> 2.42.0
> 
Hi Ben.

It is actually required that a (the better link) will be on 6 GHz and b on 5 GHz
Regarding the new exit reason, it is not really needed as we can easily differentiate between the cases (from other logs)

NACK.

Thanks,
Miri
Ben Greear Nov. 21, 2024, 8:24 p.m. UTC | #2
On 11/21/24 12:11 PM, Korenblit, Miriam Rachel wrote:
> 
> 
>> -----Original Message-----
>> From: greearb@candelatech.com <greearb@candelatech.com>
>> Sent: Wednesday, 28 August 2024 23:26
>> To: linux-wireless@vger.kernel.org
>> Cc: Ben Greear <greearb@candelatech.com>
>> Subject: [PATCH 1/2] wifi: iwlwifi: Fix eMLSR band comparison.
>>
>> From: Ben Greear <greearb@candelatech.com>
>>
>> Do not make assumptions about what band 'a' or 'b' are on.
>> And add new reason code for when eMLSR is disabled due to band.
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>   drivers/net/wireless/intel/iwlwifi/mvm/link.c | 13 ++++++++++---
>> drivers/net/wireless/intel/iwlwifi/mvm/mvm.h  |  2 ++
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/link.c
>> b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
>> index bb3de07bc6be..f3fb37fec8a8 100644
>> --- a/drivers/net/wireless/intel/iwlwifi/mvm/link.c
>> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
>> @@ -16,6 +16,7 @@
>>   	HOW(EXIT_LOW_RSSI)		\
>>   	HOW(EXIT_COEX)			\
>>   	HOW(EXIT_BANDWIDTH)		\
>> +	HOW(EXIT_BAND)			\
>>   	HOW(EXIT_CSA)			\
>>   	HOW(EXIT_LINK_USAGE)
>>
>> @@ -750,11 +751,17 @@ bool iwl_mvm_mld_valid_link_pair(struct
>> ieee80211_vif *vif,
>>   	    iwl_mvm_esr_disallowed_with_link(mvm, vif, b, false))
>>   		return false;
>>
>> -	if (a->chandef->width != b->chandef->width ||
>> -	    !(a->chandef->chan->band == NL80211_BAND_6GHZ &&
>> -	      b->chandef->chan->band == NL80211_BAND_5GHZ))
>> +	if (a->chandef->width != b->chandef->width)
>>   		ret |= IWL_MVM_ESR_EXIT_BANDWIDTH;
>>
>> +	/* Only supports 5g and 6g bands at the moment */
>> +	if (((a->chandef->chan->band != NL80211_BAND_6GHZ) &&
>> +	     (a->chandef->chan->band != NL80211_BAND_5GHZ)) ||
>> +	    ((b->chandef->chan->band != NL80211_BAND_6GHZ) &&
>> +	     (b->chandef->chan->band != NL80211_BAND_5GHZ)) ||
>> +	    (a->chandef->chan->band == b->chandef->chan->band))
>> +		ret |= IWL_MVM_ESR_EXIT_BAND;
>> +
>>   	if (ret) {
>>   		IWL_DEBUG_INFO(mvm,
>>   			       "Links %d and %d are not a valid pair for EMLSR, a-
>>> chwidth: %d  b: %d band-a: %d  band-b: %d\n", diff --git
>> a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> index ac4e135ed91b..22bec9ca46bb 100644
>> --- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
>> @@ -368,6 +368,7 @@ struct iwl_mvm_vif_link_info {
>>    *	preventing the enablement of EMLSR
>>    * @IWL_MVM_ESR_EXIT_CSA: CSA happened, so exit EMLSR
>>    * @IWL_MVM_ESR_EXIT_LINK_USAGE: Exit EMLSR due to low tpt on secondary
>> link
>> + * @IWL_MVM_ESR_EXIT_BAND: Exit EMLSR due to incompatible Bands
>>    */
>>   enum iwl_mvm_esr_state {
>>   	IWL_MVM_ESR_BLOCKED_PREVENTION	= 0x1,
>> @@ -382,6 +383,7 @@ enum iwl_mvm_esr_state {
>>   	IWL_MVM_ESR_EXIT_BANDWIDTH	= 0x80000,
>>   	IWL_MVM_ESR_EXIT_CSA		= 0x100000,
>>   	IWL_MVM_ESR_EXIT_LINK_USAGE	= 0x200000,
>> +	IWL_MVM_ESR_EXIT_BAND		= 0x400000,
>>   };
>>
>>   #define IWL_MVM_BLOCK_ESR_REASONS 0xffff
>> --
>> 2.42.0
>>
> Hi Ben.
> 
> It is actually required that a (the better link) will be on 6 GHz and b on 5 GHz
> Regarding the new exit reason, it is not really needed as we can easily differentiate between the cases (from other logs)

Hello Miri,

I tested this patch, and it fixed problems for me when I ran a test that created
interfering traffic on 5ghz and then later on 6Ghz.  I expected eMLSR mode to stay
active no matter where the interfering traffic existed.  With this patch, and a few
others I posted, the be200 then works fairly well.

6Ghz is not always better, for instance in case where it is congested with
external traffic.

Can you please let me know *why* you think the better link must always be 6ghz in this case?

Thanks,
Ben

> 
> NACK.
> 
> Thanks,
> Miri
diff mbox series

Patch

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/link.c b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
index bb3de07bc6be..f3fb37fec8a8 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/link.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/link.c
@@ -16,6 +16,7 @@ 
 	HOW(EXIT_LOW_RSSI)		\
 	HOW(EXIT_COEX)			\
 	HOW(EXIT_BANDWIDTH)		\
+	HOW(EXIT_BAND)			\
 	HOW(EXIT_CSA)			\
 	HOW(EXIT_LINK_USAGE)
 
@@ -750,11 +751,17 @@  bool iwl_mvm_mld_valid_link_pair(struct ieee80211_vif *vif,
 	    iwl_mvm_esr_disallowed_with_link(mvm, vif, b, false))
 		return false;
 
-	if (a->chandef->width != b->chandef->width ||
-	    !(a->chandef->chan->band == NL80211_BAND_6GHZ &&
-	      b->chandef->chan->band == NL80211_BAND_5GHZ))
+	if (a->chandef->width != b->chandef->width)
 		ret |= IWL_MVM_ESR_EXIT_BANDWIDTH;
 
+	/* Only supports 5g and 6g bands at the moment */
+	if (((a->chandef->chan->band != NL80211_BAND_6GHZ) &&
+	     (a->chandef->chan->band != NL80211_BAND_5GHZ)) ||
+	    ((b->chandef->chan->band != NL80211_BAND_6GHZ) &&
+	     (b->chandef->chan->band != NL80211_BAND_5GHZ)) ||
+	    (a->chandef->chan->band == b->chandef->chan->band))
+		ret |= IWL_MVM_ESR_EXIT_BAND;
+
 	if (ret) {
 		IWL_DEBUG_INFO(mvm,
 			       "Links %d and %d are not a valid pair for EMLSR, a->chwidth: %d  b: %d band-a: %d  band-b: %d\n",
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index ac4e135ed91b..22bec9ca46bb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -368,6 +368,7 @@  struct iwl_mvm_vif_link_info {
  *	preventing the enablement of EMLSR
  * @IWL_MVM_ESR_EXIT_CSA: CSA happened, so exit EMLSR
  * @IWL_MVM_ESR_EXIT_LINK_USAGE: Exit EMLSR due to low tpt on secondary link
+ * @IWL_MVM_ESR_EXIT_BAND: Exit EMLSR due to incompatible Bands
  */
 enum iwl_mvm_esr_state {
 	IWL_MVM_ESR_BLOCKED_PREVENTION	= 0x1,
@@ -382,6 +383,7 @@  enum iwl_mvm_esr_state {
 	IWL_MVM_ESR_EXIT_BANDWIDTH	= 0x80000,
 	IWL_MVM_ESR_EXIT_CSA		= 0x100000,
 	IWL_MVM_ESR_EXIT_LINK_USAGE	= 0x200000,
+	IWL_MVM_ESR_EXIT_BAND		= 0x400000,
 };
 
 #define IWL_MVM_BLOCK_ESR_REASONS 0xffff