On Sun, Dec 29, 2019 at 2:51 PM Eric Sage [off-list ref] wrote:

At Facebook we use tail calls to jump between our firewall filters and
our L4LB. This is a program I wrote to estimate per program performance
by swapping out the entries in the program array with interceptors that
take measurements and then jump to the original entries.

I found the sample programs to be invaluable in understanding how to use
the libbpf API (as well as the test env from the xdp-tutorial repo for
testing), and want to return the favor. I am currently working on
my next iteration that uses fentry/fexit to be less invasive,
but I thought it was an interesting PoC of what you can do with program
arrays.

Signed-off-by: Eric Sage <redacted>
---

Hey Eric,

Thanks for contributing this to samples. Ideally this would use BPF
skeleton to simplify all the map and program look ups, but given
samples/bpf doesn't have a infrastructure set up for this, it might be
a bit too much effort at this point. So I think it's ok to do it this
way, even though it would be really nice to try.

But please update the old-style bpf_map_def to BTF-defined maps before
this gets merged (see below).

 samples/bpf/Makefile          |   3 +
 samples/bpf/xdp_stat_common.h |  28 ++
 samples/bpf/xdp_stat_kern.c   | 169 ++++++++
 samples/bpf/xdp_stat_user.c   | 746 ++++++++++++++++++++++++++++++++++
 4 files changed, 946 insertions(+)
 create mode 100644 samples/bpf/xdp_stat_common.h
 create mode 100644 samples/bpf/xdp_stat_kern.c
 create mode 100644 samples/bpf/xdp_stat_user.c

[...]

+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+#include "xdp_stat_common.h"
+
+#define MAX_PROG_ARRAY 64
+
+/* NR is used to map interceptors to the programs that are being intercepted. */
+#define INTERCEPTOR(INDEX)                                                     \
+       SEC("xdp/interceptor_" #INDEX)                                         \
+       int interceptor_##INDEX(struct xdp_md *ctx)                            \
+       {                                                                      \
+               return interceptor_impl(ctx, INDEX);                           \
+       }
+
+       /* Required to use bpf_ktime_get_ns() */
+       char _license[] SEC("license") = "GPL";

Wrong indentation?

+
+/* interception_info holds a single record per CPU to pass global state between
+ * interceptor programs.
+ */
+struct bpf_map_def SEC("maps") interception_info = {
+       .type = BPF_MAP_TYPE_PERCPU_ARRAY,
+       .key_size = sizeof(__u32),
+       .value_size = sizeof(struct interception_info_rec),
+       .max_entries = 1,
+};

With BTF-define maps this would be

struct {
    __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
    __uint(max_entries, 1);
    __type(key, __u32);
    __type(value, struct interception_info_rec);
} interception_info SEC(".maps");

Can you please switch all the maps to this new syntax?

+
+/* interceptor_stats maps interceptor indexes to measurements of an intercepted
+ * BPF program. Index 0 maps the interceptor entrypoint to measurements of the
+ * original entrypoint.
+ */
+struct bpf_map_def SEC("maps") prog_stats = {
+       .type = BPF_MAP_TYPE_PERCPU_ARRAY,
+       .key_size = sizeof(__u32),
+       .value_size = sizeof(struct prog_stats_rec),
+       .max_entries = MAX_PROG_ARRAY,
+};
+

[...]

+static struct intercept_ctx *intercept__setup(char *mappath, int ifindex)
+{
+       struct intercept_ctx *ctx = malloc(sizeof(struct intercept_ctx));
+
+       ctx->ifindex = ifindex;
+
+       ctx->stats_enabled_oldval =
+               _update_sysctl("/proc/sys/kernel/bpf_stats_enabled", 1);
+       if (ctx->stats_enabled_oldval < 0)
+               perror("ERR: set bpf_stats_enabled sysctl failed\n");
+
+       if (bpf_prog_load(FILENAME_XDP_STAT_KERN, BPF_PROG_TYPE_XDP,
+                         &ctx->bpf_obj, &ctx->entry_fd)) {

bpf_object__open() + bpf_object__load() is preferred over
bpf_prog_load() way to open and load BPF object files, would be nice
to use that instead.

Alternative is BPF skeleton, but as I said, it will require a bit more
set up in Makefile (you'd need to generate skeleton with `bpftool gen
skeleton <your-bpf-object>.o`). Few benefits of BPF skeleton, though:
  - you wouldn't need to look up maps and programs by name, they would
be accessible as struct fields of skeleton (e.g.,
skel->maps.interception_info);
  - BPF object file would be embedded into userspace program, so when
distributing this you wouldn't need to drag along a separate .o file.

+               fprintf(stderr, "ERR: failed to load %s\n",
+                       FILENAME_XDP_STAT_KERN);
+               return NULL;
+       }
+

[...]