| Message ID | 20240828202630.2151365-1-greearb@candelatech.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] wifi: iwlwifi: Fix eMLSR band comparison. | expand |
> -----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
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
On 11/21/24 12:24, Ben Greear wrote: > 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? Hello Miriam, I wanted to check to see if you still consider this patch invalid? If so, I'll adjust it to work better as out-of-tree patch and add it to my pile. If you think core logic is fine but the patch needs some tweaks, please let me know your suggestions. Thanks, Ben
> -----Original Message----- > From: Ben Greear <greearb@candelatech.com> > Sent: Tuesday, 10 December 2024 21:55 > To: Korenblit, Miriam Rachel <miriam.rachel.korenblit@intel.com>; linux- > wireless@vger.kernel.org > Subject: Re: [PATCH 1/2] wifi: iwlwifi: Fix eMLSR band comparison. > > On 11/21/24 12:24, Ben Greear wrote: > > 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? > > Hello Miriam, > > I wanted to check to see if you still consider this patch invalid? If so, I'll adjust > it to work better as out-of-tree patch and add it to my pile. > > If you think core logic is fine but the patch needs some tweaks, please let me > know your suggestions. We are just changing this logic internally (and allow also 2.4 GHz) So I am not going to take it. > > Thanks, > Ben >
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