Re: [PATCH bpf] bpftool: Use libbpf error code for flow dissector query

[Yonghong Song]

On 6/2/26 12:43 PM, Yonghong Song wrote:
> On 6/2/26 10:03 AM, Woojin Ji wrote:
> > bpf_prog_query() returns a negative errno on failure. 
> > query_flow_dissector()
> > currently closes the namespace fd and then reads errno to decide whether
> > -EINVAL means that the running kernel does not support flow dissector 
> > queries.
> >
> > That errno check controls behavior, not just diagnostics: -EINVAL is 
> > handled
> > as a non-fatal old-kernel case, while any other error makes bpftool 
> > net fail.
> > Reading errno after close() is fragile, because close() can overwrite 
> > errno
> > before the check. Use the libbpf-returned error code instead so the
>
> Do you have evidence that close() is fragile?

The patch itself looks good to me. But it would be great in commit message
to answer why. Note that fd is readonly: fd = open("/proc/self/ns/net", O_RDONLY).

> > compatibility branch is based on the BPF_PROG_QUERY result itself.
> >
> > Keep the existing errno reset in the non-fatal path to preserve batch 
> > mode
> > behavior. The success path is unchanged.
> >
> > Fixes: 7f0c57fec80f ("bpftool: show flow_dissector attachment status")
> > Assisted-by: ChatGPT:gpt-5.5
> > Signed-off-by: Woojin Ji <random6.xyz@xxxxxxxxx>
> > ---
> >   tools/bpf/bpftool/net.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
> > index 974189da8a91..dba28755d284 100644
> > --- a/tools/bpf/bpftool/net.c
> > +++ b/tools/bpf/bpftool/net.c
> > @@ -603,14 +603,14 @@ static int query_flow_dissector(struct 
> > bpf_attach_info *attach_info)
> >                    &attach_flags, prog_ids, &prog_cnt);
> >       close(fd);
> >       if (err) {
> > -        if (errno == EINVAL) {
> > +        if (err == -EINVAL) {
> >               /* Older kernel's don't support querying
> >                * flow dissector programs.
> >                */
> >               errno = 0;
> >               return 0;
> >           }
> > -        p_err("can't query prog: %s", strerror(errno));
> > +        p_err("can't query prog: %s", strerror(-err));
> >           return -1;
> >       }