Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: bpf <bpf@vger.kernel.org>, "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Jesper Dangaard Brouer" <brouer@redhat.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	Networking <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next v2 2/8] samples: bpf: Add common infrastructure for XDP samples
Date: Thu, 22 Jul 2021 20:34:59 -0700	[thread overview]
Message-ID: <CAEf4BzaZa+0srsKKifF_Hanbojpu_GsbWc8rbzcfjb8ek49S7Q@mail.gmail.com> (raw)
In-Reply-To: <20210721212833.701342-3-memxor@gmail.com>

On Wed, Jul 21, 2021 at 2:28 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> 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@gmail.com>
> ---
>  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;                                         \
> +       })
> +

[...]

  reply	other threads:[~2021-07-23  3:35 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-21 21:28 [PATCH bpf-next v2 0/8] Improve XDP samples usability and output Kumar Kartikeya Dwivedi
2021-07-21 21:28 ` [PATCH bpf-next v2 1/8] samples: bpf: fix a couple of warnings Kumar Kartikeya Dwivedi
2021-07-21 21:28 ` [PATCH bpf-next v2 2/8] samples: bpf: Add common infrastructure for XDP samples Kumar Kartikeya Dwivedi
2021-07-23  3:34   ` Andrii Nakryiko [this message]
2021-07-21 21:28 ` [PATCH bpf-next v2 3/8] samples: bpf: Add BPF support for XDP samples helper Kumar Kartikeya Dwivedi
2021-07-23  3:42   ` Andrii Nakryiko
2021-07-21 21:28 ` [PATCH bpf-next v2 4/8] samples: bpf: Convert xdp_monitor to use " Kumar Kartikeya Dwivedi
2021-07-21 21:28 ` [PATCH bpf-next v2 5/8] samples: bpf: Convert xdp_redirect " Kumar Kartikeya Dwivedi
2021-07-23  3:49   ` Andrii Nakryiko
2021-07-21 21:28 ` [PATCH bpf-next v2 6/8] samples: bpf: Convert xdp_redirect_map to use XDP samples helpers Kumar Kartikeya Dwivedi
2021-07-21 21:28 ` [PATCH bpf-next v2 7/8] samples: bpf: Convert xdp_redirect_map_multi " Kumar Kartikeya Dwivedi
2021-07-21 21:28 ` [PATCH bpf-next v2 8/8] samples: bpf: Convert xdp_redirect_cpu " Kumar Kartikeya Dwivedi
2021-07-23 21:32 ` [PATCH bpf-next v2 0/8] Improve XDP samples usability and output Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAEf4BzaZa+0srsKKifF_Hanbojpu_GsbWc8rbzcfjb8ek49S7Q@mail.gmail.com \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=toke@redhat.com \
    --subject='Re: [PATCH bpf-next v2 2/8] samples: bpf: Add common infrastructure for XDP samples' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).