Re: [PATCH bpf-next v7 5/5] bpftool: Show parameters of BPF task iterators.

[Kui-Feng Lee <kuifeng@fb.com>]

On Fri, 2022-08-26 at 12:23 +0100, Quentin Monnet wrote:
> !-------------------------------------------------------------------|
>   This Message Is From an External Sender
> 
> > -------------------------------------------------------------------
> > !
> 
> On 26/08/2022 01:37, Kui-Feng Lee wrote:
> > Show tid or pid of iterators if giving an argument of tid or pid
> > 
> > For exampole, the commaned `bpftool link list` may list following
> 
> s/exampole/example/, s/commaned/command/
> 
> > lines.
> > 
> > 1: iter  prog 2  target_name bpf_map
> > 2: iter  prog 3  target_name bpf_prog
> > 33: iter  prog 225  target_name task_file  tid 1644
> >         pids test_progs(1644)
> > 
> > Link 33 is a task_file iterator with tid 1644.  For now, only
> > targets
> > of task, task_file and task_vma may be with tid or pid to filter
> > out
> > tasks other than that belong to a process (pid) or a thread (tid).
> 
> s/that belong/those belonging/?
> 
> > 
> > Signed-off-by: Kui-Feng Lee <kuifeng@fb.com>
> > ---
> >  tools/bpf/bpftool/link.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> > diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> > index 7a20931c3250..f96c18bb7a42 100644
> > --- a/tools/bpf/bpftool/link.c
> > +++ b/tools/bpf/bpftool/link.c
> > @@ -83,6 +83,13 @@ static bool is_iter_map_target(const char
> > *target_name)
> >                strcmp(target_name, "bpf_sk_storage_map") == 0;
> >  }
> >  
> > +static bool is_iter_task_target(const char *target_name)
> > +{
> > +       return strcmp(target_name, "task") == 0 ||
> > +               strcmp(target_name, "task_file") == 0 ||
> > +               strcmp(target_name, "task_vma") == 0;
> > +}
> > +
> >  static void show_iter_json(struct bpf_link_info *info,
> > json_writer_t *wtr)
> >  {
> >         const char *target_name = u64_to_ptr(info-
> > >iter.target_name);
> > @@ -91,6 +98,12 @@ static void show_iter_json(struct bpf_link_info
> > *info, json_writer_t *wtr)
> >  
> >         if (is_iter_map_target(target_name))
> >                 jsonw_uint_field(wtr, "map_id", info-
> > >iter.map.map_id);
> > +       else if (is_iter_task_target(target_name)) {
> > +               if (info->iter.task.tid)
> > +                       jsonw_uint_field(wtr, "tid", info-
> > >iter.task.tid);
> > +               if (info->iter.task.pid)
> > +                       jsonw_uint_field(wtr, "pid", info-
> > >iter.task.pid);
> > +       }
> >  }
> >  
> >  static int get_prog_info(int prog_id, struct bpf_prog_info *info)
> > @@ -208,6 +221,12 @@ static void show_iter_plain(struct
> > bpf_link_info *info)
> >  
> >         if (is_iter_map_target(target_name))
> >                 printf("map_id %u  ", info->iter.map.map_id);
> > +       else if (is_iter_task_target(target_name)) {
> > +               if (info->iter.task.tid)
> > +                       printf("tid %u ", info->iter.task.tid);
> > +               else if (info->iter.task.pid)
> > +                       printf("pid %u ", info->iter.task.pid);
> 
> Looks good, thanks! I note that you have an "if ... else ..." here,
> vs.
> two "if"s above for the JSON output. Could you please make this
> consistent?

Sure!