netdev
[Top] [All Lists]

Re: [PATCH] Handle shared SKBs in VLAN receive code

To: Tommy Christensen <tommy.christensen@xxxxxxxxx>
Subject: Re: [PATCH] Handle shared SKBs in VLAN receive code
From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
Date: Fri, 10 Oct 2003 16:52:43 -0700
Cc: netdev@xxxxxxxxxxx
In-reply-to: <3F87430D.2040000@xxxxxxxxx>
Organization: Candela Technologies
References: <3F87430D.2040000@xxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5b) Gecko/20030827
Tommy Christensen wrote:
As noted by Jonas Munsin, the vlan patch that went into 2.6.0-test7 did a
kfree() rather than a kfree_skb(). Hmm.

Instead of just fixing this up, I suggest we do things the right way:

The VLAN code have long been claiming to handle shared SKBs, without actually
doing so. I have now added the call to skb_share_check().

This enables us to simply do a skb_unshare() when mangling the ethernet header.


Patch is against linux-2.6.0-test7 (applies to 2.4.23-pre7 as well).

-Tommy


------------------------------------------------------------------------

diff -ru linux-2.6.0-test7/net/8021q/vlan_dev.c linux-2.6/net/8021q/vlan_dev.c
--- linux-2.6.0-test7/net/8021q/vlan_dev.c      Wed Oct  8 21:24:44 2003
+++ linux-2.6/net/8021q/vlan_dev.c      Sat Oct 11 00:45:28 2003
@@ -74,11 +74,7 @@
 static inline struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
 {
        if (VLAN_DEV_INFO(skb->dev)->flags & 1) {
-               if (skb_shared(skb) || skb_cloned(skb)) {
-                       struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
-                       kfree(skb);
-                       skb = nskb;
-               }
+               skb = skb_unshare(skb, GFP_ATOMIC);

On 2.4.22, at least, skb_unshare only checks skb_cloned(), so do we
also need to check skb_shared()?  Or was that check in the old patch above
not needed?

Ben

--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com



<Prev in Thread] Current Thread [Next in Thread>