bpf_xdp_redirect_map: Perform map lookup in eBPF helper

The bpf_redirect_map() helper used by XDP programs doesn't return any
indication of whether it can successfully redirect to the map index it was
given. Instead, BPF programs have to track this themselves, leading to
programs using duplicate maps to track which entries are populated in the
devmap.

This patch fixes this by moving the map lookup into the bpf_redirect_map()
helper, which makes it possible to return failure to the eBPF program. The
lower bits of the flags argument is used as the return code, which means
that existing users who pass a '0' flag argument will get XDP_ABORTED.

With this, a BPF program can check the return code from the helper call and
react by, for instance, substituting a different redirect. This works for
any type of map used for redirect.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 92bd192f778689fc72a49b7927da10abd6468f35..1fe53e78c7e38ad52111a674fc876d5501c56768 100644 (file)
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -580,6 +580,7 @@ struct bpf_skb_data_end {
 struct bpf_redirect_info {
        u32 flags;
        u32 tgt_index;
+       void *tgt_value;
        struct bpf_map *map;
        struct bpf_map *map_to_flush;
        u32 kern_flags;

diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 81e708c4b5138c35cd5b35c51d2fe0ec113de998..68899fdc985b9d2e5dbc174dc29f9844ebebc4fd 100644 (file)
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -175,9 +175,8 @@ struct _bpf_dtab_netdev {
 #endif /* __DEVMAP_OBJ_TYPE */
 
 #define devmap_ifindex(fwd, map)                               \
-       (!fwd ? 0 :                                             \
-        ((map->map_type == BPF_MAP_TYPE_DEVMAP) ?              \
-         ((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0))
+       ((map->map_type == BPF_MAP_TYPE_DEVMAP) ?               \
+         ((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0)
 
 #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)               \
         trace_xdp_redirect_map(dev, xdp, devmap_ifindex(fwd, map),     \

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index a396b516a2b258bb5bd3f5f6fa41fd452001cef9..cffea1826a1f2ac20b0d878c5a87f240c2f1e1ce 100644 (file)
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1571,8 +1571,11 @@ union bpf_attr {
  *             but this is only implemented for native XDP (with driver
  *             support) as of this writing).
  *
- *             All values for *flags* are reserved for future usage, and must
- *             be left at zero.
+ *             The lower two bits of *flags* are used as the return code if
+ *             the map lookup fails. This is so that the return value can be
+ *             one of the XDP program return codes up to XDP_TX, as chosen by
+ *             the caller. Any higher bits in the *flags* argument must be
+ *             unset.
  *
  *             When used to redirect packets to net devices, this helper
  *             provides a high performance increase over **bpf_redirect**\ ().

diff --git a/net/core/filter.c b/net/core/filter.c
index b4a062379bb99e058df3555d1170fc49a3f4e51e..4836264f82eec9c6e580dfd0dfe6fa504f58ffb5 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3605,17 +3605,13 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
                               struct bpf_redirect_info *ri)
 {
        u32 index = ri->tgt_index;
-       void *fwd = NULL;
+       void *fwd = ri->tgt_value;
        int err;
 
        ri->tgt_index = 0;
+       ri->tgt_value = NULL;
        WRITE_ONCE(ri->map, NULL);
 
-       fwd = __xdp_map_lookup_elem(map, index);
-       if (unlikely(!fwd)) {
-               err = -EINVAL;
-               goto err;
-       }
        if (ri->map_to_flush && unlikely(ri->map_to_flush != map))
                xdp_do_flush_map();
 
@@ -3652,18 +3648,13 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
 {
        struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
        u32 index = ri->tgt_index;
-       void *fwd = NULL;
+       void *fwd = ri->tgt_value;
        int err = 0;
 
        ri->tgt_index = 0;
+       ri->tgt_value = NULL;
        WRITE_ONCE(ri->map, NULL);
 
-       fwd = __xdp_map_lookup_elem(map, index);
-       if (unlikely(!fwd)) {
-               err = -EINVAL;
-               goto err;
-       }
-
        if (map->map_type == BPF_MAP_TYPE_DEVMAP) {
                struct bpf_dtab_netdev *dst = fwd;
 
@@ -3732,6 +3723,7 @@ BPF_CALL_2(bpf_xdp_redirect, u32, ifindex, u64, flags)
 
        ri->flags = flags;
        ri->tgt_index = ifindex;
+       ri->tgt_value = NULL;
        WRITE_ONCE(ri->map, NULL);
 
        return XDP_REDIRECT;
@@ -3750,9 +3742,21 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
 {
        struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
 
-       if (unlikely(flags))
+       /* Lower bits of the flags are used as return code on lookup failure */
+       if (unlikely(flags > XDP_TX))
                return XDP_ABORTED;
 
+       ri->tgt_value = __xdp_map_lookup_elem(map, ifindex);
+       if (unlikely(!ri->tgt_value)) {
+               /* If the lookup fails we want to clear out the state in the
+                * redirect_info struct completely, so that if an eBPF program
+                * performs multiple lookups, the last one always takes
+                * precedence.
+                */
+               WRITE_ONCE(ri->map, NULL);
+               return flags;
+       }
+
        ri->flags = flags;
        ri->tgt_index = ifindex;
        WRITE_ONCE(ri->map, map);