diff mbox series

[RFC,v2,net-next,03/12] libbpf: api for getting/setting link xdp options

Message ID 20191226023200.21389-4-prashantbhole.linux@gmail.com
State RFC
Delegated to: David Miller
Headers show
Series XDP in tx path | expand

Commit Message

Prashant Bhole Dec. 26, 2019, 2:31 a.m. UTC
This patch introduces and uses new APIs:

struct bpf_link_xdp_opts {
        struct xdp_link_info *link_info;
        size_t link_info_sz;
        __u32 flags;
        __u32 prog_id;
        int prog_fd;
};

enum bpf_link_cmd {
	BPF_LINK_GET_XDP_INFO,
	BPF_LINK_GET_XDP_ID,
	BPF_LINK_SET_XDP_FD,
};

int bpf_get_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
		      enum bpf_link_cmd cmd);
int bpf_set_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
		      enum bpf_link_cmd cmd);

The operations performed by these two functions are equivalent to
existing APIs.

BPF_LINK_GET_XDP_ID equivalent to bpf_get_link_xdp_id()
BPF_LINK_SET_XDP_FD equivalent to bpf_set_link_xdp_fd()
BPF_LINK_GET_XDP_INFO equivalent to bpf_get_link_xdp_info()

It will be easy to extend this API by adding members in struct
bpf_link_xdp_opts and adding different operations. Next patch
will extend this API to set XDP program in the tx path.

Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
 tools/lib/bpf/libbpf.h   | 36 +++++++++++++++++++
 tools/lib/bpf/libbpf.map |  2 ++
 tools/lib/bpf/netlink.c  | 77 ++++++++++++++++++++++++++++++++++++----
 3 files changed, 109 insertions(+), 6 deletions(-)

Comments

Andrii Nakryiko Dec. 30, 2019, 4:49 a.m. UTC | #1
On Wed, Dec 25, 2019 at 6:34 PM Prashant Bhole
<prashantbhole.linux@gmail.com> wrote:
>
> This patch introduces and uses new APIs:
>
> struct bpf_link_xdp_opts {
>         struct xdp_link_info *link_info;
>         size_t link_info_sz;
>         __u32 flags;
>         __u32 prog_id;
>         int prog_fd;
> };

Please see the usage of DECLARE_LIBBPF_OPTS and OPTS_VALID/OPTS_GET
(e.g., in bpf_object__open_file). This also seems like a rather
low-level API, so might be more appropriate to follow the naming of
low-level API in bpf.h (see Andrey Ignatov's recent
bpf_prog_attach_xattr() changes).

As is this is not backwards/forward compatible, unless you use
LIBBPF_OPTS approach (that's what Alexei meant).


>
> enum bpf_link_cmd {
>         BPF_LINK_GET_XDP_INFO,
>         BPF_LINK_GET_XDP_ID,
>         BPF_LINK_SET_XDP_FD,
> };
>
> int bpf_get_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
>                       enum bpf_link_cmd cmd);
> int bpf_set_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
>                       enum bpf_link_cmd cmd);
>
> The operations performed by these two functions are equivalent to
> existing APIs.
>
> BPF_LINK_GET_XDP_ID equivalent to bpf_get_link_xdp_id()
> BPF_LINK_SET_XDP_FD equivalent to bpf_set_link_xdp_fd()
> BPF_LINK_GET_XDP_INFO equivalent to bpf_get_link_xdp_info()
>
> It will be easy to extend this API by adding members in struct
> bpf_link_xdp_opts and adding different operations. Next patch
> will extend this API to set XDP program in the tx path.

Not really, and this has been extensively discussed previously. One of
the problems is old user code linked against newer libbpf version
(shared library). New libbpf will assume struct with more fields,
while old user code will provide too short struct. That's why all the
LIBBPF_OPTS stuff.

>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
> ---
>  tools/lib/bpf/libbpf.h   | 36 +++++++++++++++++++
>  tools/lib/bpf/libbpf.map |  2 ++
>  tools/lib/bpf/netlink.c  | 77 ++++++++++++++++++++++++++++++++++++----
>  3 files changed, 109 insertions(+), 6 deletions(-)
>

