linux-kernel - Re: [PATCH rtw-next 3/6] wifi: rtw89: implement C2H TX report handler

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250929121400-91aaec5c84b4868757ce3384-pchelkin@ispras>
Date: Mon, 29 Sep 2025 12:46:20 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Bitterblue Smith <rtl8821cerfe2@...il.com>
Cc: Ping-Ke Shih <pkshih@...ltek.com>, 
	Zong-Zhe Yang <kevin_yang@...ltek.com>, Po-Hao Huang <phhuang@...ltek.com>, 
	linux-wireless@...r.kernel.org, linux-kernel@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH rtw-next 3/6] wifi: rtw89: implement C2H TX report handler

On Wed, 24. Sep 22:16, Bitterblue Smith wrote:
> On 24/09/2025 01:12, Bitterblue Smith wrote:
> > On 20/09/2025 16:26, Fedor Pchelkin wrote:
> >> diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
> >> index ddebf7972068..f196088a8316 100644
> >> --- a/drivers/net/wireless/realtek/rtw89/fw.h
> >> +++ b/drivers/net/wireless/realtek/rtw89/fw.h
> >> @@ -3747,6 +3747,11 @@ struct rtw89_c2h_scanofld {
> >>  #define RTW89_GET_MAC_C2H_MCC_REQ_ACK_H2C_FUNC(c2h) \
> >>  	le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(15, 8))
> >>  
> >> +#define RTW89_GET_MAC_C2H_TX_RPT_TX_STATE(c2h) \
> >> +	le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(7, 6))
> >> +#define RTW89_GET_MAC_C2H_TX_RPT_SW_DEFINE(c2h) \
> >> +	le32_get_bits(*((const __le32 *)(c2h) + 2), GENMASK(12, 8))
> > 
> > This is only 4 bits:
> > 

Right, will be fixed.

> > #define TXCCXRPT_SW_DEFINE_SH		8
> > #define TXCCXRPT_SW_DEFINE_MSK		0xf
> > 
> > 
> > The rest of the series looks good to me. (I don't know much about
> > the RCU stuff.) I will test this tomorrow.
> > 
> 
> Actually, I found this in my notes:
> 
> "how to get just one tx report for each request? currently it seems
> to provide a report for each transmission attempt. how is the vendor
> driver coping with that?"
> 
> I think your code doesn't account for this.
> 
> Sorry I forgot about this detail. This behaviour is new in rtw89.
> The chips supported by rtw88 provide only one report for each request.

I think this part is also controlled with a couple of extra bits in TX
descriptor.

The vendor driver specifies a retry limit of 8 times.  It's probably the
default one followed by the firmware anyway because multiple retries on
failure status are seen via debug output of rtw89_mac_c2h_tx_rpt().
https://github.com/fofajardo/rtl8851bu/blob/1f1a14492fdac757c64a7efb7846be6374984d09/core/rtw_xmit.c#L7438

When there is a failed TX status reported by the firmware, the report is
ignored until the limit is reached or success status appears.
https://github.com/fofajardo/rtl8851bu/blob/1f1a14492fdac757c64a7efb7846be6374984d09/phl/hal_g6/hal_api_mac.c#L9547

The only concern is what happens if the sequence number of transmitted
frames wraps (4 bits is not a big range) when the previous frames with the
same sequence number were not processed yet - probability of the situation
increases with high retry limit being set.  I imagine the firmware has
some sort of handling for this but your reply suggests constraining retry
limit in the driver, are there maybe some other reasons we should set the
retry limit to one TX report?