* [PATCH] pktgen: fix transmission headers with frags=0
@ 2011-03-14 13:22 Daniel Turull
2011-03-14 13:56 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Turull @ 2011-03-14 13:22 UTC (permalink / raw)
To: netdev; +Cc: Robert Olsson, Jens Laas, Voravit Tanyingyong, Daniel Turull
The headers of pktgen were incorrectly added in a pktgen packet
without frags (frags=0). There was an offset in the pktgen headers.
The cause was in reusing the pgh variable as a return variable in skb_put
when adding the payload to the skb.
A rename of the variable is done.
Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
---
The PKTGEN magic (be9b e955) now starts in the correct offset.
Before the patch, it was starting at the end of the packet (be9b)
Capture from tcpdump:
before patch:
14:57:37.854812 IP 10.0.0.2.discard > 10.254.254.84.discard: UDP, length 18
0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
0x0010: 002e 0004 0000 2011 8767 0a00 0002 0afe
0x0020: fe54 0009 0009 001a 0000 0000 0000 b072
0x0030: 9102 00ea ffff 0010 0000 be9b
after patch:
14:44:32.896048 IP 10.0.0.2.discard > 10.217.234.56.discard: UDP, length 18
0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
0x0010: 002e 0000 0000 2011 9bac 0a00 0002 0ad9
0x0020: ea38 0009 0009 001a 0000 be9b e955 0000
0x0030: 0001 4d7e 1b09 0005 5f23 af00
---
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f0aec6c..5baa9d9 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2615,13 +2615,14 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
{
struct timeval timestamp;
struct pktgen_hdr *pgh;
+ void *data;
pgh = (struct pktgen_hdr *)skb_put(skb, sizeof(*pgh));
datalen -= sizeof(*pgh);
if (pkt_dev->nfrags <= 0) {
- pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
- memset(pgh + 1, 0, datalen);
+ data = skb_put(skb, datalen);
+ memset(data + 1, 0, datalen);
} else {
int frags = pkt_dev->nfrags;
int i, len;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] pktgen: fix transmission headers with frags=0
2011-03-14 13:22 [PATCH] pktgen: fix transmission headers with frags=0 Daniel Turull
@ 2011-03-14 13:56 ` Eric Dumazet
2011-03-14 14:13 ` Daniel Turull
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2011-03-14 13:56 UTC (permalink / raw)
To: Daniel Turull; +Cc: netdev, Robert Olsson, Jens Laas, Voravit Tanyingyong
Le lundi 14 mars 2011 à 14:22 +0100, Daniel Turull a écrit :
> The headers of pktgen were incorrectly added in a pktgen packet
> without frags (frags=0). There was an offset in the pktgen headers.
>
> The cause was in reusing the pgh variable as a return variable in skb_put
> when adding the payload to the skb.
>
> A rename of the variable is done.
>
> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
> ---
> The PKTGEN magic (be9b e955) now starts in the correct offset.
> Before the patch, it was starting at the end of the packet (be9b)
>
not exactly, but offseted +16 bytes
(sizeof(struct pktgen_hdr))
> Capture from tcpdump:
>
> before patch:
> 14:57:37.854812 IP 10.0.0.2.discard > 10.254.254.84.discard: UDP, length 18
> 0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
> 0x0010: 002e 0004 0000 2011 8767 0a00 0002 0afe
> 0x0020: fe54 0009 0009 001a 0000 0000 0000 b072
> 0x0030: 9102 00ea ffff 0010 0000 be9b
>
> after patch:
> 14:44:32.896048 IP 10.0.0.2.discard > 10.217.234.56.discard: UDP, length 18
> 0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
> 0x0010: 002e 0000 0000 2011 9bac 0a00 0002 0ad9
> 0x0020: ea38 0009 0009 001a 0000 be9b e955 0000
> 0x0030: 0001 4d7e 1b09 0005 5f23 af00
>
> ---
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index f0aec6c..5baa9d9 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2615,13 +2615,14 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
> {
> struct timeval timestamp;
> struct pktgen_hdr *pgh;
> + void *data;
>
> pgh = (struct pktgen_hdr *)skb_put(skb, sizeof(*pgh));
> datalen -= sizeof(*pgh);
>
> if (pkt_dev->nfrags <= 0) {
> - pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
> - memset(pgh + 1, 0, datalen);
> + data = skb_put(skb, datalen);
> + memset(data + 1, 0, datalen);
> } else {
> int frags = pkt_dev->nfrags;
> int i, len;
> --
Good catch !
Hmm this patch is not correct, why memset(data + 1, ...) ?
Also, this patch is needed for net-next-2.6 only
(bug introduced by commit 26ad787962ef84677a48c560
(pktgen: speedup fragmented skbs)
I would avoid the "void *data;" declaration and just use following :
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f0aec6c..f727c83 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2620,7 +2620,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
datalen -= sizeof(*pgh);
if (pkt_dev->nfrags <= 0) {
- pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
+ skb_put(skb, datalen);
memset(pgh + 1, 0, datalen);
} else {
int frags = pkt_dev->nfrags;
or even :
memset(skb_put(skb, datalen), 0, datalen);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] pktgen: fix transmission headers with frags=0
2011-03-14 13:56 ` Eric Dumazet
@ 2011-03-14 14:13 ` Daniel Turull
2011-03-14 14:18 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Turull @ 2011-03-14 14:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Robert Olsson, Jens Laas, Voravit Tanyingyong
On 03/14/2011 02:56 PM, Eric Dumazet wrote:
> Le lundi 14 mars 2011 à 14:22 +0100, Daniel Turull a écrit :
>> The headers of pktgen were incorrectly added in a pktgen packet
>> without frags (frags=0). There was an offset in the pktgen headers.
>>
>> The cause was in reusing the pgh variable as a return variable in skb_put
>> when adding the payload to the skb.
>>
>> A rename of the variable is done.
>>
>> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
>> ---
>> The PKTGEN magic (be9b e955) now starts in the correct offset.
>> Before the patch, it was starting at the end of the packet (be9b)
>>
>
> not exactly, but offseted +16 bytes
> (sizeof(struct pktgen_hdr))
True, I shouldn't say end.
>
>> Capture from tcpdump:
>>
>> before patch:
>> 14:57:37.854812 IP 10.0.0.2.discard > 10.254.254.84.discard: UDP, length 18
>> 0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
>> 0x0010: 002e 0004 0000 2011 8767 0a00 0002 0afe
>> 0x0020: fe54 0009 0009 001a 0000 0000 0000 b072
>> 0x0030: 9102 00ea ffff 0010 0000 be9b
>>
>> after patch:
>> 14:44:32.896048 IP 10.0.0.2.discard > 10.217.234.56.discard: UDP, length 18
>> 0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
>> 0x0010: 002e 0000 0000 2011 9bac 0a00 0002 0ad9
>> 0x0020: ea38 0009 0009 001a 0000 be9b e955 0000
>> 0x0030: 0001 4d7e 1b09 0005 5f23 af00
>>
>> ---
>> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
>> index f0aec6c..5baa9d9 100644
>> --- a/net/core/pktgen.c
>> +++ b/net/core/pktgen.c
>> @@ -2615,13 +2615,14 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
>> {
>> struct timeval timestamp;
>> struct pktgen_hdr *pgh;
>> + void *data;
>>
>> pgh = (struct pktgen_hdr *)skb_put(skb, sizeof(*pgh));
>> datalen -= sizeof(*pgh);
>>
>> if (pkt_dev->nfrags <= 0) {
>> - pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
>> - memset(pgh + 1, 0, datalen);
>> + data = skb_put(skb, datalen);
>> + memset(data + 1, 0, datalen);
>> } else {
>> int frags = pkt_dev->nfrags;
>> int i, len;
>> --
>
> Good catch !
>
> Hmm this patch is not correct, why memset(data + 1, ...) ?
I kept the +1 as it was in the original code, but I supposed it can be avoided.
> Also, this patch is needed for net-next-2.6 only
> (bug introduced by commit 26ad787962ef84677a48c560
> (pktgen: speedup fragmented skbs)
>
> I would avoid the "void *data;" declaration and just use following :
>
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index f0aec6c..f727c83 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -2620,7 +2620,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
> datalen -= sizeof(*pgh);
>
> if (pkt_dev->nfrags <= 0) {
> - pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
> + skb_put(skb, datalen);
> memset(pgh + 1, 0, datalen);
> } else {
> int frags = pkt_dev->nfrags;
>
>
>
> or even :
>
> memset(skb_put(skb, datalen), 0, datalen);
>
I think that the second is better and more compact.
Do I resend the new patch?
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pktgen: fix transmission headers with frags=0
2011-03-14 14:13 ` Daniel Turull
@ 2011-03-14 14:18 ` Eric Dumazet
2011-03-14 14:31 ` [PATCH net-next-2.6] pktgen: bug fix in " Daniel Turull
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2011-03-14 14:18 UTC (permalink / raw)
To: Daniel Turull; +Cc: netdev, Robert Olsson, Jens Laas, Voravit Tanyingyong
Le lundi 14 mars 2011 à 15:13 +0100, Daniel Turull a écrit :
> On 03/14/2011 02:56 PM, Eric Dumazet wrote:
> > Good catch !
> >
> > Hmm this patch is not correct, why memset(data + 1, ...) ?
>
> I kept the +1 as it was in the original code, but I supposed it can be avoided.
>
original code was assuming pgh was at right location.
pgh + 1 : really pointed to first byte after pktgen header.
data + 1 : points one byte off (so we leak one byte of kernel memory,
security guys would complain a lot...)
> I think that the second is better and more compact.
>
> Do I resend the new patch?
Yes please ;)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next-2.6] pktgen: bug fix in transmission headers with frags=0
2011-03-14 14:18 ` Eric Dumazet
@ 2011-03-14 14:31 ` Daniel Turull
2011-03-14 14:35 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Turull @ 2011-03-14 14:31 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Robert Olsson, Jens Laas, Voravit Tanyingyong
(bug introduced by commit 26ad787962ef84677a48c560
(pktgen: speedup fragmented skbs)
The headers of pktgen were incorrectly added in a pktgen packet
without frags (frags=0). There was an offset in the pktgen headers.
The cause was in reusing the pgh variable as a return variable in skb_put
when adding the payload to the skb.
Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
---
The PKTGEN magic (be9b e955) now starts in the correct offset.
Before the patch, it was starting with an offset of +16 bytes
(sizeof(struct pktgen_hdr)) (be9b)
Capture from tcpdump:
before patch:
14:57:37.854812 IP 10.0.0.2.discard > 10.254.254.84.discard: UDP, length 18
0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
0x0010: 002e 0004 0000 2011 8767 0a00 0002 0afe
0x0020: fe54 0009 0009 001a 0000 0000 0000 b072
0x0030: 9102 00ea ffff 0010 0000 be9b
after patch:
14:44:32.896048 IP 10.0.0.2.discard > 10.217.234.56.discard: UDP, length 18
0x0000: 001b 2157 ed84 001b 215d 01d0 0800 4500
0x0010: 002e 0000 0000 2011 9bac 0a00 0002 0ad9
0x0020: ea38 0009 0009 001a 0000 be9b e955 0000
0x0030: 0001 4d7e 1b09 0005 5f23 af00
---
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index f0aec6c..0c55eaa 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2620,8 +2620,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
datalen -= sizeof(*pgh);
if (pkt_dev->nfrags <= 0) {
- pgh = (struct pktgen_hdr *)skb_put(skb, datalen);
- memset(pgh + 1, 0, datalen);
+ memset(skb_put(skb, datalen), 0, datalen);
} else {
int frags = pkt_dev->nfrags;
int i, len;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] pktgen: bug fix in transmission headers with frags=0
2011-03-14 14:31 ` [PATCH net-next-2.6] pktgen: bug fix in " Daniel Turull
@ 2011-03-14 14:35 ` Eric Dumazet
2011-03-14 20:47 ` David Miller
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2011-03-14 14:35 UTC (permalink / raw)
To: Daniel Turull; +Cc: netdev, Robert Olsson, Jens Laas, Voravit Tanyingyong
Le lundi 14 mars 2011 à 15:31 +0100, Daniel Turull a écrit :
> (bug introduced by commit 26ad787962ef84677a48c560
> (pktgen: speedup fragmented skbs)
>
> The headers of pktgen were incorrectly added in a pktgen packet
> without frags (frags=0). There was an offset in the pktgen headers.
>
> The cause was in reusing the pgh variable as a return variable in skb_put
> when adding the payload to the skb.
>
> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks !
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next-2.6] pktgen: bug fix in transmission headers with frags=0
2011-03-14 14:35 ` Eric Dumazet
@ 2011-03-14 20:47 ` David Miller
0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2011-03-14 20:47 UTC (permalink / raw)
To: eric.dumazet; +Cc: daniel.turull, netdev, robert, jens.laas, voravit
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 14 Mar 2011 15:35:50 +0100
> Le lundi 14 mars 2011 à 15:31 +0100, Daniel Turull a écrit :
>> (bug introduced by commit 26ad787962ef84677a48c560
>> (pktgen: speedup fragmented skbs)
>>
>> The headers of pktgen were incorrectly added in a pktgen packet
>> without frags (frags=0). There was an offset in the pktgen headers.
>>
>> The cause was in reusing the pgh variable as a return variable in skb_put
>> when adding the payload to the skb.
>>
>> Signed-off-by: Daniel Turull <daniel.turull@gmail.com>
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-03-14 20:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-14 13:22 [PATCH] pktgen: fix transmission headers with frags=0 Daniel Turull
2011-03-14 13:56 ` Eric Dumazet
2011-03-14 14:13 ` Daniel Turull
2011-03-14 14:18 ` Eric Dumazet
2011-03-14 14:31 ` [PATCH net-next-2.6] pktgen: bug fix in " Daniel Turull
2011-03-14 14:35 ` Eric Dumazet
2011-03-14 20:47 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).