RE: [PATCH v11 4/7] wifi: rtw88: sdio: zero the padding added to a TX transfer

[Ping-Ke Shih]

luka.gejak@xxxxxxxxx <luka.gejak@xxxxxxxxx> wrote:
> From: Luka Gejak <luka.gejak@xxxxxxxxx>
> 
> rtw_sdio_write_port() rounds the transfer up with sdio_align_size() and
> then hands that length to sdio_memcpy_toio() while the skb still only
> holds skb->len bytes. The difference, between one and 511 bytes, is read
> from beyond the end of the frame and transmitted. Whether it stays
> inside the skb's allocation depends on how much tailroom the skb happens
> to have, so this is at best sending uninitialised memory over the air.
> 
> Pad the skb up to the transfer size first. __skb_pad() zeroes the added
> bytes, reallocates a cloned skb rather than writing into a buffer a
> clone still shares, and leaves skb->len alone, so nothing else in the
> transmit path has to change.

With __skb_pad(), it might increase CPU usage.
Could you roughly measure that?

> 
> It must not free the skb on failure: rtw_sdio_write_data() frees the skb
> itself and rtw_sdio_process_tx_queue() requeues it, so both callers
> still own it and would double free.
> 
> Found while reworking this path for the RTL8723BS. Measured on RTL8723BS
> hardware, padding the transfer costs nothing observable: uplink is
> 19.5 to 19.8 Mbit/s padded against 20.9 to 21.1 Mbit/s unpadded in an
> interleaved A/B, with scans, reconnection and a UDP flood clean in both.
> The other SDIO parts sharing this path are untested; I have only the
> RTL8723BS.
> 
> Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
> Signed-off-by: Luka Gejak <luka.gejak@xxxxxxxxx>

Acked-by: Ping-Ke Shih <pkshih@xxxxxxxxxxx>