Re: [PATCH] pktgen: fix transmission headers with frags=0

[Eric Dumazet <eric.dumazet@gmail.com>]

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);