Simplify the code to send the addba response and the delba message. Use
ieee80211 helpers if possible.
This series should be applied after the "start cleaning up
issue_action_BA" series.
Martin Kaiser (11):
staging: r8188eu: use mgmt to set resp dialog token
staging: r8188eu: use mgmt to set the addba resp status
staging: r8188eu: use mgmt to set the addba resp timeout
staging: r8188eu: use ieee80211 to set addba resp capabilities
staging: r8188eu: calculate the addba response length
staging: r8188eu: use mgmt to set the delba reason code
staging: r8188eu: use mgmt to set delba params
staging: r8188eu: clarify the contents of the delba params
staging: r8188eu: calculate the delba length
staging: r8188eu: remove the pframe variable
staging: r8188eu: use u8, u16 in issue_action_BA prototype
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 51 +++++++------------
.../staging/r8188eu/include/rtw_mlme_ext.h | 3 +-
2 files changed, 18 insertions(+), 36 deletions(-)
--
2.30.2
An addba response always ends with the timeout field. The length of the
addba response is the offset of the end of the timeout field in the struct
ieee80211_mgmt that defines the message.
Use offsetofend to calculate this offset and drop the intermediate pktlen
increments as we add addba response components.
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b391273969af..2a13546e585c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -5441,18 +5441,14 @@ void issue_action_BA(struct adapter *padapter, unsigned char *raddr, unsigned ch
break;
case WLAN_ACTION_ADDBA_RESP:
mgmt->u.action.u.addba_resp.action_code = WLAN_ACTION_ADDBA_RESP;
- pattrib->pktlen++;
mgmt->u.action.u.addba_resp.dialog_token = pmlmeinfo->ADDBA_req.dialog_token;
- pattrib->pktlen++;
mgmt->u.action.u.addba_resp.status = cpu_to_le16(status);
- pattrib->pktlen += 2;
capab = le16_to_cpu(pmlmeinfo->ADDBA_req.BA_para_set) & 0x3f;
capab |= u16_encode_bits(64, IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK);
capab |= u16_encode_bits(pregpriv->ampdu_amsdu, IEEE80211_ADDBA_PARAM_AMSDU_MASK);
mgmt->u.action.u.addba_req.capab = cpu_to_le16(capab);
- pattrib->pktlen += 2;
mgmt->u.action.u.addba_resp.timeout = pmlmeinfo->ADDBA_req.BA_timeout_value;
- pattrib->pktlen += 2;
+ pattrib->pktlen = offsetofend(struct ieee80211_mgmt, u.action.u.addba_resp.timeout);
break;
case WLAN_ACTION_DELBA:
mgmt->u.action.u.delba.action_code = WLAN_ACTION_DELBA;
--
2.30.2
The delba parameters field contains an initiator/receiver flag and the
tid. The caller of issue_action_BA passes these components in the status
parameter.
Extract the two components from status and use u16_encode_bits to copy
them into the parameters field. This should clarify what's going on and
should make it easier to replace the status parameter in the future.
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b4d22dae212a..7968674a0705 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -5377,7 +5377,7 @@ void issue_action_BA(struct adapter *padapter, unsigned char *raddr, unsigned ch
struct sta_priv *pstapriv = &padapter->stapriv;
struct registry_priv *pregpriv = &padapter->registrypriv;
struct ieee80211_mgmt *mgmt;
- u16 capab;
+ u16 capab, params;
pmgntframe = alloc_mgtxmitframe(pxmitpriv);
if (!pmgntframe)
@@ -5450,6 +5450,9 @@ void issue_action_BA(struct adapter *padapter, unsigned char *raddr, unsigned ch
mgmt->u.action.u.delba.action_code = WLAN_ACTION_DELBA;
pattrib->pktlen++;
mgmt->u.action.u.delba.params = cpu_to_le16((status & 0x1F) << 3);
+ params = u16_encode_bits((status & 0x1), IEEE80211_DELBA_PARAM_INITIATOR_MASK);
+ params |= u16_encode_bits((status >> 1) & 0xF, IEEE80211_DELBA_PARAM_TID_MASK);
+ mgmt->u.action.u.delba.params = cpu_to_le16(params);
pattrib->pktlen += 2;
mgmt->u.action.u.delba.reason_code = cpu_to_le16(WLAN_STATUS_REQUEST_DECLINED);
pattrib->pktlen += 2;
--
2.30.2
Hi Martin,
On 6/2/22 22:37, Martin Kaiser wrote:
> Simplify the code to send the addba response and the delba message. Use
> ieee80211 helpers if possible.
>
> This series should be applied after the "start cleaning up
> issue_action_BA" series.
>
> Martin Kaiser (11):
> staging: r8188eu: use mgmt to set resp dialog token
> staging: r8188eu: use mgmt to set the addba resp status
> staging: r8188eu: use mgmt to set the addba resp timeout
> staging: r8188eu: use ieee80211 to set addba resp capabilities
> staging: r8188eu: calculate the addba response length
> staging: r8188eu: use mgmt to set the delba reason code
> staging: r8188eu: use mgmt to set delba params
> staging: r8188eu: clarify the contents of the delba params
> staging: r8188eu: calculate the delba length
> staging: r8188eu: remove the pframe variable
> staging: r8188eu: use u8, u16 in issue_action_BA prototype
>
> drivers/staging/r8188eu/core/rtw_mlme_ext.c | 51 +++++++------------
> .../staging/r8188eu/include/rtw_mlme_ext.h | 3 +-
> 2 files changed, 18 insertions(+), 36 deletions(-)
>
Looks fine, thanks!
Tested-by: Pavel Skripkin <[email protected]>
With regards,
Pavel Skripkin
Use the mgmt structure and defines from ieee80211.h to set the
capabilities field of an addba response.
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index de2a1e8b1a65..b391273969af 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -5446,16 +5446,11 @@ void issue_action_BA(struct adapter *padapter, unsigned char *raddr, unsigned ch
pattrib->pktlen++;
mgmt->u.action.u.addba_resp.status = cpu_to_le16(status);
pattrib->pktlen += 2;
- BA_para_set = le16_to_cpu(pmlmeinfo->ADDBA_req.BA_para_set) & 0x3f;
- BA_para_set |= 0x1000; /* 64 buffer size */
-
- if (pregpriv->ampdu_amsdu == 0)/* disabled */
- BA_para_set = BA_para_set & ~BIT(0);
- else if (pregpriv->ampdu_amsdu == 1)/* enabled */
- BA_para_set = BA_para_set | BIT(0);
- le_tmp = cpu_to_le16(BA_para_set);
-
- pframe = rtw_set_fixed_ie(pframe, 2, (unsigned char *)&le_tmp, &pattrib->pktlen);
+ capab = le16_to_cpu(pmlmeinfo->ADDBA_req.BA_para_set) & 0x3f;
+ capab |= u16_encode_bits(64, IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK);
+ capab |= u16_encode_bits(pregpriv->ampdu_amsdu, IEEE80211_ADDBA_PARAM_AMSDU_MASK);
+ mgmt->u.action.u.addba_req.capab = cpu_to_le16(capab);
+ pattrib->pktlen += 2;
mgmt->u.action.u.addba_resp.timeout = pmlmeinfo->ADDBA_req.BA_timeout_value;
pattrib->pktlen += 2;
break;
--
2.30.2