On Wed, Jul 21, 2021 at 2:28 PM Kumar Kartikeya Dwivedi
<memxor@xxxxxxxxx> wrote:
>
> This file implements some common helpers to consolidate differences in
> features and functionality between the various XDP samples and give them
> a consistent look, feel, and reporting capabilities.
>
> Some of the key features are:
> * A concise output format accompanied by helpful text explaining its
> fields.
> * An elaborate output format building upon the concise one, and folding
> out details in case of errors and staying out of view otherwise.
> * Extended reporting of redirect errors by capturing hits for each
> errno and displaying them inline (ENETDOWN, EINVAL, ENOSPC, etc.)
> to aid debugging.
> * Reporting of each xdp_exception action for all samples that use these
> helpers (XDP_ABORTED, etc.) to aid debugging.
> * Capturing per ifindex pair devmap_xmit counts for decomposing the
> total TX count per devmap redirection.
> * Ability to jump to source locations invoking tracepoints.
> * Faster retrieval of stats per polling interval using mmap'd eBPF
> array map (through .bss).
> * Printing driver names for devices redirecting packets.
> * Printing summarized total statistics for the entire session.
> * Ability to dynamically switch between concise and verbose mode, using
> SIGQUIT (Ctrl + \).
>
> The goal is sharing these helpers that most of the XDP samples implement
> in some form but differently for each, lacking in some respect compared
> to one another.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
> ---
> samples/bpf/Makefile | 6 +-
> samples/bpf/xdp_sample_shared.h | 53 ++
> samples/bpf/xdp_sample_user.c | 1380 +++++++++++++++++++++++++++++++
> samples/bpf/xdp_sample_user.h | 202 +++++
> 4 files changed, 1640 insertions(+), 1 deletion(-)
> create mode 100644 samples/bpf/xdp_sample_shared.h
> create mode 100644 samples/bpf/xdp_sample_user.c
> create mode 100644 samples/bpf/xdp_sample_user.h
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 036998d11ded..57ccff5ccac4 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -62,6 +62,7 @@ LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
>
> CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
> TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
> +XDP_SAMPLE := xdp_sample_user.o
>
> fds_example-objs := fds_example.o
> sockex1-objs := sockex1_user.o
> @@ -116,6 +117,8 @@ xdp_sample_pkts-objs := xdp_sample_pkts_user.o
> ibumad-objs := ibumad_user.o
> hbm-objs := hbm.o $(CGROUP_HELPERS)
>
> +xdp_sample_user-objs := xdp_sample_user.o $(LIBBPFDIR)/hashmap.o
> +
nope, hashmap is an internal libbpf API, no using it from samples, please
> # Tell kbuild to always build the programs
> always-y := $(tprogs-y)
> always-y += sockex1_kern.o
> @@ -201,6 +204,7 @@ TPROGS_CFLAGS += -Wstrict-prototypes
> TPROGS_CFLAGS += -I$(objtree)/usr/include
> TPROGS_CFLAGS += -I$(srctree)/tools/testing/selftests/bpf/
> TPROGS_CFLAGS += -I$(srctree)/tools/lib/
> +TPROGS_CFLAGS += -I$(srctree)/tools/lib/bpf
> TPROGS_CFLAGS += -I$(srctree)/tools/include
> TPROGS_CFLAGS += -I$(srctree)/tools/perf
> TPROGS_CFLAGS += -DHAVE_ATTR_TEST=0
> @@ -210,7 +214,7 @@ TPROGS_CFLAGS += --sysroot=$(SYSROOT)
> TPROGS_LDFLAGS := -L$(SYSROOT)/usr/lib
> endif
>
> -TPROGS_LDLIBS += $(LIBBPF) -lelf -lz
> +TPROGS_LDLIBS += $(LIBBPF) -lelf -lz -lm
> TPROGLDLIBS_tracex4 += -lrt
> TPROGLDLIBS_trace_output += -lrt
> TPROGLDLIBS_map_perf_test += -lrt
> diff --git a/samples/bpf/xdp_sample_shared.h b/samples/bpf/xdp_sample_shared.h
> new file mode 100644
> index 000000000000..b211dca233d9
> --- /dev/null
> +++ b/samples/bpf/xdp_sample_shared.h
> @@ -0,0 +1,53 @@
> +#ifndef _XDP_SAMPLE_SHARED_H
> +#define _XDP_SAMPLE_SHARED_H
> +
> +/*
> + * Best-effort relaxed load/store
> + * __atomic_load_n/__atomic_store_n built-in is not supported for BPF target
> + */
> +#define ATOMIC_LOAD(var) ({ (*(volatile typeof(var) *)&(var)); })
> +#define ATOMIC_STORE(var, val) ({ *((volatile typeof(var) *)&(var)) = (val); })
> +/* This does a load + store instead of the expensive atomic fetch add, but store
> + * is still atomic so that userspace reading the value reads the old or the new
> + * one, but not a partial store.
> + */
> +#define ATOMIC_ADD_NORMW(var, val) \
it's rather some sort of NO_TEAR_ADD, but definitely not an atomic
add? Wouldn't this ATOMIC_ADD name be too misleading?
> + ({ \
> + typeof(val) __val = (val); \
> + if (__val) \
> + ATOMIC_STORE((var), (var) + __val); \
> + })
> +
> +#define ATOMIC_INC_NORMW(var) ATOMIC_ADD_NORMW((var), 1)
> +
> +#define MAX_CPUS 64
> +
> +/* Values being read/stored must be word sized */
> +#if __LP64__
> +typedef __u64 datarec_t;
> +#else
> +typedef __u32 datarec_t;
> +#endif
why not use size_t instead of typedefs?
> +
> +struct datarec {
> + datarec_t processed;
> + datarec_t dropped;
> + datarec_t issue;
> + union {
> + datarec_t xdp_pass;
> + datarec_t info;
> + };
> + datarec_t xdp_drop;
> + datarec_t xdp_redirect;
> +} __attribute__((aligned(64)));
> +
[...]
> +#define __attach_tp(name) \
> + ({ \
> + if (!bpf_program__is_tracing(skel->progs.name)) \
> + return -EINVAL; \
> + skel->links.name = bpf_program__attach(skel->progs.name); \
> + if (!skel->links.name) \
you need to turn on libbpf 1.0 mode for errors to use NULL check, see
libbpf_set_strict_mode(); otherwise need to use libbpf_get_error() to
check
> + return -errno; \
> + })
> +
[...]
