Skip to contentSkip to conversation

libbpf: add function to setup XDP

PATCH In review Eric Leblond 7 participants 9 messages Dec 9, 2017 – Dec 12, 2017

Subsystems (4)
Networking Network drivers BPF libbpf
lore
Share & tools
lore

Eric Leblond

author
#0 lore raw

PATCH 1/1 libbpf: add function to setup XDP

Most of the code is taken from set_link_xdp_fd() in bpf_load.c and slightly modified to be library compliant.

Signed-off-by Eric Leblond <[email protected]>
---
 tools/lib/bpf/bpf.c    | 108 ++++++++++++++++++++++++++++++++++++++++++++++++-
 tools/lib/bpf/libbpf.c |   2 +
 tools/lib/bpf/libbpf.h |   4 ++
 3 files changed, 112 insertions(+), 2 deletions(-)
tools/lib/bpf/bpf.c +106 -2
index 5128677e4117..bea173be66fc 100644
@@ -25,6 +25,10 @@
 #include <asm/unistd.h>
 #include <linux/bpf.h>
 #include "bpf.h"
+#include "libbpf.h"
+#include <linux/rtnetlink.h>
+#include <sys/socket.h>
+#include <errno.h>
 
 /*
  * When building perf, unistd.h is overridden. __NR_bpf is

@@ -46,8 +50,6 @@
 # endif
 #endif
 
-#define min(x, y) ((x) < (y) ? (x) : (y))
-
 static inline __u64 ptr_to_u64(const void *ptr)
 {
 	return (__u64) (unsigned long) ptr;

@@ -413,3 +415,105 @@ int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len)
 
 	return err;
 }
+
+int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
+{
+	struct sockaddr_nl sa;
+	int sock, seq = 0, len, ret = -1;
+	char buf[4096];
+	struct nlattr *nla, *nla_xdp;
+	struct {
+		struct nlmsghdr  nh;
+		struct ifinfomsg ifinfo;
+		char             attrbuf[64];
+	} req;
+	struct nlmsghdr *nh;
+	struct nlmsgerr *err;
+
+	memset(&sa, 0, sizeof(sa));
+	sa.nl_family = AF_NETLINK;
+
+	sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+	if (sock < 0) {
+		return -errno;
+	}
+
+	if (bind(sock, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
+		ret = -errno;
+		goto cleanup;
+	}
+
+	memset(&req, 0, sizeof(req));
+	req.nh.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
+	req.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
+	req.nh.nlmsg_type = RTM_SETLINK;
+	req.nh.nlmsg_pid = 0;
+	req.nh.nlmsg_seq = ++seq;
+	req.ifinfo.ifi_family = AF_UNSPEC;
+	req.ifinfo.ifi_index = ifindex;
+
+	/* started nested attribute for XDP */
+	nla = (struct nlattr *)(((char *)&req)
+				+ NLMSG_ALIGN(req.nh.nlmsg_len));
+	nla->nla_type = NLA_F_NESTED | 43/*IFLA_XDP*/;
+	nla->nla_len = NLA_HDRLEN;
+
+	/* add XDP fd */
+	nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
+	nla_xdp->nla_type = 1/*IFLA_XDP_FD*/;
+	nla_xdp->nla_len = NLA_HDRLEN + sizeof(int);
+	memcpy((char *)nla_xdp + NLA_HDRLEN, &fd, sizeof(fd));
+	nla->nla_len += nla_xdp->nla_len;
+
+	/* if user passed in any flags, add those too */
+	if (flags) {
+		nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
+		nla_xdp->nla_type = 3/*IFLA_XDP_FLAGS*/;
+		nla_xdp->nla_len = NLA_HDRLEN + sizeof(flags);
+		memcpy((char *)nla_xdp + NLA_HDRLEN, &flags, sizeof(flags));
+		nla->nla_len += nla_xdp->nla_len;
+	}
+
+	req.nh.nlmsg_len += NLA_ALIGN(nla->nla_len);
+
+	if (send(sock, &req, req.nh.nlmsg_len, 0) < 0) {
+		ret = -errno;
+		goto cleanup;
+	}
+
+	len = recv(sock, buf, sizeof(buf), 0);
+	if (len < 0) {
+		ret = -errno;
+		goto cleanup;
+	}
+
+	for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+	     nh = NLMSG_NEXT(nh, len)) {
+		if (nh->nlmsg_pid != getpid()) {
+			ret = -LIBBPF_ERRNO__WRNGPID;
+			goto cleanup;
+		}
+		if (nh->nlmsg_seq != seq) {
+			ret = -LIBBPF_ERRNO__INVSEQ;
+			goto cleanup;
+		}
+		switch (nh->nlmsg_type) {
+		case NLMSG_ERROR:
+			err = (struct nlmsgerr *)NLMSG_DATA(nh);
+			if (!err->error)
+				continue;
+			ret = err->error;
+			goto cleanup;
+		case NLMSG_DONE:
+			break;
+		default:
+			break;
+		}
+	}
+
+	ret = 0;
+
+cleanup:
+	close(sock);
+	return ret;
+}
tools/lib/bpf/libbpf.c +2 -0
index 5aa45f89da93..931e98c097a8 100644
@@ -106,6 +106,8 @@ static const char *libbpf_strerror_table[NR_ERRNO] = {
 	[ERRCODE_OFFSET(PROG2BIG)]	= "Program too big",
 	[ERRCODE_OFFSET(KVER)]		= "Incorrect kernel version",
 	[ERRCODE_OFFSET(PROGTYPE)]	= "Kernel doesn't support this program type",
+	[ERRCODE_OFFSET(WRNGPID)]	= "Wrong pid in netlink message",
+	[ERRCODE_OFFSET(INVSEQ)]	= "Invalid netlink sequence",
 };
 
 int libbpf_strerror(int err, char *buf, size_t size)
