> Hi and sorry for late comment.
>
Hi Stanislaw,
> On Fri, May 31, 2019 at 11:38:22AM +0200, Lorenzo Bianconi wrote:
> > Commit f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes
> > for rx") breaks A-MSDU support. When A-MSDU is enable the device can
> > receive frames up to q->buf_size but they will be discarded in
> > mt76u_process_rx_entry since there is no enough room for
> > skb_shared_info. Fix the issue reallocating the skb and copying in the
> > linear area the first 128B of the received frames and in the frag_list
> > the remaining part.
> >
> > Fixes: f8f527b16db5 ("mt76: usb: use EP max packet aligned buffer sizes for rx")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > ---
> > drivers/net/wireless/mediatek/mt76/mt76.h | 1 +
> > drivers/net/wireless/mediatek/mt76/usb.c | 52 +++++++++++++++++++----
> > 2 files changed, 44 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
> > index 97a1296562d0..74d4edf941d6 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt76.h
> > +++ b/drivers/net/wireless/mediatek/mt76/mt76.h
> > @@ -30,6 +30,7 @@
> > #define MT_TX_RING_SIZE 256
> > #define MT_MCU_RING_SIZE 32
> > #define MT_RX_BUF_SIZE 2048
> > +#define MT_SKB_HEAD_LEN 128
> >
> > struct mt76_dev;
> > struct mt76_wcid;
> > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> > index bbaa1365bbda..2bfc8214c0d8 100644
> > --- a/drivers/net/wireless/mediatek/mt76/usb.c
> > +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> > @@ -429,6 +429,48 @@ static int mt76u_get_rx_entry_len(u8 *data, u32 data_len)
> > return dma_len;
> > }
> >
> > +static struct sk_buff *
> > +mt76u_build_rx_skb(u8 *data, int len, int buf_size,
> > + int *nsgs)
> > +{
> > + int data_len = min(len, MT_SKB_HEAD_LEN);
> > + struct sk_buff *skb;
> > +
> > + if (SKB_WITH_OVERHEAD(buf_size) >= MT_DMA_HDR_LEN + len) {
> > + /* fast path */
> > + skb = build_skb(data, buf_size);
> > + if (!skb)
> > + return NULL;
> > +
> > + skb_reserve(skb, MT_DMA_HDR_LEN);
> > + __skb_put(skb, len);
> > +
> > + return skb;
> > + }
> > +
> > + /* slow path, not enough space for data and
> > + * skb_shared_info
> > + */
> > + skb = alloc_skb(data_len, GFP_ATOMIC);
> > + if (!skb)
> > + return NULL;
> > +
> > + skb_put_data(skb, data + MT_DMA_HDR_LEN, data_len);
> mt7601u and iwlmvm just copy hdrlen + 8 and put the rest
> of the buffer in fragment, which supose to be more efficient,
> see comment in iwl_mvm_pass_packet_to_mac80211().
Right here we copy 128B instead of 32 but I think it is good to have L3 and L4
header in the linear area of the skb since otherwise the stack will need to
align them
>
> > + data += (data_len + MT_DMA_HDR_LEN);
> > + len -= data_len;
> > + if (len > 0) {
> > + struct page *page = virt_to_head_page(data);
> > + int offset = data - (u8 *)page_address(page);
> u8 cast not needed.
>
> > + skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > + page, offset, len, buf_size);
> > + } else {
> > + *nsgs = 0;
> This seems to be not necessary or a bug (use first buffer 2 times).
maybe I have been a little bit too paranoid here :)
Regards,
Lorenzo
>
> Stanislaw
Attachment:
signature.asc
Description: PGP signature
