On 2021-02-12 01:33, Johannes Berg wrote:
On Mon, 2021-01-25 at 13:52 -0800, Aloka Dixit wrote:
FILS discovery and especially unsolicited probe response templates are
big. Sometimes send_and_recv() returns error due to memory
unavailability during wpa_driver_nl80211_set_ap() depending on how
many
interfaces, which elements are added.
What? Where do you get errors from? Netlink even supports vmalloc now,
I
believe, so the kernel really shouldn't care?
The error was shown on hostapd side during attribute building when
16 interfaces were added on one radio and unsolicited broadcast
probe response feature was enabled.
It was resolved with a separate command for this feature instead of
being a part of NL80211_CMD_START_AP.
Moving these to separate commands
resolves this issue along with more control over the time interval
during run-time.
I tend to agree with Arend though, we have NL80211_CMD_SET_BEACON and
since NL80211_CMD_NEW_BEACON was renamed to NL80211_CMD_START_AP to
more
accurately say what it does, I think generalizing "AP modifications" by
renaming NL80211_CMD_SET_BEACON to NL80211_CMD_UPDATE_AP (or such)
would
make a lot of sense.
Let's not conflate the two issues here.
1) memory issues - need to understand better
2) update is needed - I'd say SET_BEACON/UPDATE_AP would be a better
way
than pulling everything into separate commands. Updates can be
partial too, after all, if you include only the changed attributes,
and that might even address case 1? I.e. why wouldn't userspace be
able to do UPDATE_AP multiple times, just like with your patch it
would do NL80211_CMD_SET_FILS_DISCOVERY
and NL80211_CMD_SET_UNSOL_BCAST_PROBE_RESP?
Also, technically this constitutes an API break. One that perhaps
nobody
cares about yet, but surely somebody already has hostapd versions that
use NL80211_ATTR_FILS_DISCOVERY or NL80211_ATTR_UNSOL_BCAST_PROBE_RESP?
After all, you don't want to tell me you never tested this code ;-)
johannes
I myself added the hostapd changes corresponding to the original kernel
code,
This patch-set doesn't remove these attributes so, if accepted,
current hostapd calls will become no-op but won't break any other
functionality until I send the next version for separate commands for
hostapd.
On a different note, a line was missed in nl80211_fils_discovery_policy
definition from my side in the original change so included it in this
patch-set.
Should I send a separate patch for that while we discuss regarding the
separate
command?
Thanks.