tools/lib/bpf/libbpf.h +4 -0
index 6e20003109e0..e42f96900318 100644
@@ -42,6 +42,8 @@ enum libbpf_errno {
 	LIBBPF_ERRNO__PROG2BIG,	/* Program too big */
 	LIBBPF_ERRNO__KVER,	/* Incorrect kernel version */
 	LIBBPF_ERRNO__PROGTYPE,	/* Kernel doesn't support this program type */
+	LIBBPF_ERRNO__WRNGPID,	/* Wrong pid in netlink message */
+	LIBBPF_ERRNO__INVSEQ,	/* Invalid netlink sequence */
 	__LIBBPF_ERRNO__END,
 };
 

@@ -246,4 +248,6 @@ long libbpf_get_error(const void *ptr);
 
 int bpf_prog_load(const char *file, enum bpf_prog_type type,
 		  struct bpf_object **pobj, int *prog_fd);
+
+int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 #endif
Signature · 1 line
2.15.1
#1 lore raw

tools/lib/bpf/bpf.c int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len) · line 458

Quoted 4 lines · Eric Leblond
tools/lib/bpf/bpf.c +4 -0
+	/* started nested attribute for XDP */
+	nla = (struct nlattr *)(((char *)&req)
+				+ NLMSG_ALIGN(req.nh.nlmsg_len));
+	nla->nla_type = NLA_F_NESTED | 43/*IFLA_XDP*/;

as a part of the move into libbpf can the magic numbers be replaced by the names directly and there as a comment?

There are more below.

Quoted 18 lines
tools/lib/bpf/bpf.c +18 -0
+	nla->nla_len = NLA_HDRLEN;
+
+	/* add XDP fd */
+	nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
+	nla_xdp->nla_type = 1/*IFLA_XDP_FD*/;
+	nla_xdp->nla_len = NLA_HDRLEN + sizeof(int);
+	memcpy((char *)nla_xdp + NLA_HDRLEN, &fd, sizeof(fd));
+	nla->nla_len += nla_xdp->nla_len;
+
+	/* if user passed in any flags, add those too */
+	if (flags) {
+		nla_xdp = (struct nlattr *)((char *)nla + nla->nla_len);
+		nla_xdp->nla_type = 3/*IFLA_XDP_FLAGS*/;
+		nla_xdp->nla_len = NLA_HDRLEN + sizeof(flags);
+		memcpy((char *)nla_xdp + NLA_HDRLEN, &flags, sizeof(flags));
+		nla->nla_len += nla_xdp->nla_len;
+	}
+
#2 lore raw
Quoted 8 lines · David Ahern

