[bpf-next,v2,0/6] Various BPF helper improvements

This series adds two BPF helpers, that is, one for retrieving the classid
of an skb and another one to redirect via the neigh subsystem, and improves
also the cookie helpers by removing the atomic counter. I've also added
the bpf_tail_call_static() helper to the libbpf API that we've been using
in Cilium for a while now, and last but not least the series adds a few
selftests. For details, please check individual patches, thanks!

v1 -> v2:
  - Rework cookie generator to support nested contexts (Eric)
  - Use ip_neigh_gw6() and container_of() (David)
  - Rename __throw_build_bug() and improve comments (Andrii)
  - Use bpf_tail_call_static() also in BPF samples (Maciej)

Daniel Borkmann (6):
  bpf: add classid helper only based on skb->sk
  bpf, net: rework cookie generator as per-cpu one
  bpf: add redirect_neigh helper as redirect drop-in
  bpf, libbpf: add bpf_tail_call_static helper for bpf programs
  bpf, selftests: use bpf_tail_call_static where appropriate
  bpf, selftests: add redirect_neigh selftest

 include/linux/cookie.h                        |  50 +++
 include/linux/skbuff.h                        |   5 +
 include/linux/sock_diag.h                     |  14 +-
 include/net/net_namespace.h                   |   2 +-
 include/uapi/linux/bpf.h                      |  24 ++
 kernel/bpf/reuseport_array.c                  |   2 +-
 net/core/filter.c                             | 286 ++++++++++++++++--
 net/core/net_namespace.c                      |   9 +-
 net/core/sock_diag.c                          |   9 +-
 net/core/sock_map.c                           |   4 +-
 samples/bpf/sockex3_kern.c                    |  20 +-
 tools/include/uapi/linux/bpf.h                |  24 ++
 tools/lib/bpf/bpf_helpers.h                   |  46 +++
 tools/testing/selftests/bpf/progs/bpf_flow.c  |  12 +-
 tools/testing/selftests/bpf/progs/tailcall1.c |  28 +-
 tools/testing/selftests/bpf/progs/tailcall2.c |  14 +-
 tools/testing/selftests/bpf/progs/tailcall3.c |   4 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf1.c   |   4 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf2.c   |   6 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf3.c   |   6 +-
 .../selftests/bpf/progs/tailcall_bpf2bpf4.c   |   6 +-
 .../selftests/bpf/progs/test_tc_neigh.c       | 144 +++++++++
 tools/testing/selftests/bpf/test_tc_neigh.sh  | 168 ++++++++++
 23 files changed, 807 insertions(+), 80 deletions(-)
 create mode 100644 include/linux/cookie.h
 create mode 100644 tools/testing/selftests/bpf/progs/test_tc_neigh.c
 create mode 100755 tools/testing/selftests/bpf/test_tc_neigh.sh

On Mon, Sep 28, 2020 at 7:39 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Port of tail_call_static() helper function from Cilium's BPF code base [0]
> to libbpf, so others can easily consume it as well. We've been using this
> in production code for some time now. The main idea is that we guarantee
> that the kernel's BPF infrastructure and JIT (here: x86_64) can patch the
> JITed BPF insns with direct jumps instead of having to fall back to using
> expensive retpolines. By using inline asm, we guarantee that the compiler
> won't merge the call from different paths with potentially different
> content of r2/r3.
>
> We're also using Cilium's __throw_build_bug() macro (here as: __bpf_unreachable())
> in different places as a neat trick to trigger compilation errors when
> compiler does not remove code at compilation time. This works for the BPF
> back end as it does not implement the __builtin_trap().
>
>   [0] https://github.com/cilium/cilium/commit/f5537c26020d5297b70936c6b7d03a1e412a1035
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Andrii Nakryiko <andriin@fb.com>
> ---

few optional nits below, but looks good to me:

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  tools/lib/bpf/bpf_helpers.h | 46 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>

[...]

> +/*
> + * Helper function to perform a tail call with a constant/immediate map slot.
> + */
> +static __always_inline void
> +bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)

nit: const void *ctx would work here, right? would avoid users having
to do unnecessary casts in some cases

> +{
> +       if (!__builtin_constant_p(slot))
> +               __bpf_unreachable();
> +
> +       /*
> +        * Provide a hard guarantee that LLVM won't optimize setting r2 (map
> +        * pointer) and r3 (constant map index) from _different paths_ ending
> +        * up at the _same_ call insn as otherwise we won't be able to use the
> +        * jmpq/nopl retpoline-free patching by the x86-64 JIT in the kernel
> +        * given they mismatch. See also d2e4c1e6c294 ("bpf: Constant map key
> +        * tracking for prog array pokes") for details on verifier tracking.
> +        *
> +        * Note on clobber list: we need to stay in-line with BPF calling
> +        * convention, so even if we don't end up using r0, r4, r5, we need
> +        * to mark them as clobber so that LLVM doesn't end up using them
> +        * before / after the call.
> +        */
> +       asm volatile("r1 = %[ctx]\n\t"
> +                    "r2 = %[map]\n\t"
> +                    "r3 = %[slot]\n\t"
> +                    "call 12\n\t"

nit: it's weird to have tabs at the end of each string literal,
especially that r1 doesn't start with a tab...

> +                    :: [ctx]"r"(ctx), [map]"r"(map), [slot]"i"(slot)
> +                    : "r0", "r1", "r2", "r3", "r4", "r5");
> +}
> +
>  /*
>   * Helper structure used by eBPF C program
>   * to describe BPF map attributes to libbpf loader
> --
> 2.21.0
>

On 9/28/20 7:39 PM, Andrii Nakryiko wrote:
> On Mon, Sep 28, 2020 at 7:39 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> Port of tail_call_static() helper function from Cilium's BPF code base [0]
>> to libbpf, so others can easily consume it as well. We've been using this
>> in production code for some time now. The main idea is that we guarantee
>> that the kernel's BPF infrastructure and JIT (here: x86_64) can patch the
>> JITed BPF insns with direct jumps instead of having to fall back to using
>> expensive retpolines. By using inline asm, we guarantee that the compiler
>> won't merge the call from different paths with potentially different
>> content of r2/r3.
>>
>> We're also using Cilium's __throw_build_bug() macro (here as: __bpf_unreachable())
>> in different places as a neat trick to trigger compilation errors when
>> compiler does not remove code at compilation time. This works for the BPF
>> back end as it does not implement the __builtin_trap().
>>
>>    [0] https://github.com/cilium/cilium/commit/f5537c26020d5297b70936c6b7d03a1e412a1035
>>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Andrii Nakryiko <andriin@fb.com>
>> ---
> 
> few optional nits below, but looks good to me:
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>
[...]

Thanks!

>> +/*
>> + * Helper function to perform a tail call with a constant/immediate map slot.
>> + */
>> +static __always_inline void
>> +bpf_tail_call_static(void *ctx, const void *map, const __u32 slot)
> 
> nit: const void *ctx would work here, right? would avoid users having
> to do unnecessary casts in some cases

The bpf_tail_call() UAPI signature only has 'void *ctx' as well, so shouldn't
cause any issues as drop-in replacement wrt casts, but also ctx here is not always
a read-only [context] object either so it may potentially be a bit misleading to
users to convert it to const.