On 4/25/2025 6:44 PM, Ben Greear wrote:
On 4/24/25 22:33, Sarika Sharma wrote:
On 4/24/2025 10:32 PM, Ben Greear wrote:
On 4/24/25 09:44, Sarika Sharma wrote:
On 4/23/2025 10:22 PM, Johannes Berg wrote:
On Tue, 2025-04-15 at 09:50 +0530, Sarika Sharma wrote:
Current implementation of NL80211_GET_STATION does not work for
multi-link operation(MLO) since in case of MLO only deflink (or one
of the links) is considered and not all links.
Therefore to support for MLO, start reorganizing sinfo structure
related data elements and add link_sinfo structure for link-level
statistics and keep station related data at sinfo structure.
Currently, changes are done at the deflink(or one of the links)
level.
Actual link-level changes will be added in subsequent changes.
Also currently, mac80211 ops .sta_statistics() is mapped to fill
sinfo
structure. But to add support for station statistics at link level,
change the ops to .link_sta_statistics() to fill link_sinfo
structure.
Additionally, move connected_time before assoc_at in station_info
structure to get minimal holes.
pahole summary before this change:
- size: 232, cachelines: 4, members: 23
- sum members: 223, holes: 3, sum holes: 9
- forced alignments: 1
- last cacheline: 40 bytes
pahole summary after this change:
- size: 224, cachelines: 4, members: 23
- sum members: 223, holes: 1, sum holes: 1
- forced alignments: 1
- last cacheline: 32 bytes
Signed-off-by: Sarika Sharma <quic_sarishar@xxxxxxxxxxx>
---
NOTE:
- Included driver changes for fixing compilation issue.
Does this really need to do all the changes in mac80211 and the
drivers?
OTOH maybe if not then it would cause much more back and forth?
Yes, true this patch includes only the minimum necessary changes to
resolve the compilation issues in mac80211 and the drivers.
Without these changes, compilation issues will persist.
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1810,47 +1810,51 @@ static int ath6kl_get_station(struct wiphy
*wiphy, struct net_device *dev,
else if (left < 0)
return left;
+ sinfo->links[0] = kzalloc(sizeof(*sinfo->links[0]), GFP_KERNEL);
+ if (!sinfo->links[0])
+ return -ENOMEM;
This seems rather error-prone to me.
We already have sinfo->pertid today, allocated and freed by cfg80211,
and here you've added something that's allocated by the driver and
freed
by mac80211. That seems odd in comparison?
I'm not sure what the choices are, but I can't say I like this one ;-)
Maybe it's still the least bad option.
Options what I can think of here, other then above approach, may be
can allocate memory during get_station() call only, in cfg80211(but
this may not be memory efficient as have to allocate for all
possible links).
or, may be can introduce an API in cfg80211 to allocate the memory
for sinfo->links, and call the API from drivers/mac80211 while
filling the sinfo->links[] data.
sinfo->links[] could be an array in sinfo instead of a pointer, so
whatever allocates sinfo
automatically allocates the links memory area, and then just fill in
those values as needed
in the driver and ignore them in mac80211 if not filled?
sinfo->links[] cannot be used as an array because taking an array of
IEEE80211_MLD_MAX_NUM_LINKS (15) would make the station_info structure
too large, exceeding the maximum allowed size.
If you mean max allowed size on the stack, then you could alloc it from
the heap
and free it when done.
Or you could just alloc storage for 3 links for now since no radio has
more than that currently.
I agree, currently it's 3 only, but everywhere we are using max-15, so
explicitly keeping it 3 here isn't questionable?
Also, in future it will not be useful approach, when we need to increase
the links(that time we have to change this to pointer instead of array).
Thanks,
Ben
Instead, we can use the earlier method (define
deflink(link_station_info) for non-MLO and use the same), but Johannes
did not favor that approach, so I avoided providing this as an
additional option.
Thanks,
Ben