tools/lib/bpf/bpf.c int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len) · line 458

Quoted 4 lines · Eric Leblond
tools/lib/bpf/bpf.c +4 -0
+	/* started nested attribute for XDP */
+	nla = (struct nlattr *)(((char *)&req)
+				+ NLMSG_ALIGN(req.nh.nlmsg_len));
+	nla->nla_type = NLA_F_NESTED | 43/*IFLA_XDP*/;

as a part of the move into libbpf can the magic numbers be replaced by the names directly and there as a comment?

In general it would be nice to use names instead of numbers, but it's much bigger change then this patch, since it would require copying and syncing a bunch of headers into tools/ which may not be such a good idea in the end.

Only removal of min() looks a bit suspicious to me. Eric, is it because it now comes from some header?

#3 lore raw
In reply to Eric Leblond
Quoted 12 lines · Eric Leblond
tools/lib/bpf/bpf.c +12 -0
+	for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+	     nh = NLMSG_NEXT(nh, len)) {
+		if (nh->nlmsg_pid != getpid()) {
+			ret = -LIBBPF_ERRNO__WRNGPID;
+			goto cleanup;
+		}
+		if (nh->nlmsg_seq != seq) {
+			ret = -LIBBPF_ERRNO__INVSEQ;
+			goto cleanup;
+		}
+		switch (nh->nlmsg_type) {
+		case NLMSG_ERROR:

tools/lib/bpf/bpf.c int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len) · line 511

Quoted 10 lines
tools/lib/bpf/bpf.c +10 -0
+			err = (struct nlmsgerr *)NLMSG_DATA(nh);
+			if (!err->error)
+				continue;
+			ret = err->error;
+			goto cleanup;
+		case NLMSG_DONE:
+			break;
+		default:
+			break;
+		}

Would it be possible to print out or preferably return to the caller the ext ack error message? A couple of drivers are using it for XDP mis-configuration reporting instead of printks. We should encourage other to do the same and support it in all user space since ext ack msgs lead to much better user experience.

author
#4 lore raw

Hello,

