On Thu, 13 Jul 2017 13:14:30 +0200
Jesper Dangaard Brouer <[email protected]> wrote:
> I'm still getting crashes (but much harder to provoke), but I figured
> out why. We sort of missed one case, where map_to_flush gets set, when
> the ndo_xdp_xmit() call starts to fail, and the ixgbe driver then
> forgets to call xdp_do_flush_map, if all packets in that NAPI cycle
> failed. We could blame the driver, but yhe clean solution is making
> sure, that we don't set map_to_flush when the __bpf_tx_xdp() call
> fails. It should also handle the other case I fixed .... I'll cleanup
> my PoC-fix patch, test it and provide it here.
I changed flow in the function to be:
int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
struct bpf_prog *xdp_prog)
{
struct redirect_info *ri = this_cpu_ptr(&redirect_info);
struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
int err = -EINVAL;
ri->ifindex = 0;
ri->map = NULL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd)
goto out;
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(dev, fwd, xdp_prog, XDP_REDIRECT);
return err;
}
The diff is:
diff --git a/net/core/filter.c b/net/core/filter.c
index 4ca895d6ed51..c50a7ec2cdab 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2483,26 +2483,25 @@ 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;
+
+ ri->ifindex = 0;
+ ri->map = NULL;
fwd = __dev_map_lookup_elem(map, index);
if (!fwd)
goto out;
- ri->ifindex = 0;
-
if (ri->map_to_flush && (ri->map_to_flush != map))
xdp_do_flush_map();
- ri->map_to_flush = map;
- ri->map = NULL;
+ err = __bpf_tx_xdp(fwd, map, xdp, index);
+ if (likely(!err))
+ ri->map_to_flush = map;
- trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT);
-
- return __bpf_tx_xdp(fwd, map, xdp, index);
out:
- ri->ifindex = 0;
- ri->map = NULL;
- return -EINVAL;
+ trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT);
+ return err;
}
int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
