[PATCH 0/4] staging: r8188eu: some more cleanups

[PATCH 0/4] staging: r8188eu: some more cleanups

[Martin Kaiser]

Here's another set of simple cleanups in the code that handles incoming
frames.

Martin Kaiser (4):
  staging: r8188eu: use ieee80211 helper to read the qos tid
  staging: r8188eu: merge _mgt_dispatcher into mgt_dispatcher
  staging: r8188eu: use ieee80211 helpers in mgt_dispatcher
  staging: r8188eu: pkt_newalloc is not used

 drivers/staging/r8188eu/core/rtw_mlme_ext.c | 35 +++++++++------------
 drivers/staging/r8188eu/core/rtw_recv.c     |  3 +-
 drivers/staging/r8188eu/include/rtw_recv.h  |  1 -
 drivers/staging/r8188eu/include/wifi.h      |  2 --
 4 files changed, 16 insertions(+), 25 deletions(-)

-- 
2.30.2

[PATCH 1/4] staging: r8188eu: use ieee80211 helper to read the qos tid

[Martin Kaiser]

Replace the driver-specific GetPriority macro with ieee80211_get_tid,
which does exactly the same thing.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/core/rtw_recv.c | 2 +-
 drivers/staging/r8188eu/include/wifi.h  | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 91a6e0f035f4..16224476d18a 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -978,7 +978,7 @@ static int validate_recv_data_frame(struct adapter *adapter,
 	pattrib->ack_policy = 0;
 	/* parsing QC field */
 	if (pattrib->qos) {
-		pattrib->priority = GetPriority((ptr + 24));
+		pattrib->priority = ieee80211_get_tid(hdr);
 		pattrib->ack_policy = GetAckpolicy((ptr + 24));
 		pattrib->amsdu = GetAMsdu((ptr + 24));
 		pattrib->hdrlen = 26;
diff --git a/drivers/staging/r8188eu/include/wifi.h b/drivers/staging/r8188eu/include/wifi.h
index b26770e91fc0..0254310bdf44 100644
--- a/drivers/staging/r8188eu/include/wifi.h
+++ b/drivers/staging/r8188eu/include/wifi.h
@@ -211,8 +211,6 @@ enum WIFI_REG_DOMAIN {
 #define SetPriority(pbuf, tid)	\
 	*(__le16 *)(pbuf) |= cpu_to_le16(tid & 0xf)
 
-#define GetPriority(pbuf)	((le16_to_cpu(*(__le16 *)(pbuf))) & 0xf)
-
 #define SetEOSP(pbuf, eosp)	\
 		*(__le16 *)(pbuf) |= cpu_to_le16((eosp & 1) << 4)
 
-- 
2.30.2

[PATCH 2/4] staging: r8188eu: merge _mgt_dispatcher into mgt_dispatcher

[Martin Kaiser]

All that the _mgt_dispatcher function does is to call a function from
a function pointer. It's not worth having a separate function for this.

Merge _mgt_dispatcher into mgt_dispatcher.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 66dd06371991..ba071de4c05c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -390,19 +390,6 @@ void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
 	}
 }
 
-static void _mgt_dispatcher(struct adapter *padapter, struct mlme_handler *ptable, struct recv_frame *precv_frame)
-{
-	u8 *pframe = precv_frame->rx_data;
-
-	if (ptable->func) {
-	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
-		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
-		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
-			return;
-		ptable->func(padapter, precv_frame);
-	}
-}
-
 void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	int index;
@@ -442,7 +429,14 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 		else
 			ptable->func = &OnAuthClient;
 	}
-	_mgt_dispatcher(padapter, ptable, precv_frame);
+
+	if (ptable->func) {
+	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
+		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
+		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
+			return;
+		ptable->func(padapter, precv_frame);
+	}
 }
 
 static u32 p2p_listen_state_process(struct adapter *padapter, unsigned char *da)
-- 
2.30.2

Re: [PATCH 2/4] staging: r8188eu: merge _mgt_dispatcher into mgt_dispatcher

[Pavel Skripkin]

Hi Martin,

On 4/13/22 23:07, Martin Kaiser wrote:
> All that the _mgt_dispatcher function does is to call a function from
> a function pointer. It's not worth having a separate function for this.
> 
> Merge _mgt_dispatcher into mgt_dispatcher.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---

[code snip]

> +
> +	if (ptable->func) {
> +	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
> +		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
> +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
> +			return;
> +		ptable->func(padapter, precv_frame);
> +	}
>   }

Looks like each `mlme_sta_tbl` element has `func` member initialized. I 
think, we can remove this check.




