On 12/19/2018 08:36 AM, Jesper Dangaard Brouer wrote: > On Tue, 18 Dec 2018 18:27:06 -0800 > Stephen Hemminger <step...@networkplumber.org> wrote: > >> This is the result of a conversation about monitoring of link >> state changes with BPF. > > If you want to use this from BPF then you are in for a surprise. As > tracepoints BPF cannot read these "__string" constructs, here the netdev > name. I tried a lot of different tricks that didn't work, see [1], > until Alexei explained that it simply isn't supported. > > I instead recommend adding the ifindex to the tracepoint. The__string > and __assign_str is also a performance concern as it does strcpy behind > your back. > > I have an year old TODO list item about improving this: > ** TODO Make perf-script plugin for ifindex to name translation > SCHEDULED: <2017-11-20 Mon> > > Today, the existing network tracepoints using dev->name is not that > usable by BPF, as BPF cannot identify the interface. Thus IMHO it would > make sense to convert the existing network tracepoints dev->name into > dev->ifindex, and then let perf-script convert this to the interface > name. Either in userspace via if_indextoname(3), or (as ACME pointed > out at the time) we might want to have a lookup table stored together > with perf.data for later inspection (in-case ifindexes changed).
Hmm, why not just doing something as in your example below with napi_poll() where you pass in the napi pointer, and then use bpf_probe_read_str() on ctx->dev for fetching the name? At least there this should work and should be okay given it's rather slow-path event. Thanks, Daniel > [1] > https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/samples/bpf/napi_monitor_kern.c#L34-L130 > >> Signed-off-by: Stephen Hemminger <step...@networkplumber.org> >> --- >> include/trace/events/net.h | 115 +++++++++++++++++++++++++++++++++++++ >> net/core/dev.c | 9 ++- >> 2 files changed, 123 insertions(+), 1 deletion(-) >> >> diff --git a/include/trace/events/net.h b/include/trace/events/net.h >> index 1efd7d9b25fe..141310d24610 100644 >> --- a/include/trace/events/net.h >> +++ b/include/trace/events/net.h > [...] >> +TRACE_EVENT(net_dev_notifier_entry, >> + >> + TP_PROTO(const struct netdev_notifier_info *info, unsigned long val), >> + >> + TP_ARGS(info, val), >> + >> + TP_STRUCT__entry( >> + __string( name, info->dev->name ) >> + __field( enum netdev_cmd, event ) >> + ), >> + >> + TP_fast_assign( >> + __assign_str(name, info->dev->name); >> + __entry->event = val; >> + ), > > These __string and __assign_str are costly and behind the scenes does a > strcpy. > >> + >> + TP_printk("dev=%s event=%s", >> + __get_str(name), netdev_event_type(__entry->event)) >> +); >> + >> +TRACE_EVENT(net_dev_notifier, >> + >> + TP_PROTO(const struct netdev_notifier_info *info, int rc, unsigned long >> val), >> + >> + TP_ARGS(info, rc, val), >> + >> + TP_STRUCT__entry( >> + __string( name, info->dev->name ) >> + __field( enum netdev_cmd, event ) >> + __field( int, rc ) >> + ), >> + >> + TP_fast_assign( >> + __assign_str(name, info->dev->name); >> + __entry->event = val; >> + __entry->rc = rc; >> + ), >> + >> + TP_printk("dev=%s event=%s ret=%d", >> + __get_str(name), netdev_event_type(__entry->event), >> + __entry->rc) >> +); >> + > > The bare minimum change is to _also_ add (info->dev->)ifindex to the > tracepoint, as this makes it usable from BPF. >
