Hi,
On 12/27/2023 6:01 PM, Philo Lu wrote:
> A kfunc is needed to write into the relay channel, named
> bpf_relay_output. The usage is same as bpf_ringbuf_output helper. It
> only works after relay files are set, i.e., after calling
> map_update_elem for the created relay map.
>
> Signed-off-by: Philo Lu <lulie@xxxxxxxxxxxxxxxxx>
> ---
> kernel/bpf/helpers.c | 3 +++
> kernel/bpf/relaymap.c | 22 ++++++++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index be72824f32b2..22480b69ff27 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2617,6 +2617,9 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_null)
> BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
> BTF_ID_FLAGS(func, bpf_dynptr_size)
> BTF_ID_FLAGS(func, bpf_dynptr_clone)
> +#ifdef CONFIG_RELAY
> +BTF_ID_FLAGS(func, bpf_relay_output)
> +#endif
> BTF_SET8_END(common_btf_ids)
Could you explain the reason why bpf_relay_out is placed in
common_btf_ids instead of generic_kfunc_set ?
>
> static const struct btf_kfunc_id_set common_kfunc_set = {
> diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c
> index 02b33a8e6b6c..37280d60133c 100644
> --- a/kernel/bpf/relaymap.c
> +++ b/kernel/bpf/relaymap.c
> @@ -6,6 +6,7 @@
> #include <linux/slab.h>
> #include <linux/bpf.h>
> #include <linux/err.h>
> +#include <linux/btf.h>
>
> #define RELAY_CREATE_FLAG_MASK (BPF_F_OVERWRITE)
>
> @@ -197,3 +198,24 @@ const struct bpf_map_ops relay_map_ops = {
> .map_mem_usage = relay_map_mem_usage,
> .map_btf_id = &relay_map_btf_ids[0],
> };
> +
> +__bpf_kfunc_start_defs();
> +
> +__bpf_kfunc int bpf_relay_output(struct bpf_map *map,
> + void *data, u64 data__sz, u32 flags)
> +{
> + struct bpf_relay_map *rmap;
> +
> + /* not support any flag now */
> + if (unlikely(!map || flags))
> + return -EINVAL;
> +
> + rmap = container_of(map, struct bpf_relay_map, map);
How does bpf_relay_out() guarantee the passed map is a relay map ? And
just like bpf_map_sum_elem_count(), I think KF_TRUSTED_ARGS is also
necessary for bpf_relay_output().
> + if (!rmap->relay_chan->has_base_filename)
> + return -ENOENT;
> +
I think a comment is needed here. It needs to explain why checking
->has_base_filename is enough to guarantee the concurrently running of
bpf_relay_output() and .map_update_elem() is safe.
> + relay_write(rmap->relay_chan, data, data__sz);
> + return 0;
> +}
> +
> +__bpf_kfunc_end_defs();
