| Message ID | 20211029092539.2851b4799386.If9736d4575ee79420cbec1bd930181e1d53c7317@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | nl80211: fix radio statistics in survey dump | expand |
On Friday, 29 October 2021 09:25:39 CEST Johannes Berg wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Even if userspace specifies the NL80211_ATTR_SURVEY_RADIO_STATS > attribute, we cannot get the statistics because we're not really > parsing the incoming attributes properly any more. > > Fix this by passing the attrbuf to nl80211_prepare_wdev_dump() > and filling it there, if given, and using a local version only > if no output is desired. > > Since I'm touching it anyway, make nl80211_prepare_wdev_dump() > static. Tested-by: Sven Eckelmann <sven@narfation.org> Thanks for checking the problem. I've tested it together with https://patchwork.kernel.org/project/linux-wireless/patch/20211029072939.15767-1-johannes@sipsolutions.net/ on an AX200 and firmware version 59.601f3a66.0 cc-a0-59.ucode. $ sudo ./iw dev wlan0 survey dump --radio Survey data from wlan0 channel active time: 17205356 ms channel receive time: 653 ms channel transmit time: 74 ms But you might have noticed that the channel active time is off by a lot. Just after a couple of seconds: sudo ./iw dev wlan0 survey dump --radio Survey data from wlan0 channel active time: 38689859 ms channel receive time: 904 ms channel transmit time: 128 ms Here are the times from a loop with a delay of roughly one second: channel active time: 47284414 ms channel active time: 47284455 ms channel active time: 47284496 ms channel active time: 47284637 ms channel active time: 47284742 ms channel active time: 47284788 ms channel active time: 47284860 ms channel active time: 47284871 ms channel active time: 47284883 ms channel active time: 47284911 ms channel active time: 47285040 ms channel active time: 47285280 ms channel active time: 47285449 ms channel active time: 47285745 ms channel active time: 47285886 ms channel active time: 47286074 ms channel active time: 47286108 ms channel active time: 47286120 ms channel active time: 47286154 ms channel active time: 47286235 ms channel active time: 47286247 ms channel active time: 47286405 ms channel active time: 47286418 ms channel active time: 51581468 ms channel active time: 51581569 ms channel active time: 51581765 ms channel active time: 51581795 ms channel active time: 51581807 ms channel active time: 51581818 ms channel active time: 51581836 ms channel active time: 51581877 ms channel active time: 51582130 ms channel active time: 51582205 ms channel active time: 51582219 ms channel active time: 51582269 ms channel active time: 51582319 ms channel active time: 51582379 ms channel active time: 51582430 ms channel active time: 51582491 ms channel active time: 51582694 ms channel active time: 51582870 ms channel active time: 51582926 ms channel active time: 51582946 ms channel active time: 51583078 ms channel active time: 51583111 ms channel active time: 51583120 ms channel active time: 51583129 ms channel active time: 51583142 ms channel active time: 51583172 ms channel active time: 51583186 ms channel active time: 51583331 ms channel active time: 51583449 ms channel active time: 51583649 ms channel active time: 51583776 ms channel active time: 51583789 ms channel active time: 51583801 ms channel active time: 51583821 ms channel active time: 55878795 ms channel active time: 55878828 ms channel active time: 55878853 ms channel active time: 55878863 ms channel active time: 55879005 ms channel active time: 55879051 ms channel active time: 55879114 ms channel active time: 55879144 ms channel active time: 55879169 ms channel active time: 55879313 ms channel active time: 55879610 ms channel active time: 55879626 ms channel active time: 55879641 ms channel active time: 55879786 ms channel active time: 55879931 ms channel active time: 55880082 ms channel active time: 55880114 ms channel active time: 55880145 ms channel active time: 55880304 ms channel active time: 55880342 ms channel active time: 55880397 ms channel active time: 55880407 ms channel active time: 55880595 ms channel active time: 55880653 ms channel active time: 60175670 ms channel active time: 60175702 ms channel active time: 60175714 ms channel active time: 60175863 ms channel active time: 60175876 ms channel active time: 60175887 ms channel active time: 60175925 ms channel active time: 60175935 ms channel active time: 60175950 ms channel active time: 60175957 ms channel active time: 60175968 ms channel active time: 60176020 ms I haven't checked if these jumps are related to anything specific or not. But if you multiple the values by 1000 (msec -> usec) and then create a diff between the previous values then you see that these values are also all over the place and not roughly ~1000000. If you just read the mvm->radio_stats.on_time_rf (in usec) then you see following: channel active time: 2346292 us channel active time: 3357380 us channel active time: 4368516 us channel active time: 4567172 us channel active time: 126139 us channel active time: 196605 us channel active time: 214119 us channel active time: 234556 us channel active time: 293402 us channel active time: 311971 us channel active time: 322606 us channel active time: 368173 us channel active time: 382785 us channel active time: 420408 us channel active time: 430851 us channel active time: 442990 us channel active time: 458566 us channel active time: 54960 us channel active time: 67810 us channel active time: 187747 us channel active time: 4295188641 us channel active time: 4295229424 us channel active time: 16295 us channel active time: 160736 us channel active time: 330166 us channel active time: 340849 us channel active time: 356933 us channel active time: 562579 us channel active time: 628140 us channel active time: 678766 us channel active time: 736022 us channel active time: 751099 us channel active time: 769377 us channel active time: 787728 us channel active time: 822183 us channel active time: 832478 us channel active time: 863107 us channel active time: 915469 us channel active time: 52166 us channel active time: 201828 us channel active time: 364236 us channel active time: 387916 us channel active time: 422965 us channel active time: 8560 us channel active time: 4295006989 us channel active time: 4295020943 us channel active time: 4295051766 us channel active time: 4295086037 us channel active time: 4295119851 us channel active time: 4295157051 us channel active time: 4295193488 us channel active time: 4295247769 us channel active time: 4295302615 us channel active time: 4295315627 us channel active time: 4295352876 us channel active time: 45385 us channel active time: 121871 us channel active time: 142972 us channel active time: 262344 us channel active time: 418666 us So it also jumps all over the place. This could be investigated further but I just wanted to mention it here. Kind regards, Sven
On Friday, 29 October 2021 10:46:43 CET Sven Eckelmann wrote: > If you just read the mvm->radio_stats.on_time_rf (in usec) then you see following: [...] > channel active time: 8560 us > channel active time: 4295006989 us > channel active time: 4295020943 us > channel active time: 4295051766 us > channel active time: 4295086037 us > channel active time: 4295119851 us > channel active time: 4295157051 us > channel active time: 4295193488 us > channel active time: 4295247769 us > channel active time: 4295302615 us > channel active time: 4295315627 us > channel active time: 4295352876 us > channel active time: 45385 us > channel active time: 121871 us > channel active time: 142972 us > channel active time: 262344 us > channel active time: 418666 us > > So it also jumps all over the place. This could be investigated further but I > just wanted to mention it here. Sorry, wanted to write more about it last week but forgot about it. If I basically filter out the upper 32 bit in mvm->radio_stats.on_time_rf then it didn't look that bad on a AX210. It seems like the upper bits is sometimes 0x00000001 for unknown reasons. Like it would be some kind of flag which should indicate some kind of change/event. So maybe the firmware team could check what this means. It is not really urgent - I just got interested in the problem :) Thanks, Sven
On Tue, 2021-11-02 at 12:12 +0100, Sven Eckelmann wrote: > > Sorry, wanted to write more about it last week but forgot about it. If I > basically filter out the upper 32 bit in mvm->radio_stats.on_time_rf then it > didn't look that bad on a AX210. It seems like the upper bits is sometimes > 0x00000001 for unknown reasons. Like it would be some kind of flag which > should indicate some kind of change/event. So maybe the firmware team could > check what this means. Well, I checked, and it *is* just a u64 value. However, I suspect it sometimes underflows when powersave time is accounted into it, or something? johannes
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 25ee16558dfa..83a1ba96e172 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -937,33 +937,37 @@ nl80211_packet_pattern_policy[MAX_NL80211_PKTPAT + 1] = { [NL80211_PKTPAT_OFFSET] = { .type = NLA_U32 }, }; -int nl80211_prepare_wdev_dump(struct netlink_callback *cb, - struct cfg80211_registered_device **rdev, - struct wireless_dev **wdev) +static int nl80211_prepare_wdev_dump(struct netlink_callback *cb, + struct cfg80211_registered_device **rdev, + struct wireless_dev **wdev, + struct nlattr **attrbuf) { int err; if (!cb->args[0]) { - struct nlattr **attrbuf; + struct nlattr **attrbuf_free = NULL; - attrbuf = kcalloc(NUM_NL80211_ATTR, sizeof(*attrbuf), - GFP_KERNEL); - if (!attrbuf) - return -ENOMEM; + if (!attrbuf) { + attrbuf = kcalloc(NUM_NL80211_ATTR, sizeof(*attrbuf), + GFP_KERNEL); + if (!attrbuf) + return -ENOMEM; + attrbuf_free = attrbuf; + } err = nlmsg_parse_deprecated(cb->nlh, GENL_HDRLEN + nl80211_fam.hdrsize, attrbuf, nl80211_fam.maxattr, nl80211_policy, NULL); if (err) { - kfree(attrbuf); + kfree(attrbuf_free); return err; } rtnl_lock(); *wdev = __cfg80211_wdev_from_attrs(NULL, sock_net(cb->skb->sk), attrbuf); - kfree(attrbuf); + kfree(attrbuf_free); if (IS_ERR(*wdev)) { rtnl_unlock(); return PTR_ERR(*wdev); @@ -6198,7 +6202,7 @@ static int nl80211_dump_station(struct sk_buff *skb, int sta_idx = cb->args[2]; int err; - err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev); + err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, NULL); if (err) return err; /* nl80211_prepare_wdev_dump acquired it in the successful case */ @@ -7093,7 +7097,7 @@ static int nl80211_dump_mpath(struct sk_buff *skb, int path_idx = cb->args[2]; int err; - err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev); + err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, NULL); if (err) return err; /* nl80211_prepare_wdev_dump acquired it in the successful case */ @@ -7293,7 +7297,7 @@ static int nl80211_dump_mpp(struct sk_buff *skb, int path_idx = cb->args[2]; int err; - err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev); + err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, NULL); if (err) return err; /* nl80211_prepare_wdev_dump acquired it in the successful case */ @@ -9723,7 +9727,7 @@ static int nl80211_dump_scan(struct sk_buff *skb, struct netlink_callback *cb) int start = cb->args[2], idx = 0; int err; - err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev); + err = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, NULL); if (err) return err; /* nl80211_prepare_wdev_dump acquired it in the successful case */ @@ -9856,7 +9860,7 @@ static int nl80211_dump_survey(struct sk_buff *skb, struct netlink_callback *cb) if (!attrbuf) return -ENOMEM; - res = nl80211_prepare_wdev_dump(cb, &rdev, &wdev); + res = nl80211_prepare_wdev_dump(cb, &rdev, &wdev, attrbuf); if (res) { kfree(attrbuf); return res; diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h index a3f387770f1b..d642e3be4ee7 100644 --- a/net/wireless/nl80211.h +++ b/net/wireless/nl80211.h @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* * Portions of this file - * Copyright (C) 2018, 2020 Intel Corporation + * Copyright (C) 2018, 2020-2021 Intel Corporation */ #ifndef __NET_WIRELESS_NL80211_H #define __NET_WIRELESS_NL80211_H @@ -22,10 +22,6 @@ static inline u64 wdev_id(struct wireless_dev *wdev) ((u64)wiphy_to_rdev(wdev->wiphy)->wiphy_idx << 32); } -int nl80211_prepare_wdev_dump(struct netlink_callback *cb, - struct cfg80211_registered_device **rdev, - struct wireless_dev **wdev); - int nl80211_parse_chandef(struct cfg80211_registered_device *rdev, struct genl_info *info, struct cfg80211_chan_def *chandef);