This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
Combinating with tracepoint on dev_hard_start_xmit, we can check how long it
takes to free transmited packets. And using it, we can calculate how many
packets driver had at that time. It is useful when a drop of transmited packet
is a problem.
<idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
<idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
Signed-off-by: Koki Sanagi <[email protected]>
---
include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
net/core/dev.c | 2 ++
net/core/skbuff.c | 1 +
3 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 4b2be6d..6ab5b34 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
__entry->skbaddr, __entry->protocol, __entry->location)
);
+TRACE_EVENT(consume_skb,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p",
+ __entry->skbaddr)
+);
+
+TRACE_EVENT(dev_kfree_skb_irq,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field( void *, skbaddr )
+ ),
+
+ TP_fast_assign(
+ __entry->skbaddr = skb;
+ ),
+
+ TP_printk("skbaddr=%p",
+ __entry->skbaddr)
+);
+
TRACE_EVENT(skb_copy_datagram_iovec,
TP_PROTO(const struct sk_buff *skb, int len),
diff --git a/net/core/dev.c b/net/core/dev.c
index 4b64b21..807b1ca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -131,6 +131,7 @@
#include <linux/random.h>
#include <trace/events/napi.h>
#include <trace/events/net.h>
+#include <trace/events/skb.h>
#include <linux/pci.h>
#include "net-sysfs.h"
@@ -1580,6 +1581,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
struct softnet_data *sd;
unsigned long flags;
+ trace_dev_kfree_skb_irq(skb);
local_irq_save(flags);
sd = &__get_cpu_var(softnet_data);
skb->next = sd->completion_queue;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 34432b4..a7b4036 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
smp_rmb();
else if (likely(!atomic_dec_and_test(&skb->users)))
return;
+ trace_consume_skb(skb);
__kfree_skb(skb);
}
EXPORT_SYMBOL(consume_skb);
Le vendredi 25 juin 2010 à 09:59 +0900, Koki Sanagi a écrit :
> This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
> Combinating with tracepoint on dev_hard_start_xmit, we can check how long it
> takes to free transmited packets. And using it, we can calculate how many
> packets driver had at that time. It is useful when a drop of transmited packet
> is a problem.
>
> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>
> Signed-off-by: Koki Sanagi <[email protected]>
> ---
> include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
> net/core/dev.c | 2 ++
> net/core/skbuff.c | 1 +
> 3 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> index 4b2be6d..6ab5b34 100644
> --- a/include/trace/events/skb.h
> +++ b/include/trace/events/skb.h
> @@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
> __entry->skbaddr, __entry->protocol, __entry->location)
> );
>
> +TRACE_EVENT(consume_skb,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field( void *, skbaddr )
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p",
> + __entry->skbaddr)
> +);
> +
> +TRACE_EVENT(dev_kfree_skb_irq,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field( void *, skbaddr )
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p",
> + __entry->skbaddr)
> +);
> +
> TRACE_EVENT(skb_copy_datagram_iovec,
>
> TP_PROTO(const struct sk_buff *skb, int len),
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4b64b21..807b1ca 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -131,6 +131,7 @@
> #include <linux/random.h>
> #include <trace/events/napi.h>
> #include <trace/events/net.h>
> +#include <trace/events/skb.h>
> #include <linux/pci.h>
>
> #include "net-sysfs.h"
> @@ -1580,6 +1581,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
> struct softnet_data *sd;
> unsigned long flags;
>
> + trace_dev_kfree_skb_irq(skb);
> local_irq_save(flags);
> sd = &__get_cpu_var(softnet_data);
> skb->next = sd->completion_queue;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 34432b4..a7b4036 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
> smp_rmb();
> else if (likely(!atomic_dec_and_test(&skb->users)))
> return;
> + trace_consume_skb(skb);
> __kfree_skb(skb);
> }
> EXPORT_SYMBOL(consume_skb);
>
You might add a trace point to skb_free_datagram_locked() too, since it
contains an inlined consume_skb()
(2010/06/25 13:55), Eric Dumazet wrote:
> Le vendredi 25 juin 2010 à 09:59 +0900, Koki Sanagi a écrit :
>> This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
>> Combinating with tracepoint on dev_hard_start_xmit, we can check how long it
>> takes to free transmited packets. And using it, we can calculate how many
>> packets driver had at that time. It is useful when a drop of transmited packet
>> is a problem.
>>
>> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
>> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>>
>> Signed-off-by: Koki Sanagi <[email protected]>
>> ---
>> include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
>> net/core/dev.c | 2 ++
>> net/core/skbuff.c | 1 +
>> 3 files changed, 39 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
>> index 4b2be6d..6ab5b34 100644
>> --- a/include/trace/events/skb.h
>> +++ b/include/trace/events/skb.h
>> @@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
>> __entry->skbaddr, __entry->protocol, __entry->location)
>> );
>>
>> +TRACE_EVENT(consume_skb,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + ),
>> +
>> + TP_printk("skbaddr=%p",
>> + __entry->skbaddr)
>> +);
>> +
>> +TRACE_EVENT(dev_kfree_skb_irq,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + ),
>> +
>> + TP_printk("skbaddr=%p",
>> + __entry->skbaddr)
>> +);
>> +
>> TRACE_EVENT(skb_copy_datagram_iovec,
>>
>> TP_PROTO(const struct sk_buff *skb, int len),
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4b64b21..807b1ca 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -131,6 +131,7 @@
>> #include <linux/random.h>
>> #include <trace/events/napi.h>
>> #include <trace/events/net.h>
>> +#include <trace/events/skb.h>
>> #include <linux/pci.h>
>>
>> #include "net-sysfs.h"
>> @@ -1580,6 +1581,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
>> struct softnet_data *sd;
>> unsigned long flags;
>>
>> + trace_dev_kfree_skb_irq(skb);
>> local_irq_save(flags);
>> sd = &__get_cpu_var(softnet_data);
>> skb->next = sd->completion_queue;
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index 34432b4..a7b4036 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
>> smp_rmb();
>> else if (likely(!atomic_dec_and_test(&skb->users)))
>> return;
>> + trace_consume_skb(skb);
>> __kfree_skb(skb);
>> }
>> EXPORT_SYMBOL(consume_skb);
>>
>
>
> You might add a trace point to skb_free_datagram_locked() too, since it
> contains an inlined consume_skb()
>
I think it is contrary.
skb_free_datagram_locked() contains consume_skb(), so tracepoint isn't needed.
Because skb_free_datagram_locked() can be traced by trace_consume_skb().
Le vendredi 25 juin 2010 à 16:12 +0900, Koki Sanagi a écrit :
> > You might add a trace point to skb_free_datagram_locked() too, since it
> > contains an inlined consume_skb()
> >
>
> I think it is contrary.
I think you are _very_ wrong.
> skb_free_datagram_locked() contains consume_skb(), so tracepoint isn't needed.
> Because skb_free_datagram_locked() can be traced by trace_consume_skb().
>
>
>
Koki, it would be good if you worked on net-next-2.6, so that my comment
applies.
Also, not sending this kind of patches on netdev is not going to help
very much.
Who is supposed to review them on lkml and Ack them ?
Nobody.
Please build your future network related patches against net-next-2.6,
and send them to nedev and David Miller, the official network
maintainer, as stated in MAINTAINERS file.
NETWORKING [GENERAL]
M: "David S. Miller" <[email protected]>
L: [email protected]
W: http://www.linuxfoundation.org/en/Net
W: http://patchwork.ozlabs.org/project/netdev/list/
T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next-2.6.git
S: Maintained
F: net/
F: include/net/
F: include/linux/in.h
F: include/linux/net.h
F: include/linux/netdevice.h
(2010/06/26 5:05), Eric Dumazet wrote:
> Le vendredi 25 juin 2010 à 16:12 +0900, Koki Sanagi a écrit :
>
>>> You might add a trace point to skb_free_datagram_locked() too, since it
>>> contains an inlined consume_skb()
>>>
>>
>> I think it is contrary.
>
> I think you are _very_ wrong.
>
>> skb_free_datagram_locked() contains consume_skb(), so tracepoint isn't needed.
>> Because skb_free_datagram_locked() can be traced by trace_consume_skb().
>>
>>
>>
>
> Koki, it would be good if you worked on net-next-2.6, so that my comment
> applies.
Yes, I am very wrong... I saw skb_free_datagram_locked() at linux-2.6.34.
It would be good to add tracepoint to it.
> Also, not sending this kind of patches on netdev is not going to help
> very much.
>
> Who is supposed to review them on lkml and Ack them ?
>
> Nobody.
>
> Please build your future network related patches against net-next-2.6,
> and send them to nedev and David Miller, the official network
> maintainer, as stated in MAINTAINERS file.
>
> NETWORKING [GENERAL]
> M: "David S. Miller" <[email protected]>
> L: [email protected]
> W: http://www.linuxfoundation.org/en/Net
> W: http://patchwork.ozlabs.org/project/netdev/list/
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-2.6.git
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next-2.6.git
> S: Maintained
> F: net/
> F: include/net/
> F: include/linux/in.h
> F: include/linux/net.h
> F: include/linux/netdevice.h
Yes, my submitting this time has many improper things(address, format and description...).
I'll check and modify them next time.
Thanks,
Koki Sanagi.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
On Fri, 2010-06-25 at 09:59 +0900, Koki Sanagi wrote:
> This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
> Combinating with tracepoint on dev_hard_start_xmit, we can check how long it
> takes to free transmited packets. And using it, we can calculate how many
> packets driver had at that time. It is useful when a drop of transmited packet
> is a problem.
>
> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>
> Signed-off-by: Koki Sanagi <[email protected]>
> ---
> include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
> net/core/dev.c | 2 ++
> net/core/skbuff.c | 1 +
> 3 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> index 4b2be6d..6ab5b34 100644
> --- a/include/trace/events/skb.h
> +++ b/include/trace/events/skb.h
> @@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
> __entry->skbaddr, __entry->protocol, __entry->location)
> );
>
> +TRACE_EVENT(consume_skb,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field( void *, skbaddr )
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p",
> + __entry->skbaddr)
> +);
> +
> +TRACE_EVENT(dev_kfree_skb_irq,
> +
> + TP_PROTO(struct sk_buff *skb),
> +
> + TP_ARGS(skb),
> +
> + TP_STRUCT__entry(
> + __field( void *, skbaddr )
> + ),
> +
> + TP_fast_assign(
> + __entry->skbaddr = skb;
> + ),
> +
> + TP_printk("skbaddr=%p",
> + __entry->skbaddr)
> +);
These two tracepoints are also identical in body. Please use
DECLARE_EVENT_CLASS() and DEFINE_EVENT() instead.
-- Steve
> +
> TRACE_EVENT(skb_copy_datagram_iovec,
>
> TP_PROTO(const struct sk_buff *skb, int len),
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 4b64b21..807b1ca 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -131,6 +131,7 @@
> #include <linux/random.h>
> #include <trace/events/napi.h>
> #include <trace/events/net.h>
> +#include <trace/events/skb.h>
> #include <linux/pci.h>
>
> #include "net-sysfs.h"
> @@ -1580,6 +1581,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
> struct softnet_data *sd;
> unsigned long flags;
>
> + trace_dev_kfree_skb_irq(skb);
> local_irq_save(flags);
> sd = &__get_cpu_var(softnet_data);
> skb->next = sd->completion_queue;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 34432b4..a7b4036 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
> smp_rmb();
> else if (likely(!atomic_dec_and_test(&skb->users)))
> return;
> + trace_consume_skb(skb);
> __kfree_skb(skb);
> }
> EXPORT_SYMBOL(consume_skb);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
(2010/06/29 3:12), Steven Rostedt wrote:
> On Fri, 2010-06-25 at 09:59 +0900, Koki Sanagi wrote:
>> This patch adds tracepoint to consume_skb and dev_kfree_skb_irq.
>> Combinating with tracepoint on dev_hard_start_xmit, we can check how long it
>> takes to free transmited packets. And using it, we can calculate how many
>> packets driver had at that time. It is useful when a drop of transmited packet
>> is a problem.
>>
>> <idle>-0 [001] 241409.218333: consume_skb: skbaddr=dd6b2fb8
>> <idle>-0 [001] 241409.490555: dev_kfree_skb_irq: skbaddr=f5e29840
>>
>> Signed-off-by: Koki Sanagi <[email protected]>
>> ---
>> include/trace/events/skb.h | 36 ++++++++++++++++++++++++++++++++++++
>> net/core/dev.c | 2 ++
>> net/core/skbuff.c | 1 +
>> 3 files changed, 39 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
>> index 4b2be6d..6ab5b34 100644
>> --- a/include/trace/events/skb.h
>> +++ b/include/trace/events/skb.h
>> @@ -35,6 +35,42 @@ TRACE_EVENT(kfree_skb,
>> __entry->skbaddr, __entry->protocol, __entry->location)
>> );
>>
>> +TRACE_EVENT(consume_skb,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + ),
>> +
>> + TP_printk("skbaddr=%p",
>> + __entry->skbaddr)
>> +);
>> +
>> +TRACE_EVENT(dev_kfree_skb_irq,
>> +
>> + TP_PROTO(struct sk_buff *skb),
>> +
>> + TP_ARGS(skb),
>> +
>> + TP_STRUCT__entry(
>> + __field( void *, skbaddr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->skbaddr = skb;
>> + ),
>> +
>> + TP_printk("skbaddr=%p",
>> + __entry->skbaddr)
>> +);
>
> These two tracepoints are also identical in body. Please use
> DECLARE_EVENT_CLASS() and DEFINE_EVENT() instead.
>
> -- Steve
>
OK, I'll do that.
Thanks,
Koki Sanagi.
>> +
>> TRACE_EVENT(skb_copy_datagram_iovec,
>>
>> TP_PROTO(const struct sk_buff *skb, int len),
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 4b64b21..807b1ca 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -131,6 +131,7 @@
>> #include <linux/random.h>
>> #include <trace/events/napi.h>
>> #include <trace/events/net.h>
>> +#include <trace/events/skb.h>
>> #include <linux/pci.h>
>>
>> #include "net-sysfs.h"
>> @@ -1580,6 +1581,7 @@ void dev_kfree_skb_irq(struct sk_buff *skb)
>> struct softnet_data *sd;
>> unsigned long flags;
>>
>> + trace_dev_kfree_skb_irq(skb);
>> local_irq_save(flags);
>> sd = &__get_cpu_var(softnet_data);
>> skb->next = sd->completion_queue;
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index 34432b4..a7b4036 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -466,6 +466,7 @@ void consume_skb(struct sk_buff *skb)
>> smp_rmb();
>> else if (likely(!atomic_dec_and_test(&skb->users)))
>> return;
>> + trace_consume_skb(skb);
>> __kfree_skb(skb);
>> }
>> EXPORT_SYMBOL(consume_skb);
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
>
>