Quoted 21 lines · Jakub Kicinski
Quoted 20 lines · Eric Leblond
tools/lib/bpf/bpf.c +20 -0
+	for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+	     nh = NLMSG_NEXT(nh, len)) {
+		if (nh->nlmsg_pid != getpid()) {
+			ret = -LIBBPF_ERRNO__WRNGPID;
+			goto cleanup;
+		}
+		if (nh->nlmsg_seq != seq) {
+			ret = -LIBBPF_ERRNO__INVSEQ;
+			goto cleanup;
+		}
+		switch (nh->nlmsg_type) {
+		case NLMSG_ERROR:
+			err = (struct nlmsgerr *)NLMSG_DATA(nh);
+			if (!err->error)
+				continue;
+			ret = err->error;
+			goto cleanup;
+		case NLMSG_DONE:
+			break;
+		default:
Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+			break;
+		}

Would it be possible to print out or preferably return to the caller the ext ack error message? A couple of drivers are using it for XDP mis-configuration reporting instead of printks. We should encourage other to do the same and support it in all user space since ext ack msgs lead to much better user experience.

I've seen the kind of messages displayed by reading at kernel log. They are really useful and it looks almost mandatory to be able to display them.

Kernel code seems to not have a parser for the ext ack error message. Did I miss something here ?

Looking at tc code, it seems it is using libmnl to parse them and I doubt it is a good idea to use that in libbpf as it is introducing a dependency.

Does someone has an existing parsing code or should I write on my own ?

BR,

Signature · 2 lines
Eric Leblond <[email protected]>
Blog: https://home.regit.org/
#5 lore raw
Quoted 10 lines · Eric Leblond
Quoted 5 lines

Would it be possible to print out or preferably return to the caller the ext ack error message? A couple of drivers are using it for XDP mis-configuration reporting instead of printks. We should encourage other to do the same and support it in all user space since ext ack msgs lead to much better user experience.

I've seen the kind of messages displayed by reading at kernel log. They are really useful and it looks almost mandatory to be able to display them.

Quoted 8 lines

Kernel code seems to not have a parser for the ext ack error message. Did I miss something here ?

Looking at tc code, it seems it is using libmnl to parse them and I doubt it is a good idea to use that in libbpf as it is introducing a dependency.

Does someone has an existing parsing code or should I write on my own ?

I had worked on extack for libbpf but seem to have lost the changes.

Look at the commits here:
https://github.com/dsahern/iproute2/commits/ext-ack

I suggest using this:

https://github.com/dsahern/iproute2/commit/b61e4c7dd54a5d3ff98640da4b480441cee497b2

to bring in nlattr from lib/nlattr (as I recall lib/nlattr can not be used directly). From there, use this one:

https://github.com/dsahern/iproute2/commit/261f7251e6704d565b91e310faabbbb7e18d14a1

to see what is needed for extack support.

Really not that much code to add.

maintainer
#6 lore raw
Quoted 29 lines · David Ahern
Quoted 10 lines · Eric Leblond
Quoted 5 lines

Would it be possible to print out or preferably return to the caller the ext ack error message? A couple of drivers are using it for XDP mis-configuration reporting instead of printks. We should encourage other to do the same and support it in all user space since ext ack msgs lead to much better user experience.

I've seen the kind of messages displayed by reading at kernel log. They are really useful and it looks almost mandatory to be able to display them.

Quoted 8 lines

Kernel code seems to not have a parser for the ext ack error message. Did I miss something here ?

Looking at tc code, it seems it is using libmnl to parse them and I doubt it is a good idea to use that in libbpf as it is introducing a dependency.

Does someone has an existing parsing code or should I write on my own ?

I had worked on extack for libbpf but seem to have lost the changes.

Look at the commits here:
https://github.com/dsahern/iproute2/commits/ext-ack

I suggest using this:

https://github.com/dsahern/iproute2/commit/b61e4c7dd54a5d3ff98640da4b480441cee497b2

Quoted 8 lines

to bring in nlattr from lib/nlattr (as I recall lib/nlattr can not be used directly). From there, use this one:

https://github.com/dsahern/iproute2/commit/261f7251e6704d565b91e310faabbbb7e18d14a1

to see what is needed for extack support.

Really not that much code to add.

+1, ext ack support would improve troubleshooting a lot here; please add and respin. Thanks, Eric!

#7 lore raw
In reply to Eric Leblond
Quoted 4 lines · Eric Leblond

Most of the code is taken from set_link_xdp_fd() in bpf_load.c and slightly modified to be library compliant.

Signed-off-by Eric Leblond <[email protected]>

...

Quoted 1 line

+int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)

...

tools/lib/bpf/bpf.c int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len) · line 492

Quoted 3 lines
tools/lib/bpf/bpf.c +3 -0
+	for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+	     nh = NLMSG_NEXT(nh, len)) {
+		if (nh->nlmsg_pid != getpid()) {

Generally nlmsg_pid should not be compared with process id. See man netlink and
https://github.com/iovisor/bcc/pull/1275/commits/69ce96a54c55960c8de3392061254c97b6306a6d

Quoted 3 lines
tools/lib/bpf/bpf.c +3 -0
+			ret = -LIBBPF_ERRNO__WRNGPID;
+			goto cleanup;
+		}
Signature · 1 line
Toshiaki Makita
maintainer
#8 lore raw

From: Toshiaki Makita <[email protected]>
Date: Mon, 11 Dec 2017 11:24:12 +0900

Quoted 7 lines · Eric Leblond
Quoted 4 lines

Most of the code is taken from set_link_xdp_fd() in bpf_load.c and slightly modified to be library compliant.

Signed-off-by Eric Leblond <[email protected]>

...

Quoted 1 line

+int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)

Quoted 8 lines

...

tools/lib/bpf/bpf.c int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len) · line 492

Quoted 3 lines
tools/lib/bpf/bpf.c +3 -0
+	for (nh = (struct nlmsghdr *)buf; NLMSG_OK(nh, len);
+	     nh = NLMSG_NEXT(nh, len)) {
+		if (nh->nlmsg_pid != getpid()) {

Generally nlmsg_pid should not be compared with process id. See man netlink and
https://github.com/iovisor/bcc/pull/1275/commits/69ce96a54c55960c8de3392061254c97b6306a6d

Right. I wish we had never named this "pid", it gets misinterpreted way too easily.