Re: [PATCH bpf-next 1/3] bpf: introduce helper bpf_get_task_stack_trace()

On Jun 23, 2020, at 11:45 AM, Andrii Nakryiko [off-list ref] wrote:

On Tue, Jun 23, 2020 at 12:08 AM Song Liu [off-list ref] wrote:

quoted

This helper can be used with bpf_iter__task to dump all /proc/*/stack to
a seq_file.

Signed-off-by: Song Liu <redacted>
---
include/uapi/linux/bpf.h       | 10 +++++++++-
kernel/trace/bpf_trace.c       | 21 +++++++++++++++++++++
scripts/bpf_helpers_doc.py     |  2 ++
tools/include/uapi/linux/bpf.h | 10 +++++++++-
4 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 19684813faaed..a30416b822fe3 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h

@@ -3252,6 +3252,13 @@ union bpf_attr {
 *             case of **BPF_CSUM_LEVEL_QUERY**, the current skb->csum_level
 *             is returned or the error code -EACCES in case the skb is not
 *             subject to CHECKSUM_UNNECESSARY.
+ *
+ * int bpf_get_task_stack_trace(struct task_struct *task, void *entries, u32 size)
+ *     Description
+ *             Save a task stack trace into array *entries*. This is a wrapper
+ *             over stack_trace_save_tsk().
+ *     Return
+ *             Number of trace entries stored.
 */

#define __BPF_FUNC_MAPPER(FN)          \
       FN(unspec),                     \

@@ -3389,7 +3396,8 @@ union bpf_attr {
       FN(ringbuf_submit),             \
       FN(ringbuf_discard),            \
       FN(ringbuf_query),              \
-       FN(csum_level),
+       FN(csum_level),                 \
+       FN(get_task_stack_trace),

We have get_stackid and get_stack, I think to stay consistent it
should be named get_task_stack

quoted

/* integer value in 'imm' field of BPF_CALL instruction selects which helper
 * function eBPF program intends to call

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index e729c9e587a07..2c13bcb5c2bce 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c

@@ -1488,6 +1488,23 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = {
       .arg4_type      = ARG_ANYTHING,

};

+BPF_CALL_3(bpf_get_task_stack_trace, struct task_struct *, task,
+          void *, entries, u32, size)

See get_stack definition, this one needs to support flags as well. And
we should probably support BPF_F_USER_BUILD_ID as well. And
BPF_F_USER_STACK is also a good idea, I presume?

This will be a different direction that is similar to stackmap implementation.
Current version follows the implementation behind /proc/<pid>/stack . Let me 
check which of them is better. 

Thanks,
Song