> -----Original Message-----
> From: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> Sent: Tuesday, April 18, 2023 9:05 PM
> To: linux-wireless@xxxxxxxxxxxxxxx
> Cc: Jes Sorensen <Jes.Sorensen@xxxxxxxxx>; Ping-Ke Shih <pkshih@xxxxxxxxxxx>
> Subject: [PATCH] wifi: rtl8xxxu: Support USB RX aggregation for the newer chips
>
> The driver can receive several frames in the same USB transfer.
> Add the code to handle this in rtl8xxxu_parse_rxdesc24(), even though
> currently all the relevant chips send only one frame per USB transfer
> (RTL8723BU, RTL8192EU, RTL8188FU, RTL8710BU).
>
> rtl8xxxu_parse_rxdesc16() used by RTL8723AU, RTL8192CU, and RTL8188EU
> already handles RX aggregation.
>
> This was tested with RTL8188FU, RTL8192EU, RTL8710BU, and RTL8192FU.
>
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx>
> ---
> .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 117 ++++++++++++------
> 1 file changed, 76 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> index fd8c8c6d53d6..5fccd898d607 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
> @@ -6197,61 +6197,96 @@ int rtl8xxxu_parse_rxdesc16(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
> int rtl8xxxu_parse_rxdesc24(struct rtl8xxxu_priv *priv, struct sk_buff *skb)
> {
> struct ieee80211_hw *hw = priv->hw;
> - struct ieee80211_rx_status *rx_status = IEEE80211_SKB_RXCB(skb);
> - struct rtl8xxxu_rxdesc24 *rx_desc =
> - (struct rtl8xxxu_rxdesc24 *)skb->data;
> + struct ieee80211_rx_status *rx_status;
> + struct rtl8xxxu_rxdesc24 *rx_desc;
> struct rtl8723au_phy_stats *phy_stats;
> - __le32 *_rx_desc_le = (__le32 *)skb->data;
> - u32 *_rx_desc = (u32 *)skb->data;
> + struct sk_buff *next_skb = NULL;
> + __le32 *_rx_desc_le;
> + u32 *_rx_desc;
> int drvinfo_sz, desc_shift;
> - int i;
> + int i, pkt_len, urb_len, pkt_offset;
>
> - for (i = 0; i < (sizeof(struct rtl8xxxu_rxdesc24) / sizeof(u32)); i++)
> - _rx_desc[i] = le32_to_cpu(_rx_desc_le[i]);
> + urb_len = skb->len;
>
> - memset(rx_status, 0, sizeof(struct ieee80211_rx_status));
> + if (urb_len < sizeof(struct rtl8xxxu_rxdesc24)) {
> + kfree_skb(skb);
> + return RX_TYPE_ERROR;
> + }
>
> - skb_pull(skb, sizeof(struct rtl8xxxu_rxdesc24));
> + do {
> + rx_desc = (struct rtl8xxxu_rxdesc24 *)skb->data;
> + _rx_desc_le = (__le32 *)skb->data;
> + _rx_desc = (u32 *)skb->data;
>
> - phy_stats = (struct rtl8723au_phy_stats *)skb->data;
> + for (i = 0; i < (sizeof(struct rtl8xxxu_rxdesc24) / sizeof(u32)); i++)
> + _rx_desc[i] = le32_to_cpu(_rx_desc_le[i]);
>
> - drvinfo_sz = rx_desc->drvinfo_sz * 8;
> - desc_shift = rx_desc->shift;
> - skb_pull(skb, drvinfo_sz + desc_shift);
> + pkt_len = rx_desc->pktlen;
>
> - if (rx_desc->rpt_sel) {
> - struct device *dev = &priv->udev->dev;
> - dev_dbg(dev, "%s: C2H packet\n", __func__);
> - rtl8723bu_handle_c2h(priv, skb);
> - return RX_TYPE_C2H;
> - }
> + drvinfo_sz = rx_desc->drvinfo_sz * 8;
> + desc_shift = rx_desc->shift;
> + pkt_offset = roundup(pkt_len + drvinfo_sz + desc_shift +
> + sizeof(struct rtl8xxxu_rxdesc24), 8);
>
> - if (rx_desc->phy_stats)
> - priv->fops->parse_phystats(priv, rx_status, phy_stats,
> - rx_desc->rxmcs, (struct ieee80211_hdr *)skb->data,
> - rx_desc->crc32 || rx_desc->icverr);
> + /*
> + * Only clone the skb if there's enough data at the end to
> + * at least cover the rx descriptor
> + */
> + if (urb_len >= (pkt_offset + sizeof(struct rtl8xxxu_rxdesc24)))
That means you use 'sizeof(struct rtl8xxxu_rxdesc24)' as clue to know if this
is the last packet or not, right?
Do you think if it is needed to handle malformed packet for the last skb if
its length is shorter than pkt_offset or pkt_len?
> + next_skb = skb_clone(skb, GFP_ATOMIC);
>
> - rx_status->mactime = rx_desc->tsfl;
> - rx_status->flag |= RX_FLAG_MACTIME_START;
> + rx_status = IEEE80211_SKB_RXCB(skb);
> + memset(rx_status, 0, sizeof(struct ieee80211_rx_status));
>
> - if (!rx_desc->swdec)
> - rx_status->flag |= RX_FLAG_DECRYPTED;
> - if (rx_desc->crc32)
> - rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
> - if (rx_desc->bw)
> - rx_status->bw = RATE_INFO_BW_40;
> + skb_pull(skb, sizeof(struct rtl8xxxu_rxdesc24));
>
> - if (rx_desc->rxmcs >= DESC_RATE_MCS0) {
> - rx_status->encoding = RX_ENC_HT;
> - rx_status->rate_idx = rx_desc->rxmcs - DESC_RATE_MCS0;
> - } else {
> - rx_status->rate_idx = rx_desc->rxmcs;
> - }
> + phy_stats = (struct rtl8723au_phy_stats *)skb->data;
> +
> + skb_pull(skb, drvinfo_sz + desc_shift);
> +
> + skb_trim(skb, pkt_len);
> +
> + if (rx_desc->rpt_sel) {
> + struct device *dev = &priv->udev->dev;
> + dev_dbg(dev, "%s: C2H packet\n", __func__);
> + rtl8723bu_handle_c2h(priv, skb);
> + } else {
> + if (rx_desc->phy_stats)
> + priv->fops->parse_phystats(priv, rx_status, phy_stats,
> + rx_desc->rxmcs, (struct ieee80211_hdr
> *)skb->data,
> + rx_desc->crc32 || rx_desc->icverr);
This statement is too long. Add a helper 'struct ieee80211_hdr *hdr', like
if (rx_desc->phy_stats) {
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
priv->fops->parse_phystats(priv, rx_status, phy_stats,
rx_desc->rxmcs, hdr,
rx_desc->crc32 || rx_desc->icverr);
}
> +
> + rx_status->mactime = rx_desc->tsfl;
> + rx_status->flag |= RX_FLAG_MACTIME_START;
> +
> + if (!rx_desc->swdec)
> + rx_status->flag |= RX_FLAG_DECRYPTED;
> + if (rx_desc->crc32)
> + rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
> + if (rx_desc->bw)
> + rx_status->bw = RATE_INFO_BW_40;
>
> - rx_status->freq = hw->conf.chandef.chan->center_freq;
> - rx_status->band = hw->conf.chandef.chan->band;
> + if (rx_desc->rxmcs >= DESC_RATE_MCS0) {
> + rx_status->encoding = RX_ENC_HT;
> + rx_status->rate_idx = rx_desc->rxmcs - DESC_RATE_MCS0;
> + } else {
> + rx_status->rate_idx = rx_desc->rxmcs;
> + }
> +
> + rx_status->freq = hw->conf.chandef.chan->center_freq;
> + rx_status->band = hw->conf.chandef.chan->band;
> +
> + ieee80211_rx_irqsafe(hw, skb);
> + }
> +
> + skb = next_skb;
> + if (skb)
> + skb_pull(next_skb, pkt_offset);
> +
> + urb_len -= pkt_offset;
> + next_skb = NULL;
> + } while (skb && urb_len >= sizeof(struct rtl8xxxu_rxdesc24));
>
> - ieee80211_rx_irqsafe(hw, skb);
> return RX_TYPE_DATA_PKT;
> }
>
> --
> 2.39.2
>
> ------Please consider the environment before printing this e-mail.
