[Click] 2.6 update - Bug + Fix

[Paine, Thomas Asa]

Beyers,
	I applied your patch to my 2.6.16.13 this morning and is working
well, thanks Beyers (is that something you will check into the CVS?).
The memory leaks from my previous post are fixed as well, thanks Eddie.


Thanks, 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  
   Thomas Paine (paineta at uwec.edu) 
   University of Wisconsin - Eau Claire 
   garbage foo(garbage g){return(g);} 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 



-----Original Message-----
From: click-bounces at pdos.csail.mit.edu
[mailto:click-bounces at pdos.csail.mit.edu] On Behalf Of Beyers Cronje
Sent: Wednesday, June 28, 2006 12:37 AM
To: Eddie Kohler
Cc: Click
Subject: Re: [Click] 2.6 update - Bug + Fix

Hi Eddie,

I found the bug I reported earlier on the new etc/linux-2.6.16.13-patch
you posted below. The problem is that skb_recycle does a
atomic_sub_return on the skb dataref if the skb is cloned. If the skb
data-reference is more than 1, ie shared by another skb and the
recycling condition fails, skb_recycle calls  kfree_skbmem, which in
turns  calls skb_release_data which does another atomic_sub_return on
the dataref of the skb. This double subtraction on dataref on the same
skb will obviously cause skb_release_data to prematurely free the packet
data.

The attached patch fixes this by first checking using atomic_read on
dataref, and only calls atomic_sub after we know we can recycle the skb.

I'm not sure what performance impact the additional atomic operation
will have.

Cheers

Beyers Cronje

         if (skb->fclone == SKB_FCLONE_UNAVAILABLE
             && (!skb->cloned ||
-            !atomic_sub_return(skb->nohdr ? (1 << SKB_DATAREF_SHIFT) +
1 :
1,
-                       &skb_shinfo(skb)->dataref))) {
+            !(atomic_read(&skb_shinfo(skb)->dataref) - skb->nohdr ? (1 
+ <<
SKB_DATAREF_SHIFT) + 1 : 1))) {
+            if (skb->cloned)
+                atomic_sub(skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 :
1,&skb_shinfo(skb)->dataref);
             if (skb_shinfo(skb)->nr_frags) {
                 int i;
                 for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)



On 6/23/06, Eddie Kohler <kohler at cs.ucla.edu> wrote:
>
> Hi all,
>
> A brief update on Linux 2.6 kernel module support.  We've just checked

> in a fix that plugs a large memory leak of sk_buffs.  You will need to

> re-patch your kernels with the current etc/linux-2.6.16.13-patch.  
> FYI.
>
> Eddie
> _______________________________________________
> click mailing list
> click at amsterdam.lcs.mit.edu
> https://amsterdam.lcs.mit.edu/mailman/listinfo/click
>