Re: [PATCH bpf-next v8 3/5] bpf: Handle show_fdinfo for the parameterized task BPF iterators

[Yonghong Song <yhs@fb.com>]

On 8/29/22 12:23 PM, Kui-Feng Lee wrote:
> Show information of iterators in the respective files under
> /proc/<pid>/fdinfo/.
> 
> For example, for a task file iterator with 1723 as the value of tid
> parameter, its fdinfo would look like the following lines.
> 
>      pos:    0
>      flags:  02000000
>      mnt_id: 14
>      ino:    38
>      link_type:      iter
>      link_id:        51
>      prog_tag:       a590ac96db22b825
>      prog_id:        299
>      target_name:    task_file
>      task_type:      TID
>      tid: 1723
> 
> This patch add the last three fields.  task_type is the type of the
> task parameter.  TID means the iterator visit only the thread
> specified by tid.  The value of tid in the above example is 1723.  For
> the case of PID task_type, it means the iterator visits only threads
> of a process and will show the pid value of the process instead of a
> tid.
> 
> Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>

Ack with a nit below.

Acked-by: Yonghong Song <yhs@fb.com>

> ---
>   kernel/bpf/task_iter.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> index 5140117447e5..c10aeeffe1d6 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -10,6 +10,12 @@
>   #include <linux/btf_ids.h>
>   #include "mmap_unlock_work.h"
>   
> +static const char * const iter_task_type_names[] = {
> +	"ALL",
> +	"TID",
> +	"PID",
> +};
> +
>   struct bpf_iter_seq_task_common {
>   	struct pid_namespace *ns;
>   	enum bpf_iter_task_type	type;
> @@ -683,6 +689,15 @@ static int bpf_iter_fill_link_info(const struct bpf_iter_aux_info *aux, struct b
>   	return 0;
>   }
>   
> +static void bpf_iter_task_show_fdinfo(const struct bpf_iter_aux_info *aux, struct seq_file *seq)
> +{
> +	seq_printf(seq, "task_type:\t%s\n", iter_task_type_names[aux->task.type]);
> +	if (aux->task.type == BPF_TASK_ITER_TID)
> +		seq_printf(seq, "tid:\t%d\n", aux->task.pid);
> +	else if (aux->task.type == BPF_TASK_ITER_TGID)
> +		seq_printf(seq, "pid:\t%d\n", aux->task.pid);
> +}

Let us use '%u' instead of '%d' for aux->type.pid since its type is u32.

> +
>   static struct bpf_iter_reg task_reg_info = {
>   	.target			= "task",
>   	.attach_target		= bpf_iter_attach_task,
> @@ -694,6 +709,7 @@ static struct bpf_iter_reg task_reg_info = {
>   	},
>   	.seq_info		= &task_seq_info,
>   	.fill_link_info		= bpf_iter_fill_link_info,
> +	.show_fdinfo		= bpf_iter_task_show_fdinfo,
>   };
>   
[...]