* [PATCH net-next] net: skb_orphan() changes
@ 2013-07-30 23:11 Eric Dumazet
2013-07-31 22:24 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2013-07-30 23:11 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
It is illegal to set skb->sk without corresponding destructor.
Its therefore safe for skb_orphan() to not clear skb->sk if
skb->destructor is not set.
Also avoid clearing skb->destructor if already NULL.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/skbuff.h | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5afefa0..a95547a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1805,10 +1805,11 @@ static inline void pskb_trim_unique(struct sk_buff *skb, unsigned int len)
*/
static inline void skb_orphan(struct sk_buff *skb)
{
- if (skb->destructor)
+ if (skb->destructor) {
skb->destructor(skb);
- skb->destructor = NULL;
- skb->sk = NULL;
+ skb->destructor = NULL;
+ skb->sk = NULL;
+ }
}
/**
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: skb_orphan() changes
2013-07-30 23:11 [PATCH net-next] net: skb_orphan() changes Eric Dumazet
@ 2013-07-31 22:24 ` David Miller
2013-07-31 22:44 ` Eric Dumazet
0 siblings, 1 reply; 4+ messages in thread
From: David Miller @ 2013-07-31 22:24 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 30 Jul 2013 16:11:15 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> It is illegal to set skb->sk without corresponding destructor.
>
> Its therefore safe for skb_orphan() to not clear skb->sk if
> skb->destructor is not set.
>
> Also avoid clearing skb->destructor if already NULL.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, but personally I would have considered adding an
else clause checking BUG_ON(skb->sk), or are you not so
confident in your assertion? :-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: skb_orphan() changes
2013-07-31 22:24 ` David Miller
@ 2013-07-31 22:44 ` Eric Dumazet
2013-07-31 23:44 ` David Miller
0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2013-07-31 22:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev
On Wed, 2013-07-31 at 15:24 -0700, David Miller wrote:
> Applied, but personally I would have considered adding an
> else clause checking BUG_ON(skb->sk), or are you not so
> confident in your assertion? :-)
I considered this but this was adding a significant overhead, as this
is an inline function.
What about doing that for a limited time, say until 3.12 is released ?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: skb_orphan() changes
2013-07-31 22:44 ` Eric Dumazet
@ 2013-07-31 23:44 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2013-07-31 23:44 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 31 Jul 2013 15:44:51 -0700
> On Wed, 2013-07-31 at 15:24 -0700, David Miller wrote:
>
>> Applied, but personally I would have considered adding an
>> else clause checking BUG_ON(skb->sk), or are you not so
>> confident in your assertion? :-)
>
> I considered this but this was adding a significant overhead, as this
> is an inline function.
>
> What about doing that for a limited time, say until 3.12 is released ?
That sounds like a great idea.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-31 23:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-30 23:11 [PATCH net-next] net: skb_orphan() changes Eric Dumazet
2013-07-31 22:24 ` David Miller
2013-07-31 22:44 ` Eric Dumazet
2013-07-31 23:44 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).