[PATCH] bpf: Expose bpf_sk_storage_* to iterator programs

[PATCH] bpf: Expose bpf_sk_storage_* to iterator programs

[Florent Revest]

From: Florent Revest <revest@google.com>

Iterators are currently used to expose kernel information to userspace
over fast procfs-like files but iterators could also be used to
initialize local storage. For example, the task_file iterator could be
used to store associations between processes and sockets.

This exposes the socket local storage helpers to all iterators. Martin
Kafai checked that this was safe to call these helpers from the
sk_storage_map iterators.

Signed-off-by: Florent Revest <revest@google.com>
---
 kernel/trace/bpf_trace.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e4515b0f62a8..3530120fa280 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -17,6 +17,8 @@
 #include <linux/error-injection.h>
 #include <linux/btf_ids.h>
 
+#include <net/bpf_sk_storage.h>
+
 #include <uapi/linux/bpf.h>
 #include <uapi/linux/btf.h>
 
@@ -1750,6 +1752,14 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		       NULL;
 	case BPF_FUNC_d_path:
 		return &bpf_d_path_proto;
+	case BPF_FUNC_sk_storage_get:
+		return prog->expected_attach_type == BPF_TRACE_ITER ?
+		       &bpf_sk_storage_get_proto :
+		       NULL;
+	case BPF_FUNC_sk_storage_delete:
+		return prog->expected_attach_type == BPF_TRACE_ITER ?
+		       &bpf_sk_storage_delete_proto :
+		       NULL;
 	default:
 		return raw_tp_prog_func_proto(func_id, prog);
 	}
-- 
2.29.2.222.g5d2a92d10f8-goog

Re: [PATCH] bpf: Expose bpf_sk_storage_* to iterator programs

[Martin KaFai Lau]

On Thu, Nov 12, 2020 at 09:09:14PM +0100, Florent Revest wrote:
> From: Florent Revest <revest@google.com>
> 
> Iterators are currently used to expose kernel information to userspace
> over fast procfs-like files but iterators could also be used to
> initialize local storage. For example, the task_file iterator could be
> used to store associations between processes and sockets.
> 
> This exposes the socket local storage helpers to all iterators. Martin
> Kafai checked that this was safe to call these helpers from the
> sk_storage_map iterators.
> 
> Signed-off-by: Florent Revest <revest@google.com>
> ---
>  kernel/trace/bpf_trace.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index e4515b0f62a8..3530120fa280 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -17,6 +17,8 @@
>  #include <linux/error-injection.h>
>  #include <linux/btf_ids.h>
>  
> +#include <net/bpf_sk_storage.h>
> +
>  #include <uapi/linux/bpf.h>
>  #include <uapi/linux/btf.h>
>  
> @@ -1750,6 +1752,14 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		       NULL;
>  	case BPF_FUNC_d_path:
>  		return &bpf_d_path_proto;
> +	case BPF_FUNC_sk_storage_get:
> +		return prog->expected_attach_type == BPF_TRACE_ITER ?
> +		       &bpf_sk_storage_get_proto :
> +		       NULL;
> +	case BPF_FUNC_sk_storage_delete:
> +		return prog->expected_attach_type == BPF_TRACE_ITER ?
> +		       &bpf_sk_storage_delete_proto :
> +		       NULL;
Test(s) is needed.  e.g. iterating a bpf_sk_storage_map and also
calling bpf_sk_storage_get/delete.

I would expect to see another test/example
showing how it works end-to-end to solve the problem you have in hand.
This patch probably belongs to a longer series.

BTW, I am also enabling bpf_sk_storage_(get|delete) for FENTRY/FEXIT/RAW_TP
but I think the conflict should be manageable.
https://patchwork.ozlabs.org/project/netdev/patch/20201112211313.2587383-1-kafai@fb.com/

Re: [PATCH] bpf: Expose bpf_sk_storage_* to iterator programs

[Florent Revest]

On Thu, 2020-11-12 at 13:57 -0800, Martin KaFai Lau wrote:
> Test(s) is needed.  e.g. iterating a bpf_sk_storage_map and also
> calling bpf_sk_storage_get/delete.
> 
> I would expect to see another test/example showing how it works end-
> to-end to solve the problem you have in hand.
> This patch probably belongs to a longer series.

Fair point, I'll get that done, thank you!

> BTW, I am also enabling bpf_sk_storage_(get|delete) for
> FENTRY/FEXIT/RAW_TP but I think the conflict should be manageable.
> https://patchwork.ozlabs.org/project/netdev/patch/20201112211313.2587383-1-kafai@fb.com/

Thanks for the heads up, should be no problem :)