Messages in this thread Patch in this message | | | From | Eric Dumazet <> | Date | Thu, 25 Aug 2022 08:31:56 -0700 | Subject | Re: [PATCH net-next] net: skb: fix kfree_skb event output error in perf |
| |
On Wed, Aug 24, 2022 at 10:37 PM <menglong8.dong@gmail.com> wrote: > > From: Menglong Dong <imagedong@tencent.com> > > As Eric reported, the 'reason' field is not presented when trace the > kfree_skb event by perf: > > $ perf record -e skb:kfree_skb -a sleep 10 > $ perf script > ip_defrag 14605 [021] 221.614303: skb:kfree_skb: > skbaddr=0xffff9d2851242700 protocol=34525 location=0xffffffffa39346b1 > reason: > > The cause seems to be passing kernel address directly to TP_printk(), > which is not right.
Why ?
It seems this adds an expensive copy of a string that should reside in rodata section of vmlinux, thus can not disappear... (Also the ring buffer entry will have a dynamic size ?)
trace_safe_str() is using is_kernel_rodata() which should return true for drop_reasons[X] ?
$ grep drop_reasons net/core/dropreason_str.c const char * const drop_reasons[] = { ...
> > Therefore, fix this by adding a '__string' field to the TP_STRUCT of > kfree_skb, which is 'reason_str', and passing it to TP_printk(). > > (Not sure if we should still keep the 'reason' field in > TP_STRUCT__entry)
Maybe for event/trace filtering purposes ?
> > Reported-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Menglong Dong <imagedong@tencent.com> > --- > include/trace/events/skb.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h > index 45264e4bb254..7235554141c3 100644 > --- a/include/trace/events/skb.h > +++ b/include/trace/events/skb.h > @@ -24,6 +24,7 @@ TRACE_EVENT(kfree_skb, > __field(void *, location) > __field(unsigned short, protocol) > __field(enum skb_drop_reason, reason) > + __string(reason_str, drop_reasons[reason]) > ), > > TP_fast_assign( > @@ -31,11 +32,12 @@ TRACE_EVENT(kfree_skb, > __entry->location = location; > __entry->protocol = ntohs(skb->protocol); > __entry->reason = reason; > + __assign_str(reason_str, drop_reasons[reason]); > ), > > TP_printk("skbaddr=%p protocol=%u location=%p reason: %s", > __entry->skbaddr, __entry->protocol, __entry->location, > - drop_reasons[__entry->reason]) > + __get_str(reason_str)) > ); > > TRACE_EVENT(consume_skb, > -- > 2.37.2 >
| |