Received-SPF: pass (google.com: domain of linux-wireless+bounces-2134-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Ping-Ke Shih <pkshih@realtek.com>
To: Martin Kaistra <martin.kaistra@linutronix.de>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
CC: Jes Sorensen <Jes.Sorensen@gmail.com>, Kalle Valo <kvalo@kernel.org>,
        Bitterblue Smith <rtl8821cerfe2@gmail.com>,
        Sebastian Andrzej Siewior
	<bigeasy@linutronix.de>
Subject: RE: [PATCH] wifi: rtl8xxxu: update rate mask per sta
Thread-Topic: [PATCH] wifi: rtl8xxxu: update rate mask per sta
Thread-Index: AQHaSVU1/kXlkXq/OEm3ZacX5mGlCLDevu1w
Date: Thu, 18 Jan 2024 01:37:58 +0000
Message-ID: <4d5f06f2407042f2862af7559ed66eac@realtek.com>
References: <20240117145516.497966-1-martin.kaistra@linutronix.de>
In-Reply-To: <20240117145516.497966-1-martin.kaistra@linutronix.de>
Accept-Language: en-US, zh-TW
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0


> -----Original Message-----
> From: Martin Kaistra <martin.kaistra@linutronix.de>
> Sent: Wednesday, January 17, 2024 10:55 PM
> To: linux-wireless@vger.kernel.org
> Cc: Jes Sorensen <Jes.Sorensen@gmail.com>; Kalle Valo <kvalo@kernel.org>;=
 Ping-Ke Shih
> <pkshih@realtek.com>; Bitterblue Smith <rtl8821cerfe2@gmail.com>; Sebasti=
an Andrzej Siewior
> <bigeasy@linutronix.de>
> Subject: [PATCH] wifi: rtl8xxxu: update rate mask per sta
>=20
> Until now, rtl8xxxu_watchdog_callback() only fetches RSSI and updates
> the rate mask in station mode. This means, in AP mode only the default
> rate mask is used.
>=20
> In order to have the rate mask reflect the actual connection quality,
> extend rtl8xxxu_watchdog_callback() to iterate over every sta. Like in
> the rtw88 driver, add a function to collect all currently present stas
> and then iterate over a list of copies to ensure no RCU lock problems
> for register access via USB. Remove the existing RCU lock in
> rtl8xxxu_refresh_rate_mask().
>=20
> Since the currently used ieee80211_ave_rssi() is only for 'vif', add
> driver-level tracking of RSSI per sta.
>=20
> Signed-off-by: Martin Kaistra <martin.kaistra@linutronix.de>

[...]

> @@ -6317,6 +6318,76 @@ static void rtl8188e_c2hcmd_callback(struct work_s=
truct *work)
>         }
>  }
>=20
> +#define rtl8xxxu_iterate_vifs_atomic(priv, iterator, data)              =
       \
> +       ieee80211_iterate_active_interfaces_atomic((priv)->hw,           =
       \
> +                       IEEE80211_IFACE_ITER_NORMAL, iterator, data)
> +
> +struct rtl8xxxu_rx_addr_match_data {
> +       struct rtl8xxxu_priv *priv;
> +       struct ieee80211_hdr *hdr;
> +       struct ieee80211_rx_status *rx_status;
> +       u8 *bssid;
> +};
> +
> +static void rtl8xxxu_rx_addr_match_iter(void *data, u8 *mac,
> +                                       struct ieee80211_vif *vif)
> +{
> +       struct rtl8xxxu_rx_addr_match_data *iter_data =3D data;
> +       struct ieee80211_sta *sta;
> +       struct ieee80211_hdr *hdr =3D iter_data->hdr;
> +       struct rtl8xxxu_priv *priv =3D iter_data->priv;
> +       struct rtl8xxxu_sta_info *sta_info;
> +       struct ieee80211_rx_status *rx_status =3D iter_data->rx_status;
> +       u8 *bssid =3D iter_data->bssid;
> +
> +       if (!ether_addr_equal(vif->bss_conf.bssid, bssid))
> +               return;
> +
> +       if (!(ether_addr_equal(vif->addr, hdr->addr1) ||
> +             ieee80211_is_beacon(hdr->frame_control)))
> +               return;
> +
> +       sta =3D ieee80211_find_sta_by_ifaddr(priv->hw, hdr->addr2,
> +                                          vif->addr);

Can't we search for 'sta' by rx_desc->mac_id? Then, you don't need a lot of
code to check address.=20

> +       if (!sta)
> +               return;
> +
> +       sta_info =3D (struct rtl8xxxu_sta_info *)sta->drv_priv;
> +       ewma_rssi_add(&sta_info->avg_rssi, -rx_status->signal);
> +}
> +

[...]

> @@ -7119,7 +7203,7 @@ static void rtl8xxxu_refresh_rate_mask(struct rtl8x=
xxu_priv *priv,
>         u8 go_up_gap =3D 5;
>         u8 macid =3D rtl8xxxu_get_macid(priv, sta);
>=20
> -       rssi_level =3D priv->rssi_level;
> +       rssi_level =3D priv->rssi_level[macid];

Is it possible to move 'rssi_level' into struct rtl8xxxu_sta_info?

>         snr =3D rtl8xxxu_signal_to_snr(signal);
>         snr_thresh_high =3D RTL8XXXU_SNR_THRESH_HIGH;
>         snr_thresh_low =3D RTL8XXXU_SNR_THRESH_LOW;

[...]

> @@ -7329,40 +7411,60 @@ static void rtl8xxxu_track_cfo(struct rtl8xxxu_pr=
iv *priv)
>         rtl8xxxu_set_atc_status(priv, abs(cfo_average) >=3D CFO_TH_ATC);
>  }
>=20
> -static void rtl8xxxu_watchdog_callback(struct work_struct *work)
> +static void rtl8xxxu_ra_iter(void *data, struct ieee80211_sta *sta)
>  {
> -       struct ieee80211_vif *vif;
> -       struct rtl8xxxu_priv *priv;
> -       int i;
> +       struct rtl8xxxu_sta_info *sta_info =3D (struct rtl8xxxu_sta_info =
*)sta->drv_priv;
> +       struct rtl8xxxu_priv *priv =3D data;
> +       int signal =3D -ewma_rssi_read(&sta_info->avg_rssi);

The unit conversion of signal is a little complicated --
from physt to rx_status->signal to sta_info->avg_rssi.

I think you did it well. Just want to confirm have you checked the final re=
sult
is equal to before at runtime?

[...]