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

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

[Martin Kaiser]

Clean up the mgt_dispatcher function and remove unnecessary info from the
mlme_sta_tbl array.

Martin Kaiser (6):
  staging: r8188eu: check receiver address only once
  staging: r8188eu: replace the GetFrameSubType call
  staging: r8188eu: the frame type is shifted out
  staging: r8188eu: replace mlme_handler with function pointer
  staging: r8188eu: don't call empty DoReserved function
  staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

 drivers/staging/r8188eu/core/rtw_mlme_ext.c   | 60 ++++++++-----------
 .../staging/r8188eu/include/rtw_mlme_ext.h    |  6 +-
 2 files changed, 25 insertions(+), 41 deletions(-)

-- 
2.30.2

[PATCH 1/6] staging: r8188eu: check receiver address only once

[Martin Kaiser]

Check only once in mgt_dispatcher that the receiver address is the local
address or the broadcast address. The second identical check can be
removed.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index b6ee6a24930a..3afd06120cb1 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -431,13 +431,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 			ptable->func = &OnAuthClient;
 	}
 
-	if (ptable->func) {
-	/* receive the frames that ra(a1) is my address or ra(a1) is bc address. */
-		if (memcmp(hdr->addr1, myid(&padapter->eeprompriv), ETH_ALEN) &&
-		    !is_broadcast_ether_addr(hdr->addr1))
-			return;
+	if (ptable->func)
 		ptable->func(padapter, precv_frame);
-	}
 }
 
 static u32 p2p_listen_state_process(struct adapter *padapter, unsigned char *da)
-- 
2.30.2

[PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call

[Martin Kaiser]

The driver's local GetFrameSubType macro returns both frame type and
subtype.

Use the ieee80211 framework to extract the two fields. This shows more
clearly that both type and subtype are read.

Convert everything to host endianness before we use bit operations.

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 3afd06120cb1..be1afabe77d1 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -395,7 +395,6 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 	int index;
 	struct mlme_handler *ptable;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
-	u8 *pframe = precv_frame->rx_data;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)precv_frame->rx_data;
 	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, hdr->addr2);
 
@@ -409,8 +408,8 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 
 	ptable = mlme_sta_tbl;
 
-	index = GetFrameSubType(pframe) >> 4;
-
+	index = (le16_to_cpu(hdr->frame_control) &
+		 (IEEE80211_FCTL_STYPE | IEEE80211_FCTL_FTYPE)) >> 4;
 	if (index > 13)
 		return;
 	ptable += index;
-- 
2.30.2

[PATCH 3/6] staging: r8188eu: the frame type is shifted out

[Martin Kaiser]

The index calculation in mgt_dispatcher does not use the frame type. The
4-bit right shift ignores protocol version and frame type.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index be1afabe77d1..77a14f55b743 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -408,8 +408,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 
 	ptable = mlme_sta_tbl;
 
-	index = (le16_to_cpu(hdr->frame_control) &
-		 (IEEE80211_FCTL_STYPE | IEEE80211_FCTL_FTYPE)) >> 4;
+	index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
 	if (index > 13)
 		return;
 	ptable += index;
-- 
2.30.2

[PATCH 4/6] staging: r8188eu: replace mlme_handler with function pointer

[Martin Kaiser]

mlme_sta_tbl is an array of struct mlme_handler. mlme_handler's num and
str components are not used. The code in mgt_dispatcher uses the subtype
number of the incoming frame to select the array entry for compiling the
repsonse.

We can remove struct mlme_handler and make mlme_sta_tbl an array of
function pointers.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 77a14f55b743..492f481b61ec 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -14,25 +14,22 @@
 #include "../include/rtl8188e_xmit.h"
 #include "../include/rtl8188e_dm.h"
 
