Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220516032519.29831-1-ryazanov.s.a@gmail.com> <20220516032519.29831-2-ryazanov.s.a@gmail.com>
In-Reply-To: <20220516032519.29831-2-ryazanov.s.a@gmail.com>
From:   Sergey Ryazanov <ryazanov.s.a@gmail.com>
Date:   Mon, 16 May 2022 23:57:34 +0300
Message-ID: <CAHNKnsS2_vVPZT-PSjNDzfQmyXEaOJNO8MHqVfntN=GSG3P_Ng@mail.gmail.com>
Subject: Re: [PATCH 1/4] ath10k: improve tx status reporting
To:     Kalle Valo <kvalo@kernel.org>
Cc:     ath10k@lists.infradead.org, linux-wireless@vger.kernel.org,
        Edward Matijevic <motolav@gmail.com>,
        John Crispin <john@phrozen.org>,
        =?UTF-8?B?T2xkxZlpY2ggSmVkbGnEjWth?= <oldium.pro@gmail.com>,
        Tom Psyborg <pozega.tomislav@gmail.com>,
        Vasanthakumar Thiagarajan <vthiagar@qti.qualcomm.com>,
        Zhijun You <hujy652@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, May 16, 2022 at 6:25 AM Sergey Ryazanov <ryazanov.s.a@gmail.com> wr=
ote:
> We use ieee80211_tx_status() to report each completed tx frame.
> Internally, this function calls sta_info_get_by_addrs(), what has a
> couple of drawbacks:
> 1. additional station lookup causes a performance degradation;
> 2. mac80211 can not properly account Ethernet encapsulated frames due
>    to the inability to properly determine the destination (station) MAC
>    address since ieee80211_tx_status() assumes the frame has a 802.11
>    header.
>
> The latter is especially destructive if we want to use hardware frames
> encapsulation.
>
> To fix both of these issues, replace ieee80211_tx_status() with
> ieee80211_tx_status_ext() call and feed it station pointer from the tx
> queue associated with the transmitted frame.
>
> Tested-on: QCA9888 hw 2.0 10.4-3.9.0.2-00131
> Tested-on: QCA6174 hw 3.2 PCI WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>
> Tested-by: Old=C5=99ich Jedli=C4=8Dka <oldium.pro@gmail.com> # TP-Link Ar=
cher C7 v4 & v5 (QCA9563 + QCA9880)
> Tested-by: Edward Matijevic <motolav@gmail.com> # TP-Link Archer C2600 (I=
PQ8064 + QCA9980 10.4.1.00030-1)
> Tested-by: Edward Matijevic <motolav@gmail.com> # QCA9377 PCI in Sta mode
> Tested-by: Zhijun You <hujy652@gmail.com> # NETGEAR R7800 (QCA9984 10.4-3=
.9.0.2-00159)
> ---
>
> Changes since RFC:
>  * new Tested-on and Tested-by tags
>
>  drivers/net/wireless/ath/ath10k/txrx.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireles=
s/ath/ath10k/txrx.c
> index 10123974c3da..72540434c75b 100644
> --- a/drivers/net/wireless/ath/ath10k/txrx.c
> +++ b/drivers/net/wireless/ath/ath10k/txrx.c
> @@ -43,6 +43,7 @@ static void ath10k_report_offchan_tx(struct ath10k *ar,=
 struct sk_buff *skb)
>  int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>                          const struct htt_tx_done *tx_done)
>  {
> +       struct ieee80211_tx_status status;
>         struct ath10k *ar =3D htt->ar;
>         struct device *dev =3D ar->dev;
>         struct ieee80211_tx_info *info;
> @@ -128,7 +129,16 @@ int ath10k_txrx_tx_unref(struct ath10k_htt *htt,
>                 info->status.flags |=3D IEEE80211_TX_STATUS_ACK_SIGNAL_VA=
LID;
>         }
>
> -       ieee80211_tx_status(htt->ar->hw, msdu);
> +       memset(&status, 0, sizeof(status));
> +       status.skb =3D msdu;
> +       status.info =3D info;
> +
> +       rcu_read_lock();
> +       if (txq && txq->sta)
> +               status.sta =3D txq->sta;

Just noticed that since we do not dereference the txq->sta pointer
here, the above code can be simplified to:

if (txq)
        status.sta =3D txq->sta;

Kalle, should I send V2 or can you change this in your tree?

> +       ieee80211_tx_status_ext(htt->ar->hw, &status);
> +       rcu_read_unlock();
> +
>         /* we do not own the msdu anymore */
>
>         return 0;

--=20
Sergey