Linux-Wireless Archive on lore.kernel.org
 help / Atom feed
* [RFC] mt76x02u: correct pad for fragments
@ 2019-02-11  8:37 Stanislaw Gruszka
  2019-02-11  9:47 ` Lorenzo Bianconi
  0 siblings, 1 reply; 2+ messages in thread
From: Stanislaw Gruszka @ 2019-02-11  8:37 UTC (permalink / raw)
  To: Felix Fietkau; +Cc: linux-wireless, Lorenzo Bianconi

Modifying skb->len & data_len doesn't look correct. Issue is not critical
we just pad 2 times, except first padding is not filled with zeros.

However I'm not sure if we should not add pad to all skb's in frags
list.

Additionally remove unlikely(pad) condition, we always pad for at least
four bytes what is needed by HW.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 .../wireless/mediatek/mt76/mt76x02_usb_core.c | 22 +++++++------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
index dc2226c722dd..c7ca2d93720a 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
@@ -49,21 +49,15 @@ int mt76x02u_skb_dma_info(struct sk_buff *skb, int port, u32 flags)
 	       FIELD_PREP(MT_TXD_INFO_DPORT, port) | flags;
 	put_unaligned_le32(info, skb_push(skb, sizeof(info)));
 
-	pad = round_up(skb->len, 4) + 4 - skb->len;
-	skb_walk_frags(skb, iter) {
+	skb_walk_frags(skb, iter)
 		last = iter;
-		if (!iter->next) {
-			skb->data_len += pad;
-			skb->len += pad;
-			break;
-		}
-	}
-
-	if (unlikely(pad)) {
-		if (skb_pad(last, pad))
-			return -ENOMEM;
-		__skb_put(last, pad);
-	}
+
+	/* Add zero pad of 4 - 7 bytes at the end of buffer */
+	pad = round_up(skb->len, 4) + 4 - skb->len;
+	if (skb_pad(last, pad))
+		return -ENOMEM;
+	__skb_put(last, pad);
+
 	return 0;
 }
 
-- 
2.19.2


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [RFC] mt76x02u: correct pad for fragments
  2019-02-11  8:37 [RFC] mt76x02u: correct pad for fragments Stanislaw Gruszka
@ 2019-02-11  9:47 ` Lorenzo Bianconi
  0 siblings, 0 replies; 2+ messages in thread
From: Lorenzo Bianconi @ 2019-02-11  9:47 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Felix Fietkau, linux-wireless

> Modifying skb->len & data_len doesn't look correct. Issue is not critical
> we just pad 2 times, except first padding is not filled with zeros.
> 
> However I'm not sure if we should not add pad to all skb's in frags
> list.

Hi Stanislaw,

IIRC the first packet of a A-MSDU burst keeps track of the burst length 
so if we add the a pad to last one we need to update the skb->len/data_len
of the first one. No need to pad all skbs in the frag list, just last one

Regards,
Lorenzo

> Additionally remove unlikely(pad) condition, we always pad for at least
> four bytes what is needed by HW.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
>  .../wireless/mediatek/mt76/mt76x02_usb_core.c | 22 +++++++------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> index dc2226c722dd..c7ca2d93720a 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt76x02_usb_core.c
> @@ -49,21 +49,15 @@ int mt76x02u_skb_dma_info(struct sk_buff *skb, int port, u32 flags)
>  	       FIELD_PREP(MT_TXD_INFO_DPORT, port) | flags;
>  	put_unaligned_le32(info, skb_push(skb, sizeof(info)));
>  
> -	pad = round_up(skb->len, 4) + 4 - skb->len;
> -	skb_walk_frags(skb, iter) {
> +	skb_walk_frags(skb, iter)
>  		last = iter;
> -		if (!iter->next) {
> -			skb->data_len += pad;
> -			skb->len += pad;
> -			break;
> -		}
> -	}
> -
> -	if (unlikely(pad)) {
> -		if (skb_pad(last, pad))
> -			return -ENOMEM;
> -		__skb_put(last, pad);
> -	}
> +
> +	/* Add zero pad of 4 - 7 bytes at the end of buffer */
> +	pad = round_up(skb->len, 4) + 4 - skb->len;
> +	if (skb_pad(last, pad))
> +		return -ENOMEM;
> +	__skb_put(last, pad);
> +
>  	return 0;
>  }
>  
> -- 
> 2.19.2
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11  8:37 [RFC] mt76x02u: correct pad for fragments Stanislaw Gruszka
2019-02-11  9:47 ` Lorenzo Bianconi

Linux-Wireless Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-wireless/0 linux-wireless/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-wireless linux-wireless/ https://lore.kernel.org/linux-wireless \
		linux-wireless@vger.kernel.org linux-wireless@archiver.kernel.org
	public-inbox-index linux-wireless


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-wireless


AGPL code for this site: git clone https://public-inbox.org/ public-inbox