2020-12-09 13:30:24

by Florent Revest

[permalink] [raw]
Subject: [PATCH bpf-next v4 2/4] bpf: Expose bpf_get_socket_cookie to tracing programs

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 <[email protected]>
---
include/linux/bpf.h | 1 +
include/uapi/linux/bpf.h | 7 +++++++
kernel/trace/bpf_trace.c | 2 ++
net/core/filter.c | 12 ++++++++++++
tools/include/uapi/linux/bpf.h | 7 +++++++
5 files changed, 29 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 07cb5d15e743..5a858e8c3f1a 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1860,6 +1860,7 @@ 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_sock_from_file_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 52ddd217d6a1..be5e96de306d 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1760,6 +1760,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_delete_tracing_proto;
case BPF_FUNC_sock_from_file:
return &bpf_sock_from_file_proto;
+ case BPF_FUNC_get_socket_cookie:
+ return &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 255aeee72402..13ad9a64f04f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4631,6 +4631,18 @@ 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, 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
--
2.29.2.576.ga3fc446d84-goog


2020-12-09 15:39:00

by KP Singh

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 2/4] bpf: Expose bpf_get_socket_cookie to tracing programs

On Wed, Dec 9, 2020 at 2:29 PM Florent Revest <[email protected]> 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 <[email protected]>

Acked-by: KP Singh <[email protected]>

2020-12-09 16:39:39

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 2/4] bpf: Expose bpf_get_socket_cookie to tracing programs

On 12/9/20 2:26 PM, Florent Revest 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.

Looks like this commit description is now stale and needs to be updated
since we only really add one helper?

> Signed-off-by: Florent Revest <[email protected]>
> ---
> include/linux/bpf.h | 1 +
> include/uapi/linux/bpf.h | 7 +++++++
> kernel/trace/bpf_trace.c | 2 ++
> net/core/filter.c | 12 ++++++++++++
> tools/include/uapi/linux/bpf.h | 7 +++++++
> 5 files changed, 29 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 07cb5d15e743..5a858e8c3f1a 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1860,6 +1860,7 @@ 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_sock_from_file_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**.

Maybe add a small comment that this one also works for sleepable [tracing] progs?

> + * Return
> + * A 8-byte long unique number.

... or 0 if *sk* is NULL.

> * 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 52ddd217d6a1..be5e96de306d 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1760,6 +1760,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_sk_storage_delete_tracing_proto;
> case BPF_FUNC_sock_from_file:
> return &bpf_sock_from_file_proto;
> + case BPF_FUNC_get_socket_cookie:
> + return &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 255aeee72402..13ad9a64f04f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4631,6 +4631,18 @@ 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, 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
>

2021-01-19 16:08:07

by Florent Revest

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 2/4] bpf: Expose bpf_get_socket_cookie to tracing programs

On Wed, Dec 9, 2020 at 5:35 PM Daniel Borkmann <[email protected]> wrote:
>
> On 12/9/20 2:26 PM, Florent Revest 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.
>
> Looks like this commit description is now stale and needs to be updated
> since we only really add one helper?
>
> > Signed-off-by: Florent Revest <[email protected]>
> > ---
> > include/linux/bpf.h | 1 +
> > include/uapi/linux/bpf.h | 7 +++++++
> > kernel/trace/bpf_trace.c | 2 ++
> > net/core/filter.c | 12 ++++++++++++
> > tools/include/uapi/linux/bpf.h | 7 +++++++
> > 5 files changed, 29 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 07cb5d15e743..5a858e8c3f1a 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1860,6 +1860,7 @@ 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_sock_from_file_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**.
>
> Maybe add a small comment that this one also works for sleepable [tracing] progs?
>
> > + * Return
> > + * A 8-byte long unique number.
>
> ... or 0 if *sk* is NULL.

Argh, I somehow missed this email during my holidays, I'm sending a
v5. Thank you Daniel!