This makes the networking layer warn when something tries to

charge an skb to a socket that already is charged to another

socket (or has a destructor from somewhere else.)
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---

Not sure if the code size increase is prohibitive, haven't really

checked. Might have helped in early mac80211 days (when we didn't

have the skb_orphan call) but for normal skb usage this shouldn't

ever happen.
 include/net/sock.h |    2 ++

 1 file changed, 2 insertions(+)
--- everything.orig/include/net/sock.h	2008-05-03 15:51:36.000000000 +0200

+++ everything/include/net/sock.h	2008-05-03 15:51:56.000000000 +0200

@@ -1167,6 +1167,7 @@ static inline void skb_set_owner_w(struc

 {

 	sock_hold(sk);

 	skb->sk = sk;

+	WARN_ON(skb->destructor);

 	skb->destructor = sock_wfree;

 	atomic_add(skb->truesize, &sk->sk_wmem_alloc);

 }

@@ -1174,6 +1175,7 @@ static inline void skb_set_owner_w(struc

 static inline void skb_set_owner_r(struct sk_buff *skb, struct sock *sk)

 {

 	skb->sk = sk;

+	WARN_ON(skb->destructor);

 	skb->destructor = sock_rfree;

 	atomic_add(skb->truesize, &sk->sk_rmem_alloc);

 	sk_mem_charge(sk, skb->truesize);
--

From: Johannes Berg <johannes@sipsolutions.net>
I completely agree that this a correct assertion, that

absolutely should never happen in the current tree.
But if we are going to do this, I'm pretty sure we'll

need to uninline these two routines.
I'll leave it up to you, if you think it's still worthwhile

with the uninlining, please resubmit :-)

--

Given that it's a corner case and no regular net drivers would ever want

to do something that could possibly cause this condition to be true,

it's probably not worth it either way. Even mac80211 doesn't run into

it, although it might have helped a bit to remember to add skb_orphan()

to the re-injection.
If we decide to uninline those functions for another reason (used too

much, code size, ...) then we can still do that.
johannes

From: Johannes Berg <johannes@sipsolutions.net>
Agreed.

--

According to my measurements the size bloat of those two is

(x86/32bit, gcc 4.1.2 redhat something):
-1091  40 funcs, 89 +, 1180 -, diff: -1091 --- skb_set_owner_r

-495  46 funcs, 70 +, 565 -, diff: -495 --- skb_set_owner_w
--

 i.

--

From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
That's not too bad, but adding the WARN_ON() we're discussing

will plump that up a bit, percentage wise, which is why I

said we should inline it in such a case.

--

s/inline/uninline/g
According to some long e-mail today its worth ~198 unexport patches! 8-)
- Arnaldo

--

> > From: "Ilpo_J

So do we want to out-line them? I don't really know the scale involved.

If so I can resubmit this patch with them outlined and the warnings

added (but did you account for the EXPORT_SYMBOL space?)
johannes

On Mon, 5 May 2008, Johannes Berg wrote:
> On Mon, 2008-05-05 at 12:43 +0300, Ilpo J

I guess it's not much, it seems to be an unsigned long, a pointer and

the name string.
johannes