Re: [PATCH bpf-next v2 5/8] samples: bpf: Convert xdp_redirect to use XDP samples helper

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Wed, Jul 21, 2021 at 2:28 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> This change converts XDP redirect tool to use the XDP samples support
> introduced in previous changes.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  samples/bpf/Makefile            |  10 +-
>  samples/bpf/xdp_redirect.bpf.c  |  52 +++++++
>  samples/bpf/xdp_redirect_kern.c |  90 -----------
>  samples/bpf/xdp_redirect_user.c | 262 +++++++++++++-------------------
>  4 files changed, 160 insertions(+), 254 deletions(-)
>  create mode 100644 samples/bpf/xdp_redirect.bpf.c
>  delete mode 100644 samples/bpf/xdp_redirect_kern.c
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 1b4838b2beb0..b94b6dac09ff 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -39,7 +39,6 @@ tprogs-y += lwt_len_hist
>  tprogs-y += xdp_tx_iptunnel
>  tprogs-y += test_map_in_map
>  tprogs-y += per_socket_stats_example
> -tprogs-y += xdp_redirect
>  tprogs-y += xdp_redirect_map
>  tprogs-y += xdp_redirect_map_multi
>  tprogs-y += xdp_redirect_cpu
> @@ -56,6 +55,7 @@ tprogs-y += xdp_sample_pkts
>  tprogs-y += ibumad
>  tprogs-y += hbm
>
> +tprogs-y += xdp_redirect
>  tprogs-y += xdp_monitor
>
>  # Libbpf dependencies
> @@ -100,7 +100,6 @@ lwt_len_hist-objs := lwt_len_hist_user.o
>  xdp_tx_iptunnel-objs := xdp_tx_iptunnel_user.o
>  test_map_in_map-objs := test_map_in_map_user.o
>  per_socket_stats_example-objs := cookie_uid_helper_example.o
> -xdp_redirect-objs := xdp_redirect_user.o
>  xdp_redirect_map-objs := xdp_redirect_map_user.o
>  xdp_redirect_map_multi-objs := xdp_redirect_map_multi_user.o
>  xdp_redirect_cpu-objs := xdp_redirect_cpu_user.o
> @@ -118,6 +117,7 @@ ibumad-objs := ibumad_user.o
>  hbm-objs := hbm.o $(CGROUP_HELPERS)
>
>  xdp_sample_user-objs := xdp_sample_user.o $(LIBBPFDIR)/hashmap.o
> +xdp_redirect-objs := xdp_redirect_user.o $(XDP_SAMPLE)
>  xdp_monitor-objs := xdp_monitor_user.o $(XDP_SAMPLE)
>
>  # Tell kbuild to always build the programs
> @@ -164,7 +164,6 @@ always-y += tcp_clamp_kern.o
>  always-y += tcp_basertt_kern.o
>  always-y += tcp_tos_reflect_kern.o
>  always-y += tcp_dumpstats_kern.o
> -always-y += xdp_redirect_kern.o
>  always-y += xdp_redirect_map_kern.o
>  always-y += xdp_redirect_map_multi_kern.o
>  always-y += xdp_redirect_cpu_kern.o
> @@ -315,6 +314,7 @@ verify_target_bpf: verify_cmds
>  $(BPF_SAMPLES_PATH)/*.c: verify_target_bpf $(LIBBPF)
>  $(src)/*.c: verify_target_bpf $(LIBBPF)
>
> +$(obj)/xdp_redirect_user.o: $(obj)/xdp_redirect.skel.h
>  $(obj)/xdp_monitor_user.o: $(obj)/xdp_monitor.skel.h
>
>  $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
> @@ -358,6 +358,7 @@ endef
>
>  CLANG_SYS_INCLUDES = $(call get_sys_includes,$(CLANG))
>
> +$(obj)/xdp_redirect.bpf.o: $(obj)/xdp_sample.bpf.o
>  $(obj)/xdp_monitor.bpf.o: $(obj)/xdp_sample.bpf.o
>
>  $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h $(src)/xdp_sample_shared.h
> @@ -368,9 +369,10 @@ $(obj)/%.bpf.o: $(src)/%.bpf.c $(obj)/vmlinux.h $(src)/xdp_sample.bpf.h $(src)/x
>                 -I$(srctree)/tools/lib $(CLANG_SYS_INCLUDES) \
>                 -c $(filter %.bpf.c,$^) -o $@
>
> -LINKED_SKELS := xdp_monitor.skel.h
> +LINKED_SKELS := xdp_redirect.skel.h xdp_monitor.skel.h
>  clean-files += $(LINKED_SKELS)
>
> +xdp_redirect.skel.h-deps := xdp_redirect.bpf.o xdp_sample.bpf.o
>  xdp_monitor.skel.h-deps := xdp_monitor.bpf.o xdp_sample.bpf.o
>
>  LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.bpf.c,$(foreach skel,$(LINKED_SKELS),$($(skel)-deps)))
> diff --git a/samples/bpf/xdp_redirect.bpf.c b/samples/bpf/xdp_redirect.bpf.c
> new file mode 100644
> index 000000000000..f5098812db36
> --- /dev/null
> +++ b/samples/bpf/xdp_redirect.bpf.c
> @@ -0,0 +1,52 @@
> +/* Copyright (c) 2016 John Fastabend <john.r.fastabend@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + */
> +#define KBUILD_MODNAME "foo"
> +
> +#include "vmlinux.h"
> +#include "xdp_sample.bpf.h"
> +#include "xdp_sample_shared.h"
> +
> +const volatile int ifindex_out;
> +
> +SEC("xdp_redirect")

can you please use "canonical" SEC("xdp") everywhere and use function
names as unique BPF program identifiers. Section name is the program
type specification (plus extra type-specific argument where
applicable, like tracepoint name), not a unique identifier.

> +int xdp_redirect_prog(struct xdp_md *ctx)
> +{
> +       void *data_end = (void *)(long)ctx->data_end;
> +       void *data = (void *)(long)ctx->data;
> +       u32 key = bpf_get_smp_processor_id();
> +       struct ethhdr *eth = data;
> +       struct datarec *rec;
> +       u64 nh_off;
> +

[...]