On Wed, 2020-12-23 at 14:46 +0200, Jouni Malinen wrote:

On Fri, Nov 06, 2020 at 11:41:06AM +0100, Johannes Berg wrote:

quoted

I suspect that SET_POWERSAVE might be confusing.

Why? Isn't the use case here very similar to the existing station mode
use of power save even if the power saving mechanism is more of a vendor
specific extension that applies while there are no associated stations?

Yeah, true, fair point.

However, set-powersave is a bit of a legacy API with state in the
kernel, and sometimes restrictions on how/when you can set it etc. I'm
not sure it's a good idea for those reasons alone?

quoted

Perhaps just with an attribute used in START_AP (and CHANGE_BEACON if
needed) would be sufficient?

NL80211_CMD_START_AP with a new attribute (or even re-use of
NL80211_ATTR_PS_STATE) might work for a case where this does not need to
be changed dynamically during the lifetime of the BSS.
NL80211_CMD_SET_BEACON (which maps to the change_beacon() callback)
feels like something that is currently limited to Beacon data updates
with its use of struct cfg80211_beacon_data instead of struct
cfg80211_ap_settings..

That SET_BEACON name is still from the old NEW/SET/DEL_BEACON time.
Should that be renamed to NL80211_CMD_UPDATE_AP if we extend this to
changes that are not really targeting the Beacon frame payload itself?

I'd be surprised if we don't already have non-beacon state there ... but
it looks like only very little non-beacon state, namely the FTM
responder state.

Renaming seems reasonable, we've done it before with START_AP.

And should the cfg80211_beacon_data argument be replaced with
cfg80211_ap_settings? It looks like we already have some struct
cfg80211_ap_settings values like inactivity_timeout and beacon_rate (and
maybe some HE parameters?) that one might want to update during the
lifetime of the BSS..

That also seems reasonable.

johannes