Re: [PATCH v5 bpf-next 5/8] libbpf: add API to get XDP/XSK supported features

From: Yonghong Song <yhs@meta.com>
To: Lorenzo Bianconi <lorenzo@kernel.org>, bpf@vger.kernel.org
Cc: netdev@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, kuba@kernel.org,
	Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH v5 bpf-next 5/8] libbpf: add API to get XDP/XSK supported features
Date: Mon, 27 Feb 2023 12:38:47 -0800	[thread overview]
Message-ID: <e519f15d-cdd0-9362-34f3-3e6b8c8a4762@meta.com> (raw)
In-Reply-To: <a72609ef4f0de7fee5376c40dbf54ad7f13bfb8d.1675245258.git.lorenzo@kernel.org>

On 2/1/23 2:24 AM, Lorenzo Bianconi wrote:
> Extend bpf_xdp_query routine in order to get XDP/XSK supported features
> of netdev over route netlink interface.
> Extend libbpf netlink implementation in order to support netlink_generic
> protocol.
> 
> Co-developed-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> Co-developed-by: Marek Majtyka <alardam@gmail.com>
> Signed-off-by: Marek Majtyka <alardam@gmail.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   tools/lib/bpf/libbpf.h  |  3 +-
>   tools/lib/bpf/netlink.c | 96 +++++++++++++++++++++++++++++++++++++++++
>   tools/lib/bpf/nlattr.h  | 12 ++++++
>   3 files changed, 110 insertions(+), 1 deletion(-)
> 
[...]
> +
>   int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts)
>   {
>   	struct libbpf_nla_req req = {
> @@ -366,6 +433,10 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts)
>   		.ifinfo.ifi_family = AF_PACKET,
>   	};
>   	struct xdp_id_md xdp_id = {};
> +	struct xdp_features_md md = {
> +		.ifindex = ifindex,
> +	};
> +	__u16 id;
>   	int err;
>   
>   	if (!OPTS_VALID(opts, bpf_xdp_query_opts))
> @@ -393,6 +464,31 @@ int bpf_xdp_query(int ifindex, int xdp_flags, struct bpf_xdp_query_opts *opts)
>   	OPTS_SET(opts, skb_prog_id, xdp_id.info.skb_prog_id);
>   	OPTS_SET(opts, attach_mode, xdp_id.info.attach_mode);
>   
> +	if (!OPTS_HAS(opts, feature_flags))
> +		return 0;
> +
> +	err = libbpf_netlink_resolve_genl_family_id("netdev", sizeof("netdev"), &id);
> +	if (err < 0)
> +		return libbpf_err(err);

Hi, Lorenzo,

Using latest libbpf repo (https://github.com/libbpf/libbpf, sync'ed from 
source), looks like the above change won't work if the program is 
running on an old kernel, e.g., 5.12 kernel.

In this particular combination, in user space, bpf_xdp_query_opts does
have 'feature_flags' member, so the control can reach
libbpf_netlink_resolve_genl_family_id(). However, the family 'netdev'
is only available in latest kernel (after this patch set). So
the error will return in the above.

This breaks backward compatibility since old working application won't
work any more with a refresh of libbpf.

I could not come up with an easy solution for this. One thing we could
do is to treat 'libbpf_netlink_resolve_genl_family_id()' as a probe, so
return 0 if probe fails.

   err = libbpf_netlink_resolve_genl_family_id("netdev", 
sizeof("netdev"), &id);
   if (err < 0)
	return 0;

Please let me know whether my suggestion makes sense or there could be a
better solution.

> +
> +	memset(&req, 0, sizeof(req));
> +	req.nh.nlmsg_len = NLMSG_LENGTH(GENL_HDRLEN);
> +	req.nh.nlmsg_flags = NLM_F_REQUEST;
> +	req.nh.nlmsg_type = id;
> +	req.gnl.cmd = NETDEV_CMD_DEV_GET;
> +	req.gnl.version = 2;
> +
> +	err = nlattr_add(&req, NETDEV_A_DEV_IFINDEX, &ifindex, sizeof(ifindex));
> +	if (err < 0)
> +		return err;
> +
> +	err = libbpf_netlink_send_recv(&req, NETLINK_GENERIC,
> +				       parse_xdp_features, NULL, &md);
> +	if (err)
> +		return libbpf_err(err);
> +
> +	opts->feature_flags = md.flags;
> +
>   	return 0;
>   }
>   
[...]