netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>, ast@kernel.org
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org,
	bjorn.topel@intel.com, magnus.karlsson@intel.com
Subject: Re: [PATCH v6 bpf-next 0/6] bpf: tailcalls in BPF subprograms
Date: Sat, 1 Aug 2020 03:03:19 +0200	[thread overview]
Message-ID: <fbe6e5ca-65ba-7698-3b8d-1214b5881e88@iogearbox.net> (raw)
In-Reply-To: <20200731000324.2253-1-maciej.fijalkowski@intel.com>

On 7/31/20 2:03 AM, Maciej Fijalkowski wrote:
> v5->v6:
> - propagate only those poke descriptors that individual subprogram is
>    actually using (Daniel)
> - drop the cumbersome check if poke desc got filled in map_poke_run()
> - move poke->ip renaming in bpf_jit_add_poke_descriptor() from patch 4
>    to patch 3 to provide bisectability (Daniel)

I did a basic test with Cilium on K8s with this set, spawning a few Pods
and checking connectivity & whether we're not crashing since it has bit more
elaborate tail call use. So far so good. I was inclined to push the series
out, but there is one more issue I noticed and didn't notice earlier when
reviewing, and that is overall stack size:

What happens when you create a single program that has nested BPF to BPF
calls e.g. either up to the maximum nesting or one call that is using up
the max stack size which is then doing another BPF to BPF call that contains
the tail call. In the tail call map, you have the same program in there.
This means we create a worst case stack from BPF size of max_stack_size *
max_tail_call_size, that is, 512*32. So that adds 16k worst case. For x86
we have a stack of arch/x86/include/asm/page_64_types.h:

   #define THREAD_SIZE_ORDER       (2 + KASAN_STACK_ORDER)
  #define THREAD_SIZE  (PAGE_SIZE << THREAD_SIZE_ORDER)

So we end up with 16k in a typical case. And this will cause kernel stack
overflow; I'm at least not seeing where we handle this situation in the
set. Hm, need to think more, but maybe this needs tracking of max stack
across tail calls to force an upper limit..

Thanks,
Daniel

  parent reply	other threads:[~2020-08-01  1:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-31  0:03 [PATCH v6 bpf-next 0/6] bpf: tailcalls in BPF subprograms Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 1/6] bpf, x64: use %rcx instead of %rax for tail call retpolines Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 2/6] bpf: propagate poke descriptors to subprograms Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 3/6] bpf: rename poke descriptor's 'ip' member to 'tailcall_target' Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 4/6] bpf, x64: rework pro/epilogue and tailcall handling in JIT Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 5/6] bpf: allow for tailcalls in BPF subprograms for x64 JIT Maciej Fijalkowski
2020-07-31  0:03 ` [PATCH v6 bpf-next 6/6] selftests: bpf: add dummy prog for bpf2bpf with tailcall Maciej Fijalkowski
2020-08-01  1:03 ` Daniel Borkmann [this message]
2020-08-01  7:13   ` [PATCH v6 bpf-next 0/6] bpf: tailcalls in BPF subprograms Maciej Fijalkowski
2020-08-02  3:07     ` Alexei Starovoitov
2020-08-03 14:00       ` Daniel Borkmann
2020-08-21 17:38         ` Maciej Fijalkowski
2020-08-26 21:35           ` Alexei Starovoitov
2020-08-29 23:19             ` Maciej Fijalkowski
2020-09-01 16:24               ` Alexei Starovoitov
2020-09-02 20:01                 ` Maciej Fijalkowski

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=fbe6e5ca-65ba-7698-3b8d-1214b5881e88@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=ast@kernel.org \
    --cc=bjorn.topel@intel.com \
    --cc=bpf@vger.kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).