Re: [PATCH] nl80211: Update ERP info using NL80211_CMD_UPDATE_CONNECT_PARAMS

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

+ Jithu, Eylon

On 3/29/2018 1:16 PM, Johannes Berg wrote:
> Hi Arend,
>
>> Picking up a somewhat old thread as I did not see a follow-up on this
>> patch. I got queried about it over here by our FILS team. So what is
>> needed for this patch to pass the bar?
>
> That's indeed a bit old :-)
>
>>>> + * @UPDATE_FILS_ERP_INFO: Indicates that FILS connection parameters (realm,
>>>> + *	username, erp sequence number and rrk) are updated
>>>> + * @UPDATE_AUTH_TYPE: Indicates that Authentication type is updated
>>>
>>> These are new here, but you don't know if they were actually supported:
>>>
>>>> +	if (wiphy_ext_feature_isset(&rdev->wiphy,
>>>> +				    NL80211_EXT_FEATURE_FILS_SK_OFFLOAD) &&
>>>
>>> here.
>>
>> The description of the FILS_SK_OFFLOAD currently says:
>>
>>    * @NL80211_EXT_FEATURE_FILS_SK_OFFLOAD: Driver SME supports FILS
>> shared key
>>    *      authentication with %NL80211_CMD_CONNECT.
>>
>> Are you suggesting a new flag to cover the new update attributes?
>
> [snip]
>
>>> Again, how do you know the driver will actually look at
>>> UPDATE_AUTH_TYPE?
>>
>> If they don't they are broken, right? And if they are broken, the
>> connection will drop and regular connect will happen anyway, no?
>>
>> We could add a new flag to signal driver will handle the extra
>> parameters in UPDATE_CONNECT_PARAMS, but it is not clear why it would be
>> needed. Seems to me user-space has all the info needed with the existing
>> flag(s).
>
> Agree, and we don't even have any drivers that are setting the
> FILS_SK_OFFLOAD flag anyway, so we can still redefine its semantics to
> some extent.

There is some implied behavior about the UPDATE_AUTH_TYPE. The 
FILS_SK_OFFLOAD only seems to cover NL80211_AUTHTYPE_FILS_SK. So to me 
it seems that changing the auth type really means the driver should give 
up on roaming and let user-space handle it.

> So yeah, I'd argue that what the patch needed was somebody taking a
> critical look at my review ;-)
>
> And perhaps fixing the weird flags thing I pointed out.

Yup. That made sense.

Also there is a DOC section about FILS shared key authentication 
offload" so I suppose that should be extended as well.

Regards,
Arend