* [bpf-next V2 PATCH 1/3] xdp: unlikely instrumentation for xdp map redirect
2018-09-03 7:54 [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Jesper Dangaard Brouer
@ 2018-09-03 7:54 ` Jesper Dangaard Brouer
2018-09-03 7:55 ` [bpf-next V2 PATCH 2/3] xdp: explicit inline __xdp_map_lookup_elem Jesper Dangaard Brouer
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2018-09-03 7:54 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, Alexei Starovoitov, Jesper Dangaard Brouer
Notice the compiler generated ASM code layout was suboptimal. It
assumed map enqueue errors as the likely case, which is shouldn't.
It assumed that xdp_do_flush_map() was a likely case, due to maps
changing between packets, which should be very unlikely.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/filter.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index c25eb36f1320..520f5e9e0b73 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3182,7 +3182,7 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
struct bpf_dtab_netdev *dst = fwd;
err = dev_map_enqueue(dst, xdp, dev_rx);
- if (err)
+ if (unlikely(err))
return err;
__dev_map_insert_ctx(map, index);
break;
@@ -3191,7 +3191,7 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
struct bpf_cpu_map_entry *rcpu = fwd;
err = cpu_map_enqueue(rcpu, xdp, dev_rx);
- if (err)
+ if (unlikely(err))
return err;
__cpu_map_insert_ctx(map, index);
break;
@@ -3279,7 +3279,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
err = -EINVAL;
goto err;
}
- if (ri->map_to_flush && ri->map_to_flush != map)
+ if (ri->map_to_flush && unlikely(ri->map_to_flush != map))
xdp_do_flush_map();
err = __bpf_tx_xdp_map(dev, fwd, map, xdp, index);
^ permalink raw reply related [flat|nested] 5+ messages in thread* [bpf-next V2 PATCH 2/3] xdp: explicit inline __xdp_map_lookup_elem
2018-09-03 7:54 [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Jesper Dangaard Brouer
2018-09-03 7:54 ` [bpf-next V2 PATCH 1/3] xdp: unlikely instrumentation for xdp map redirect Jesper Dangaard Brouer
@ 2018-09-03 7:55 ` Jesper Dangaard Brouer
2018-09-03 7:55 ` [bpf-next V2 PATCH 3/3] xdp: split code for map vs non-map redirect Jesper Dangaard Brouer
2018-09-06 17:55 ` [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Alexei Starovoitov
3 siblings, 0 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2018-09-03 7:55 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, Alexei Starovoitov, Jesper Dangaard Brouer
The compiler chooses to not-inline the function __xdp_map_lookup_elem,
because it can see that it is used by both Generic-XDP and native-XDP
do redirect calls (xdp_do_generic_redirect_map and xdp_do_redirect_map).
The compiler cannot know that this is a bad choice, as it cannot know
that a net device cannot run both XDP modes (Generic or Native) at the
same time. Thus, mark this function inline, even-though we normally
leave this up-to the compiler.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/filter.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 520f5e9e0b73..ec1b4eb0d3d4 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3232,7 +3232,7 @@ void xdp_do_flush_map(void)
}
EXPORT_SYMBOL_GPL(xdp_do_flush_map);
-static void *__xdp_map_lookup_elem(struct bpf_map *map, u32 index)
+static inline void *__xdp_map_lookup_elem(struct bpf_map *map, u32 index)
{
switch (map->map_type) {
case BPF_MAP_TYPE_DEVMAP:
@@ -3275,7 +3275,7 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
WRITE_ONCE(ri->map, NULL);
fwd = __xdp_map_lookup_elem(map, index);
- if (!fwd) {
+ if (unlikely(!fwd)) {
err = -EINVAL;
goto err;
}
@@ -3303,7 +3303,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
u32 index = ri->ifindex;
int err;
- if (map)
+ if (likely(map))
return xdp_do_redirect_map(dev, xdp, xdp_prog, map);
fwd = dev_get_by_index_rcu(dev_net(dev), index);
^ permalink raw reply related [flat|nested] 5+ messages in thread* [bpf-next V2 PATCH 3/3] xdp: split code for map vs non-map redirect
2018-09-03 7:54 [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Jesper Dangaard Brouer
2018-09-03 7:54 ` [bpf-next V2 PATCH 1/3] xdp: unlikely instrumentation for xdp map redirect Jesper Dangaard Brouer
2018-09-03 7:55 ` [bpf-next V2 PATCH 2/3] xdp: explicit inline __xdp_map_lookup_elem Jesper Dangaard Brouer
@ 2018-09-03 7:55 ` Jesper Dangaard Brouer
2018-09-06 17:55 ` [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Alexei Starovoitov
3 siblings, 0 replies; 5+ messages in thread
From: Jesper Dangaard Brouer @ 2018-09-03 7:55 UTC (permalink / raw)
To: netdev; +Cc: Daniel Borkmann, Alexei Starovoitov, Jesper Dangaard Brouer
The compiler does an efficient job of inlining static C functions.
Perf top clearly shows that almost everything gets inlined into the
function call xdp_do_redirect.
The function xdp_do_redirect end-up containing and interleaving the
map and non-map redirect code. This is sub-optimal, as it would be
strange for an XDP program to use both types of redirect in the same
program. The two use-cases are separate, and interleaving the code
just cause more instruction-cache pressure.
I would like to stress (again) that the non-map variant bpf_redirect
is very slow compared to the bpf_redirect_map variant, approx half the
speed. Measured with driver i40e the difference is:
- map redirect: 13,250,350 pps
- non-map redirect: 7,491,425 pps
For this reason, the function name of the non-map variant of redirect
have been called xdp_do_redirect_slow. This hopefully gives a hint
when using perf, that this is not the optimal XDP redirect operating mode.
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
net/core/filter.c | 52 ++++++++++++++++++++++++++++++----------------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index ec1b4eb0d3d4..7738d1d70154 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3170,6 +3170,32 @@ static int __bpf_tx_xdp(struct net_device *dev,
return 0;
}
+static noinline int
+xdp_do_redirect_slow(struct net_device *dev, struct xdp_buff *xdp,
+ struct bpf_prog *xdp_prog, struct bpf_redirect_info *ri)
+{
+ struct net_device *fwd;
+ u32 index = ri->ifindex;
+ int err;
+
+ fwd = dev_get_by_index_rcu(dev_net(dev), index);
+ ri->ifindex = 0;
+ if (unlikely(!fwd)) {
+ err = -EINVAL;
+ goto err;
+ }
+
+ err = __bpf_tx_xdp(fwd, NULL, xdp, 0);
+ 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;
+}
+
static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
struct bpf_map *map,
struct xdp_buff *xdp,
@@ -3264,9 +3290,9 @@ void bpf_clear_redirect_map(struct bpf_map *map)
}
static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
- struct bpf_prog *xdp_prog, struct bpf_map *map)
+ struct bpf_prog *xdp_prog, struct bpf_map *map,
+ struct bpf_redirect_info *ri)
{
- struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
u32 index = ri->ifindex;
void *fwd = NULL;
int err;
@@ -3299,29 +3325,11 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
{
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
struct bpf_map *map = READ_ONCE(ri->map);
- struct net_device *fwd;
- u32 index = ri->ifindex;
- int err;
if (likely(map))
- return xdp_do_redirect_map(dev, xdp, xdp_prog, map);
+ return xdp_do_redirect_map(dev, xdp, xdp_prog, map, ri);
- fwd = dev_get_by_index_rcu(dev_net(dev), index);
- ri->ifindex = 0;
- if (unlikely(!fwd)) {
- err = -EINVAL;
- goto err;
- }
-
- err = __bpf_tx_xdp(fwd, NULL, xdp, 0);
- 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;
+ return xdp_do_redirect_slow(dev, xdp, xdp_prog, ri);
}
EXPORT_SYMBOL_GPL(xdp_do_redirect);
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect
2018-09-03 7:54 [bpf-next V2 PATCH 0/3] XDP micro optimizations for redirect Jesper Dangaard Brouer
` (2 preceding siblings ...)
2018-09-03 7:55 ` [bpf-next V2 PATCH 3/3] xdp: split code for map vs non-map redirect Jesper Dangaard Brouer
@ 2018-09-06 17:55 ` Alexei Starovoitov
3 siblings, 0 replies; 5+ messages in thread
From: Alexei Starovoitov @ 2018-09-06 17:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, Daniel Borkmann
On Mon, Sep 03, 2018 at 09:54:52AM +0200, Jesper Dangaard Brouer wrote:
> This patchset contains XDP micro optimizations for the redirect core.
> These are not functional changes. The optimizations revolve around
> getting the compiler to layout the code in a way that reflect how XDP
> redirect is used.
>
> Today the compiler chooses to inline and uninline (static C functions)
> in a suboptimal way, compared to how XDP redirect can be used. Perf
> top clearly shows that almost everything gets inlined into the
> function call xdp_do_redirect.
>
> The way the compiler chooses to inlines, does not reflect how XDP
> redirect is used, as the compile cannot know this.
Applied, Thanks
^ permalink raw reply [flat|nested] 5+ messages in thread