[PATCH 0/6] staging: r8188eu: clean up ap2sta_data_frame

[PATCH 0/6] staging: r8188eu: clean up ap2sta_data_frame

[Martin Kaiser]

Start cleaning up ap2sta_data_frame. Remove duplicate assignments, use an
ieee80211 helper in one place.

Martin Kaiser (6):
  staging: r8188eu: set bssid only once
  staging: r8188eu: don't set pattrib->dst again
  staging: r8188eu: don't set pattrib->ra again
  staging: r8188eu: don't set pattrib->ta again
  staging: r8188eu: remove a misleading comment
  staging: r8188eu: use ieee80211 helper to check for nullfunc frame

 drivers/staging/r8188eu/core/rtw_recv.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

-- 
2.30.2

[PATCH 1/6] staging: r8188eu: set bssid only once

[Martin Kaiser]

In function ap2sta_data_frame, for WIFI_MP_STATE && _FW_LINKED, the code
sets pattrib->bssid twice. Remove the first one of these assignments.
Remove an empty comment before the second assignment.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 020bc212532f..9c63319a7d48 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -706,11 +706,9 @@ static int ap2sta_data_frame(
 		   check_fwstate(pmlmepriv, _FW_LINKED)) {
 		memcpy(pattrib->dst, GetAddr1Ptr(ptr), ETH_ALEN);
 		memcpy(pattrib->src, GetAddr2Ptr(ptr), ETH_ALEN);
-		memcpy(pattrib->bssid, GetAddr3Ptr(ptr), ETH_ALEN);
 		memcpy(pattrib->ra, pattrib->dst, ETH_ALEN);
 		memcpy(pattrib->ta, pattrib->src, ETH_ALEN);
 
-		/*  */
 		memcpy(pattrib->bssid,  mybssid, ETH_ALEN);
 
 		*psta = rtw_get_stainfo(pstapriv, pattrib->bssid); /*  get sta_info */
-- 
2.30.2

[PATCH 2/6] staging: r8188eu: don't set pattrib->dst again

[Martin Kaiser]

Do not set pattrib->dst again in ap2sta_data_frame. This function is
called by validate_recv_data_frame when the packet's ToDS==0 and
FromDS==1.

In this case, validate_recv_data_frame has already set pattrib->dst
to addr1.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 9c63319a7d48..ea81501b3286 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -704,7 +704,6 @@ static int ap2sta_data_frame(
 		}
 	} else if (check_fwstate(pmlmepriv, WIFI_MP_STATE) &&
 		   check_fwstate(pmlmepriv, _FW_LINKED)) {
-		memcpy(pattrib->dst, GetAddr1Ptr(ptr), ETH_ALEN);
 		memcpy(pattrib->src, GetAddr2Ptr(ptr), ETH_ALEN);
 		memcpy(pattrib->ra, pattrib->dst, ETH_ALEN);
 		memcpy(pattrib->ta, pattrib->src, ETH_ALEN);
-- 
2.30.2

[PATCH 3/6] staging: r8188eu: don't set pattrib->ra again

[Martin Kaiser]

Do not set pattrib->ra again in ap2sta_data_frame. This function is
called by validate_recv_data_frame when the packet's ToDS==0 and
FromDS==1.

validate_recv_data_frame has already set pattrib->ra to addr1. This is in
line with the table in ieee80211.h.

ap2sta_data_frame sets pattrib->ra to pattrib->dst. For ToDS==0, FromDS==1,
dst is addr1, i.e. ap2sta_data_frame sets the same value again that was
already set by validate_recv_data_frame.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index ea81501b3286..3619b1ab0267 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -705,7 +705,6 @@ static int ap2sta_data_frame(
 	} else if (check_fwstate(pmlmepriv, WIFI_MP_STATE) &&
 		   check_fwstate(pmlmepriv, _FW_LINKED)) {
 		memcpy(pattrib->src, GetAddr2Ptr(ptr), ETH_ALEN);
-		memcpy(pattrib->ra, pattrib->dst, ETH_ALEN);
 		memcpy(pattrib->ta, pattrib->src, ETH_ALEN);
 
 		memcpy(pattrib->bssid,  mybssid, ETH_ALEN);
-- 
2.30.2

[PATCH 4/6] staging: r8188eu: don't set pattrib->ta again

[Martin Kaiser]

The table in ieee80211.h shows that TA is addr2. This value was already
set by validate_recv_data_frame, we don't have to set it again in
ap2sta_data_frame.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 3619b1ab0267..a761d88d4711 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -705,7 +705,6 @@ static int ap2sta_data_frame(
 	} else if (check_fwstate(pmlmepriv, WIFI_MP_STATE) &&
 		   check_fwstate(pmlmepriv, _FW_LINKED)) {
 		memcpy(pattrib->src, GetAddr2Ptr(ptr), ETH_ALEN);
-		memcpy(pattrib->ta, pattrib->src, ETH_ALEN);
 
 		memcpy(pattrib->bssid,  mybssid, ETH_ALEN);
 
-- 
2.30.2

[PATCH 5/6] staging: r8188eu: remove a misleading comment

[Martin Kaiser]

Remove a misleading comment in ap2sta_data_frame. We are not checking
for QOS frames.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index a761d88d4711..8b21ec8f111f 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -693,9 +693,6 @@ static int ap2sta_data_frame(
 			goto exit;
 		}
 
-		/* if ((GetFrameSubType(ptr) & WIFI_QOS_DATA_TYPE) == WIFI_QOS_DATA_TYPE) { */
-		/*  */
-
 		if (GetFrameSubType(ptr) & BIT(6)) {
 			/* No data, will not indicate to upper layer, temporily count it here */
 			count_rx_stats(adapter, precv_frame, *psta);
-- 
2.30.2

[PATCH 6/6] staging: r8188eu: use ieee80211 helper to check for nullfunc frame

[Martin Kaiser]

Use the ieee80211_is_nullfunc function from ieee80211.h to check for a
nullfunc frame. This is a data frame whose subtype has bit6 set.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 8b21ec8f111f..32cd452d1f3d 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -649,6 +649,7 @@ static int ap2sta_data_frame(
 	struct sta_info **psta)
 {
 	u8 *ptr = precv_frame->rx_data;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)precv_frame->rx_data;
 	struct rx_pkt_attrib *pattrib = &precv_frame->attrib;
 	int ret = _SUCCESS;
 	struct	sta_priv *pstapriv = &adapter->stapriv;
@@ -693,8 +694,8 @@ static int ap2sta_data_frame(
 			goto exit;
 		}
 
-		if (GetFrameSubType(ptr) & BIT(6)) {
-			/* No data, will not indicate to upper layer, temporily count it here */
+		if (ieee80211_is_nullfunc(hdr->frame_control)) {
+			/* We count the nullfunc frame, but we'll not pass it on to higher layers. */
 			count_rx_stats(adapter, precv_frame, *psta);
 			ret = RTW_RX_HANDLED;
 			goto exit;
-- 
2.30.2

Re: [PATCH 0/6] staging: r8188eu: clean up ap2sta_data_frame

[Philipp Hortmann]

On 7/4/22 22:16, Martin Kaiser wrote:
> Start cleaning up ap2sta_data_frame. Remove duplicate assignments, use an
> ieee80211 helper in one place.
> 
> Martin Kaiser (6):
>    staging: r8188eu: set bssid only once
>    staging: r8188eu: don't set pattrib->dst again
>    staging: r8188eu: don't set pattrib->ra again
>    staging: r8188eu: don't set pattrib->ta again
>    staging: r8188eu: remove a misleading comment
>    staging: r8188eu: use ieee80211 helper to check for nullfunc frame
> 
>   drivers/staging/r8188eu/core/rtw_recv.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
> 

Tested-by: Philipp Hortmann <philipp.g.hortmann@gmail.com> # Edimax N150