-static struct mlme_handler mlme_sta_tbl[] = {
-	{WIFI_ASSOCREQ,		"OnAssocReq",	&OnAssocReq},
-	{WIFI_ASSOCRSP,		"OnAssocRsp",	&OnAssocRsp},
-	{WIFI_REASSOCREQ,	"OnReAssocReq",	&OnAssocReq},
-	{WIFI_REASSOCRSP,	"OnReAssocRsp",	&OnAssocRsp},
-	{WIFI_PROBEREQ,		"OnProbeReq",	&OnProbeReq},
-	{WIFI_PROBERSP,		"OnProbeRsp",		&OnProbeRsp},
-
-	/*----------------------------------------------------------
-					below 2 are reserved
-	-----------------------------------------------------------*/
-	{0,					"DoReserved",		&DoReserved},
-	{0,					"DoReserved",		&DoReserved},
-	{WIFI_BEACON,		"OnBeacon",		&OnBeacon},
-	{WIFI_ATIM,			"OnATIM",		&OnAtim},
-	{WIFI_DISASSOC,		"OnDisassoc",		&OnDisassoc},
-	{WIFI_AUTH,			"OnAuth",		&OnAuthClient},
-	{WIFI_DEAUTH,		"OnDeAuth",		&OnDeAuth},
-	{WIFI_ACTION,		"OnAction",		&OnAction},
+/* response function for each management frame subtype, do not reorder */
+static mlme_handler mlme_sta_tbl[] = {
+	OnAssocReq,
+	OnAssocRsp,
+	OnAssocReq,
+	OnAssocRsp,
+	OnProbeReq,
+	OnProbeRsp,
+	DoReserved,
+	DoReserved,
+	OnBeacon,
+	OnAtim,
+	OnDisassoc,
+	OnAuthClient,
+	OnDeAuth,
+	OnAction,
 };
 
 static struct action_handler OnAction_tbl[] = {
@@ -393,7 +390,7 @@ void free_mlme_ext_priv(struct mlme_ext_priv *pmlmeext)
 void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 {
 	int index;
-	struct mlme_handler *ptable;
+	mlme_handler fct;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)precv_frame->rx_data;
 	struct sta_info *psta = rtw_get_stainfo(&padapter->stapriv, hdr->addr2);
@@ -406,12 +403,10 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 	    !is_broadcast_ether_addr(hdr->addr1))
 		return;
 
-	ptable = mlme_sta_tbl;
-
 	index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
 	if (index > 13)
 		return;
-	ptable += index;
+	fct = mlme_sta_tbl[index];
 
 	if (psta) {
 		if (ieee80211_has_retry(hdr->frame_control)) {
@@ -424,13 +419,13 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 
 	if (ieee80211_is_auth(hdr->frame_control)) {
 		if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
-			ptable->func = &OnAuth;
+			fct = OnAuth;
 		else
-			ptable->func = &OnAuthClient;
+			fct = OnAuthClient;
 	}
 
-	if (ptable->func)
-		ptable->func(padapter, precv_frame);
+	if (fct)
+		fct(padapter, precv_frame);
 }
 
 static u32 p2p_listen_state_process(struct adapter *padapter, unsigned char *da)
diff --git a/drivers/staging/r8188eu/include/rtw_mlme_ext.h b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
index 0875472dbb72..dcf91e7894a5 100644
--- a/drivers/staging/r8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/r8188eu/include/rtw_mlme_ext.h
@@ -184,11 +184,7 @@ enum SCAN_STATE {
 	SCAN_STATE_MAX,
 };
 
-struct mlme_handler {
-	unsigned int   num;
-	char *str;
-	unsigned int (*func)(struct adapter *adapt, struct recv_frame *frame);
-};
+typedef unsigned int (*mlme_handler)(struct adapter *adapt, struct recv_frame *frame);
 
 struct action_handler {
 	unsigned int   num;
-- 
2.30.2

[PATCH 5/6] staging: r8188eu: don't call empty DoReserved function

[Martin Kaiser]

Replace the DoReserved function pointer with NULL in mlme_sta_tbl. We can
skip the function call for reserved subtypes.

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

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 492f481b61ec..abb910f33c1c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -22,8 +22,8 @@ static mlme_handler mlme_sta_tbl[] = {
 	OnAssocRsp,
 	OnProbeReq,
 	OnProbeRsp,
-	DoReserved,
-	DoReserved,
+	NULL,
+	NULL,
 	OnBeacon,
 	OnAtim,
 	OnDisassoc,
-- 
2.30.2

[PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

[Martin Kaiser]

Use ARRAY_SIZE instead of hard-coding the number of entries in the
mlme_sta_tbl array.

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 abb910f33c1c..973adebdd69c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
 		return;
 
 	index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
-	if (index > 13)
+	if (index > ARRAY_SIZE(mlme_sta_tbl))
 		return;
 	fct = mlme_sta_tbl[index];
 
-- 
2.30.2

Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

[Dan Carpenter]

On Sun, Apr 17, 2022 at 12:22:21PM +0200, Martin Kaiser wrote:
> Use ARRAY_SIZE instead of hard-coding the number of entries in the
> mlme_sta_tbl array.
> 
> 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 abb910f33c1c..973adebdd69c 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
>  		return;
>  
>  	index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
> -	if (index > 13)
> +	if (index > ARRAY_SIZE(mlme_sta_tbl))
                  ^
This is an off by one.  Should be >=.  The auto builders would have
caught this eventually.

>  		return;
>  	fct = mlme_sta_tbl[index];

regards,
dan carpenter

Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl

[Martin Kaiser]

Thus wrote Dan Carpenter (dan.carpenter@oracle.com):

> On Sun, Apr 17, 2022 at 12:22:21PM +0200, Martin Kaiser wrote:
> > Use ARRAY_SIZE instead of hard-coding the number of entries in the
> > mlme_sta_tbl array.

> > 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 abb910f33c1c..973adebdd69c 100644
> > --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> > +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> > @@ -404,7 +404,7 @@ void mgt_dispatcher(struct adapter *padapter, struct recv_frame *precv_frame)
> >  		return;

> >  	index = (le16_to_cpu(hdr->frame_control) & IEEE80211_FCTL_STYPE) >> 4;
> > -	if (index > 13)
> > +	if (index > ARRAY_SIZE(mlme_sta_tbl))
>                   ^
> This is an off by one.  Should be >=.  The auto builders would have
> caught this eventually.

Thanks for spotting this, Dan. You're right, the array has 14 elements :-(

This has already made it into staging-next, I'll fix it in a new patch.

Best regards,
Martin

> >  		return;
> >  	fct = mlme_sta_tbl[index];

> regards,
> dan carpenter