linux-kernel - Re: [PATCH rtw-next v2 6/7] wifi: rtw89: handle IEEE80211_TX_CTL_REQ_TX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251015103403-08f19f981c70546d56ebd07a-pchelkin@ispras>
Date: Wed, 15 Oct 2025 10:51:21 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Ping-Ke Shih <pkshih@...ltek.com>
Cc: Bitterblue Smith <rtl8821cerfe2@...il.com>, 
	Zong-Zhe Yang <kevin_yang@...ltek.com>, Bernie Huang <phhuang@...ltek.com>, 
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"lvc-project@...uxtesting.org" <lvc-project@...uxtesting.org>
Subject: Re: [PATCH rtw-next v2 6/7] wifi: rtw89: handle
 IEEE80211_TX_CTL_REQ_TX_STATUS frames for USB

On Wed, 15. Oct 01:43, Ping-Ke Shih wrote:
> > > > +
> > > > +               if (sw_define != skb_data->tx_rpt_sn)
> > > > +                       continue;
> > > > +               if (tx_status != RTW89_TX_DONE &&
> > > > +                   data_txcnt != skb_data->tx_pkt_cnt_lmt)
> > >
> > > As commit message of previous patch, "When there is a failed
> > > TX status reported by the firmware, the report is ignored until the limit
> > > is reached or success status appears."
> > >
> > > Do you still need to check data_txcnt for failed cases?
> > 
> > The question also concerns
> > 
> >   tx_req->desc_info.tx_cnt_lmt = 8;
> > 
> > line in rtw89_tx_rpt_enable().  'tx_cnt_lmt' is written to TX descriptor
> > and processed by firmware.  The value defines how many times the firmware
> > will retry transmission attempts, it will not retry more times than that.
> > 
> > 'data_txcnt' C2H field determines the retry attempt counter for the frame
> > returned by the firmware.  If it reaches the limit, this means we got
> > the last report from the firmware and there would be no other firmware
> > reports for the sent frame.  So a final tx_status should be taken
> > uncondionally in this case.
> > 
> > E.g. if 'tx_cnt_lmt' is set to 1, the firmware will try only once,
> > 'data_txcnt' will be 1, too.  The limit is reached and we should take
> > tx_status immediately as is.  So there's a higher chance of getting a
> > failed status eventually.
> > 
> > I set it currently to 8 as the vendor driver does.  In local testing it
> > looks more than enough.  I've seen maximum of 5 retry attempts for the
> > same frame (usually there are no retries at all) though my network radio
> > environment is quite noisy.
> > 
> > I'll add the tx_cnt_lmt related info to commit message for clarity.
> > 
> 
> Thanks for the detail. 
> 
>     1 2 3 4 5 6 7 8
> (a) x x x x x x x x  ==> retry 8 times, but all failure. Report at 8th C2H.
> (b) x x x o          ==> retry 3 times, and 4th done. Report at 4th C2H.
> (c) o                ==> just done at first one. Report at first C2H.
> 
> For every attempt, firmware reports a C2H with tx_status, right?

Yes.

> Can I say (a) case is why we should check data_txcnt? 
> For cases (b)/(c), they rely on 'tx_status == RTW89_TX_DONE'.

We should somehow determine in case (a) when those 8 attempts for the
frame have passed and then promptly give the report with a failed status
up to the wireless stack.  To my mind, without checking data_txcnt
rtw89_mac_c2h_tx_rpt() can't determine the time when to do an actual
report if every retry attempt has failed.

Otherwise skb would remain in the queue being unreported until HCI reset
takes place, though we already had a chance to report it as failed.