Search Linux Wireless

Re: [PATCH RFC 2/7] wifi: cfg80211: reorg sinfo structure elements for MLO

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/14/2025 4:49 PM, Johannes Berg wrote:
On Sun, 2025-01-12 at 13:40 +0530, Sarika Sharma wrote:
On 1/10/2025 2:49 PM, Johannes Berg wrote:
On Fri, 2025-01-10 at 09:54 +0530, Sarika Sharma wrote:

   struct station_info {

+	struct link_station_info deflink;

Having a deflink here seems kind of questionable?

ohh! why so? In other structures like vif, sdata, sta, we did the same
way. So why can't we have the same way here as well?

Well, (a) I tend to think in hindsight it was sort of a mistake, but (b)
we couldn't really do it differently due to all drivers. But here you're
already touching all the code anyway, and it's mostly cfg80211/mac80211,
so why bother?

Sure, I will remove deflink and instead could use link pointer (link[0]) to fill statistics for non-ML stations.


Why not pass multiple pointers?

Sorry, I didn't get you.
Did you mean keep the sinfo structure and call this for filling all link
level, non-mL, ML information?

I think I meant that we could have per link pointers passed to the
method, but we might as well just have them in the sinfo struct as you
introduce in later patches.

Sure, will introduce the link pointer here and use link[0] for non-ML stations and corresponding link pointer for ML stations.

For drivers that offload link decisions and do not provide per-link statistics, a flag could be used to indicate whether the per-link statistics is filled from driver or not, if not can use link[0] to fill the station information and if yes then can use the corresponding link pointers to fill link level information.


-	if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&
+	if (!(link_sinfo->filled & BIT_ULL(NL80211_STA_INFO_TX_BITRATE)) &&


A little less renaming would probably also make this easier to review.

Sure, but this is corresponding to filling link level data, that's why
just renaming sinfo to link_sinfo.

Yeah, but does it matter so much? :)

Not really, this is just done to differentiate the structures used(station_info or link_station_info) and code readability purpose.


I guess the flipside is that this way it's easier to review that all
values fall into the right place, but I think the data structure
organisation you did mostly ensures that anyway? Not sure though.

Yes from data structure organization it is taken care that it falls at right place.
If it found to be difficult to review and required, I can rename to sinfo?


johannes





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux