netdev - Re: [PATCH v2] samples/bpf: Add xdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Jan 2020 12:35:22 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Eric Sage <eric@...e.org>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Martin Lau <kafai@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        john fastabend <john.fastabend@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>
Subject: Re: [PATCH v2] samples/bpf: Add xdp_stat sample program

On Sat, Jan 11, 2020 at 8:05 PM Eric Sage <eric@...e.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@...e.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");
> +

[...]