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

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

[Yonghong Song]

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);
+	}
 
 	memset(&req, 0, sizeof(req));
 	req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
-- 
2.30.2

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

[Andrii Nakryiko]

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;
 }

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

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 27 Feb 2023 14:49:43 -0800 you 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.
> 
> [...]

Here is the summary with links:
  - [bpf] libbpf: Fix bpf_xdp_query() in old kernels
    https://git.kernel.org/bpf/bpf-next/c/c8ee37bde402

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Yonghong Song]

On 2/27/23 3:35 PM, Andrii Nakryiko wrote:
> 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;
>   }

Sounds good to me. Thanks!

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

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

[Lorenzo Bianconi]

[-- 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 --]