Re: [PATCH bpf] libbpf: Fix bpf_xdp_query() in old kernels

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3153 bytes --]

> On Mon, Feb 27, 2023 at 2:50 PM Yonghong Song <yhs@fb.com> wrote:
> >
> > Commit 04d58f1b26a4("libbpf: add API to get XDP/XSK supported features")
> > added feature_flags to struct bpf_xdp_query_opts. If a user uses
> > bpf_xdp_query_opts with feature_flags member, the bpf_xdp_query()
> > will check whether 'netdev' family exists or not in the kernel.
> > If it does not exist, the bpf_xdp_query() will return -ENOENT.
> >
> > But 'netdev' family does not exist in old kernels as it is
> > introduced in the same patch set as Commit 04d58f1b26a4.
> > So old kernel with newer libbpf won't work properly with
> > bpf_xdp_query() api call.
> >
> > To fix this issue, if the return value of
> > libbpf_netlink_resolve_genl_family_id() is -ENOENT, bpf_xdp_query()
> > will just return 0, skipping the rest of xdp feature query.
> > This preserves backward compatibility.
> >
> > Fixes: 04d58f1b26a4 ("libbpf: add API to get XDP/XSK supported features")
> > Cc: Lorenzo Bianconi <lorenzo@kernel.org>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
> > ---
> >  tools/lib/bpf/netlink.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
> > index 1653e7a8b0a1..4c1b3502f88d 100644
> > --- a/tools/lib/bpf/netlink.c
> > +++ b/tools/lib/bpf/netlink.c
> > @@ -468,8 +468,11 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts)
> >                 return 0;
> >
> >         err = libbpf_netlink_resolve_genl_family_id("netdev", sizeof("netdev"), &id);
> > -       if (err < 0)
> > +       if (err < 0) {
> > +               if (err == -ENOENT)
> > +                       return 0;
> >                 return libbpf_err(err);
> > +       }
> >
> 
> As I mentioned in another thread, I'm a bit worried of this early
> return, because query_opts might be extended and then we'll forget
> about this early return. So I did these changes and pushed to
> bpf-next:
> 
> diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
> index 4c1b3502f88d..84dd5fa14905 100644
> --- a/tools/lib/bpf/netlink.c
> +++ b/tools/lib/bpf/netlink.c
> @@ -469,8 +469,10 @@ int bpf_xdp_query(int ifindex, int xdp_flags,
> struct bpf_xdp_query_opts *opts)
> 
>         err = libbpf_netlink_resolve_genl_family_id("netdev",
> sizeof("netdev"), &id);
>         if (err < 0) {
> -               if (err == -ENOENT)
> -                       return 0;
> +               if (err == -ENOENT) {
> +                       opts->feature_flags = 0;
> +                       goto skip_feature_flags;
> +               }
>                 return libbpf_err(err);
>         }
> 
> @@ -492,6 +494,7 @@ int bpf_xdp_query(int ifindex, int xdp_flags,
> struct bpf_xdp_query_opts *opts)
> 
>         opts->feature_flags = md.flags;
> 
> +skip_feature_flags:
>         return 0;

thx for fixing this:
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

>  }
> 
> >         memset(&req, 0, sizeof(req));
> >         req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
> > --
> > 2.30.2
> >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]