Re: [PATCH v5 bpf-next 05/10] bpf: tcp: Check BPF_SOCK_OPS_TEST_FLAG() after cgroup_bpf_enabled(CGROUP_SOCK_OPS).

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 10/08, Kuniyuki Iwashima wrote:
> BPF_CGROUP_RUN_PROG_XXX() macros guard __cgroup_bpf_run_filter_XXX()
> with cgroup_bpf_enabled().
> 
> However, even when no SOCK_OPS prog is attached, callers still
> initialise struct bpf_sock_ops_kern (memset(), etc.) or evaluate
> BPF_SOCK_OPS_TEST_FLAG(), which loads tp->bpf_sock_ops_cb_flags
> from a cold cacheline near the end of struct tcp_sock.
> 
> Similar to bpf_tcp_ops, let's check cgroup_bpf_enabled() before
> BPF_SOCK_OPS_TEST_FLAG() and struct bpf_sock_ops_kern setup, and
> rename BPF_CGROUP_RUN_PROG_SOCK_OPS{,_SK}() with __ prefix.
> 
> Since all direct callers of tcp_call_bpf() pass 0 and NULL for
> nargs and args, they can be folded into the new tcp_call_bpf()
> macro.
> 
> All callers of tcp_call_bpf_{2,3}arg() check BPF_SOCK_OPS_TEST_FLAG()
> and do not need the return value.  These are replaced with the new
> tcp_call_bpf_flag() macro.
> 
> Now, bpf_skops_hdr_opt_len() and bpf_tcp_ops_hdr_opt_len() are
> just NOPs without bpf prog attached.
> 
>   $ llvm-objdump -S -D --disassemble=tcp_established_options vmlinux
>   ...
>   ; 	asm goto(ARCH_STATIC_BRANCH_ASM("%c0 + %c1", "%l[l_yes]")
> >>ffffffff825127e6: 66 90                	nop
> >>ffffffff825127e8: 66 90                	nop
>   ; 	size = MAX_TCP_OPTION_SPACE - remaining;
>   ffffffff825127ea: b9 28 00 00 00       	movl	$0x28, %ecx
>   ffffffff825127ef: 29 c1                	subl	%eax, %ecx
>   ; 	return size;
>   ffffffff825127f1: 89 c8                	movl	%ecx, %eax
>   ...
>   ffffffff825127fb: 2e e9 6f 02 43 00    	jmp	0xffffffff82942a70 <__x86_return_thunk>
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
> v5: Always inline static key to tcp_established_options()

Acked-by: Stanislav Fomichev <sdf@fomichev.me>