RFC: pskb_expand_head instead of skb_copy in skb_checksum_help

RFC: pskb_expand_head instead of skb_copy in skb_checksum_help

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

Hi:

Here is the patch that you wanted to shoot holes at :)

The idea is simple.  None of the callers of skb_checksum are passing it
skb's which are shared.  They may be cloned however.  But the application
checksum is always in the skb header so there is no need to linearise it.

Supposing all these assumptions are correct, then we can avoid the overhead
of skb_copy() and get away with pskb_expand_head().

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 586 bytes --]

===== net/core/dev.c 1.155 vs edited =====
--- 1.155/net/core/dev.c	2004-07-31 07:23:04 +10:00
+++ edited/net/core/dev.c	2004-08-24 20:59:57 +10:00
@@ -1144,16 +1144,10 @@
 		goto out;
 	}
 
-	if (skb_shared(*pskb)  || skb_cloned(*pskb)) {
-		struct sk_buff *newskb = skb_copy(*pskb, GFP_ATOMIC);
-		if (!newskb) {
-			ret = -ENOMEM;
+	if (skb_cloned(*pskb)) {
+		ret = pskb_expand_head(*pskb, 0, 0, GFP_ATOMIC);
+		if (ret)
 			goto out;
-		}
-		if ((*pskb)->sk)
-			skb_set_owner_w(newskb, (*pskb)->sk);
-		kfree_skb(*pskb);
-		*pskb = newskb;
 	}
 
 	if (offset > (int)(*pskb)->len)

Re: RFC: pskb_expand_head instead of skb_copy in skb_checksum_help

[David S. Miller]

On Tue, 24 Aug 2004 21:05:32 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> The idea is simple.  None of the callers of skb_checksum are passing it
> skb's which are shared.  They may be cloned however.  But the application
> checksum is always in the skb header so there is no need to linearise it.

What about looping back of multicast packets via netif_rx(),
is that handled correctly too?

I only mention it because that is the case that causes
problems elsewhere.

I'm going to think over this later today some more.

Re: RFC: pskb_expand_head instead of skb_copy in skb_checksum_help

[James Morris]

On Tue, 24 Aug 2004, David S. Miller wrote:

> On Tue, 24 Aug 2004 21:05:32 +1000
> Herbert Xu <herbert@gondor.apana.org.au> wrote:
> 
> > The idea is simple.  None of the callers of skb_checksum are passing it
> > skb's which are shared.  They may be cloned however.  But the application
> > checksum is always in the skb header so there is no need to linearise it.
> 
> What about looping back of multicast packets via netif_rx(),
> is that handled correctly too?

They are clones.  I can easily regression test it if needed.


- James
-- 
James Morris
<jmorris@redhat.com>

Re: RFC: pskb_expand_head instead of skb_copy in skb_checksum_help

[David S. Miller]

On Tue, 24 Aug 2004 21:05:32 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> Here is the patch that you wanted to shoot holes at :)
> 
> The idea is simple.  None of the callers of skb_checksum are passing it
> skb's which are shared.  They may be cloned however.  But the application
> checksum is always in the skb header so there is no need to linearise it.
> 
> Supposing all these assumptions are correct, then we can avoid the overhead
> of skb_copy() and get away with pskb_expand_head().

I'm going to apply this.

If your assertion is wrong, then the skb_shared() BUG() will trigger
in pskb_expand_head() and we will know :-)