From 5acaee0a8964c9bab7775ab8bedcd1f66a2a1011 Mon Sep 17 00:00:00 2001 From: John Fastabend Date: Mon, 17 Jul 2017 09:28:35 -0700 Subject: xdp: add trace event for xdp redirect This adds a trace event for xdp redirect which may help when debugging XDP programs that use redirect bpf commands. Signed-off-by: John Fastabend Acked-by: Daniel Borkmann Acked-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 1b61357d3f57..7b1eb7b4be41 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -12,7 +12,8 @@ FN(ABORTED) \ FN(DROP) \ FN(PASS) \ - FN(TX) + FN(TX) \ + FN(REDIRECT) #define __XDP_ACT_TP_FN(x) \ TRACE_DEFINE_ENUM(XDP_##x); @@ -48,6 +49,34 @@ TRACE_EVENT(xdp_exception, __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB)) ); +TRACE_EVENT(xdp_redirect, + + TP_PROTO(const struct net_device *from, + const struct net_device *to, + const struct bpf_prog *xdp, u32 act), + + TP_ARGS(from, to, xdp, act), + + TP_STRUCT__entry( + __string(name_from, from->name) + __string(name_to, to->name) + __array(u8, prog_tag, 8) + __field(u32, act) + ), + + TP_fast_assign( + BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); + memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); + __assign_str(name_from, from->name); + __assign_str(name_to, to->name); + __entry->act = act; + ), + + TP_printk("prog=%s from=%s to=%s action=%s", + __print_hex_str(__entry->prog_tag, 8), + __get_str(name_from), __get_str(name_to), + __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB)) +); #endif /* _TRACE_XDP_H */ #include -- cgit v1.2.3 From 4c03bdd7b5c084c3c6973cb2419edac5363c051f Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Thu, 17 Aug 2017 18:22:37 +0200 Subject: xdp: adjust xdp redirect tracepoint to include return error code The return error code need to be included in the tracepoint xdp:xdp_redirect, else its not possible to distinguish successful or failed XDP_REDIRECT transmits. XDP have no queuing mechanism. Thus, it is fairly easily to overrun a NIC transmit queue. The eBPF program invoking helpers (bpf_redirect or bpf_redirect_map) to redirect a packet doesn't get any feedback whether the packet was actually transmitted. Info on failed transmits in the tracepoint xdp:xdp_redirect, is interesting as this opens for providing a feedback-loop to the receiving XDP program. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 11 +++++++---- net/core/filter.c | 19 ++++++++++++------- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 7b1eb7b4be41..0e42e69f773b 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -53,15 +53,16 @@ TRACE_EVENT(xdp_redirect, TP_PROTO(const struct net_device *from, const struct net_device *to, - const struct bpf_prog *xdp, u32 act), + const struct bpf_prog *xdp, u32 act, int err), - TP_ARGS(from, to, xdp, act), + TP_ARGS(from, to, xdp, act, err), TP_STRUCT__entry( __string(name_from, from->name) __string(name_to, to->name) __array(u8, prog_tag, 8) __field(u32, act) + __field(int, err) ), TP_fast_assign( @@ -70,12 +71,14 @@ TRACE_EVENT(xdp_redirect, __assign_str(name_from, from->name); __assign_str(name_to, to->name); __entry->act = act; + __entry->err = err; ), - TP_printk("prog=%s from=%s to=%s action=%s", + TP_printk("prog=%s from=%s to=%s action=%s err=%d", __print_hex_str(__entry->prog_tag, 8), __get_str(name_from), __get_str(name_to), - __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB)) + __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), + __entry->err) ); #endif /* _TRACE_XDP_H */ diff --git a/net/core/filter.c b/net/core/filter.c index 0f4df86d936a..fa2115695037 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2535,14 +2535,16 @@ int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, struct bpf_map *map = ri->map; u32 index = ri->ifindex; struct net_device *fwd; - int err = -EINVAL; + int err; ri->ifindex = 0; ri->map = NULL; fwd = __dev_map_lookup_elem(map, index); - if (!fwd) + if (!fwd) { + err = -EINVAL; goto out; + } if (ri->map_to_flush && (ri->map_to_flush != map)) xdp_do_flush_map(); @@ -2552,7 +2554,7 @@ int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, ri->map_to_flush = map; out: - trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT); + trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err); return err; } @@ -2562,6 +2564,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, struct redirect_info *ri = this_cpu_ptr(&redirect_info); struct net_device *fwd; u32 index = ri->ifindex; + int err; if (ri->map) return xdp_do_redirect_map(dev, xdp, xdp_prog); @@ -2570,12 +2573,14 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, ri->ifindex = 0; if (unlikely(!fwd)) { bpf_warn_invalid_xdp_redirect(index); - return -EINVAL; + err = -EINVAL; + goto out; } - trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT); - - return __bpf_tx_xdp(fwd, NULL, xdp, 0); + err = __bpf_tx_xdp(fwd, NULL, xdp, 0); +out: + trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err); + return err; } EXPORT_SYMBOL_GPL(xdp_do_redirect); -- cgit v1.2.3 From a873585587205750e7accfb2c93c29239ffa6e09 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Thu, 24 Aug 2017 12:33:18 +0200 Subject: xdp: remove net_device names from xdp_redirect tracepoint There is too much overhead in the current trace_xdp_redirect tracepoint as it does strcpy and strlen on the net_device names. Besides, exposing the ifindex/index is actually the information that is needed in the tracepoint to diagnose issues. When a lookup fails (either ifindex or devmap index) then there is a need for saying which to_index that have issues. V2: Adjust args to be aligned with trace_xdp_exception. Signed-off-by: Jesper Dangaard Brouer Acked-by: Daniel Borkmann Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 24 ++++++++++++------------ net/core/filter.c | 6 +++--- 2 files changed, 15 insertions(+), 15 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 0e42e69f773b..cd37706c6f55 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -51,33 +51,33 @@ TRACE_EVENT(xdp_exception, TRACE_EVENT(xdp_redirect, - TP_PROTO(const struct net_device *from, - const struct net_device *to, - const struct bpf_prog *xdp, u32 act, int err), + TP_PROTO(const struct net_device *dev, + const struct bpf_prog *xdp, u32 act, + int to_index, int err), - TP_ARGS(from, to, xdp, act, err), + TP_ARGS(dev, xdp, act, to_index, err), TP_STRUCT__entry( - __string(name_from, from->name) - __string(name_to, to->name) __array(u8, prog_tag, 8) __field(u32, act) + __field(int, ifindex) + __field(int, to_index) __field(int, err) ), TP_fast_assign( BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); - __assign_str(name_from, from->name); - __assign_str(name_to, to->name); - __entry->act = act; - __entry->err = err; + __entry->act = act; + __entry->ifindex = dev->ifindex; + __entry->to_index = to_index; + __entry->err = err; ), - TP_printk("prog=%s from=%s to=%s action=%s err=%d", + TP_printk("prog=%s action=%s ifindex=%d to_index=%d err=%d", __print_hex_str(__entry->prog_tag, 8), - __get_str(name_from), __get_str(name_to), __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), + __entry->ifindex, __entry->to_index, __entry->err) ); #endif /* _TRACE_XDP_H */ diff --git a/net/core/filter.c b/net/core/filter.c index a04680331033..4bcd6baa80c9 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2524,7 +2524,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, if (likely(!err)) ri->map_to_flush = map; out: - trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err); + trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); return err; } @@ -2548,7 +2548,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, err = __bpf_tx_xdp(fwd, NULL, xdp, 0); out: - trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err); + trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_redirect); @@ -2582,7 +2582,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, skb->dev = fwd; out: - trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err); + trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_generic_redirect); -- cgit v1.2.3 From 315ec3990efd71f87e556cf7827a1ac2d565d5e8 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Thu, 24 Aug 2017 12:33:23 +0200 Subject: xdp: get tracepoints xdp_exception and xdp_redirect in sync Remove the net_device string name from the xdp_exception tracepoint, like the xdp_redirect tracepoint. Align the TP_STRUCT to have common entries between these two tracepoint. Signed-off-by: Jesper Dangaard Brouer Acked-by: Daniel Borkmann Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index cd37706c6f55..27cf2ef35f5f 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -31,22 +31,22 @@ TRACE_EVENT(xdp_exception, TP_ARGS(dev, xdp, act), TP_STRUCT__entry( - __string(name, dev->name) __array(u8, prog_tag, 8) __field(u32, act) + __field(int, ifindex) ), TP_fast_assign( BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); - __assign_str(name, dev->name); - __entry->act = act; + __entry->act = act; + __entry->ifindex = dev->ifindex; ), - TP_printk("prog=%s device=%s action=%s", + TP_printk("prog=%s action=%s ifindex=%d", __print_hex_str(__entry->prog_tag, 8), - __get_str(name), - __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB)) + __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), + __entry->ifindex) ); TRACE_EVENT(xdp_redirect, -- cgit v1.2.3 From c31e5a4876cdfbfcc28c4fb201ad872c65671067 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 29 Aug 2017 16:37:40 +0200 Subject: xdp: remove redundant argument to trace_xdp_redirect Supplying the action argument XDP_REDIRECT to the tracepoint xdp_redirect is redundant as it is only called in-case this action was specified. Remove the argument, but keep "act" member of the tracepoint struct and populate it with XDP_REDIRECT. This makes it easier to write a common bpf_prog processing events. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 6 +++--- net/core/filter.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 27cf2ef35f5f..f684f3b36bca 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -52,10 +52,10 @@ TRACE_EVENT(xdp_exception, TRACE_EVENT(xdp_redirect, TP_PROTO(const struct net_device *dev, - const struct bpf_prog *xdp, u32 act, + const struct bpf_prog *xdp, int to_index, int err), - TP_ARGS(dev, xdp, act, to_index, err), + TP_ARGS(dev, xdp, to_index, err), TP_STRUCT__entry( __array(u8, prog_tag, 8) @@ -68,7 +68,7 @@ TRACE_EVENT(xdp_redirect, TP_fast_assign( BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); - __entry->act = act; + __entry->act = XDP_REDIRECT; __entry->ifindex = dev->ifindex; __entry->to_index = to_index; __entry->err = err; diff --git a/net/core/filter.c b/net/core/filter.c index 4bcd6baa80c9..de31fb684ad4 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2524,7 +2524,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, if (likely(!err)) ri->map_to_flush = map; out: - trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); + trace_xdp_redirect(dev, xdp_prog, index, err); return err; } @@ -2548,7 +2548,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, err = __bpf_tx_xdp(fwd, NULL, xdp, 0); out: - trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); + trace_xdp_redirect(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_redirect); @@ -2582,7 +2582,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, skb->dev = fwd; out: - trace_xdp_redirect(dev, xdp_prog, XDP_REDIRECT, index, err); + trace_xdp_redirect(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_generic_redirect); -- cgit v1.2.3 From 8d3b778ff544b369f0847e6c15f3e73057298aa4 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 29 Aug 2017 16:37:45 +0200 Subject: xdp: tracepoint xdp_redirect also need a map argument To make sense of the map index, the tracepoint user also need to know that map we are talking about. Supply the map pointer but only expose the map->id. The 'to_index' is renamed 'to_ifindex'. In the xdp_redirect_map case, this is the result of the devmap lookup. The map lookup key is exposed as map_index, which is needed to troubleshoot in case the lookup failed. The 'to_ifindex' is placed after 'err' to keep TP_STRUCT as common as possible. This also keeps the TP_STRUCT similar enough, that userspace can write a monitor program, that doesn't need to care about whether bpf_redirect or bpf_redirect_map were used. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 38 ++++++++++++++++++++++++++++++-------- net/core/filter.c | 6 +++--- 2 files changed, 33 insertions(+), 11 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index f684f3b36bca..573dcfa1aeaa 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -49,20 +49,23 @@ TRACE_EVENT(xdp_exception, __entry->ifindex) ); -TRACE_EVENT(xdp_redirect, +DECLARE_EVENT_CLASS(xdp_redirect_template, TP_PROTO(const struct net_device *dev, const struct bpf_prog *xdp, - int to_index, int err), + int to_ifindex, int err, + const struct bpf_map *map, u32 map_index), - TP_ARGS(dev, xdp, to_index, err), + TP_ARGS(dev, xdp, to_ifindex, err, map, map_index), TP_STRUCT__entry( __array(u8, prog_tag, 8) __field(u32, act) __field(int, ifindex) - __field(int, to_index) __field(int, err) + __field(int, to_ifindex) + __field(u32, map_id) + __field(int, map_index) ), TP_fast_assign( @@ -70,16 +73,35 @@ TRACE_EVENT(xdp_redirect, memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); __entry->act = XDP_REDIRECT; __entry->ifindex = dev->ifindex; - __entry->to_index = to_index; __entry->err = err; + __entry->to_ifindex = to_ifindex; + __entry->map_id = map ? map->id : 0; + __entry->map_index = map_index; ), - TP_printk("prog=%s action=%s ifindex=%d to_index=%d err=%d", + TP_printk("prog=%s action=%s ifindex=%d to_ifindex=%d err=%d" + " map_id=%d map_index=%d", __print_hex_str(__entry->prog_tag, 8), __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), - __entry->ifindex, __entry->to_index, - __entry->err) + __entry->ifindex, __entry->to_ifindex, + __entry->err, + __entry->map_id, __entry->map_index) ); + +DEFINE_EVENT(xdp_redirect_template, xdp_redirect, + TP_PROTO(const struct net_device *dev, + const struct bpf_prog *xdp, + int to_ifindex, int err, + const struct bpf_map *map, u32 map_index), + TP_ARGS(dev, xdp, to_ifindex, err, map, map_index) +); + +#define _trace_xdp_redirect(dev, xdp, to, err) \ + trace_xdp_redirect(dev, xdp, to, err, NULL, 0); + +#define trace_xdp_redirect_map(dev, xdp, fwd, err, map, idx) \ + trace_xdp_redirect(dev, xdp, fwd ? fwd->ifindex : 0, err, map, idx); + #endif /* _TRACE_XDP_H */ #include diff --git a/net/core/filter.c b/net/core/filter.c index de31fb684ad4..31eab77cc842 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2524,7 +2524,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, if (likely(!err)) ri->map_to_flush = map; out: - trace_xdp_redirect(dev, xdp_prog, index, err); + trace_xdp_redirect_map(dev, xdp_prog, fwd, err, map, index); return err; } @@ -2548,7 +2548,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, err = __bpf_tx_xdp(fwd, NULL, xdp, 0); out: - trace_xdp_redirect(dev, xdp_prog, index, err); + _trace_xdp_redirect(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_redirect); @@ -2582,7 +2582,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, skb->dev = fwd; out: - trace_xdp_redirect(dev, xdp_prog, index, err); + _trace_xdp_redirect(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_generic_redirect); -- cgit v1.2.3 From b06337dfdb16bc3f668326b6a618c472c671182a Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 29 Aug 2017 16:37:51 +0200 Subject: xdp: make xdp tracepoints report bpf prog id instead of prog_tag Given previous patch expose the map_id, it seems natural to also report the bpf prog id. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 573dcfa1aeaa..89eba564e199 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -31,20 +31,19 @@ TRACE_EVENT(xdp_exception, TP_ARGS(dev, xdp, act), TP_STRUCT__entry( - __array(u8, prog_tag, 8) + __field(int, prog_id) __field(u32, act) __field(int, ifindex) ), TP_fast_assign( - BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); - memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); + __entry->prog_id = xdp->aux->id; __entry->act = act; __entry->ifindex = dev->ifindex; ), - TP_printk("prog=%s action=%s ifindex=%d", - __print_hex_str(__entry->prog_tag, 8), + TP_printk("prog_id=%d action=%s ifindex=%d", + __entry->prog_id, __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), __entry->ifindex) ); @@ -59,7 +58,7 @@ DECLARE_EVENT_CLASS(xdp_redirect_template, TP_ARGS(dev, xdp, to_ifindex, err, map, map_index), TP_STRUCT__entry( - __array(u8, prog_tag, 8) + __field(int, prog_id) __field(u32, act) __field(int, ifindex) __field(int, err) @@ -69,8 +68,7 @@ DECLARE_EVENT_CLASS(xdp_redirect_template, ), TP_fast_assign( - BUILD_BUG_ON(sizeof(__entry->prog_tag) != sizeof(xdp->tag)); - memcpy(__entry->prog_tag, xdp->tag, sizeof(xdp->tag)); + __entry->prog_id = xdp->aux->id; __entry->act = XDP_REDIRECT; __entry->ifindex = dev->ifindex; __entry->err = err; @@ -79,9 +77,9 @@ DECLARE_EVENT_CLASS(xdp_redirect_template, __entry->map_index = map_index; ), - TP_printk("prog=%s action=%s ifindex=%d to_ifindex=%d err=%d" + TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d" " map_id=%d map_index=%d", - __print_hex_str(__entry->prog_tag, 8), + __entry->prog_id, __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), __entry->ifindex, __entry->to_ifindex, __entry->err, -- cgit v1.2.3 From f5836ca5e9867fa6ab88cadb9873af56d9ceb589 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 29 Aug 2017 16:37:56 +0200 Subject: xdp: separate xdp_redirect tracepoint in error case There is a need to separate the xdp_redirect tracepoint into two tracepoints, for separating the error case from the normal forward case. Due to the extreme speeds XDP is operating at, loading a tracepoint have a measurable impact. Single core XDP REDIRECT (ethtool tuned rx-usecs 25) can do 13.7 Mpps forwarding, but loading a simple bpf_prog at the tracepoint (with a return 0) reduce perf to 10.2 Mpps (CPU E5-1650 v4 @ 3.60GHz, driver: ixgbe) The overhead of loading a bpf-based tracepoint can be calculated to cost 25 nanosec ((1/13782002-1/10267937)*10^9 = -24.83 ns). Using perf record on the tracepoint event, with a non-matching --filter expression, the overhead is much larger. Performance drops to 8.3 Mpps, cost 48 nanosec ((1/13782002-1/8312497)*10^9 = -47.74)) Having a separate tracepoint for err cases, which should be less frequent, allow running a continuous monitor for errors while not affecting the redirect forward performance (this have also been verified by measurements). Signed-off-by: Jesper Dangaard Brouer Acked-by: Alexei Starovoitov Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 22 ++++++++++++++++++---- net/core/filter.c | 37 ++++++++++++++++++++++++------------- 2 files changed, 42 insertions(+), 17 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 89eba564e199..1eebad55ebd4 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -94,11 +94,25 @@ DEFINE_EVENT(xdp_redirect_template, xdp_redirect, TP_ARGS(dev, xdp, to_ifindex, err, map, map_index) ); -#define _trace_xdp_redirect(dev, xdp, to, err) \ - trace_xdp_redirect(dev, xdp, to, err, NULL, 0); +DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err, + TP_PROTO(const struct net_device *dev, + const struct bpf_prog *xdp, + int to_ifindex, int err, + const struct bpf_map *map, u32 map_index), + TP_ARGS(dev, xdp, to_ifindex, err, map, map_index) +); + +#define _trace_xdp_redirect(dev, xdp, to) \ + trace_xdp_redirect(dev, xdp, to, 0, NULL, 0); + +#define _trace_xdp_redirect_err(dev, xdp, to, err) \ + trace_xdp_redirect_err(dev, xdp, to, err, NULL, 0); + +#define trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \ + trace_xdp_redirect(dev, xdp, fwd ? fwd->ifindex : 0, 0, map, idx); -#define trace_xdp_redirect_map(dev, xdp, fwd, err, map, idx) \ - trace_xdp_redirect(dev, xdp, fwd ? fwd->ifindex : 0, err, map, idx); +#define trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \ + trace_xdp_redirect_err(dev, xdp, fwd ? fwd->ifindex : 0, err, map, idx); #endif /* _TRACE_XDP_H */ diff --git a/net/core/filter.c b/net/core/filter.c index 31eab77cc842..096e78de0b97 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2515,16 +2515,20 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, fwd = __dev_map_lookup_elem(map, index); if (!fwd) { err = -EINVAL; - goto out; + goto err; } if (ri->map_to_flush && ri->map_to_flush != map) xdp_do_flush_map(); err = __bpf_tx_xdp(fwd, map, xdp, index); - if (likely(!err)) - ri->map_to_flush = map; -out: - trace_xdp_redirect_map(dev, xdp_prog, fwd, err, map, index); + if (unlikely(err)) + goto err; + + ri->map_to_flush = map; + trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index); + return 0; +err: + trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err); return err; } @@ -2543,12 +2547,17 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp, ri->ifindex = 0; if (unlikely(!fwd)) { err = -EINVAL; - goto out; + goto err; } err = __bpf_tx_xdp(fwd, NULL, xdp, 0); -out: - _trace_xdp_redirect(dev, xdp_prog, index, err); + if (unlikely(err)) + goto err; + + _trace_xdp_redirect(dev, xdp_prog, index); + return 0; +err: + _trace_xdp_redirect_err(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_redirect); @@ -2566,23 +2575,25 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb, ri->ifindex = 0; if (unlikely(!fwd)) { err = -EINVAL; - goto out; + goto err; } if (unlikely(!(fwd->flags & IFF_UP))) { err = -ENETDOWN; - goto out; + goto err; } len = fwd->mtu + fwd->hard_header_len + VLAN_HLEN; if (skb->len > len) { err = -EMSGSIZE; - goto out; + goto err; } skb->dev = fwd; -out: - _trace_xdp_redirect(dev, xdp_prog, index, err); + _trace_xdp_redirect(dev, xdp_prog, index); + return 0; +err: + _trace_xdp_redirect_err(dev, xdp_prog, index, err); return err; } EXPORT_SYMBOL_GPL(xdp_do_generic_redirect); -- cgit v1.2.3 From 59a308967589f5b3f1f42793ab49bc2e18069769 Mon Sep 17 00:00:00 2001 From: Jesper Dangaard Brouer Date: Tue, 29 Aug 2017 16:38:01 +0200 Subject: xdp: separate xdp_redirect tracepoint in map case Creating as specific xdp_redirect_map variant of the xdp tracepoints allow users to write simpler/faster BPF progs that get attached to these tracepoints. Goal is to still keep the tracepoints in xdp_redirect and xdp_redirect_map similar enough, that a tool can read the top part of the TP_STRUCT and produce similar monitor statistics. Signed-off-by: Jesper Dangaard Brouer Signed-off-by: David S. Miller --- include/trace/events/xdp.h | 46 ++++++++++++++++++++++++++++++++++++++-------- net/core/filter.c | 4 ++-- 2 files changed, 40 insertions(+), 10 deletions(-) (limited to 'include/trace/events/xdp.h') diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h index 1eebad55ebd4..862575ac8da9 100644 --- a/include/trace/events/xdp.h +++ b/include/trace/events/xdp.h @@ -77,13 +77,11 @@ DECLARE_EVENT_CLASS(xdp_redirect_template, __entry->map_index = map_index; ), - TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d" - " map_id=%d map_index=%d", + TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d", __entry->prog_id, __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), __entry->ifindex, __entry->to_ifindex, - __entry->err, - __entry->map_id, __entry->map_index) + __entry->err) ); DEFINE_EVENT(xdp_redirect_template, xdp_redirect, @@ -108,11 +106,43 @@ DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err, #define _trace_xdp_redirect_err(dev, xdp, to, err) \ trace_xdp_redirect_err(dev, xdp, to, err, NULL, 0); -#define trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \ - trace_xdp_redirect(dev, xdp, fwd ? fwd->ifindex : 0, 0, map, idx); +DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map, + TP_PROTO(const struct net_device *dev, + const struct bpf_prog *xdp, + int to_ifindex, int err, + const struct bpf_map *map, u32 map_index), + TP_ARGS(dev, xdp, to_ifindex, err, map, map_index), + TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d" + " map_id=%d map_index=%d", + __entry->prog_id, + __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), + __entry->ifindex, __entry->to_ifindex, + __entry->err, + __entry->map_id, __entry->map_index) +); + +DEFINE_EVENT_PRINT(xdp_redirect_template, xdp_redirect_map_err, + TP_PROTO(const struct net_device *dev, + const struct bpf_prog *xdp, + int to_ifindex, int err, + const struct bpf_map *map, u32 map_index), + TP_ARGS(dev, xdp, to_ifindex, err, map, map_index), + TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d" + " map_id=%d map_index=%d", + __entry->prog_id, + __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB), + __entry->ifindex, __entry->to_ifindex, + __entry->err, + __entry->map_id, __entry->map_index) +); + +#define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \ + trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0, \ + 0, map, idx); -#define trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \ - trace_xdp_redirect_err(dev, xdp, fwd ? fwd->ifindex : 0, err, map, idx); +#define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \ + trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0, \ + err, map, idx); #endif /* _TRACE_XDP_H */ diff --git a/net/core/filter.c b/net/core/filter.c index 096e78de0b97..c6a37fe0285b 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2525,10 +2525,10 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp, goto err; ri->map_to_flush = map; - trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index); + _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index); return 0; err: - trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err); + _trace_xdp_redirect_map_err(dev, xdp_prog, fwd, map, index, err); return err; } -- cgit v1.2.3