[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPhsuW7U6twQbjXLTaEkCpddAaauaEb0k=g5YnCjmo5TtOx6iA@mail.gmail.com>
Date: Tue, 6 Sep 2022 15:02:23 -0700
From: Song Liu <song@...nel.org>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Jesper Dangaard Brouer <hawk@...nel.org>,
Björn Töpel <bjorn@...nel.org>,
Magnus Karlsson <magnus.karlsson@...el.com>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Jonathan Lemon <jonathan.lemon@...il.com>,
Eric Dumazet <edumazet@...gle.com>,
Paolo Abeni <pabeni@...hat.com>, bpf <bpf@...r.kernel.org>,
Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 2/3] bpf: Expand map key argument of
bpf_redirect_map to u64
On Mon, Sep 5, 2022 at 12:34 PM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> For queueing packets in XDP we want to add a new redirect map type with
> support for 64-bit indexes. To prepare fore this, expand the width of the
> 'key' argument to the bpf_redirect_map() helper. Since BPF registers are
> always 64-bit, this should be safe to do after the fact.
>
> Reviewed-by: Stanislav Fomichev <sdf@...gle.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
Acked-by: Song Liu <song@...nel.org>
> ---
> include/linux/bpf.h | 2 +-
> include/linux/filter.h | 12 ++++++------
> include/uapi/linux/bpf.h | 2 +-
> kernel/bpf/cpumap.c | 4 ++--
> kernel/bpf/devmap.c | 4 ++--
> kernel/bpf/verifier.c | 2 +-
> net/core/filter.c | 4 ++--
> net/xdp/xskmap.c | 4 ++--
> 8 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 9c1674973e03..222cba23e6d9 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -134,7 +134,7 @@ struct bpf_map_ops {
> struct bpf_local_storage __rcu ** (*map_owner_storage_ptr)(void *owner);
>
> /* Misc helpers.*/
> - int (*map_redirect)(struct bpf_map *map, u32 ifindex, u64 flags);
> + int (*map_redirect)(struct bpf_map *map, u64 key, u64 flags);
>
> /* map_meta_equal must be implemented for maps that can be
> * used as an inner map. It is a runtime check to ensure
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 527ae1d64e27..eff295509f03 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -637,13 +637,13 @@ struct bpf_nh_params {
> };
>
> struct bpf_redirect_info {
> - u32 flags;
> - u32 tgt_index;
> + u64 tgt_index;
> void *tgt_value;
> struct bpf_map *map;
> + u32 flags;
> + u32 kern_flags;
> u32 map_id;
> enum bpf_map_type map_type;
> - u32 kern_flags;
> struct bpf_nh_params nh;
> };
>
> @@ -1493,7 +1493,7 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
> }
> #endif /* IS_ENABLED(CONFIG_IPV6) */
>
> -static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifindex,
> +static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u64 index,
> u64 flags, const u64 flag_mask,
> void *lookup_elem(struct bpf_map *map, u32 key))
> {
> @@ -1504,7 +1504,7 @@ static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifind
> if (unlikely(flags & ~(action_mask | flag_mask)))
> return XDP_ABORTED;
>
> - ri->tgt_value = lookup_elem(map, ifindex);
> + ri->tgt_value = lookup_elem(map, index);
> if (unlikely(!ri->tgt_value) && !(flags & BPF_F_BROADCAST)) {
> /* If the lookup fails we want to clear out the state in the
> * redirect_info struct completely, so that if an eBPF program
> @@ -1516,7 +1516,7 @@ static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u32 ifind
> return flags & action_mask;
> }
>
> - ri->tgt_index = ifindex;
> + ri->tgt_index = index;
> ri->map_id = map->id;
> ri->map_type = map->map_type;
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 837c0f9b7fdd..c6d37ac2b87c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2629,7 +2629,7 @@ union bpf_attr {
> * Return
> * 0 on success, or a negative error in case of failure.
> *
> - * long bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
> + * long bpf_redirect_map(struct bpf_map *map, u64 key, u64 flags)
> * Description
> * Redirect the packet to the endpoint referenced by *map* at
> * index *key*. Depending on its type, this *map* can contain
> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index b5ba34ddd4b6..39ed08a2bb52 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -668,9 +668,9 @@ static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> return 0;
> }
>
> -static int cpu_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
> +static int cpu_map_redirect(struct bpf_map *map, u64 index, u64 flags)
> {
> - return __bpf_xdp_redirect_map(map, ifindex, flags, 0,
> + return __bpf_xdp_redirect_map(map, index, flags, 0,
> __cpu_map_lookup_elem);
> }
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index f9a87dcc5535..d01e4c55b376 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -992,14 +992,14 @@ static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
> map, key, value, map_flags);
> }
>
> -static int dev_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
> +static int dev_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
> {
> return __bpf_xdp_redirect_map(map, ifindex, flags,
> BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,
> __dev_map_lookup_elem);
> }
>
> -static int dev_hash_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
> +static int dev_hash_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
> {
> return __bpf_xdp_redirect_map(map, ifindex, flags,
> BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 068b20ed34d2..844a44694b6f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14169,7 +14169,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> BUILD_BUG_ON(!__same_type(ops->map_peek_elem,
> (int (*)(struct bpf_map *map, void *value))NULL));
> BUILD_BUG_ON(!__same_type(ops->map_redirect,
> - (int (*)(struct bpf_map *map, u32 ifindex, u64 flags))NULL));
> + (int (*)(struct bpf_map *map, u64 index, u64 flags))NULL));
> BUILD_BUG_ON(!__same_type(ops->map_for_each_callback,
> (int (*)(struct bpf_map *map,
> bpf_callback_t callback_fn,
> diff --git a/net/core/filter.c b/net/core/filter.c
> index ee768bb5b5ab..285eaee2b373 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4408,10 +4408,10 @@ static const struct bpf_func_proto bpf_xdp_redirect_proto = {
> .arg2_type = ARG_ANYTHING,
> };
>
> -BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
> +BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u64, key,
> u64, flags)
> {
> - return map->ops->map_redirect(map, ifindex, flags);
> + return map->ops->map_redirect(map, key, flags);
> }
>
> static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
> diff --git a/net/xdp/xskmap.c b/net/xdp/xskmap.c
> index acc8e52a4f5f..771d0fa90ef5 100644
> --- a/net/xdp/xskmap.c
> +++ b/net/xdp/xskmap.c
> @@ -231,9 +231,9 @@ static int xsk_map_delete_elem(struct bpf_map *map, void *key)
> return 0;
> }
>
> -static int xsk_map_redirect(struct bpf_map *map, u32 ifindex, u64 flags)
> +static int xsk_map_redirect(struct bpf_map *map, u64 index, u64 flags)
> {
> - return __bpf_xdp_redirect_map(map, ifindex, flags, 0,
> + return __bpf_xdp_redirect_map(map, index, flags, 0,
> __xsk_map_lookup_elem);
> }
>
> --
> 2.37.2
>
Powered by blists - more mailing lists