[PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c

Atharva Tiwari posted 1 patch 1 year, 1 month ago
drivers/staging/rtl8723bs/core/rtw_mlme.c | 28 +++++++++++++++--------
1 file changed, 19 insertions(+), 9 deletions(-)
[PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
Posted by Atharva Tiwari 1 year, 1 month ago
this patch fixes the network selection logic to avoid selecting a network with the same ESSID as the oldest scanned network
if it was scanned within the last second. it also improves A-MPDU reordering bu enabling it only if the AP supports it,and disabling it otherwise

and also i have a question what does "new enough" mean on line 481?

Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_mlme.c | 28 +++++++++++++++--------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 5ded183aa08c..b33846f88680 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -481,7 +481,9 @@ void rtw_update_scanned_network(struct adapter *adapter, struct wlan_bssid_ex *t
 		}
 
 		if (rtw_roam_flags(adapter)) {
-			/* TODO: don't select network in the same ess as oldest if it's new enough*/
+			if (is_same_ess(&pnetwork->network, &oldest->network) &&
+				time_after(pnetwork->last_scanned, (unsigned long)msecs_to_jiffies(1000)))
+				continue;
 		}
 
 		if (!oldest || time_after(oldest->last_scanned, pnetwork->last_scanned))
@@ -1000,15 +1002,23 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
 
 		/* for A-MPDU Rx reordering buffer control for bmc_sta & sta_info */
 		/* if A-MPDU Rx is enabled, resetting  rx_ordering_ctrl wstart_b(indicate_seq) to default value = 0xffff */
-		/* todo: check if AP can send A-MPDU packets */
-		for (i = 0; i < 16 ; i++) {
-			preorder_ctrl = &psta->recvreorder_ctrl[i];
-			preorder_ctrl->enable = false;
-			preorder_ctrl->indicate_seq = 0xffff;
-			preorder_ctrl->wend_b = 0xffff;
-			preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
+		if (rtw_check_ap_supports_ampdu(pnetwork)) {
+			for (i = 0; i < 16 ; i++) {
+				preorder_ctrl = &psta->recvreorder_ctrl[i];
+				preorder_ctrl->enable = true; /* Enable A-MPDU reordering */
+				preorder_ctrl->indicate_seq = 0; /* Starting sequence number */
+				preorder_ctrl->wend_b = 0xffff;
+				preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
+			}
+		} else {
+			for (i = 0; i < 16; i++) {
+				preorder_ctrl = &psta->recvreorder_ctrl[i];
+				preorder_ctrl->enable = false;
+				preorder_ctrl->indicate_seq = 0xffff;
+				preorder_ctrl->wend_b = 0xffff;
+				preorder_ctrl->wsize_b = 64;  /* max_ampdu_sz; adjust as needed */
+			}
 		}
-
 		bmc_sta = rtw_get_bcmc_stainfo(padapter);
 		if (bmc_sta) {
 			for (i = 0; i < 16 ; i++) {
-- 
2.39.5
Re: [PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
Posted by kernel test robot 1 year, 1 month ago
Hi Atharva,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Atharva-Tiwari/staging-rtl8723bs-fix-network-selection-and-A-MPDU-reordering-in-rtw_mlme-c/20241224-152918
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20241224072754.2352-1-evepolonium%40gmail.com
patch subject: [PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241225/202412250814.36DjhClh-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241225/202412250814.36DjhClh-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412250814.36DjhClh-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/staging/rtl8723bs/core/rtw_mlme.c:1005:7: error: call to undeclared function 'rtw_check_ap_supports_ampdu'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1005 |                 if (rtw_check_ap_supports_ampdu(pnetwork)) {
         |                     ^
   1 error generated.


vim +/rtw_check_ap_supports_ampdu +1005 drivers/staging/rtl8723bs/core/rtw_mlme.c

   946	
   947	static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, struct wlan_network *pnetwork)
   948	{
   949		int i;
   950		struct sta_info *bmc_sta, *psta = NULL;
   951		struct recv_reorder_ctrl *preorder_ctrl;
   952		struct sta_priv *pstapriv = &padapter->stapriv;
   953		struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
   954	
   955		psta = rtw_get_stainfo(pstapriv, pnetwork->network.mac_address);
   956		if (!psta)
   957			psta = rtw_alloc_stainfo(pstapriv, pnetwork->network.mac_address);
   958	
   959		if (psta) { /* update ptarget_sta */
   960	
   961			psta->aid  = pnetwork->join_res;
   962	
   963			update_sta_info(padapter, psta);
   964	
   965			/* update station supportRate */
   966			psta->bssratelen = rtw_get_rateset_len(pnetwork->network.supported_rates);
   967			memcpy(psta->bssrateset, pnetwork->network.supported_rates, psta->bssratelen);
   968			rtw_hal_update_sta_rate_mask(padapter, psta);
   969	
   970			psta->wireless_mode = pmlmeext->cur_wireless_mode;
   971			psta->raid = networktype_to_raid_ex(padapter, psta);
   972	
   973			/* sta mode */
   974			rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
   975	
   976			/* security related */
   977			if (padapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
   978				padapter->securitypriv.binstallGrpkey = false;
   979				padapter->securitypriv.busetkipkey = false;
   980				padapter->securitypriv.bgrpkey_handshake = false;
   981	
   982				psta->ieee8021x_blocked = true;
   983				psta->dot118021XPrivacy = padapter->securitypriv.dot11PrivacyAlgrthm;
   984	
   985				memset((u8 *)&psta->dot118021x_UncstKey, 0, sizeof(union Keytype));
   986	
   987				memset((u8 *)&psta->dot11tkiprxmickey, 0, sizeof(union Keytype));
   988				memset((u8 *)&psta->dot11tkiptxmickey, 0, sizeof(union Keytype));
   989	
   990				memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
   991				psta->dot11txpn.val = psta->dot11txpn.val + 1;
   992				memset((u8 *)&psta->dot11wtxpn, 0, sizeof(union pn48));
   993				memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
   994			}
   995	
   996			/* When doing the WPS, the wps_ie_len won't equal to 0 */
   997			/* And the Wi-Fi driver shouldn't allow the data packet to be transmitted. */
   998			if (padapter->securitypriv.wps_ie_len != 0) {
   999				psta->ieee8021x_blocked = true;
  1000				padapter->securitypriv.wps_ie_len = 0;
  1001			}
  1002	
  1003			/* for A-MPDU Rx reordering buffer control for bmc_sta & sta_info */
  1004			/* if A-MPDU Rx is enabled, resetting  rx_ordering_ctrl wstart_b(indicate_seq) to default value = 0xffff */
> 1005			if (rtw_check_ap_supports_ampdu(pnetwork)) {
  1006				for (i = 0; i < 16 ; i++) {
  1007					preorder_ctrl = &psta->recvreorder_ctrl[i];
  1008					preorder_ctrl->enable = true; /* Enable A-MPDU reordering */
  1009					preorder_ctrl->indicate_seq = 0; /* Starting sequence number */
  1010					preorder_ctrl->wend_b = 0xffff;
  1011					preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
  1012				}
  1013			} else {
  1014				for (i = 0; i < 16; i++) {
  1015					preorder_ctrl = &psta->recvreorder_ctrl[i];
  1016					preorder_ctrl->enable = false;
  1017					preorder_ctrl->indicate_seq = 0xffff;
  1018					preorder_ctrl->wend_b = 0xffff;
  1019					preorder_ctrl->wsize_b = 64;  /* max_ampdu_sz; adjust as needed */
  1020				}
  1021			}
  1022			bmc_sta = rtw_get_bcmc_stainfo(padapter);
  1023			if (bmc_sta) {
  1024				for (i = 0; i < 16 ; i++) {
  1025					preorder_ctrl = &bmc_sta->recvreorder_ctrl[i];
  1026					preorder_ctrl->enable = false;
  1027					preorder_ctrl->indicate_seq = 0xffff;
  1028					preorder_ctrl->wend_b = 0xffff;
  1029					preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
  1030				}
  1031			}
  1032		}
  1033	
  1034		return psta;
  1035	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
Posted by kernel test robot 1 year, 1 month ago
Hi Atharva,

kernel test robot noticed the following build errors:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/intel-lab-lkp/linux/commits/Atharva-Tiwari/staging-rtl8723bs-fix-network-selection-and-A-MPDU-reordering-in-rtw_mlme-c/20241224-152918
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20241224072754.2352-1-evepolonium%40gmail.com
patch subject: [PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
config: csky-randconfig-001-20241224 (https://download.01.org/0day-ci/archive/20241225/202412250035.fWv25gIa-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241225/202412250035.fWv25gIa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412250035.fWv25gIa-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/staging/rtl8723bs/core/rtw_mlme.c: In function 'rtw_joinbss_update_stainfo':
>> drivers/staging/rtl8723bs/core/rtw_mlme.c:1005:21: error: implicit declaration of function 'rtw_check_ap_supports_ampdu' [-Wimplicit-function-declaration]
    1005 |                 if (rtw_check_ap_supports_ampdu(pnetwork)) {
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/rtw_check_ap_supports_ampdu +1005 drivers/staging/rtl8723bs/core/rtw_mlme.c

   946	
   947	static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, struct wlan_network *pnetwork)
   948	{
   949		int i;
   950		struct sta_info *bmc_sta, *psta = NULL;
   951		struct recv_reorder_ctrl *preorder_ctrl;
   952		struct sta_priv *pstapriv = &padapter->stapriv;
   953		struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
   954	
   955		psta = rtw_get_stainfo(pstapriv, pnetwork->network.mac_address);
   956		if (!psta)
   957			psta = rtw_alloc_stainfo(pstapriv, pnetwork->network.mac_address);
   958	
   959		if (psta) { /* update ptarget_sta */
   960	
   961			psta->aid  = pnetwork->join_res;
   962	
   963			update_sta_info(padapter, psta);
   964	
   965			/* update station supportRate */
   966			psta->bssratelen = rtw_get_rateset_len(pnetwork->network.supported_rates);
   967			memcpy(psta->bssrateset, pnetwork->network.supported_rates, psta->bssratelen);
   968			rtw_hal_update_sta_rate_mask(padapter, psta);
   969	
   970			psta->wireless_mode = pmlmeext->cur_wireless_mode;
   971			psta->raid = networktype_to_raid_ex(padapter, psta);
   972	
   973			/* sta mode */
   974			rtw_hal_set_odm_var(padapter, HAL_ODM_STA_INFO, psta, true);
   975	
   976			/* security related */
   977			if (padapter->securitypriv.dot11AuthAlgrthm == dot11AuthAlgrthm_8021X) {
   978				padapter->securitypriv.binstallGrpkey = false;
   979				padapter->securitypriv.busetkipkey = false;
   980				padapter->securitypriv.bgrpkey_handshake = false;
   981	
   982				psta->ieee8021x_blocked = true;
   983				psta->dot118021XPrivacy = padapter->securitypriv.dot11PrivacyAlgrthm;
   984	
   985				memset((u8 *)&psta->dot118021x_UncstKey, 0, sizeof(union Keytype));
   986	
   987				memset((u8 *)&psta->dot11tkiprxmickey, 0, sizeof(union Keytype));
   988				memset((u8 *)&psta->dot11tkiptxmickey, 0, sizeof(union Keytype));
   989	
   990				memset((u8 *)&psta->dot11txpn, 0, sizeof(union pn48));
   991				psta->dot11txpn.val = psta->dot11txpn.val + 1;
   992				memset((u8 *)&psta->dot11wtxpn, 0, sizeof(union pn48));
   993				memset((u8 *)&psta->dot11rxpn, 0, sizeof(union pn48));
   994			}
   995	
   996			/* When doing the WPS, the wps_ie_len won't equal to 0 */
   997			/* And the Wi-Fi driver shouldn't allow the data packet to be transmitted. */
   998			if (padapter->securitypriv.wps_ie_len != 0) {
   999				psta->ieee8021x_blocked = true;
  1000				padapter->securitypriv.wps_ie_len = 0;
  1001			}
  1002	
  1003			/* for A-MPDU Rx reordering buffer control for bmc_sta & sta_info */
  1004			/* if A-MPDU Rx is enabled, resetting  rx_ordering_ctrl wstart_b(indicate_seq) to default value = 0xffff */
> 1005			if (rtw_check_ap_supports_ampdu(pnetwork)) {
  1006				for (i = 0; i < 16 ; i++) {
  1007					preorder_ctrl = &psta->recvreorder_ctrl[i];
  1008					preorder_ctrl->enable = true; /* Enable A-MPDU reordering */
  1009					preorder_ctrl->indicate_seq = 0; /* Starting sequence number */
  1010					preorder_ctrl->wend_b = 0xffff;
  1011					preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
  1012				}
  1013			} else {
  1014				for (i = 0; i < 16; i++) {
  1015					preorder_ctrl = &psta->recvreorder_ctrl[i];
  1016					preorder_ctrl->enable = false;
  1017					preorder_ctrl->indicate_seq = 0xffff;
  1018					preorder_ctrl->wend_b = 0xffff;
  1019					preorder_ctrl->wsize_b = 64;  /* max_ampdu_sz; adjust as needed */
  1020				}
  1021			}
  1022			bmc_sta = rtw_get_bcmc_stainfo(padapter);
  1023			if (bmc_sta) {
  1024				for (i = 0; i < 16 ; i++) {
  1025					preorder_ctrl = &bmc_sta->recvreorder_ctrl[i];
  1026					preorder_ctrl->enable = false;
  1027					preorder_ctrl->indicate_seq = 0xffff;
  1028					preorder_ctrl->wend_b = 0xffff;
  1029					preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
  1030				}
  1031			}
  1032		}
  1033	
  1034		return psta;
  1035	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] staging: rtl8723bs: fix network selection and A-MPDU reordering in rtw_mlme.c
Posted by Greg Kroah-Hartman 1 year, 1 month ago
On Tue, Dec 24, 2024 at 12:57:52PM +0530, Atharva Tiwari wrote:
> this patch fixes the network selection logic to avoid selecting a network with the same ESSID as the oldest scanned network
> if it was scanned within the last second. it also improves A-MPDU reordering bu enabling it only if the AP supports it,and disabling it otherwise

What commit id does this fix?

And please wrap your changelog at 72 columns like your editor asked you
to do when making the commit :)

> and also i have a question what does "new enough" mean on line 481?

This doesn't belong in a changelog, but rather below the --- line,
right?

> 
> Signed-off-by: Atharva Tiwari <evepolonium@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_mlme.c | 28 +++++++++++++++--------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> index 5ded183aa08c..b33846f88680 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
> @@ -481,7 +481,9 @@ void rtw_update_scanned_network(struct adapter *adapter, struct wlan_bssid_ex *t
>  		}
>  
>  		if (rtw_roam_flags(adapter)) {
> -			/* TODO: don't select network in the same ess as oldest if it's new enough*/
> +			if (is_same_ess(&pnetwork->network, &oldest->network) &&
> +				time_after(pnetwork->last_scanned, (unsigned long)msecs_to_jiffies(1000)))

Where did this magic number come from?

> +				continue;
>  		}
>  
>  		if (!oldest || time_after(oldest->last_scanned, pnetwork->last_scanned))
> @@ -1000,15 +1002,23 @@ static struct sta_info *rtw_joinbss_update_stainfo(struct adapter *padapter, str
>  
>  		/* for A-MPDU Rx reordering buffer control for bmc_sta & sta_info */
>  		/* if A-MPDU Rx is enabled, resetting  rx_ordering_ctrl wstart_b(indicate_seq) to default value = 0xffff */
> -		/* todo: check if AP can send A-MPDU packets */
> -		for (i = 0; i < 16 ; i++) {
> -			preorder_ctrl = &psta->recvreorder_ctrl[i];
> -			preorder_ctrl->enable = false;
> -			preorder_ctrl->indicate_seq = 0xffff;
> -			preorder_ctrl->wend_b = 0xffff;
> -			preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
> +		if (rtw_check_ap_supports_ampdu(pnetwork)) {
> +			for (i = 0; i < 16 ; i++) {
> +				preorder_ctrl = &psta->recvreorder_ctrl[i];
> +				preorder_ctrl->enable = true; /* Enable A-MPDU reordering */
> +				preorder_ctrl->indicate_seq = 0; /* Starting sequence number */
> +				preorder_ctrl->wend_b = 0xffff;
> +				preorder_ctrl->wsize_b = 64;/* max_ampdu_sz;ex. 32(kbytes) -> wsize_b =32 */
> +			}
> +		} else {
> +			for (i = 0; i < 16; i++) {
> +				preorder_ctrl = &psta->recvreorder_ctrl[i];
> +				preorder_ctrl->enable = false;
> +				preorder_ctrl->indicate_seq = 0xffff;

So only two fields are different, right?  Why not just set those in a
separate loop instead?

> +				preorder_ctrl->wend_b = 0xffff;
> +				preorder_ctrl->wsize_b = 64;  /* max_ampdu_sz; adjust as needed */

When is this "needed"?

And you have tested all of this, right?

thanks,

greg k-h