On Thu, Sep 17, 2020 at 08:26:34PM -0700, Alexei Starovoitov wrote: > On Wed, Sep 16, 2020 at 2:16 PM Maciej Fijalkowski > <maciej.fijalkowski@xxxxxxxxx> wrote: > > > > Changelog: > > > > v7->v8: > > - teach bpf_patch_insn_data to adjust insn_idx of tailcall insn > > - address case of clearing tail call counter when bpf2bpf with tailcalls > > are mixed > > - drop unnecessary checks against cbpf in JIT > > - introduce more tailcall_bpf2bpf[X] selftests that are making sure the > > combination of tailcalls with bpf2bpf calls works correctly > > - add test cases to test_verifier to make sure logic from > > check_max_stack_depth that limits caller's stack depth down to 256 is > > correct > > - move the main patch to appear after the one that limits the caller's > > stack depth so that 'has_tail_call' can be used by 'tail_call_reachable' > > logic > > Thanks a lot for your hard work on this set. 5 month effort! Thanks for the whole collaboration! It was quite a ride :) > I think it's a huge milestone that will enable cilium, cloudflare, katran to > use bpf functions. Removing always_inline will improve performance. > Switching to global functions with function-by-function verification > will drastically improve program load times. > libbpf has full support for subprogram composition and call relocations. > Until now these verifier and libbpf features were impossible to use in XDP > programs, since most of them use tail_calls. > It's great to see all these building blocks finally coming together. > > I've applied the set with few changes. > In patch 4 I've removed ifdefs and redundant (). > In patch 5 removed redundant !tail_call_reachable check. > In patch 6 replaced CONFIG_JIT_ALWAYS_ON dependency with > jit_requested && IS_ENABLED(CONFIG_X86_64). > It's more user friendly. > I also added patch 7 that checks that ld_abs and tail_call are only > allowed in subprograms that return 'int'. Thank you for this last touch! I went through patches and I agree with the changes. > I felt that the fix is simple enough, so I just pushed it, since > without it the set is not safe. Please review it here: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=09b28d76eac48e922dc293da1aa2b2b85c32aeee LGTM. > I'll address any issues in the followups. > Because of the above changes I tweaked patch 8 to increase test coverage > with ld_abs and combination of global/static subprogs. > Also did s/__attribute__((noinline))/__noinline/. > > John and Daniel, > I wasn't able to test it on cilium programs. > When you have a chance please give it a thorough run. > tail_call poke logic is delicate. > > Lorenz, > if you can test it on cloudflare progs would be awesome. > > Thanks a lot everyone!
