Re: [PATCH 2/2] ath10k: support MAC address randomization in scan

[Arend van Spriel <arend.vanspriel@broadcom.com>]

+ Johannes (to confirm/correct my understanding regarding "supported 
commands")

On 4/18/2018 4:35 AM, Dan Williams wrote:
> On Tue, 2018-04-17 at 15:26 -0700, Brian Norris wrote:
>> On Tue, Apr 17, 2018 at 2:49 PM, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 4/17/2018 6:07 PM, Brian Norris wrote:
>>>> On Tue, Apr 17, 2018 at 10:22:13AM +0200, Arend van Spriel wrote:
>>>>> 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.
>>
>> I see what you're saying (though your comparison doesn't seem that
>> fair either; private ioctls are nothing like a well-defined nl80211
>> support list), and I'm totally good on looking at the new flag
>> eventually. But you still haven't answered my question ("why not?").
>> Is there a problem with the "supported commands" list?

My understanding is that in general the "supported commands" list is not 
well-maintained. Not every nl80211 command is represented in the list so 
user-space can not know whether it is missing or not supported.
For this particular START_SCHED_SCAN command it can be used still and 
indeed probably for a long time. I just wanted to point out that it is 
not recommended for new user-space functionality.

>>> The START_SCHED_SCAN command is indeed still provided to user-
>>> space:
>>
>> And as I see it, it probably needs to be for essentially forever. Or
>> at least a significant amount of time after wpa_supplicant stops
>> relying on it. (Hint: it's still using it today, with no reference to
>> NL80211_ATTR_SCHED_SCAN_MAX_REQS.) There's a reason the kernel has
>> ABI
>> guarantees. I suspect you only get a chance to rewrite the world
>> (WEXT
>> -> nl80211) a few times in the life of kernel ABIs.
>
> It sometimes feels like wpa_supplicant gets treated as a static entity
> that can never be changed.  In fact, send a patch to Jouni implementing
> the best practice, with a fallback to preserve compat for old kernels,
> and I'm sure he'd entertain it.  Just because the supplicant does
> something a certain way, doesn't mean it's the *best* way, but it too
> evolves.

I was actually considering to do that. The netlink messages are easily 
checked for presence of an attribute so deciding on whether to use the 
fallback is trivial.

Regards,
Arend