Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Brian Norris writes: > Hi, > > On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote: >> Ganapathi Bhat writes: >> >> > Hi Kalle, >> >> >> >> > Avoid calculating random MAC address in driver. Instead make use of >> >> > 'get_random_mask_addr()' function. >> >> > >> >> > Signed-off-by: Ganapathi Bhat >> >> >> >> I don't see 1/2 anywhere. Did it get lost? >> > >> > Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C > > It's dependent on this patch though, which kinda should be '1/2': > > [PATCH] mwifiex: avoid storing random_mac in private Thanks for pointing out, I'll make sure that I commit these in correct order. >> > (to correct a typo); and then tried sending it again. I think that >> > created some problem here. Kindly let me know how to proceed. >> >> Ok. I'll wait for review comments and if all goes well I'll apply it in >> few days. > > FWIW, this looks OK to me: > > Reviewed-by: Brian Norris > > It's just a bit strange that we have to keep our own on-stack temporary > buffer for this. Maybe this could use an in-place helper too? Or (if > it's really legal for us to modify the cfg80211_scan_request in-place) > why doesn't the upper-layer nl80211 code do the randomization for us? > Many (all?) drivers I see implementing randomization have to do this > anyway; they don't use request->mac_addr directly. (Or I suppose some > firmware could implement the randomization on its own someday...but > would we really trust it?) Good questions and I was wondering the same when looking at this patch. But I wasn't involved in the interface design so I don't really know the background here. I'm planning to apply this patch anyway, any improvements can be done as a followup patch. -- Kalle Valo
Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Hi, On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote: > Ganapathi Bhat writes: > > > Hi Kalle, > >> > >> > Avoid calculating random MAC address in driver. Instead make use of > >> > 'get_random_mask_addr()' function. > >> > > >> > Signed-off-by: Ganapathi Bhat > >> > >> I don't see 1/2 anywhere. Did it get lost? > > > > Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C It's dependent on this patch though, which kinda should be '1/2': [PATCH] mwifiex: avoid storing random_mac in private > > (to correct a typo); and then tried sending it again. I think that > > created some problem here. Kindly let me know how to proceed. > > Ok. I'll wait for review comments and if all goes well I'll apply it in > few days. FWIW, this looks OK to me: Reviewed-by: Brian Norris It's just a bit strange that we have to keep our own on-stack temporary buffer for this. Maybe this could use an in-place helper too? Or (if it's really legal for us to modify the cfg80211_scan_request in-place) why doesn't the upper-layer nl80211 code do the randomization for us? Many (all?) drivers I see implementing randomization have to do this anyway; they don't use request->mac_addr directly. (Or I suppose some firmware could implement the randomization on its own someday...but would we really trust it?) Brian
Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Ganapathi Bhat writes: > Hi Kalle, >> >> > Avoid calculating random MAC address in driver. Instead make use of >> > 'get_random_mask_addr()' function. >> > >> > Signed-off-by: Ganapathi Bhat >> >> I don't see 1/2 anywhere. Did it get lost? > > Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C > (to correct a typo); and then tried sending it again. I think that > created some problem here. Kindly let me know how to proceed. Ok. I'll wait for review comments and if all goes well I'll apply it in few days. -- Kalle Valo
RE: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Hi Kalle, > > > Avoid calculating random MAC address in driver. Instead make use of > > 'get_random_mask_addr()' function. > > > > Signed-off-by: Ganapathi Bhat > > I don't see 1/2 anywhere. Did it get lost? Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C (to correct a typo); and then tried sending it again. I think that created some problem here. Kindly let me know how to proceed. Thanks, Ganapathi > > -- > Kalle Valo
Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Ganapathi Bhat writes: > Avoid calculating random MAC address in driver. Instead make > use of 'get_random_mask_addr()' function. > > Signed-off-by: Ganapathi Bhat I don't see 1/2 anywhere. Did it get lost? -- Kalle Valo
[PATCH 2/2] mwifiex: use get_random_mask_addr() helper
Avoid calculating random MAC address in driver. Instead make
use of 'get_random_mask_addr()' function.
Signed-off-by: Ganapathi Bhat
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 84c1518..2b293b1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2503,6 +2503,7 @@ static int mwifiex_set_ibss_params(struct mwifiex_private
*priv,
struct ieee80211_channel *chan;
struct ieee_types_header *ie;
struct mwifiex_user_scan_cfg *user_scan_cfg;
+ u8 mac_addr[ETH_ALEN];
mwifiex_dbg(priv->adapter, CMD,
"info: received scan request on %s\n", dev->name);
@@ -2529,12 +2530,10 @@ static int mwifiex_set_ibss_params(struct
mwifiex_private *priv,
priv->scan_request = request;
if (request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) {
- for (i = 0; i < ETH_ALEN; i++) {
- request->mac_addr[i] &= request->mac_addr_mask[i];
- request->mac_addr[i] |=
- get_random_int() & ~(request->mac_addr_mask[i]);
- }
- ether_addr_copy(user_scan_cfg->random_mac, request->mac_addr);
+ get_random_mask_addr(mac_addr, request->mac_addr,
+request->mac_addr_mask);
+ ether_addr_copy(request->mac_addr, mac_addr);
+ ether_addr_copy(user_scan_cfg->random_mac, mac_addr);
}
user_scan_cfg->num_ssids = request->n_ssids;
--
1.9.1
