On Thu, 2025-02-13 at 22:46 +0530, Sarika Sharma wrote:
> Currently, statistics is supported at deflink( or one of the links)
> level for station. This has problems when applied to multi-link(ML)
> connections.
>
> Hence, add changes to support link level statistics in sinfo structure.
> Additionally, remove mlo_params_valid from the sinfo structure and
> add valid_links to indicate bitmap of valid links for MLO.
>
> This will be helpful to check the link related statistics during MLO.
>
> The statistics could be embedded into NL message as below:
"could be"?
> For MLO:
> cmd ->
> NL80211_ATTR_IFINDEX
> NL80211_ATTR_MAC
> NL80211_ATTR_GENERATION
> .......etc
> NL80211_ATTR_STA_INFO | nest flag
> NL80211_STA_INFO_CONNECTED_TIME,
> NL80211_STA_INFO_STA_FLAGS,
> ........etc
> NL80211_ATTR_MLO_LINK_ID,
> NL80211_ATTR_MLD_ADDR,
> NL80211_ATTR_MLO_LINKS | nested
you're being inconsistent with "| nested" and "| nest flag" but I'm not
sure it's necessary anyway?
> link_id-1 | nested
> NL80211_ATTR_MLO_LINK_ID,
And that should be indented further, perhaps? Maybe not use tabs then
but just 4 spaces or so :)
>
> Station 00:03:7f:04:31:78 (on wlan0)
> authorized: yes
> authenticated: yes
> associated: yes
> preamble: long
> WMM/WME: yes
> MFP: yes
> TDLS peer: no
> connected time: 383 seconds
> associated at [boottime]: 93.740s
> associated at: 93685 ms
> current time: 340046 ms
> MLD address: 00:03:7f:04:31:78
the indentation seems odd, but maybe that's just a copy/paste thing?
> Link 0:
> Address: 00:03:7f:04:31:78
> inactive time: 330120 ms
> rx bytes: 116
> rx packets: 3
> tx bytes: 0
> tx packets: 0
> tx retries: 0
> tx failed: 0
> rx drop misc: 0
> signal: -95 dBm
> tx bitrate: 6.0 MBit/s
> tx duration: 2669 us
> rx duration: 0 us
> DTIM period: 2
> beacon interval:100
> Link 1:
> Address: 00:03:7f:04:31:79
> inactive time: 81268 ms
> rx bytes: 1323
> rx packets: 12
> tx bytes: 1538
> tx packets: 8
> tx retries: 0
> tx failed: 0
> rx drop misc: 0
> signal: -95 dBm
> tx bitrate: 6.0 MBit/s
> tx duration: 2669 us
> rx bitrate: 6.0 MBit/s
> rx duration: 0 us
> DTIM period: 2
> beacon interval:100
This looks like it's missing the roll-up to the global counters and
timestamps? Why would that not break backward compatibility?
> static inline void cfg80211_sinfo_release_content(struct station_info *sinfo)
> {
> - if (sinfo->links[0]) {
> - kfree(sinfo->links[0]->pertid);
> - kfree(sinfo->links[0]);
> + int link_id;
> +
> + if (sinfo->valid_links) {
> + for_each_valid_link(sinfo, link_id) {
> + if (sinfo->links[link_id]) {
> + kfree(sinfo->links[link_id]->pertid);
> + kfree(sinfo->links[link_id]);
> + }
> + }
> + } else {
> + if (sinfo->links[0]) {
> + kfree(sinfo->links[0]->pertid);
> + kfree(sinfo->links[0]);
> + }
> }
Don't be so complicated ... check what "for_each_valid_link()" does if
valid_links is 0.
johannes
