Re: [PATCH v2 1/2] wifi: cfg80211/mac80211: Add support to rx retry stats

Hari Chandrakanthan
2024-03-27T10:09:45Z

On 3/25/2024 9:13 PM, Johannes Berg wrote:
> On Tue, 2024-03-19 at 19:15 +0530, Hari Chandrakanthan wrote:
>> Add support to count station level rx retries.
> Should the subject say "for ... stats"?
>
>> +++ b/net/mac80211/sta_info.c
>> @@ -2653,6 +2653,11 @@ void sta_set_sinfo(struct sta_info *sta, struct station_info *sinfo,
>> sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_RETRIES);
>> }
>>
>> + if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_RETRIES))) {
>> + sinfo->rx_retries = sta->deflink.rx_stats.rx_retries;
>> + sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_RETRIES);
>> + }
> The use of deflink here seems ... questionable?
>
> I know we've not really done any stats properly here for link STA
> (patches welcome), but I guess this could be a first one that at least
> sums up all the links like all of these should, and then find a way to
> expose per-link as well?
>
> Although possibly we should just expose per-link to cfg80211, and then
> have cfg80211 sum up for the MLD representation...
>
>
> Either way, seems odd to add something now that absolutely cannot work
> for MLO?
>
ok.

Fields such packet count, retries etc can be summed up for the MLD
representation and the existing NL
attribute can be used for exposing the summed up value.

But there are fields such as signal avg, bitrate etc which cannot be
summed up.
Should we expose such fields of each link STA through NL?

Sample station dump log for reference:
inactive time:  271110 ms
rx bytes:       1129
rx packets:     13
tx bytes:       219
tx packets:     3
tx retries:     38
tx failed:      0
rx drop misc:   0
signal:         -16 dBm
signal avg:     -20 dBm
tx bitrate:     6.0 MBit/s
tx duration:    452 us
rx bitrate:     260.0 MBit/s VHT-MCS 6 short GI VHT-NSS 4
rx duration:    792 us
last ack signal:0 dBm
avg ack signal: 0 dBm
authorized:     yes
authenticated:  yes
associated:     yes
preamble:       long
WMM/WME:        yes
MFP:            no
TDLS peer:      no
DTIM period:    2
beacon interval:100
short slot time:yes
connected time: 869 seconds
associated at [boottime]:       1040062.600s
associated at:  1695979678173 ms
current time:   1695980547138 ms

> johannes