From: Ben Greear <[email protected]>
While testing with wifi-7 radio put into AX mode, link-0 is created.
sdata->deflink.u.mgd.bssid was 00 in this case, so sta was not
found.
Use link-0 for sta if it is available to do a better job of reporting
ethtool stats.
Signed-off-by: Ben Greear <[email protected]>
---
net/mac80211/ethtool.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
index 6bd7fba8a867..1b58304fc68a 100644
--- a/net/mac80211/ethtool.c
+++ b/net/mac80211/ethtool.c
@@ -96,6 +96,7 @@ static void ieee80211_get_stats2(struct net_device *dev,
struct ieee80211_local *local = sdata->local;
struct station_info sinfo;
struct survey_info survey;
+ struct ieee80211_link_data *link;
int i, q;
int z;
#define STA_STATS_SURVEY_LEN 7
@@ -128,6 +129,12 @@ static void ieee80211_get_stats2(struct net_device *dev,
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
sta = sta_info_get_bss(sdata, sdata->deflink.u.mgd.bssid);
+ if (!sta) {
+ link = sdata_dereference(sdata->link[0], sdata);
+ if (link)
+ sta = sta_info_get_bss(sdata, link->u.mgd.bssid);
+ }
+
if (!(sta && !WARN_ON(sta->sdata->dev != dev)))
goto do_survey;
--
2.40.0
On Tue, 2023-10-03 at 14:58 -0700, [email protected] wrote:
> From: Ben Greear <[email protected]>
>
> While testing with wifi-7 radio put into AX mode, link-0 is created.
> sdata->deflink.u.mgd.bssid was 00 in this case, so sta was not
> found.
>
> Use link-0 for sta if it is available to do a better job of reporting
> ethtool stats.
>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> net/mac80211/ethtool.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
> index 6bd7fba8a867..1b58304fc68a 100644
> --- a/net/mac80211/ethtool.c
> +++ b/net/mac80211/ethtool.c
> @@ -96,6 +96,7 @@ static void ieee80211_get_stats2(struct net_device *dev,
> struct ieee80211_local *local = sdata->local;
> struct station_info sinfo;
> struct survey_info survey;
> + struct ieee80211_link_data *link;
> int i, q;
> int z;
> #define STA_STATS_SURVEY_LEN 7
> @@ -128,6 +129,12 @@ static void ieee80211_get_stats2(struct net_device *dev,
> if (sdata->vif.type == NL80211_IFTYPE_STATION) {
> sta = sta_info_get_bss(sdata, sdata->deflink.u.mgd.bssid);
Just don't even try this, link[0] is always valid.
> + if (!sta) {
> + link = sdata_dereference(sdata->link[0], sdata);
but link[0] is a bad idea anyway ... what if the AP only assigned link 1
and 2? Or you connected only there?
I'm not even sure this is really worth fixing, do you really want a
random link's statistics?
johannes
On 10/4/23 12:36, Johannes Berg wrote:
> On Tue, 2023-10-03 at 14:58 -0700, [email protected] wrote:
>> From: Ben Greear <[email protected]>
>>
>> While testing with wifi-7 radio put into AX mode, link-0 is created.
>> sdata->deflink.u.mgd.bssid was 00 in this case, so sta was not
>> found.
>>
>> Use link-0 for sta if it is available to do a better job of reporting
>> ethtool stats.
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> net/mac80211/ethtool.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/net/mac80211/ethtool.c b/net/mac80211/ethtool.c
>> index 6bd7fba8a867..1b58304fc68a 100644
>> --- a/net/mac80211/ethtool.c
>> +++ b/net/mac80211/ethtool.c
>> @@ -96,6 +96,7 @@ static void ieee80211_get_stats2(struct net_device *dev,
>> struct ieee80211_local *local = sdata->local;
>> struct station_info sinfo;
>> struct survey_info survey;
>> + struct ieee80211_link_data *link;
>> int i, q;
>> int z;
>> #define STA_STATS_SURVEY_LEN 7
>> @@ -128,6 +129,12 @@ static void ieee80211_get_stats2(struct net_device *dev,
>> if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>> sta = sta_info_get_bss(sdata, sdata->deflink.u.mgd.bssid);
>
> Just don't even try this, link[0] is always valid.
What is 'deflink' even used for then? Is that only for non MLO connections?
Or you mean link[0] is valid even in non MLO connections?
Note I'm not changing that line above in my patch, though maybe it should
be changed...
>
>> + if (!sta) {
>> + link = sdata_dereference(sdata->link[0], sdata);
>
> but link[0] is a bad idea anyway ... what if the AP only assigned link 1
> and 2? Or you connected only there?
>
> I'm not even sure this is really worth fixing, do you really want a
> random link's statistics?
I'd like it to at least have a chance of reporting something useful.
So maybe 'sta' can be the connected link with highest band. And then I can add
new ethtool stats for link-0, link-1, link-2 (and one of those would be duplicated,
since the 'best' stats are already added). Or I could have the default stats always
be link-0, and add new ethtool stats for link-1, link-2. That would make it less
backwards compatible since link-0 may not be connected and would show no useful stats for the
old default ethtool strings.
I do not want to change existing ethtool stats strings since that would break
my user-space API and I'd rather not hack around that if possible.
Thanks,
Ben
>
> johannes
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com