Received-SPF: pass (google.com: domain of linux-wireless+bounces-5210-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <d364e872eb29f03236630bab49a3243e2118ab22.camel@sipsolutions.net>
Subject: Re: [PATCH v2 1/2] wifi: cfg80211/mac80211: Add support to rx retry
 stats
From: Johannes Berg <johannes@sipsolutions.net>
To: Hari Chandrakanthan <quic_haric@quicinc.com>, ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Date: Mon, 25 Mar 2024 16:43:39 +0100
In-Reply-To: <20240319134522.4021062-2-quic_haric@quicinc.com>
References: <20240319134522.4021062-1-quic_haric@quicinc.com>
	 <20240319134522.4021062-2-quic_haric@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.4 (3.50.4-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

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 st=
ation_info *sinfo,
>  		sinfo->filled |=3D BIT_ULL(NL80211_STA_INFO_TX_RETRIES);
>  	}
> =20
> +	if (!(sinfo->filled & BIT_ULL(NL80211_STA_INFO_RX_RETRIES))) {
> +		sinfo->rx_retries =3D sta->deflink.rx_stats.rx_retries;
> +		sinfo->filled |=3D 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?


johannes