2022-07-24 15:41:59

by Martin Kaiser

[permalink] [raw]
Subject: [PATCH 1/5] staging: r8188eu: get da from ieee80211_mgmt

Define a struct ieee80211_mgmt in the OnAssocRsp function. Read the
destination address (da) from this struct.

Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 771910763fec..48ed329cec72 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -1268,6 +1268,7 @@ unsigned int OnAssocReq(struct adapter *padapter, struct recv_frame *precv_frame

unsigned int OnAssocRsp(struct adapter *padapter, struct recv_frame *precv_frame)
{
+ struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)precv_frame->rx_data;
uint i;
int res;
unsigned short status;
@@ -1279,7 +1280,7 @@ unsigned int OnAssocRsp(struct adapter *padapter, struct recv_frame *precv_frame
uint pkt_len = precv_frame->len;

/* check A1 matches or not */
- if (memcmp(myid(&padapter->eeprompriv), get_da(pframe), ETH_ALEN))
+ if (memcmp(myid(&padapter->eeprompriv), mgmt->da, ETH_ALEN))
return _SUCCESS;

if (!(pmlmeinfo->state & (WIFI_FW_AUTH_SUCCESS | WIFI_FW_ASSOC_STATE)))
--
2.30.2


2022-07-27 06:46:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: r8188eu: get da from ieee80211_mgmt

On Sun, Jul 24, 2022 at 05:39:13PM +0200, Martin Kaiser wrote:
> Define a struct ieee80211_mgmt in the OnAssocRsp function. Read the
> destination address (da) from this struct.

This explains (sortta) what this patch does, but not why you are doing
this.

To me this looks like a step backwards, why is this change needed at
all?

thanks,

greg k-h

2022-07-28 05:36:24

by Martin Kaiser

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: r8188eu: get da from ieee80211_mgmt

Thus wrote Greg Kroah-Hartman ([email protected]):

> On Sun, Jul 24, 2022 at 05:39:13PM +0200, Martin Kaiser wrote:
> > Define a struct ieee80211_mgmt in the OnAssocRsp function. Read the
> > destination address (da) from this struct.

> This explains (sortta) what this patch does, but not why you are doing
> this.

There's a bunch of macros and functions in the r8188eu driver to parse
standard messages. I'm trying to replace these driver-specific versions
with the generic ones that are shared between several drivers.

This patch removs one get_da call. It's not much but hopefully, we can
eventually remove get_da itself.

I know that we should leave the parsing of messages to mac80211 and get
rid of OnAssocRsp etc. Until we're at this point, I hope that doing the
parsing ourselves and using the generic helpers is a useful intermediate
step.

Would it make sense to rewrite the patch description or to summarize the
patches in this series to something like "read some response message
fields from struct ieee80211_mgmt"?

Thanks,
Martin

> To me this looks like a step backwards, why is this change needed at
> all?

> thanks,

> greg k-h

2022-07-28 07:04:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/5] staging: r8188eu: get da from ieee80211_mgmt

On Thu, Jul 28, 2022 at 07:25:15AM +0200, Martin Kaiser wrote:
> Thus wrote Greg Kroah-Hartman ([email protected]):
>
> > On Sun, Jul 24, 2022 at 05:39:13PM +0200, Martin Kaiser wrote:
> > > Define a struct ieee80211_mgmt in the OnAssocRsp function. Read the
> > > destination address (da) from this struct.
>
> > This explains (sortta) what this patch does, but not why you are doing
> > this.
>
> There's a bunch of macros and functions in the r8188eu driver to parse
> standard messages. I'm trying to replace these driver-specific versions
> with the generic ones that are shared between several drivers.
>
> This patch removs one get_da call. It's not much but hopefully, we can
> eventually remove get_da itself.
>
> I know that we should leave the parsing of messages to mac80211 and get
> rid of OnAssocRsp etc. Until we're at this point, I hope that doing the
> parsing ourselves and using the generic helpers is a useful intermediate
> step.
>
> Would it make sense to rewrite the patch description or to summarize the
> patches in this series to something like "read some response message
> fields from struct ieee80211_mgmt"?

As is, the patch description is not acceptable, so yes, it needs to
change :)

thanks,

greg k-h