mac80211: ethtool: check link sta if deflink sta not found.

From: Ben Greear <greearb@candelatech.com>

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 <greearb@candelatech.com>
---
 net/mac80211/ethtool.c | 7 +++++++
 1 file changed, 7 insertions(+)

On Tue, 2023-10-03 at 14:58 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
> 
> 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 <greearb@candelatech.com>
> ---
>  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, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> 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 <greearb@candelatech.com>
>> ---
>>   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
>