On 1/19/2023 12:12 PM, Johannes Berg wrote:
On Thu, 2023-01-19 at 11:40 -0800, Aloka Dixit wrote:
On 1/18/2023 7:57 AM, Johannes Berg wrote:
On Wed, 2022-11-09 at 13:47 -0800, Aloka Dixit wrote:
FILS discovery and unsolicited broadcast probe response transmissions
are configured as part of NL80211_CMD_START_AP, however both stop
after userspace uses the NL80211_CMD_SET_BEACON command as these
attributes are not processed in that command.
That seems odd. Why would that *stop*? Nothing in the driver should
actually update it to _remove_ it during change_beacon()?
Are you sure you didn't mean to "just" say "however both cannot be
updated as these attributes ..."?
johannes
FILS discovery has primary channel, center frequency in the frame format
which should be changed depending on why the beacon changed.
Hmm? Sure, the frequency is important, but beacon can change for so many
other reasons? Even just the greenfield bit in HT would cause a beacon
change as far as cfg80211/mac80211 are concerned.
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.
Surely you can't be right - that'd basically mean the whole feature is
useless right now because even the greenfield update or similar that
basically *always* happens in hostapd would disable it, since the beacon
changes and we don't have these patches?
Pretty much, yeah.
Should I send a follow-up with a different commit log?
Well seems like we need to first figure out the correct semantics here,
and possibly fix things...
johannes
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.
Thanks.