[...]
Prashant Bhole Jan. 3, 2020, 11:04 a.m. UTC | #2
On 12/30/2019 1:49 PM, Andrii Nakryiko wrote:
> On Wed, Dec 25, 2019 at 6:34 PM Prashant Bhole
> <prashantbhole.linux@gmail.com> wrote:
>>
>> This patch introduces and uses new APIs:
>>
>> struct bpf_link_xdp_opts {
>>          struct xdp_link_info *link_info;
>>          size_t link_info_sz;
>>          __u32 flags;
>>          __u32 prog_id;
>>          int prog_fd;
>> };
> 
> Please see the usage of DECLARE_LIBBPF_OPTS and OPTS_VALID/OPTS_GET
> (e.g., in bpf_object__open_file). This also seems like a rather
> low-level API, so might be more appropriate to follow the naming of
> low-level API in bpf.h (see Andrey Ignatov's recent
> bpf_prog_attach_xattr() changes).
> 
> As is this is not backwards/forward compatible, unless you use
> LIBBPF_OPTS approach (that's what Alexei meant).

Got it.

> 
> 
>>
>> enum bpf_link_cmd {
>>          BPF_LINK_GET_XDP_INFO,
>>          BPF_LINK_GET_XDP_ID,
>>          BPF_LINK_SET_XDP_FD,
>> };
>>
>> int bpf_get_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
>>                        enum bpf_link_cmd cmd);
>> int bpf_set_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
>>                        enum bpf_link_cmd cmd);
>>
>> The operations performed by these two functions are equivalent to
>> existing APIs.
>>
>> BPF_LINK_GET_XDP_ID equivalent to bpf_get_link_xdp_id()
>> BPF_LINK_SET_XDP_FD equivalent to bpf_set_link_xdp_fd()
>> BPF_LINK_GET_XDP_INFO equivalent to bpf_get_link_xdp_info()
>>
>> It will be easy to extend this API by adding members in struct
>> bpf_link_xdp_opts and adding different operations. Next patch
>> will extend this API to set XDP program in the tx path.
> 
> Not really, and this has been extensively discussed previously. One of
> the problems is old user code linked against newer libbpf version
> (shared library). New libbpf will assume struct with more fields,
> while old user code will provide too short struct. That's why all the
> LIBBPF_OPTS stuff.

Got it. Thanks for reviewing.

Prashant

> 
>>
>> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
>> ---
>>   tools/lib/bpf/libbpf.h   | 36 +++++++++++++++++++
>>   tools/lib/bpf/libbpf.map |  2 ++
>>   tools/lib/bpf/netlink.c  | 77 ++++++++++++++++++++++++++++++++++++----
>>   3 files changed, 109 insertions(+), 6 deletions(-)
>>
> 
> [...]
>
diff mbox series

Patch

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 0dbf4bfba0c4..8178fd5a1e8f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -443,10 +443,46 @@  struct xdp_link_info {
 	__u8 attach_mode;
 };
 
+struct bpf_link_xdp_opts {
+	struct xdp_link_info *link_info;
+	size_t link_info_sz;
+	__u32 flags;
+	__u32 prog_id;
+	int prog_fd;
+};
+
+/*
+ * enum values below are set of commands to get and set/get XDP related
+ * attributes to a link. These are used along with struct bpf_link_xdp_opts.
+ *
+ * BPF_LINK_GET_XDP_INFO uses fields:
+ *	- link_info
+ *	- link_info_sz
+ *	- flags
+ *
+ * BPF_LINK_SET_XDP_FD uses fields:
+ *	- flags
+ *
+ * BPF_LINK_SET_XDP_FD uses fields:
+ *	- prog_fd
+ *	- flags
+ */
+enum bpf_link_cmd {
+	BPF_LINK_GET_XDP_INFO,
+	BPF_LINK_GET_XDP_ID,
+	BPF_LINK_SET_XDP_FD,
+};
+
 LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
 LIBBPF_API int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 				     size_t info_size, __u32 flags);
+LIBBPF_API __u32 bpf_get_link_xdp_info_id(struct xdp_link_info *info,
+					  __u32 flags);
+LIBBPF_API int bpf_get_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
+				 enum bpf_link_cmd cmd);
+LIBBPF_API int bpf_set_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
+				 enum bpf_link_cmd cmd);
 
 struct perf_buffer;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 8ddc2c40e482..332522fb5853 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -207,4 +207,6 @@  LIBBPF_0.0.6 {
 		bpf_program__size;
 		btf__find_by_name_kind;
 		libbpf_find_vmlinux_btf_id;
+		bpf_set_link_opts;
+		bpf_get_link_opts;
 } LIBBPF_0.0.5;
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 5065c1aa1061..1274b540a9ad 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -129,8 +129,10 @@  static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
 	return ret;
 }
 
