+ 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