[PATCH 2/2] mwifiex: use get_random_mask_addr() helper

[PATCH 2/2] mwifiex: use get_random_mask_addr() helper

[Ganapathi Bhat]

Avoid calculating random MAC address in driver. Instead make
use of 'get_random_mask_addr()' function.

Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
 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

Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> writes:

> Avoid calculating random MAC address in driver. Instead make
> use of 'get_random_mask_addr()' function.
>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

I don't see 1/2 anywhere. Did it get lost?

-- 
Kalle Valo

RE: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

[Ganapathi Bhat]

Hi Kalle,
> 
> > Avoid calculating random MAC address in driver. Instead make use of
> > 'get_random_mask_addr()' function.
> >
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> 
> 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

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> writes:

> Hi Kalle,
>> 
>> > Avoid calculating random MAC address in driver. Instead make use of
>> > 'get_random_mask_addr()' function.
>> >
>> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>> 
>> 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

[Brian Norris]

Hi,

On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote:
> Ganapathi Bhat <gbhat@marvell.com> writes:
> 
> > Hi Kalle,
> >> 
> >> > Avoid calculating random MAC address in driver. Instead make use of
> >> > 'get_random_mask_addr()' function.
> >> >
> >> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> >> 
> >> 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 <briannorris@chromium.org>

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

[Kalle Valo]

Brian Norris <briannorris@chromium.org> writes:

> Hi,
>
> On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote:
>> Ganapathi Bhat <gbhat@marvell.com> writes:
>> 
>> > Hi Kalle,
>> >> 
>> >> > Avoid calculating random MAC address in driver. Instead make use of
>> >> > 'get_random_mask_addr()' function.
>> >> >
>> >> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>> >> 
>> >> 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 <briannorris@chromium.org>
>
> 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: [2/2] mwifiex: use get_random_mask_addr() helper

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> wrote:

> Avoid calculating random MAC address in driver. Instead make
> use of 'get_random_mask_addr()' function.
> 
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

e9a3846afaa4 mwifiex: use get_random_mask_addr() helper

-- 
https://patchwork.kernel.org/patch/9955631/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches