Re: [PATCH v2 3/5] bpf: Expose bpf_sk_storage_* to iterator programs

2020-11-19 Thread Martin KaFai Lau
On Thu, Nov 19, 2020 at 05:26:52PM +0100, Florent Revest wrote:
> From: Florent Revest 
> 
> Iterators are currently used to expose kernel information to userspace
> over fast procfs-like files but iterators could also be used to
> manipulate local storage. For example, the task_file iterator could be
> used to initialize a socket local storage with associations between
> processes and sockets or to selectively delete local storage values.
> 
> This exposes both socket local storage helpers to all iterators.
> Alternatively we could expose it to only certain iterators with strcmps
> on prog->aux->attach_func_name.
I cannot see any hole to iter the bpf_sk_storage_map and also
bpf_sk_storage_get/delete() itself for now.

I have looked at other iters (e.g. tcp, udp, and sock_map iter).
It will be good if you can double check them also.

I think at least one more test on the tcp iter is needed.

Other than that,

Acked-by: Martin KaFai Lau 


Re: [PATCH v2 3/5] bpf: Expose bpf_sk_storage_* to iterator programs

2020-11-19 Thread KP Singh
On Thu, Nov 19, 2020 at 5:27 PM Florent Revest  wrote:
>
> From: Florent Revest 
>
> Iterators are currently used to expose kernel information to userspace
> over fast procfs-like files but iterators could also be used to
> manipulate local storage. For example, the task_file iterator could be
> used to initialize a socket local storage with associations between
> processes and sockets or to selectively delete local storage values.
>
> This exposes both socket local storage helpers to all iterators.
> Alternatively we could expose it to only certain iterators with strcmps
> on prog->aux->attach_func_name.

Since you mentioned the alternative here, maybe you can also
explain why you chose the current approach.


[PATCH v2 3/5] bpf: Expose bpf_sk_storage_* to iterator programs

2020-11-19 Thread Florent Revest
From: Florent Revest 

Iterators are currently used to expose kernel information to userspace
over fast procfs-like files but iterators could also be used to
manipulate local storage. For example, the task_file iterator could be
used to initialize a socket local storage with associations between
processes and sockets or to selectively delete local storage values.

This exposes both socket local storage helpers to all iterators.
Alternatively we could expose it to only certain iterators with strcmps
on prog->aux->attach_func_name.

Signed-off-by: Florent Revest 
---
 net/core/bpf_sk_storage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/bpf_sk_storage.c b/net/core/bpf_sk_storage.c
index a32037daa933..4edd033e899c 100644
--- a/net/core/bpf_sk_storage.c
+++ b/net/core/bpf_sk_storage.c
@@ -394,6 +394,7 @@ static bool bpf_sk_storage_tracing_allowed(const struct 
bpf_prog *prog)
 * use the bpf_sk_storage_(get|delete) helper.
 */
switch (prog->expected_attach_type) {
+   case BPF_TRACE_ITER:
case BPF_TRACE_RAW_TP:
/* bpf_sk_storage has no trace point */
return true;
-- 
2.29.2.299.gdc1121823c-goog