[Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/linux/skbuff.h |    4 +++-
 net/core/datagram.c    |    2 +-
 net/core/skbuff.c      |   22 ++++++++++++++++++++++
 net/ipv4/arp.c         |    2 +-
 net/ipv4/udp.c         |    2 +-
 net/packet/af_packet.c |    2 +-
 6 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1f659e8..d328267 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -421,6 +421,7 @@ extern void skb_dma_unmap(struct device *dev, struct sk_buff *skb,
 #endif
 
 extern void kfree_skb(struct sk_buff *skb);
+extern void kfree_skb_clean(struct sk_buff *skb);
 extern void	       __kfree_skb(struct sk_buff *skb);
 extern struct sk_buff *__alloc_skb(unsigned int size,
 				   gfp_t priority, int fclone, int node);
@@ -459,7 +460,8 @@ extern int	       skb_to_sgvec(struct sk_buff *skb,
 extern int	       skb_cow_data(struct sk_buff *skb, int tailbits,
 				    struct sk_buff **trailer);
 extern int	       skb_pad(struct sk_buff *skb, int pad);
-#define dev_kfree_skb(a)	kfree_skb(a)
+#define dev_kfree_skb(a)	kfree_skb_clean(a)
+#define dev_kfree_skb_clean(a)	kfree_skb_clean(a)
 extern void	      skb_over_panic(struct sk_buff *skb, int len,
 				     void *here);
 extern void	      skb_under_panic(struct sk_buff *skb, int len,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 5e2ac0c..6cb2723 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -208,7 +208,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags,
 
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
 {
-	kfree_skb(skb);
+	kfree_skb_clean(skb);
 	sk_mem_reclaim_partial(sk);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e5e2111..4458ec8 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -65,6 +65,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/system.h>
+#include <trace/skb.h>
 
 #include "kmap_skb.h"
 
@@ -442,11 +443,32 @@ void kfree_skb(struct sk_buff *skb)
 		smp_rmb();
 	else if (likely(!atomic_dec_and_test(&skb->users)))
 		return;
+	trace_kfree_skb(skb, __builtin_return_address(0));
 	__kfree_skb(skb);
 }
 EXPORT_SYMBOL(kfree_skb);
 
 /**
+ *	kfree_skb_clean - free an skbuff
+ *	@skb: buffer to free
+ *
+ *	Drop a reference to the buffer and free it if the usage count has hit zero
+ *	Functions identically to kfree_skb, but kfree_skb assumes that the frame
+ *	is being dropped after a failure and notes that
+ */
+void kfree_skb_clean(struct sk_buff *skb)
+{
+	if (unlikely(!skb))
+		return;
+	if (likely(atomic_read(&skb->users) == 1))
+		smp_rmb();
+	else if (likely(!atomic_dec_and_test(&skb->users)))
+		return;
+	__kfree_skb(skb);
+}
+EXPORT_SYMBOL(kfree_skb_clean);
+
+/**
  *	skb_recycle_check - check if skb can be reused for receive
  *	@skb: buffer
  *	@skb_size: minimum receive buffer size
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 3f6b735..61e45c1 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -892,7 +892,7 @@ static int arp_process(struct sk_buff *skb)
 out:
 	if (in_dev)
 		in_dev_put(in_dev);
-	kfree_skb(skb);
+	kfree_skb_clean(skb);
 	return 0;
 }
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 4bd178a..e99ee8c 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1184,7 +1184,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sk = sknext;
 		} while (sknext);
 	} else
-		kfree_skb(skb);
+		kfree_skb_clean(skb);
 	spin_unlock(&hslot->lock);
 	return 0;
 }
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d8cc006..fdc9817 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -584,7 +584,7 @@ drop_n_restore:
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb(skb);
+	kfree_skb_clean(skb);
 	return 0;
 }
 

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Arnaldo Carvalho de Melo]

Em Tue, Mar 03, 2009 at 12:03:07PM -0500, Neil Horman escreveu:
> 
> Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Why just not add a "drop_skb()" instead? kfree_skb_clean sounds as if we
will do some cleanup on the skb being freed. "drop_skb()" seems much
clearer.

- Arnaldo

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

On Wed, Mar 04, 2009 at 10:54:15AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 03, 2009 at 12:03:07PM -0500, Neil Horman escreveu:
> > 
> > Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> 
> Why just not add a "drop_skb()" instead? kfree_skb_clean sounds as if we
> will do some cleanup on the skb being freed. "drop_skb()" seems much
> clearer.
> 
> - Arnaldo
> 

Check the RFC thread.  The cases in which we are dropping far outnumber the set
of cases in which we free the skb simply because its the end of the line.
Theres far less churn this way

Neil

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Arnaldo Carvalho de Melo]

Em Wed, Mar 04, 2009 at 09:18:31AM -0500, Neil Horman escreveu:
> On Wed, Mar 04, 2009 at 10:54:15AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Mar 03, 2009 at 12:03:07PM -0500, Neil Horman escreveu:
> > > 
> > > Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > Why just not add a "drop_skb()" instead? kfree_skb_clean sounds as if we
> > will do some cleanup on the skb being freed. "drop_skb()" seems much
> > clearer.
> > 
> > - Arnaldo
> > 
> 
> Check the RFC thread.  The cases in which we are dropping far outnumber the set
> of cases in which we free the skb simply because its the end of the line.
> Theres far less churn this way

I wouldn't mind the churn, as I really think drop_skb() would be
clearer, but then this is up to Dave.

- Arnaldo

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[David Miller]

From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Wed, 4 Mar 2009 13:13:35 -0300

> I wouldn't mind the churn, as I really think drop_skb() would be
> clearer, but then this is up to Dave.

I mind the churn, I have to apply this thing and deal with
the merge conflicts :-)

Why not come up with a clever alternative name for kfree_skb_clean()?

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Arnaldo Carvalho de Melo]

Em Wed, Mar 04, 2009 at 01:06:14PM -0800, David Miller escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Wed, 4 Mar 2009 13:13:35 -0300
> 
> > I wouldn't mind the churn, as I really think drop_skb() would be
> > clearer, but then this is up to Dave.
> 
> I mind the churn, I have to apply this thing and deal with
> the merge conflicts :-)
> 
> Why not come up with a clever alternative name for kfree_skb_clean()?

Oh well, sometimes churn is inevitable, but lemme try to figure out some
more clearer names for kfree_skb_clean:

rest_in_peace_skb()
here_lies_a_productive_skb()
kfree_not_dropped_skb()
kfree_worthy_skb()
consumed_skb()
hasta_la_vista_skbaby()
kfree_used_skb()

Perhaps the last one, huh? :-)

- Arnaldo

P.S.: </joke'ishy moment>

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[David Miller]

From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Wed, 4 Mar 2009 19:45:05 -0300

> hasta_la_vista_skbaby()

This is my personal favorite, it's very Mexifornia.

> consumed_skb()

But more seriously I like something like "consume_skb()" the best (ie.
drop the 'd').  It has the implication that an application or other
entity "consumed" and used the data before we freed the SKB.

What do you think?

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Arnaldo Carvalho de Melo]

Em Wed, Mar 04, 2009 at 02:47:22PM -0800, David Miller escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Wed, 4 Mar 2009 19:45:05 -0300
> 
> > hasta_la_vista_skbaby()
> 
> This is my personal favorite, it's very Mexifornia.

ROTFL, I have to get that book someday... For now I'm reading a brick
about post-'45 Europe :-)
 
> > consumed_skb()
> 
> But more seriously I like something like "consume_skb()" the best (ie.
> drop the 'd').  It has the implication that an application or other
> entity "consumed" and used the data before we freed the SKB.
> 
> What do you think?

That would be better than kfree_skb_clean, yes.

- Arnaldo

P.S.: Neil, I didn't managed to get to the other aspects of your work,
sorry about being so picky :-\

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Patrick McHardy]

David Miller wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Wed, 4 Mar 2009 19:45:05 -0300
> 
>> hasta_la_vista_skbaby()
> 
> This is my personal favorite, it's very Mexifornia.

My clear favourite as well. Maybe for some procedure that leaks
the skb :)

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

Ok, as requested, new version of this patch to add in bifurcation of kfree_skb
into two versions, one for legitimate kfree's, and one for drops

Modification Notes:
1) Renamed kfree_skb_clean to consume_skb, to better identify points where we
are finished with an skb

2) checkpatch cleanups

Thanks!

Neil

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/linux/skbuff.h |    4 +++-
 net/core/datagram.c    |    2 +-
 net/core/skbuff.c      |   22 ++++++++++++++++++++++
 net/ipv4/arp.c         |    2 +-
 net/ipv4/udp.c         |    2 +-
 net/packet/af_packet.c |    2 +-
 6 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1f659e8..1fbab2a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -421,6 +421,7 @@ extern void skb_dma_unmap(struct device *dev, struct sk_buff *skb,
 #endif
 
 extern void kfree_skb(struct sk_buff *skb);
+extern void consume_skb(struct sk_buff *skb);
 extern void	       __kfree_skb(struct sk_buff *skb);
 extern struct sk_buff *__alloc_skb(unsigned int size,
 				   gfp_t priority, int fclone, int node);
@@ -459,7 +460,8 @@ extern int	       skb_to_sgvec(struct sk_buff *skb,
 extern int	       skb_cow_data(struct sk_buff *skb, int tailbits,
 				    struct sk_buff **trailer);
 extern int	       skb_pad(struct sk_buff *skb, int pad);
-#define dev_kfree_skb(a)	kfree_skb(a)
+#define dev_kfree_skb(a)	consume_skb(a)
+#define dev_consume_skb(a)	kfree_skb_clean(a)
 extern void	      skb_over_panic(struct sk_buff *skb, int len,
 				     void *here);
 extern void	      skb_under_panic(struct sk_buff *skb, int len,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 5e2ac0c..d0de644 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -208,7 +208,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags,
 
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
 {
-	kfree_skb(skb);
+	consume_skb(skb);
 	sk_mem_reclaim_partial(sk);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e5e2111..6acbf9e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -65,6 +65,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/system.h>
+#include <trace/skb.h>
 
 #include "kmap_skb.h"
 
@@ -442,11 +443,32 @@ void kfree_skb(struct sk_buff *skb)
 		smp_rmb();
 	else if (likely(!atomic_dec_and_test(&skb->users)))
 		return;
+	trace_kfree_skb(skb, __builtin_return_address(0));
 	__kfree_skb(skb);
 }
 EXPORT_SYMBOL(kfree_skb);
 
 /**
+ *	consume_skb - free an skbuff
+ *	@skb: buffer to free
+ *
+ *	Drop a ref to the buffer and free it if the usage count has hit zero
+ *	Functions identically to kfree_skb, but kfree_skb assumes that the frame
+ *	is being dropped after a failure and notes that
+ */
+void consume_skb(struct sk_buff *skb)
+{
+	if (unlikely(!skb))
+		return;
+	if (likely(atomic_read(&skb->users) == 1))
+		smp_rmb();
+	else if (likely(!atomic_dec_and_test(&skb->users)))
+		return;
+	__kfree_skb(skb);
+}
+EXPORT_SYMBOL(consume_skb);
+
+/**
  *	skb_recycle_check - check if skb can be reused for receive
  *	@skb: buffer
  *	@skb_size: minimum receive buffer size
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 3f6b735..e89f7b7 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -892,7 +892,7 @@ static int arp_process(struct sk_buff *skb)
 out:
 	if (in_dev)
 		in_dev_put(in_dev);
-	kfree_skb(skb);
+	consume_skb(skb);
 	return 0;
 }
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 4bd178a..05b7abb 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1184,7 +1184,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sk = sknext;
 		} while (sknext);
 	} else
-		kfree_skb(skb);
+		consume_skb(skb);
 	spin_unlock(&hslot->lock);
 	return 0;
 }
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d8cc006..74776de 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -584,7 +584,7 @@ drop_n_restore:
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb(skb);
+	consume_skb(skb);
 	return 0;
 }
 

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 5 Mar 2009 16:01:05 -0500

> Ok, as requested, new version of this patch to add in bifurcation of kfree_skb
> into two versions, one for legitimate kfree's, and one for drops
> 
> Modification Notes:
> 1) Renamed kfree_skb_clean to consume_skb, to better identify points where we
> are finished with an skb
> 
> 2) checkpatch cleanups

Looks good.

Respin everything with the requested changes I asked for in #4
and I can likely toss this into net-next-2.6, thanks!

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Eric Dumazet]

David Miller a écrit :
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Thu, 5 Mar 2009 16:01:05 -0500
> 
>> Ok, as requested, new version of this patch to add in bifurcation of kfree_skb
>> into two versions, one for legitimate kfree's, and one for drops
>>
>> Modification Notes:
>> 1) Renamed kfree_skb_clean to consume_skb, to better identify points where we
>> are finished with an skb
>>
>> 2) checkpatch cleanups
> 
> Looks good.
> 
> Respin everything with the requested changes I asked for in #4
> and I can likely toss this into net-next-2.6, thanks!

I was wondering if packets droped at NIC level are handled by "Network Drop Monitor".
(See recent discussion about Multicast packet loss)

I believe not, maybe I missed something ...

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[David Miller]

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed, 11 Mar 2009 17:34:47 +0100

> David Miller a écrit :
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Thu, 5 Mar 2009 16:01:05 -0500
> > 
> >> Ok, as requested, new version of this patch to add in bifurcation of kfree_skb
> >> into two versions, one for legitimate kfree's, and one for drops
> >>
> >> Modification Notes:
> >> 1) Renamed kfree_skb_clean to consume_skb, to better identify points where we
> >> are finished with an skb
> >>
> >> 2) checkpatch cleanups
> > 
> > Looks good.
> > 
> > Respin everything with the requested changes I asked for in #4
> > and I can likely toss this into net-next-2.6, thanks!
> 
> I was wondering if packets droped at NIC level are handled by "Network Drop Monitor".
> (See recent discussion about Multicast packet loss)
> 
> I believe not, maybe I missed something ...

No, it's just a "software network drop monitor"

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

On Wed, Mar 11, 2009 at 05:34:47PM +0100, Eric Dumazet wrote:
> David Miller a écrit :
> > From: Neil Horman <nhorman@tuxdriver.com>
> > Date: Thu, 5 Mar 2009 16:01:05 -0500
> > 
> >> Ok, as requested, new version of this patch to add in bifurcation of kfree_skb
> >> into two versions, one for legitimate kfree's, and one for drops
> >>
> >> Modification Notes:
> >> 1) Renamed kfree_skb_clean to consume_skb, to better identify points where we
> >> are finished with an skb
> >>
> >> 2) checkpatch cleanups
> > 
> > Looks good.
> > 
> > Respin everything with the requested changes I asked for in #4
> > and I can likely toss this into net-next-2.6, thanks!
> 
> I was wondering if packets droped at NIC level are handled by "Network Drop Monitor".
> (See recent discussion about Multicast packet loss)
> 
> I believe not, maybe I missed something ...
> 
No, unfortunately, theres no really good way to detect drops at the driver level
without polling them periodically, which I think is somewhat in opposition to the
point of this utility.  I'd welcome suggestions however.
Neil

> 

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

On Tue, Mar 03, 2009 at 12:03:07PM -0500, Neil Horman wrote:
> 
> 
As requested, respinning against latest net-next tree.

Modification notes:

None, simple respin

Neil


Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>


 include/linux/skbuff.h |    4 +++-
 net/core/datagram.c    |    2 +-
 net/core/skbuff.c      |   22 ++++++++++++++++++++++
 net/ipv4/arp.c         |    2 +-
 net/ipv4/udp.c         |    2 +-
 net/packet/af_packet.c |    2 +-
 6 files changed, 29 insertions(+), 5 deletions(-)


diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1f659e8..1fbab2a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -421,6 +421,7 @@ extern void skb_dma_unmap(struct device *dev, struct sk_buff *skb,
 #endif
 
 extern void kfree_skb(struct sk_buff *skb);
+extern void consume_skb(struct sk_buff *skb);
 extern void	       __kfree_skb(struct sk_buff *skb);
 extern struct sk_buff *__alloc_skb(unsigned int size,
 				   gfp_t priority, int fclone, int node);
@@ -459,7 +460,8 @@ extern int	       skb_to_sgvec(struct sk_buff *skb,
 extern int	       skb_cow_data(struct sk_buff *skb, int tailbits,
 				    struct sk_buff **trailer);
 extern int	       skb_pad(struct sk_buff *skb, int pad);
-#define dev_kfree_skb(a)	kfree_skb(a)
+#define dev_kfree_skb(a)	consume_skb(a)
+#define dev_consume_skb(a)	kfree_skb_clean(a)
 extern void	      skb_over_panic(struct sk_buff *skb, int len,
 				     void *here);
 extern void	      skb_under_panic(struct sk_buff *skb, int len,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 5e2ac0c..d0de644 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -208,7 +208,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags,
 
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
 {
-	kfree_skb(skb);
+	consume_skb(skb);
 	sk_mem_reclaim_partial(sk);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e5e2111..6acbf9e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -65,6 +65,7 @@
 
 #include <asm/uaccess.h>
 #include <asm/system.h>
+#include <trace/skb.h>
 
 #include "kmap_skb.h"
 
@@ -442,11 +443,32 @@ void kfree_skb(struct sk_buff *skb)
 		smp_rmb();
 	else if (likely(!atomic_dec_and_test(&skb->users)))
 		return;
+	trace_kfree_skb(skb, __builtin_return_address(0));
 	__kfree_skb(skb);
 }
 EXPORT_SYMBOL(kfree_skb);
 
 /**
+ *	consume_skb - free an skbuff
+ *	@skb: buffer to free
+ *
+ *	Drop a ref to the buffer and free it if the usage count has hit zero
+ *	Functions identically to kfree_skb, but kfree_skb assumes that the frame
+ *	is being dropped after a failure and notes that
+ */
+void consume_skb(struct sk_buff *skb)
+{
+	if (unlikely(!skb))
+		return;
+	if (likely(atomic_read(&skb->users) == 1))
+		smp_rmb();
+	else if (likely(!atomic_dec_and_test(&skb->users)))
+		return;
+	__kfree_skb(skb);
+}
+EXPORT_SYMBOL(consume_skb);
+
+/**
  *	skb_recycle_check - check if skb can be reused for receive
  *	@skb: buffer
  *	@skb_size: minimum receive buffer size
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 3d67d1f..9c22032 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -892,7 +892,7 @@ static int arp_process(struct sk_buff *skb)
 out:
 	if (in_dev)
 		in_dev_put(in_dev);
-	kfree_skb(skb);
+	consume_skb(skb);
 	return 0;
 }
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 4bd178a..05b7abb 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1184,7 +1184,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 			sk = sknext;
 		} while (sknext);
 	} else
-		kfree_skb(skb);
+		consume_skb(skb);
 	spin_unlock(&hslot->lock);
 	return 0;
 }
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index d8cc006..74776de 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -584,7 +584,7 @@ drop_n_restore:
 		skb->len = skb_len;
 	}
 drop:
-	kfree_skb(skb);
+	consume_skb(skb);
 	return 0;
 }
 

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Evgeniy Polyakov]

On Wed, Mar 11, 2009 at 01:07:01PM -0400, Neil Horman (nhorman@tuxdriver.com) wrote:
> > I was wondering if packets droped at NIC level are handled by "Network Drop Monitor".
> > (See recent discussion about Multicast packet loss)
> > 
> > I believe not, maybe I missed something ...
> > 
> No, unfortunately, theres no really good way to detect drops at the driver level
> without polling them periodically, which I think is somewhat in opposition to the
> point of this utility.  I'd welcome suggestions however.

It could be possible to check softnet stats difference when napi is
scheduled or drop monitor is about to send a packet to the userspace
about different drop.

-- 
	Evgeniy Polyakov

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[Neil Horman]

On Thu, Mar 12, 2009 at 12:39:02AM +0300, Evgeniy Polyakov wrote:
> On Wed, Mar 11, 2009 at 01:07:01PM -0400, Neil Horman (nhorman@tuxdriver.com) wrote:
> > > I was wondering if packets droped at NIC level are handled by "Network Drop Monitor".
> > > (See recent discussion about Multicast packet loss)
> > > 
> > > I believe not, maybe I missed something ...
> > > 
> > No, unfortunately, theres no really good way to detect drops at the driver level
> > without polling them periodically, which I think is somewhat in opposition to the
> > point of this utility.  I'd welcome suggestions however.
> 
> It could be possible to check softnet stats difference when napi is
> scheduled or drop monitor is about to send a packet to the userspace
> about different drop.
> 
Those are both good ideas.  I think I like the former suggestion, since the
latter requires that a drop in the stack must occur to detect a stack closer to
the hardware.  Thank you, I'll keep that in mind for a subsequent enhancement.

Regards
Neil

> -- 
> 	Evgeniy Polyakov
> 

Re: [Patch 3/5] Network Drop Monitor: Adding kfree_skb_clean for non-drops and modifying end-of-line points for skbs

[David Miller]

From: Neil Horman <nhorman@tuxdriver.com>
Date: Wed, 11 Mar 2009 15:49:55 -0400

> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Applied.