All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arend van Spriel <arend.vanspriel@broadcom.com>
To: Brian Norris <briannorris@chromium.org>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	cjhuang@codeaurora.org, linux-wireless@vger.kernel.org,
	ath10k@lists.infradead.org
Subject: Re: [PATCH 2/2] ath10k: support MAC address randomization in scan
Date: Tue, 17 Apr 2018 23:49:13 +0200	[thread overview]
Message-ID: <5AD66BD9.4000706@broadcom.com> (raw)
In-Reply-To: <20180417160715.GA255263@rodete-desktop-imager.corp.google.com>

On 4/17/2018 6:07 PM, Brian Norris wrote:
> On Tue, Apr 17, 2018 at 10:22:13AM +0200, Arend van Spriel wrote:
>> On 4/17/2018 2:28 AM, Brian Norris wrote:
>>> It looks like the status quo for looking for SCHED_SCAN support is to
>>> check if NL80211_CMD_START_SCHED_SCAN shows up in the command support
>>> list. (IOW, that's what wpa_supplicant does.) We'll probably need to
>>> imitate that.
>>
>> I believe checking command support is not really recommended. Instead, you
>> better check NL80211_ATTR_SCHED_SCAN_MAX_REQS being non-zero (since kernel
>> 4.12 that is).
>
> Why not? Command support checking is what wpa_supplicant is doing.

That's not really a good argument. A couple (or more) years ago 
wpa_supplicant was not doing nl80211 but wext and some other using 
driver private ioctls, but that did not make it the best approach.

The START_SCHED_SCAN command is indeed still provided to user-space:

@@ -1376,7 +1377,7 @@ static int nl80211_add_commands_unsplit(struct 
cfg80211_r
                 CMD(tdls_mgmt, TDLS_MGMT);
                 CMD(tdls_oper, TDLS_OPER);
         }
-       if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
+       if (rdev->wiphy.max_sched_scan_reqs)
                 CMD(sched_scan_start, START_SCHED_SCAN);
         CMD(probe_client, PROBE_CLIENT);
         CMD(set_noack_map, SET_NOACK_MAP);

It was left in because existing user-space apps might depend on it.

> I noticed NL80211_ATTR_SCHED_SCAN_MAX_REQS, but unfortunately, we have
> to support older kernels. It looks like randomization was added in
> v3.19, and as you point out, that's only available in v4.12.

I figured that would be the issue so I dug up the kernel version.

> I welcome other alternatives if you have them to offer.

Nope. Not if you want to be kernel version agnostic.

Regards,
Arend

WARNING: multiple messages have this Message-ID (diff)
From: Arend van Spriel <arend.vanspriel@broadcom.com>
To: Brian Norris <briannorris@chromium.org>
Cc: cjhuang@codeaurora.org, linux-wireless@vger.kernel.org,
	ath10k@lists.infradead.org, Kalle Valo <kvalo@codeaurora.org>
Subject: Re: [PATCH 2/2] ath10k: support MAC address randomization in scan
Date: Tue, 17 Apr 2018 23:49:13 +0200	[thread overview]
Message-ID: <5AD66BD9.4000706@broadcom.com> (raw)
In-Reply-To: <20180417160715.GA255263@rodete-desktop-imager.corp.google.com>

On 4/17/2018 6:07 PM, Brian Norris wrote:
> On Tue, Apr 17, 2018 at 10:22:13AM +0200, Arend van Spriel wrote:
>> On 4/17/2018 2:28 AM, Brian Norris wrote:
>>> It looks like the status quo for looking for SCHED_SCAN support is to
>>> check if NL80211_CMD_START_SCHED_SCAN shows up in the command support
>>> list. (IOW, that's what wpa_supplicant does.) We'll probably need to
>>> imitate that.
>>
>> I believe checking command support is not really recommended. Instead, you
>> better check NL80211_ATTR_SCHED_SCAN_MAX_REQS being non-zero (since kernel
>> 4.12 that is).
>
> Why not? Command support checking is what wpa_supplicant is doing.

That's not really a good argument. A couple (or more) years ago 
wpa_supplicant was not doing nl80211 but wext and some other using 
driver private ioctls, but that did not make it the best approach.

The START_SCHED_SCAN command is indeed still provided to user-space:

@@ -1376,7 +1377,7 @@ static int nl80211_add_commands_unsplit(struct 
cfg80211_r
                 CMD(tdls_mgmt, TDLS_MGMT);
                 CMD(tdls_oper, TDLS_OPER);
         }
-       if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_SCHED_SCAN)
+       if (rdev->wiphy.max_sched_scan_reqs)
                 CMD(sched_scan_start, START_SCHED_SCAN);
         CMD(probe_client, PROBE_CLIENT);
         CMD(set_noack_map, SET_NOACK_MAP);

It was left in because existing user-space apps might depend on it.

> I noticed NL80211_ATTR_SCHED_SCAN_MAX_REQS, but unfortunately, we have
> to support older kernels. It looks like randomization was added in
> v3.19, and as you point out, that's only available in v4.12.

I figured that would be the issue so I dug up the kernel version.

> I welcome other alternatives if you have them to offer.

Nope. Not if you want to be kernel version agnostic.

Regards,
Arend

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

  reply	other threads:[~2018-04-17 21:49 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-30  3:13 [PATCH 0/2] ath10k: support MAC address randomization in scan Carl Huang
2018-03-30  3:13 ` Carl Huang
2018-03-30  3:13 ` [PATCH 1/2] ath10k: Add WMI_SERVICE_AVAILABLE_EVENT support Carl Huang
2018-03-30  3:13   ` Carl Huang
2018-04-16 13:40   ` [1/2] " Kalle Valo
2018-04-16 13:40   ` Kalle Valo
2018-04-24  5:46   ` Kalle Valo
2018-04-24  5:46   ` Kalle Valo
2018-03-30  3:14 ` [PATCH 2/2] ath10k: support MAC address randomization in scan Carl Huang
2018-03-30  3:14   ` Carl Huang
2018-04-12 20:59   ` Brian Norris
2018-04-12 20:59     ` Brian Norris
2018-04-13  6:53     ` cjhuang
2018-04-13  6:53       ` cjhuang
2018-04-13 11:28       ` Kalle Valo
2018-04-13 11:28         ` Kalle Valo
2018-04-13 21:13         ` Arend van Spriel
2018-04-13 21:13           ` Arend van Spriel
2018-04-16  5:16           ` cjhuang
2018-04-16  5:16             ` cjhuang
2018-04-16 11:32             ` Kalle Valo
2018-04-16 11:32               ` Kalle Valo
2018-04-17  0:28               ` Brian Norris
2018-04-17  0:28                 ` Brian Norris
2018-04-17  8:22                 ` Arend van Spriel
2018-04-17  8:22                   ` Arend van Spriel
2018-04-17 16:07                   ` Brian Norris
2018-04-17 16:07                     ` Brian Norris
2018-04-17 21:49                     ` Arend van Spriel [this message]
2018-04-17 21:49                       ` Arend van Spriel
2018-04-17 22:26                       ` Brian Norris
2018-04-17 22:26                         ` Brian Norris
2018-04-18  2:35                         ` Dan Williams
2018-04-18  2:35                           ` Dan Williams
2018-04-18  8:29                           ` Arend van Spriel
2018-04-18  8:29                             ` Arend van Spriel
2018-04-18  9:58                             ` Johannes Berg
2018-04-18  9:58                               ` Johannes Berg
2018-04-16  5:17         ` cjhuang
2018-04-16  5:17           ` cjhuang
2018-04-16 13:42   ` [2/2] " Kalle Valo
2018-04-16 13:42   ` Kalle Valo
     [not found]   ` <20180416134238.B16596076A@smtp.codeaurora.org>
2018-04-17  7:29     ` cjhuang
2018-04-17  7:29       ` cjhuang
2018-04-20 10:30       ` Kalle Valo
2018-04-20 10:30         ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5AD66BD9.4000706@broadcom.com \
    --to=arend.vanspriel@broadcom.com \
    --cc=ath10k@lists.infradead.org \
    --cc=briannorris@chromium.org \
    --cc=cjhuang@codeaurora.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.