* [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher
@ 2022-04-17 10:22 Martin Kaiser
2022-04-17 10:22 ` [PATCH 1/6] staging: r8188eu: check receiver address only once Martin Kaiser
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging,
linux-kernel, 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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/6] staging: r8188eu: check receiver address only once 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-17 10:22 ` [PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call Martin Kaiser ` (4 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser 2022-04-17 10:22 ` [PATCH 1/6] staging: r8188eu: check receiver address only once Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-17 10:22 ` [PATCH 3/6] staging: r8188eu: the frame type is shifted out Martin Kaiser ` (3 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/6] staging: r8188eu: the frame type is shifted out 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser 2022-04-17 10:22 ` [PATCH 1/6] staging: r8188eu: check receiver address only once Martin Kaiser 2022-04-17 10:22 ` [PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-17 10:22 ` [PATCH 4/6] staging: r8188eu: replace mlme_handler with function pointer Martin Kaiser ` (2 subsequent siblings) 5 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/6] staging: r8188eu: replace mlme_handler with function pointer 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser ` (2 preceding siblings ...) 2022-04-17 10:22 ` [PATCH 3/6] staging: r8188eu: the frame type is shifted out Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-17 10:22 ` [PATCH 5/6] staging: r8188eu: don't call empty DoReserved function Martin Kaiser 2022-04-17 10:22 ` [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl Martin Kaiser 5 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/6] staging: r8188eu: don't call empty DoReserved function 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser ` (3 preceding siblings ...) 2022-04-17 10:22 ` [PATCH 4/6] staging: r8188eu: replace mlme_handler with function pointer Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-17 10:22 ` [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl Martin Kaiser 5 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser ` (4 preceding siblings ...) 2022-04-17 10:22 ` [PATCH 5/6] staging: r8188eu: don't call empty DoReserved function Martin Kaiser @ 2022-04-17 10:22 ` Martin Kaiser 2022-04-22 10:08 ` Dan Carpenter 5 siblings, 1 reply; 9+ messages in thread From: Martin Kaiser @ 2022-04-17 10:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl 2022-04-17 10:22 ` [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl Martin Kaiser @ 2022-04-22 10:08 ` Dan Carpenter 2022-04-22 10:20 ` Martin Kaiser 0 siblings, 1 reply; 9+ messages in thread From: Dan Carpenter @ 2022-04-22 10:08 UTC (permalink / raw) To: Martin Kaiser Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl 2022-04-22 10:08 ` Dan Carpenter @ 2022-04-22 10:20 ` Martin Kaiser 0 siblings, 0 replies; 9+ messages in thread From: Martin Kaiser @ 2022-04-22 10:20 UTC (permalink / raw) To: Dan Carpenter Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Michael Straube, linux-staging, linux-kernel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-04-22 10:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-04-17 10:22 [PATCH 0/6] staging: r8188eu: clean up mgt_dispatcher Martin Kaiser 2022-04-17 10:22 ` [PATCH 1/6] staging: r8188eu: check receiver address only once Martin Kaiser 2022-04-17 10:22 ` [PATCH 2/6] staging: r8188eu: replace the GetFrameSubType call Martin Kaiser 2022-04-17 10:22 ` [PATCH 3/6] staging: r8188eu: the frame type is shifted out Martin Kaiser 2022-04-17 10:22 ` [PATCH 4/6] staging: r8188eu: replace mlme_handler with function pointer Martin Kaiser 2022-04-17 10:22 ` [PATCH 5/6] staging: r8188eu: don't call empty DoReserved function Martin Kaiser 2022-04-17 10:22 ` [PATCH 6/6] staging: r8188eu: use ARRAY_SIZE for mlme_sta_tbl Martin Kaiser 2022-04-22 10:08 ` Dan Carpenter 2022-04-22 10:20 ` Martin Kaiser
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox