On 3/31/2023 2:23 AM, Johannes Berg wrote:
On Thu, 2023-03-30 at 15:40 -0700, Aloka Dixit wrote:
On 3/30/2023 3:44 AM, Johannes Berg wrote:
On Tue, 2023-03-28 at 17:09 -0700, Aloka Dixit wrote:
+ switch (wdev->iftype) {
+ case NL80211_IFTYPE_AP:
+ width = wdev->u.ap.preset_chandef.width;
This seems a bit awkward and annoying, it means that we have to keep
using the API that sets the preset_chandef first, and it also means it
won't work for multi-link.
Okay, I can do it the same with it is done in he_get_txmcsmap():
chandef = wdev_chandef(wdev, link_id);
Right, that should be better.
Though not sure that works already when you're doing START_AP, I'd think
we don't set the chandef before parsing the rate attribute?
Which would mean userspace would have to separately set the beacon rate,
which means even having the whole thing inside start_ap() is pointless?
Couldn't you link it with the config in start_ap/join_mesh? And per-link
config in set_tx_bitrate_mask()?
Please correct me if I'm wrong but ieee80211_set_bitrate_mask() need not
be changed for setting the beacon rate in non-MLO case, right?
Because it does not have anything related to he_mcs currently too.
How is ieee80211_set_bitrate_mask() related, it's in mac80211?
Oh you mean it doesn't need to be changed to support EHT? Looks like to
me that it would have to be changed, and even HE today only works in
non-mac80211 drivers that implement it correctly?
But again set_bitrate_mask() isn't even related to beacon rate?
Regarding beacon rate, it looks like mac80211 _only_ accepts legacy
rates, and then also _only_ in start_ap(), so I'm not sure how the whole
thing even works ...??
I agree with all of your points and as said above, I mainly tested by
checking the final mask generated in nl80211.c.
Do you remember a similar discussion when HE code was added (I wasn't
the submitter). May be that will have some pointers regarding how it was
tested.
Do you think we can continue with this EHT support for non-mac80211
drivers similar to HE for now?
Or should we just skip this patch?
Thanks.
