Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <5b1aeb36dbc7316e5de0b0cfdf429b754c5cfd82.camel@sipsolutions.net>
Subject: Re: [PATCH] mac80211:  ethtool:  check link sta if deflink sta not
 found.
From:   Johannes Berg <johannes@sipsolutions.net>
To:     greearb@candelatech.com, linux-wireless@vger.kernel.org
Date:   Wed, 04 Oct 2023 21:36:17 +0200
In-Reply-To: <20231003215839.981227-1-greearb@candelatech.com>
References: <20231003215839.981227-1-greearb@candelatech.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2023-10-03 at 14:58 -0700, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>=20
> 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.
>=20
> Use link-0 for sta if it is available to do a better job of reporting
> ethtool stats.
>=20
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>  net/mac80211/ethtool.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>=20
> 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 =3D 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 =3D=3D NL80211_IFTYPE_STATION) {
>  		sta =3D sta_info_get_bss(sdata, sdata->deflink.u.mgd.bssid);

Just don't even try this, link[0] is always valid.

> +		if (!sta) {
> +			link =3D 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