Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1635993 > unrolled thread

[PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding Jim Baxter <jim_baxter@mentor.com> - 2017-05-04 21:50 +0200
    Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding Bjørn Mork <bjorn@mork.no> - 2017-05-04 23:10 +0200
      Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding "Baxter, Jim" <jim_baxter@mentor.com> - 2017-05-08 12:50 +0200
        Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding Bjørn Mork <bjorn@mork.no> - 2017-05-08 13:10 +0200
          Re: [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding "Baxter, Jim" <jim_baxter@mentor.com> - 2017-05-08 14:10 +0200

#1635993 — [PATCH V1 1/1] net: cdc_ncm: Fix TX zero padding

The zero padding that is added to NTB's does
not zero the memory correctly.
This is because the skb_put modifies the value
of skb_out->len which results in the memset
command not setting any memory to zero as
(ctx->tx_max - skb_out->len) == 0.

I have resolved this by storing the size of
the memory to be zeroed before the skb_put
and using this in the memset call.

Signed-off-by: Jim Baxter <jim_baxter@mentor.com>
---
 drivers/net/usb/cdc_ncm.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index f317984..e2a48d7 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -1087,6 +1087,7 @@ struct sk_buff *
 	u16 n = 0, index, ndplen;
 	u8 ready2send = 0;
 	u32 delayed_ndp_size;
+	size_t padding_count;
 
 	/* When our NDP gets written in cdc_ncm_ndp(), then skb_out->len gets updated
 	 * accordingly. Otherwise, we should check here.
@@ -1243,11 +1244,13 @@ struct sk_buff *
 	 * a ZLP after full sized NTBs.
 	 */
 	if (!(dev->driver_info->flags & FLAG_SEND_ZLP) &&
-	    skb_out->len > ctx->min_tx_pkt)
-		memset(skb_put(skb_out, ctx->tx_max - skb_out->len), 0,
-		       ctx->tx_max - skb_out->len);
-	else if (skb_out->len < ctx->tx_max && (skb_out->len % dev->maxpacket) == 0)
+	    skb_out->len > ctx->min_tx_pkt) {
+		padding_count = ctx->tx_max - skb_out->len;
+		memset(skb_put(skb_out, padding_count), 0, padding_count);
+	} else if (skb_out->len < ctx->tx_max &&
+		   (skb_out->len % dev->maxpacket) == 0) {
 		*skb_put(skb_out, 1) = 0;	/* force short packet */
+	}
 
 	/* set final frame length */
 	nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data;
-- 
1.9.1

[toc] | [next] | [standalone]

#1636046

Jim Baxter <jim_baxter@mentor.com> writes:

> The zero padding that is added to NTB's does
> not zero the memory correctly.
> This is because the skb_put modifies the value
> of skb_out->len which results in the memset
> command not setting any memory to zero as
> (ctx->tx_max - skb_out->len) == 0.
>
> I have resolved this by storing the size of
> the memory to be zeroed before the skb_put
> and using this in the memset call.
>
> Signed-off-by: Jim Baxter <jim_baxter@mentor.com>

Ouch! Thanks for finding this.  This should go to the stable queue as
well.

Reviewed-by: Bjørn Mork <bjorn@mork.no>

[toc] | [prev] | [next] | [standalone]

#1637368

Bjørn Mork <bjorn@mork.no> writes:
> 
> Ouch! Thanks for finding this.  This should go to the stable queue as
> well.
> 
> Reviewed-by: Bjørn Mork <bjorn@mork.no>
> 

Do I need to submit this to the stable queue myself?

-- Jim

[toc] | [prev] | [next] | [standalone]

#1637377

"Baxter, Jim" <jim_baxter@mentor.com> writes:

> Bjørn Mork <bjorn@mork.no> writes:
>> 
>> Ouch! Thanks for finding this.  This should go to the stable queue as
>> well.
>> 
>> Reviewed-by: Bjørn Mork <bjorn@mork.no>
>> 
>
> Do I need to submit this to the stable queue myself?

No, davem will handle that.

That is, assuming that you had posted this to netdev in the first
place... Sorry, I just assumed you did without verifying it when I
replied.  You will have to repost the patch to the appropriate
recepients.  Use

 scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c

for that.  Feel free to include my Reviewed-by tag. And if you mark the
patch for "stable" somewhere outside the commit message, then davem will
pick that up and add it to his stable queue in patchwork. I don't know
if there is any officially documented way to do this, but a "stable" tag
inside the [] brackets in the subject, or a comment below the --- marker,
usually works fine.

Note that the "Cc: stable..." method isn't used for netdev patches. See
https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt


Bjørn

[toc] | [prev] | [next] | [standalone]

#1637412

Bjørn Mork <bjorn@mork.no> writes:

> "Baxter, Jim" <jim_baxter@mentor.com> writes:
> 
>>
>> Do I need to submit this to the stable queue myself?
> 
> No, davem will handle that.
> 
> That is, assuming that you had posted this to netdev in the first
> place... Sorry, I just assumed you did without verifying it when I
> replied.  You will have to repost the patch to the appropriate
> recepients.  Use
> 
>  scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c
> 
> for that.  Feel free to include my Reviewed-by tag. And if you mark the
> patch for "stable" somewhere outside the commit message, then davem will
> pick that up and add it to his stable queue in patchwork. I don't know
> if there is any officially documented way to do this, but a "stable" tag
> inside the [] brackets in the subject, or a comment below the --- marker,
> usually works fine.
> 
> Note that the "Cc: stable..." method isn't used for netdev patches. See
> https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
> 
> 
> Bjørn
> 

Thank you, I will resend this to netdev.

I had used the command:
scripts/get_maintainer.pl drivers/net/usb/cdc_ncm.c --sections

so only sent it to the lists in the first section "USB CDC ETHERNET DRIVER".
I will remove the --sections in future.

Jim

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web