[PATCH 0/7] staging: r8188eu: more cleanups in OnAction_back

[PATCH 0/7] staging: r8188eu: more cleanups in OnAction_back

[Martin Kaiser]

Simplify some more places where fields are extracted from an incoming
block ack frame.

This series should be applied after the "start cleaning up OnAction_back"
series.

Martin Kaiser (7):
  staging: r8188eu: get addba resp status from ieee80211_mgmt
  staging: r8188eu: read addba resp tid from ieee80211_mgmt
  staging: r8188eu: merge process_addba_req into OnAction_back
  staging: r8188eu: replace if with ternary operator
  staging: r8188eu: use BIT(tid) instead of manual shift
  staging: r8188eu: check for initiator vs recipient
  staging: r8188eu: use the tid in delba processing

 drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 36 ++++++++++---------
 drivers/staging/r8188eu/core/rtw_wlan_util.c  | 22 ------------
 .../staging/r8188eu/include/rtw_mlme_ext.h    |  2 --
 3 files changed, 20 insertions(+), 40 deletions(-)

-- 
2.30.2

[PATCH 1/7] staging: r8188eu: get addba resp status from ieee80211_mgmt

[Martin Kaiser]

Read the addba response status from struct ieee80211_mgmt instead of
parsing the incoming frame ourselves.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 8a9236ba7c66..4d8337d0dc20 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1471,7 +1471,7 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 	struct sta_info *psta = NULL;
 	struct recv_reorder_ctrl *preorder_ctrl;
 	unsigned char		*frame_body;
-	unsigned short	tid, status;
+	unsigned short	tid;
 	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
 	u8 *pframe = precv_frame->rx_data;
@@ -1505,9 +1505,8 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 37);/* reject ADDBA Req */
 		break;
 	case WLAN_ACTION_ADDBA_RESP:
-		status = get_unaligned_le16(&frame_body[3]);
 		tid = ((frame_body[5] >> 2) & 0x7);
