On Thu, 2025-05-08 at 14:34 +0530, Sarika Sharma wrote: > > > > But to be honest I completely forgot all the history here. Looks like > > your initial patches couldn't even build, so I can't compare well with > > what it would've looked like before. > > Oops! if require can send new version V7, in building state(as already > prepared the patch as per last comments)? No no, I meant *initial* patches, I was just trying to look at v1/v2/... to compare how much driver changes they all needed, but in the earlier versions you just skipped the driver changes entirely, so I couldn't compare :) As far as I can tell this version builds fine, the robot seemed happy. > > > We can also prohibit filling entries that make no sense for multi-link > > statistics when provided on a global level, though honestly I feel like > > for most of them we should actually provide some value, even if tx/rx > > rate is just the best rate of all the links, or something like that? > > Yes, but then document for station_info structure needed to be updated here. > As for some of fields it will be similar for both non-Ml and MLO case. > But some of fields like signal,rates, etc, meaning will be different for > non-ML and MLO stats. Agree, and maybe some of the fields should just be prohibited for MLO (like signal/rate) until we figure out what semantics we want for them for MLO. But we can document and implement that, I'd think? > > This would also not yet require any drivers or mac80211 to change at > > all, since it's just adding a new capability. > > Oh! Do you mean not add all the things now at link level? and add > afterwards as per requirement/ while adding new capabilities? > > or add as of now and let driver/mac80211 filling those capabilities > handle it later part on. I just mean wrt. the patch sequence. As it is now you're making a lot of changes to drivers/mac80211 during the cfg80211 patches (specifically patch 4), but then in some sense you *undo* the need for those later (after patch 7 a lot of the driver changes aren't _really_ needed, particularly for non-MLO drivers). That seems harder than it should be. We later have a lot of the fields back that were modified earlier, so I'm suggesting to just not do it that way, but rather keep _all_ the fields, then you don't have any driver changes. Then add all the fields for each link, and the nl80211 code, aggregation code, etc. And _then_ you can start moving things over much more easily, like mac80211 would start providing per-link stats for all the fields, and accumulated stats about removed links in the original "non-MLO- statistics" fields (regardless of whether they're reported that way because the connection isn't MLO, or because the driver wasn't updated.) > > Oh, in terms of allocation - the struct isn't huge now, do we even > > care about simply adding 15 links? It still stays well below a > > page as far as I can tell ... keep it simpler for now? > > Okay! but although many fields are more applicable at link level. So > isn't keeping the pointer is better then keeping array[15](seeing error > with array [15], mentioned below)? Right ... hm. I actually thought this struct was already not on the stack, not sure why. That's why I was talking about the size of allocations etc. Alright, since we already have cfg80211_sinfo_release_content() it seems easier now to actually allocate the link stats dynamically, so instead of what I just said in my other email: struct link_station_info links[15]; we'd have struct link_station_info *links; and then just make sure we allocate a 15 links array, and free it in cfg80211_sinfo_release_content(). That actually it looks the same for the users, e.g. "sinfo.links[3]". johannes