With regards,
Pavel Skripkin

Re: [PATCH 2/4] staging: r8188eu: merge _mgt_dispatcher into mgt_dispatcher

[Martin Kaiser]

Hi Pavel,

Thus wrote Pavel Skripkin (paskripkin@gmail.com):

> On 4/13/22 23:07, Martin Kaiser wrote:
> > All that the _mgt_dispatcher function does is to call a function from
> > a function pointer. It's not worth having a separate function for this.

> > Merge _mgt_dispatcher into mgt_dispatcher.

> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---

> [code snip]

> > +
> > +	if (ptable->func) {
> > +	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
> > +		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
> > +		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
> > +			return;
> > +		ptable->func(padapter, precv_frame);
> > +	}
> >   }

> Looks like each `mlme_sta_tbl` element has `func` member initialized. I
> think, we can remove this check.

I've just sent a series to refactor mgt_dispatcher.

All mlme_sta_tbl entries have a function pointer, but some of the point
to a dummy function DoReserved. I guess we should use NULL and keep the
check instead of calling a function that does nothing.

Best regards,
Martin

[PATCH 3/4] staging: r8188eu: use ieee80211 helpers in mgt_dispatcher

[Martin Kaiser]

Use structs and functions from ieee80211.h in mgt_dispatcher to parse
the incoming frame.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index ba071de4c05c..b6ee6a24930a 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -396,14 +396,15 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 	struct mlme_handler *ptable;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	u8 *pframe = precv_frame->rx_data;
-	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, GetAddr2Ptr(pframe));
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)precv_frame->rx_data;
+	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, hdr->addr2);
 
-	if (GetFrameType(pframe) != IEEE80211_FTYPE_MGMT)
+	if (!ieee80211_is_mgmt(hdr->frame_control))
 		return;
 
 	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
-	if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
-	    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
+	if (memcmp(hdr->addr1, myid(&padapter->eeprompriv), ETH_ALEN) &&
+	    !is_broadcast_ether_addr(hdr->addr1))
 		return;
 
 	ptable = mlme_sta_tbl;
@@ -415,7 +416,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 	ptable += index;
 
 	if (psta) {
-		if (GetRetry(pframe)) {
+		if (ieee80211_has_retry(hdr->frame_control)) {
 			if (precv_frame->attrib.seq_num == psta->RxMgmtFrameSeqNum)
 				/* drop the duplicate management frame */
 				return;
@@ -423,7 +424,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 		psta->RxMgmtFrameSeqNum = precv_frame->attrib.seq_num;
 	}
 
-	if (GetFrameSubType(pframe) == WIFI_AUTH) {
+	if (ieee80211_is_auth(hdr->frame_control)) {
 		if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
 			ptable->func = &OnAuth;
 		else
@@ -432,8 +433,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 
 	if (ptable->func) {
 	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
-		if (memcmp(GetAddr1Ptr(pframe), myid(&padapter->eeprompriv), ETH_ALEN) &&
-		    !is_broadcast_ether_addr(GetAddr1Ptr(pframe)))
+		if (memcmp(hdr->addr1, myid(&padapter->eeprompriv), ETH_ALEN) &&
+		    !is_broadcast_ether_addr(hdr->addr1))
 			return;
 		ptable->func(padapter, precv_frame);
 	}
-- 
2.30.2

[PATCH 4/4] staging: r8188eu: pkt_newalloc is not used

[Martin Kaiser]

pkg_newalloc in struct recv_buf is not used. Remove it.

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

diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index 16224476d18a..c1005ddaa249 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -71,7 +71,6 @@ int _rtw_init_recv_priv(struct recv_priv *precvpriv, struct adapter *padapter)
 
 		list_add_tail(&precvframe->list, &precvpriv->free_recv_queue.queue);
 
-		precvframe->pkt_newalloc = NULL;
 		precvframe->pkt = NULL;
 
 		precvframe->len = 0;
diff --git a/drivers/staging/r8188eu/include/rtw_recv.h b/drivers/staging/r8188eu/include/rtw_recv.h
index 59b4773bc3c9..e57e48a51f17 100644
--- a/drivers/staging/r8188eu/include/rtw_recv.h
+++ b/drivers/staging/r8188eu/include/rtw_recv.h
@@ -229,7 +229,6 @@ struct recv_buf {
 struct recv_frame {
 	struct list_head list;
 	struct sk_buff	 *pkt;
-	struct sk_buff	 *pkt_newalloc;
 	struct adapter  *adapter;
 	u8 fragcnt;
 	int frame_tag;
-- 
2.30.2