Re: [PATCH v2] samples/bpf: Add xdp_stat sample program

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

On Sat, Jan 11, 2020 at 8:05 PM Eric Sage <eric@sage.org> 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 <eric@sage.org>
> ---
> Changes in v2:
> - Upped max number of interceptors to 32 and mentioned the max in the
>   help.
> - Fixed license formatting.
> - Requested change to convert BPF map definitions to BTF.
>

I should have mentioned a gotcha with PROG_ARRAY earlier, sorry about
that. Use key_size/value_size for it. Other than that, looks good.

[...]

> diff --git a/samples/bpf/xdp_stat_kern.c b/samples/bpf/xdp_stat_kern.c
> new file mode 100644
> index 000000000000..83cff4807f72
> --- /dev/null
> +++ b/samples/bpf/xdp_stat_kern.c
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2019 Facebook
> + */
> +
> +/* Conceptually interception looks like this for a single packet:
> + *
> + * interceptor_0 -> entrypoint -> interceptor_1 -> prog_1 -> ... ->
> + * interceptor_N -> prog_N -> XDP_ACTION
> + *
> + * At any point in the chain, including in the entrypoint, an XDP_ACTION can
> + * be returned. It is also not assumed that the order of jumps will not change
> + * (except that the entrypoint always comes first).
> + *
> + * Because there is no way to hook into the return of the XDP action, the
> + * entrpoint (interceptor_0) is also used to record the terminal run of the

typo: entrypoint

> + * previous BPF program on the same CPU. Conceputally:

typo: Conceptually

> + *
> + * ... -> prog_N -> XDP_ACTION -> interceptor_0 -> ...
> + *
> + * FIXME: A bad side effect of this is that the reported stats will always be
> + * behind in tracking terminal runs which is confusing to the user.
> + */
> +

[...]

> +/* jmp_table_entry has a single entry - the original XDP entrpoint - so that
> + * the intercpetor entrypoint can jump to it.
> + */
> +struct {
> +       __uint(type, BPF_MAP_TYPE_PROG_ARRAY);
> +       __uint(max_entries, 1);
> +       __type(key, __u32);
> +       __type(value, __u32);

Did this actually work, when you run your tool? Few special maps still
don't support specifying type information, I think PROG_ARRAY is one
of those. For such cases you have to specify key_size and value_size,
instead of key/value types:

__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(__u32));


> +} jmp_table_entrypoint SEC(".maps");
> +

[...]