| Message ID | 20201208201533.1312057-2-revest@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v3,1/4] bpf: Be less specific about socket cookies guarantees | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | warning | Series does not have a cover letter |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 15659 this patch: 15659 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: please, no space before tabs |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 15324 this patch: 15324 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Tue, Dec 8, 2020 at 9:20 PM Florent Revest <revest@chromium.org> wrote: > > This needs two new helpers, one that works in a sleepable context (using > sock_gen_cookie which disables/enables preemption) and one that does not > (for performance reasons). Both take a struct sock pointer and need to > check it for NULLness. > > This helper could also be useful to other BPF program types such as LSM. > > Signed-off-by: Florent Revest <revest@chromium.org> > --- > include/linux/bpf.h | 2 ++ > include/uapi/linux/bpf.h | 7 +++++++ > kernel/trace/bpf_trace.c | 4 ++++ > net/core/filter.c | 24 ++++++++++++++++++++++++ > tools/include/uapi/linux/bpf.h | 7 +++++++ > 5 files changed, 44 insertions(+) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index d05e75ed8c1b..2ecda549b773 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1859,6 +1859,8 @@ extern const struct bpf_func_proto bpf_snprintf_btf_proto; > extern const struct bpf_func_proto bpf_per_cpu_ptr_proto; > extern const struct bpf_func_proto bpf_this_cpu_ptr_proto; > extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto; > +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_sleepable_proto; > +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto; > > const struct bpf_func_proto *bpf_tracing_func_proto( > enum bpf_func_id func_id, const struct bpf_prog *prog); > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index ba59309f4d18..9ac66cf25959 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -1667,6 +1667,13 @@ union bpf_attr { > * Return > * A 8-byte long unique number. > * > + * u64 bpf_get_socket_cookie(void *sk) > + * Description > + * Equivalent to **bpf_get_socket_cookie**\ () helper that accepts > + * *sk*, but gets socket from a BTF **struct sock**. > + * Return > + * A 8-byte long unique number. > + * > * u32 bpf_get_socket_uid(struct sk_buff *skb) > * Return > * The owner UID of the socket associated to *skb*. If the socket > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c > index 0cf0a6331482..99accc2146bc 100644 > --- a/kernel/trace/bpf_trace.c > +++ b/kernel/trace/bpf_trace.c > @@ -1778,6 +1778,10 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > return &bpf_sk_storage_get_tracing_proto; > case BPF_FUNC_sk_storage_delete: > return &bpf_sk_storage_delete_tracing_proto; > + case BPF_FUNC_get_socket_cookie: > + return prog->aux->sleepable ? > + &bpf_get_socket_ptr_cookie_sleepable_proto : > + &bpf_get_socket_ptr_cookie_proto; > #endif > case BPF_FUNC_seq_printf: > return prog->expected_attach_type == BPF_TRACE_ITER ? > diff --git a/net/core/filter.c b/net/core/filter.c > index 77001a35768f..34877796ab5b 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -4631,6 +4631,30 @@ static const struct bpf_func_proto bpf_get_socket_cookie_sock_proto = { > .arg1_type = ARG_PTR_TO_CTX, > }; > > +BPF_CALL_1(bpf_get_socket_ptr_cookie_sleepable, struct sock *, sk) > +{ > + return sk ? sock_gen_cookie(sk) : 0; My understanding is you can simply always call sock_gen_cookie and not have two protos. This will disable preemption in sleepable programs and not have any effect in non-sleepable programs since preemption will already be disabled.
On Tue, 2020-12-08 at 23:08 +0100, KP Singh wrote: > My understanding is you can simply always call sock_gen_cookie and > not have two protos. > > This will disable preemption in sleepable programs and not have any > effect in non-sleepable programs since preemption will already be > disabled. Sure, that works. I thought that providing two helper implems would slightly improve performances on non-sleepable programs but I can send a v4 with only one helper that calls sock_gen_cookie.
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index d05e75ed8c1b..2ecda549b773 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1859,6 +1859,8 @@ extern const struct bpf_func_proto bpf_snprintf_btf_proto; extern const struct bpf_func_proto bpf_per_cpu_ptr_proto; extern const struct bpf_func_proto bpf_this_cpu_ptr_proto; extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto; +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_sleepable_proto; +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto; const struct bpf_func_proto *bpf_tracing_func_proto( enum bpf_func_id func_id, const struct bpf_prog *prog); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index ba59309f4d18..9ac66cf25959 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -1667,6 +1667,13 @@ union bpf_attr { * Return * A 8-byte long unique number. * + * u64 bpf_get_socket_cookie(void *sk) + * Description + * Equivalent to **bpf_get_socket_cookie**\ () helper that accepts + * *sk*, but gets socket from a BTF **struct sock**. + * Return + * A 8-byte long unique number. + * * u32 bpf_get_socket_uid(struct sk_buff *skb) * Return * The owner UID of the socket associated to *skb*. If the socket diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 0cf0a6331482..99accc2146bc 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1778,6 +1778,10 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) return &bpf_sk_storage_get_tracing_proto; case BPF_FUNC_sk_storage_delete: return &bpf_sk_storage_delete_tracing_proto; + case BPF_FUNC_get_socket_cookie: + return prog->aux->sleepable ? + &bpf_get_socket_ptr_cookie_sleepable_proto : + &bpf_get_socket_ptr_cookie_proto; #endif case BPF_FUNC_seq_printf: return prog->expected_attach_type == BPF_TRACE_ITER ? diff --git a/net/core/filter.c b/net/core/filter.c index 77001a35768f..34877796ab5b 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -4631,6 +4631,30 @@ static const struct bpf_func_proto bpf_get_socket_cookie_sock_proto = { .arg1_type = ARG_PTR_TO_CTX, }; +BPF_CALL_1(bpf_get_socket_ptr_cookie_sleepable, struct sock *, sk) +{ + return sk ? sock_gen_cookie(sk) : 0; +} + +const struct bpf_func_proto bpf_get_socket_ptr_cookie_sleepable_proto = { + .func = bpf_get_socket_ptr_cookie_sleepable, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON, +}; + +BPF_CALL_1(bpf_get_socket_ptr_cookie, struct sock *, sk) +{ + return sk ? __sock_gen_cookie(sk) : 0; +} + +const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto = { + .func = bpf_get_socket_ptr_cookie, + .gpl_only = false, + .ret_type = RET_INTEGER, + .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON, +}; + BPF_CALL_1(bpf_get_socket_cookie_sock_ops, struct bpf_sock_ops_kern *, ctx) { return __sock_gen_cookie(ctx->sk); diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index ba59309f4d18..9ac66cf25959 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -1667,6 +1667,13 @@ union bpf_attr { * Return * A 8-byte long unique number. * + * u64 bpf_get_socket_cookie(void *sk) + * Description + * Equivalent to **bpf_get_socket_cookie**\ () helper that accepts + * *sk*, but gets socket from a BTF **struct sock**. + * Return + * A 8-byte long unique number. + * * u32 bpf_get_socket_uid(struct sk_buff *skb) * Return * The owner UID of the socket associated to *skb*. If the socket
This needs two new helpers, one that works in a sleepable context (using sock_gen_cookie which disables/enables preemption) and one that does not (for performance reasons). Both take a struct sock pointer and need to check it for NULLness. This helper could also be useful to other BPF program types such as LSM. Signed-off-by: Florent Revest <revest@chromium.org> --- include/linux/bpf.h | 2 ++ include/uapi/linux/bpf.h | 7 +++++++ kernel/trace/bpf_trace.c | 4 ++++ net/core/filter.c | 24 ++++++++++++++++++++++++ tools/include/uapi/linux/bpf.h | 7 +++++++ 5 files changed, 44 insertions(+)