-int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
+static int __bpf_set_link_xdp_fd(int ifindex, struct bpf_link_xdp_opts *opts)
 {
+	int fd = opts->prog_fd;
+	__u32 flags = opts->flags;
 	int sock, seq = 0, ret;
 	struct nlattr *nla, *nla_xdp;
 	struct {
@@ -188,6 +190,16 @@  int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
 	return ret;
 }
 
+int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
+{
+	struct bpf_link_xdp_opts opts = {};
+
+	opts.prog_fd = fd;
+	opts.flags = flags;
+
+	return bpf_set_link_opts(ifindex, &opts, BPF_LINK_SET_XDP_FD);
+}
+
 static int __dump_link_nlmsg(struct nlmsghdr *nlh,
 			     libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
 {
@@ -248,10 +260,12 @@  static int get_xdp_info(void *cookie, void *msg, struct nlattr **tb)
 	return 0;
 }
 
-int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
-			  size_t info_size, __u32 flags)
+static int __bpf_get_link_xdp_info(int ifindex, struct bpf_link_xdp_opts *opts)
 {
+	struct xdp_link_info *info = opts->link_info;
+	size_t info_size = opts->link_info_sz;
 	struct xdp_id_md xdp_id = {};
+	__u32 flags = opts->flags;
 	int sock, ret;
 	__u32 nl_pid;
 	__u32 mask;
@@ -284,6 +298,18 @@  int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
 	return ret;
 }
 
+int bpf_get_link_xdp_info(int ifindex, struct xdp_link_info *info,
+			  size_t info_size, __u32 flags)
+{
+	struct bpf_link_xdp_opts opts = {};
+
+	opts.link_info = info;
+	opts.link_info_sz = info_size;
+	opts.flags = flags;
+
+	return bpf_get_link_opts(ifindex, &opts, BPF_LINK_GET_XDP_INFO);
+}
+
 static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
 {
 	if (info->attach_mode != XDP_ATTACHED_MULTI)
@@ -300,12 +326,13 @@  static __u32 get_xdp_id(struct xdp_link_info *info, __u32 flags)
 
 int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
 {
-	struct xdp_link_info info;
+	struct bpf_link_xdp_opts opts = {};
 	int ret;
 
-	ret = bpf_get_link_xdp_info(ifindex, &info, sizeof(info), flags);
+	opts.flags = flags;
+	ret =  bpf_get_link_opts(ifindex, &opts, BPF_LINK_GET_XDP_ID);
 	if (!ret)
-		*prog_id = get_xdp_id(&info, flags);
+		*prog_id = opts.prog_id;
 
 	return ret;
 }
@@ -449,3 +476,41 @@  int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
 	return bpf_netlink_recv(sock, nl_pid, seq, __dump_filter_nlmsg,
 				dump_filter_nlmsg, cookie);
 }
+
+int bpf_set_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
+		      enum bpf_link_cmd cmd)
+{
+	switch (cmd) {
+	case BPF_LINK_SET_XDP_FD:
+		return __bpf_set_link_xdp_fd(ifindex, opts);
+	case BPF_LINK_GET_XDP_INFO:
+	case BPF_LINK_GET_XDP_ID:
+	default:
+		return -EINVAL;
+	}
+}
+
+int bpf_get_link_opts(int ifindex, struct bpf_link_xdp_opts *opts,
+		      enum bpf_link_cmd cmd)
+{
+	switch (cmd) {
+	case BPF_LINK_GET_XDP_INFO:
+		return __bpf_get_link_xdp_info(ifindex, opts);
+	case BPF_LINK_GET_XDP_ID: {
+		struct bpf_link_xdp_opts tmp_opts = {};
+		struct xdp_link_info link_info = {};
+		int ret;
+
+		tmp_opts.flags = opts->flags;
+		tmp_opts.link_info = &link_info;
+		tmp_opts.link_info_sz = sizeof(link_info);
+		ret = __bpf_get_link_xdp_info(ifindex, &tmp_opts);
+		if (!ret)
+			opts->prog_id = get_xdp_id(&link_info, opts->flags);
+		return ret;
+	}
+	case BPF_LINK_SET_XDP_FD:
+	default:
+		return -EINVAL;
+	}
+}