On Thu, 2023-01-19 at 12:43 -0800, Aloka Dixit wrote: > > > > > Hence the > > > current design, at least ath11k, assumes that BSS_CHANGED_FILS_DISCOVERY > > > and BSS_CHANGED_UNSOL_BCAST_PROBE_RESP "not being set", when beacon is > > > changed, means disable these features. > > > What do you think? > > > > I think that makes no sense? If mac80211 didn't clear struct > > ieee80211_bss_conf::fils_discovery, then surely it should stick around > > even if the beacon changes??? > > > "max_interval" was be used as the enable/disable knob for these > features. Non-zero = enable, zero = disable. > But the side effect of that is if NL80211 does not receive these > attributes then by default the interval is set to 0. But it doesn't change in bss_conf if you send change beacon, at least not before these patches? Or am I confusing myself? > I can think of following: > (1) max_interval cannot be the enable/disable knob because then every > code path in the userspace would still need to set it to non-zero to > continue transmission. Are you okay with having extra members in enum > nl80211_fils_discovery_attributes to ENABLE/DISABLE? I think that is > what you suggested in your comment for the next patch in this series as > well. > > (2) If the template needs changing for any reason then the userspace > will be responsible to send a new one. Until then the driver will > continue the transmission with existing template and interval unless > DISABLE is used. > Were those meant to be mutually exclusive, because it doesn't seem like that to me? I think (2) must be what happens now, before these patches, because I don't see where it would be changed? Like I said above. I agree that we'd now need an explicit way to indicate "disable", but we could for example say you disable by adding a nested NL80211_ATTR_FILS_DISCOVERY attribute without any of the sub-attributes, which logically kind of makes sense - you're changing NL80211_ATTR_FILS_DISCOVERY, but you're not including a new set of parameters, so logically that means disable? johannes