-		if (status == 0) {	/* successful */
+		if (mgmt->u.action.u.addba_resp.status == 0) {	/* successful */
 			psta->htpriv.agg_enable_bitmap |= 1 << tid;
 			psta->htpriv.candidate_tid_bitmap &= ~BIT(tid);
 		} else {
-- 
2.30.2

[PATCH 2/7] staging: r8188eu: read addba resp tid from ieee80211_mgmt

[Martin Kaiser]

Use the same code as ieee80211_process_addba_resp to calculate the tid
from the capability field of an addba response message.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 4d8337d0dc20..03c9b2d0d4e8 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1505,7 +1505,8 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 37);/* reject ADDBA Req */
 		break;
 	case WLAN_ACTION_ADDBA_RESP:
-		tid = ((frame_body[5] >> 2) & 0x7);
+		tid = u16_get_bits(le16_to_cpu(mgmt->u.action.u.addba_resp.capab),
+				   IEEE80211_ADDBA_PARAM_TID_MASK);
 		if (mgmt->u.action.u.addba_resp.status == 0) {	/* successful */
 			psta->htpriv.agg_enable_bitmap |= 1 << tid;
 			psta->htpriv.candidate_tid_bitmap &= ~BIT(tid);
-- 
2.30.2

[PATCH 3/7] staging: r8188eu: merge process_addba_req into OnAction_back

[Martin Kaiser]

The process_addba_req function is called only by OnAction_back. Merge the
two functions.

OnAction_back has already checked that psta != NULL, we can drop the check
in process_addba_req before merging the functions. We can also simplify
the parsing of the tid by using struct ieee80211.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c   |  6 ++++-
 drivers/staging/r8188eu/core/rtw_wlan_util.c  | 22 -------------------
 .../staging/r8188eu/include/rtw_mlme_ext.h    |  2 --
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 03c9b2d0d4e8..77f229121b4d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1497,7 +1497,11 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 	switch (mgmt->u.action.u.addba_req.action_code) {
 	case WLAN_ACTION_ADDBA_REQ:
 		memcpy(&pmlmeinfo->ADDBA_req, &frame_body[2], sizeof(struct ADDBA_request));
-		process_addba_req(padapter, (u8 *)&pmlmeinfo->ADDBA_req, mgmt->sa);
+		tid = u16_get_bits(le16_to_cpu(mgmt->u.action.u.addba_req.capab),
+				   IEEE80211_ADDBA_PARAM_TID_MASK);
+		preorder_ctrl = &psta->recvreorder_ctrl[tid];
+		preorder_ctrl->indicate_seq = 0xffff;
+		preorder_ctrl->enable = pmlmeinfo->bAcceptAddbaReq;
 
 		if (pmlmeinfo->bAcceptAddbaReq)
 			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 0);
diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index 27035eac6e61..9585ab41cd3f 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -1464,28 +1464,6 @@ int update_sta_support_rate(struct adapter *padapter, u8 *pvar_ie, uint var_ie_l
 	return _SUCCESS;
 }
 
-void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr)
-{
-	struct sta_info *psta;
-	u16 tid;
-	u16 param;
-	struct recv_reorder_ctrl *preorder_ctrl;
-	struct sta_priv *pstapriv = &padapter->stapriv;
-	struct ADDBA_request	*preq = (struct ADDBA_request *)paddba_req;
-	struct mlme_ext_priv	*pmlmeext = &padapter->mlmeextpriv;
-	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
-
-	psta = rtw_get_stainfo(pstapriv, addr);
-
-	if (psta) {
-		param = le16_to_cpu(preq->BA_para_set);
-		tid = (param >> 2) & 0x0f;
-		preorder_ctrl = &psta->recvreorder_ctrl[tid];
-		preorder_ctrl->indicate_seq = 0xffff;
-		preorder_ctrl->enable = pmlmeinfo->bAcceptAddbaReq;
-	}
-}
-
 void update_TSF(struct mlme_ext_priv *pmlmeext, u8 *pframe, uint len)
 {
 	u8 *pIE;
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index 888b12a9f755..573d65b175cc 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -592,8 +592,6 @@ void addba_timer_hdl(struct sta_info *psta);
 bool cckrates_included(unsigned char *rate, int ratelen);
 bool cckratesonly_included(unsigned char *rate, int ratelen);
 
-void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr);
-
 void update_TSF(struct mlme_ext_priv *pmlmeext, u8 *pframe, uint len);
 void correct_TSF(struct adapter *padapter, struct mlme_ext_priv *pmlmeext);
 
-- 
2.30.2

[PATCH 4/7] staging: r8188eu: replace if with ternary operator

[Martin Kaiser]

Replace an if statement with a ternary operator, making the code a tiny
bit shorter.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 77f229121b4d..d5ea074c62f4 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1503,10 +1503,8 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 		preorder_ctrl->indicate_seq = 0xffff;
 		preorder_ctrl->enable = pmlmeinfo->bAcceptAddbaReq;
 
-		if (pmlmeinfo->bAcceptAddbaReq)
-			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 0);
-		else
-			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 37);/* reject ADDBA Req */
+		issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP,
+				pmlmeinfo->bAcceptAddbaReq ? 0 : 37);
 		break;
 	case WLAN_ACTION_ADDBA_RESP:
 		tid = u16_get_bits(le16_to_cpu(mgmt->u.action.u.addba_resp.capab),
-- 
2.30.2

[PATCH 5/7] staging: r8188eu: use BIT(tid) instead of manual shift

[Martin Kaiser]

Use BIT(tid) instead of 1 << tid when we process an addba response.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index d5ea074c62f4..9aa6d09b2e74 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1510,7 +1510,7 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 		tid = u16_get_bits(le16_to_cpu(mgmt->u.action.u.addba_resp.capab),
 				   IEEE80211_ADDBA_PARAM_TID_MASK);
 		if (mgmt->u.action.u.addba_resp.status == 0) {	/* successful */
-			psta->htpriv.agg_enable_bitmap |= 1 << tid;
+			psta->htpriv.agg_enable_bitmap |= BIT(tid);
 			psta->htpriv.candidate_tid_bitmap &= ~BIT(tid);
 		} else {
 			psta->htpriv.agg_enable_bitmap &= ~BIT(tid);
-- 
2.30.2

[PATCH 6/7] staging: r8188eu: check for initiator vs recipient

[Martin Kaiser]

For an incoming delba message, the code checks if the delba was sent by
the data initiator or by the data recipient.

Rewrite this check using the ieee80211 helpers, making the code easier to
understand.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 9aa6d09b2e74..af525d633fd7 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1517,10 +1517,11 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 		}
 		break;
 	case WLAN_ACTION_DELBA:
-		if ((frame_body[3] & BIT(3)) == 0) {
+		if (u16_get_bits(le16_to_cpu(mgmt->u.action.u.delba.params),
+				 IEEE80211_DELBA_PARAM_INITIATOR_MASK) == WLAN_BACK_RECIPIENT) {
 			psta->htpriv.agg_enable_bitmap &= ~(1 << ((frame_body[3] >> 4) & 0xf));
 			psta->htpriv.candidate_tid_bitmap &= ~(1 << ((frame_body[3] >> 4) & 0xf));
-		} else if ((frame_body[3] & BIT(3)) == BIT(3)) {
+		} else {
 			tid = (frame_body[3] >> 4) & 0x0F;
 			preorder_ctrl =  &psta->recvreorder_ctrl[tid];
 			preorder_ctrl->enable = false;
-- 
2.30.2

[PATCH 7/7] staging: r8188eu: use the tid in delba processing

[Martin Kaiser]

Extract the tid from the delba frame regardless of the recipient/initiator
setting. Use the extracted tid when we process the delba frame.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index af525d633fd7..b24c6a79646d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1517,12 +1517,13 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
 		}
 		break;
 	case WLAN_ACTION_DELBA:
+		tid = u16_get_bits(le16_to_cpu(mgmt->u.action.u.delba.params),
+				   IEEE80211_DELBA_PARAM_TID_MASK);
 		if (u16_get_bits(le16_to_cpu(mgmt->u.action.u.delba.params),
 				 IEEE80211_DELBA_PARAM_INITIATOR_MASK) == WLAN_BACK_RECIPIENT) {
-			psta->htpriv.agg_enable_bitmap &= ~(1 << ((frame_body[3] >> 4) & 0xf));
-			psta->htpriv.candidate_tid_bitmap &= ~(1 << ((frame_body[3] >> 4) & 0xf));
+			psta->htpriv.agg_enable_bitmap &= ~BIT(tid);
+			psta->htpriv.candidate_tid_bitmap &= ~BIT(tid);
 		} else {
-			tid = (frame_body[3] >> 4) & 0x0F;
 			preorder_ctrl =  &psta->recvreorder_ctrl[tid];
 			preorder_ctrl->enable = false;
 			preorder_ctrl->indicate_seq = 0xffff;
-- 
2.30.2

Re: [PATCH 4/7] staging: r8188eu: replace if with ternary operator

[Joe Perches]

On Sat, 2022-05-14 at 18:47 +0200, Martin Kaiser wrote:
> Replace an if statement with a ternary operator, making the code a tiny
> bit shorter.
[]
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
[]
> @@ -1503,10 +1503,8 @@ unsigned int OnAction_back(struct adapter *padapter, struct recv_frame *precv_fr
>  		preorder_ctrl->indicate_seq = 0xffff;
>  		preorder_ctrl->enable = pmlmeinfo->bAcceptAddbaReq;
>  
> -		if (pmlmeinfo->bAcceptAddbaReq)
> -			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 0);
> -		else
> -			issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP, 37);/* reject ADDBA Req */
> +		issue_action_BA(padapter, mgmt->sa, WLAN_ACTION_ADDBA_RESP,
> +				pmlmeinfo->bAcceptAddbaReq ? 0 : 37);

Seems like magic numbers.  Maybe add/use defines?