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. 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. johannes
