On 4/28/2025 7:39 PM, Ben Greear wrote:
On 4/27/25 22:20, Sarika Sharma wrote:
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).
You can change allocation scheme in the future easily enough, and maybe
there will never exist any linux
driver with more than 3 links anyway? And you can allocate quite large
amounts of memory in the kernel,
so you could always have it a single struct if you wanted.
Anyway, Johannes makes the real decisions on this, so hopefully you can
get his approval before you
code something up he dislikes, I was just voicing my suggestion.
Thanks,
Ben
Sure, lets see what's Johannes opinion on this.
