Two minor BPF updates

PATCH Applied Daniel Borkmann 2 participants 6 messages Sep 5, 2017

Subsystems (3)

Networking Network drivers Library code

lore

Share & tools

lore

Daniel Borkmann

author

2017-09-05 00:24 UTC #0 lore raw

cover Two minor BPF updates

Two minor updates including a small cleanup for dumping the trace pipe and one for consolidating prog dumps for tc and xdp to use bpf_prog_info_by_fd() when possible.

Thanks!

Daniel Borkmann (2):
bpf: minor cleanups for bpf_trace_pipe
bpf: consolidate dumps to use bpf_dump_prog_info

 include/bpf_util.h |  2 +-
 ip/ipaddress.c     |  6 ++++--
 ip/iplink_xdp.c    | 19 +++++++++++++++----
 ip/xdp.h           |  2 +-
 lib/bpf.c          | 31 ++++++++++++++++++-------------
 tc/f_bpf.c         |  8 ++++----
 tc/m_bpf.c         |  8 ++++----
 7 files changed, 47 insertions(+), 29 deletions(-)

Signature · 1 line

1.9.3

Daniel Borkmann

author

2017-09-05 00:24 UTC #1 lore raw

PATCH 1/2 bpf: minor cleanups for bpf_trace_pipe

Just minor nits, e.g. no need to fflush() and instead of returning right away, just break and close the fd.

Signed-off-by Daniel Borkmann <[email protected]>

---
 lib/bpf.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

lib/bpf.c +9 ~~-10~~

index 5fd4928..7463fdc 100644
@@ -569,9 +569,9 @@ int bpf_trace_pipe(void)
 		"/trace",
 		0,
 	};
+	int fd_in, fd_out = STDERR_FILENO;
 	char tpipe[PATH_MAX];
 	const char *mnt;
-	int fd;
 
 	mnt = bpf_find_mntpt("tracefs", TRACEFS_MAGIC, tracefs_mnt,
 			     sizeof(tracefs_mnt), tracefs_known_mnts);

@@ -582,8 +582,8 @@ int bpf_trace_pipe(void)
 
 	snprintf(tpipe, sizeof(tpipe), "%s/trace_pipe", mnt);
 
-	fd = open(tpipe, O_RDONLY);
-	if (fd < 0)
+	fd_in = open(tpipe, O_RDONLY);
+	if (fd_in < 0)
 		return -1;
 
 	fprintf(stderr, "Running! Hang up with ^C!\n\n");

@@ -591,15 +591,14 @@ int bpf_trace_pipe(void)
 		static char buff[4096];
 		ssize_t ret;
 
-		ret = read(fd, buff, sizeof(buff) - 1);
-		if (ret > 0) {
-			if (write(STDERR_FILENO, buff, ret) != ret)
-				return -1;
-			fflush(stderr);
-		}
+		ret = read(fd_in, buff, sizeof(buff));
+		if (ret > 0 && write(fd_out, buff, ret) == ret)
+			continue;
+		break;
 	}
 
-	return 0;
+	close(fd_in);
+	return -1;
 }
 
 static int bpf_gen_global(const char *bpf_sub_dir)

Signature · 1 line

1.9.3

Daniel Borkmann

author

2017-09-05 00:24 UTC #2 lore raw

PATCH 2/2 bpf: consolidate dumps to use bpf_dump_prog_info

In reply to Daniel Borkmann

Consolidate dump of prog info to use bpf_dump_prog_info() when possible. Moving forward, we want to have a consistent output for BPF progs when being dumped. E.g. in cls/act case we used to dump tag as a separate netlink attribute before we had BPF_OBJ_GET_INFO_BY_FD bpf(2) command.

Move dumping tag into bpf_dump_prog_info() as well, and only dump the netlink attribute for older kernels. Also, reuse bpf_dump_prog_info() for XDP case, so we can dump tag and whether program was jited, which we currently don't show.

Signed-off-by Daniel Borkmann <[email protected]>

---
 include/bpf_util.h |  2 +-
 ip/ipaddress.c     |  6 ++++--
 ip/iplink_xdp.c    | 19 +++++++++++++++----
 ip/xdp.h           |  2 +-
 lib/bpf.c          | 12 +++++++++---
 tc/f_bpf.c         |  8 ++++----
 tc/m_bpf.c         |  8 ++++----
 7 files changed, 38 insertions(+), 19 deletions(-)

include/bpf_util.h +1 -1

index 6582ec8..e818221 100644
@@ -261,7 +261,7 @@ int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
 int bpf_prog_attach_fd(int prog_fd, int target_fd, enum bpf_attach_type type);
 int bpf_prog_detach_fd(int target_fd, enum bpf_attach_type type);
 
-void bpf_dump_prog_info(FILE *f, uint32_t id);
+int bpf_dump_prog_info(FILE *f, uint32_t id);
 
 #ifdef HAVE_ELF
 int bpf_send_map_fds(const char *path, const char *obj);

ip/ipaddress.c +4 -2

index c9312f0..dbdd839 100644
@@ -837,7 +837,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (tb[IFLA_MTU])
 		fprintf(fp, "mtu %u ", rta_getattr_u32(tb[IFLA_MTU]));
 	if (tb[IFLA_XDP])
-		xdp_dump(fp, tb[IFLA_XDP]);
+		xdp_dump(fp, tb[IFLA_XDP], do_link, false);
 	if (tb[IFLA_QDISC])
 		fprintf(fp, "qdisc %s ", rta_getattr_str(tb[IFLA_QDISC]));
 	if (tb[IFLA_MASTER]) {

@@ -951,12 +951,14 @@ int print_linkinfo(const struct sockaddr_nl *who,
 		}
 	}
 
-
 	if ((do_link || show_details) && tb[IFLA_IFALIAS]) {
 		fprintf(fp, "%s    alias %s", _SL_,
 			rta_getattr_str(tb[IFLA_IFALIAS]));
 	}
 
+	if ((do_link || show_details) && tb[IFLA_XDP])
+		xdp_dump(fp, tb[IFLA_XDP], true, true);
+
 	if (do_link && show_stats) {
 		fprintf(fp, "%s", _SL_);
 		__print_link_stats(fp, tb);

ip/iplink_xdp.c +15 -4

index 9ae9ee5..5aa66fe 100644
@@ -81,9 +81,10 @@ int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 	return 0;
 }
 
-void xdp_dump(FILE *fp, struct rtattr *xdp)
+void xdp_dump(FILE *fp, struct rtattr *xdp, bool link, bool details)
 {
 	struct rtattr *tb[IFLA_XDP_MAX + 1];
+	__u32 prog_id = 0;
 	__u8 mode;
 
 	parse_rtattr_nested(tb, IFLA_XDP_MAX, xdp);

@@ -94,6 +95,8 @@ void xdp_dump(FILE *fp, struct rtattr *xdp)
 	mode = rta_getattr_u8(tb[IFLA_XDP_ATTACHED]);
 	if (mode == XDP_ATTACHED_NONE)
 		return;
+	else if (details && link)
+		fprintf(fp, "%s    prog/xdp", _SL_);
 	else if (mode == XDP_ATTACHED_DRV)
 		fprintf(fp, "xdp");
 	else if (mode == XDP_ATTACHED_SKB)

@@ -104,8 +107,16 @@ void xdp_dump(FILE *fp, struct rtattr *xdp)
 		fprintf(fp, "xdp[%u]", mode);
 
 	if (tb[IFLA_XDP_PROG_ID])
-		fprintf(fp, "/id:%u",
-			rta_getattr_u32(tb[IFLA_XDP_PROG_ID]));
+		prog_id = rta_getattr_u32(tb[IFLA_XDP_PROG_ID]);
+	if (!details) {
+		if (prog_id && !link)
+			fprintf(fp, "/id:%u", prog_id);
+		fprintf(fp, " ");
+		return;
+	}
 
-	fprintf(fp, " ");
+	if (prog_id) {
+		fprintf(fp, " ");
+		bpf_dump_prog_info(fp, prog_id);
+	}
 }

ip/xdp.h +1 -1

index ba897a2..1efd591 100644
@@ -5,6 +5,6 @@
 
 int xdp_parse(int *argc, char ***argv, struct iplink_req *req, bool generic,
 	      bool drv, bool offload);
-void xdp_dump(FILE *fp, struct rtattr *tb);
+void xdp_dump(FILE *fp, struct rtattr *tb, bool link, bool details);
 
 #endif /* __XDP__ */

lib/bpf.c +9 -3

index 7463fdc..cfa1f79 100644
@@ -179,25 +179,31 @@ static int bpf_prog_info_by_fd(int fd, struct bpf_prog_info *info,
 	return ret;
 }
 
-void bpf_dump_prog_info(FILE *f, uint32_t id)
+int bpf_dump_prog_info(FILE *f, uint32_t id)
 {
 	struct bpf_prog_info info = {};
 	uint32_t len = sizeof(info);
-	int fd, ret;
+	int fd, ret, dump_ok = 0;
+	SPRINT_BUF(tmp);
 
 	fprintf(f, "id %u ", id);
 
 	fd = bpf_prog_fd_by_id(id);
 	if (fd < 0)
-		return;
+		return dump_ok;
 
 	ret = bpf_prog_info_by_fd(fd, &info, &len);
 	if (!ret && len) {
+		fprintf(f, "tag %s ",
+			hexstring_n2a(info.tag, sizeof(info.tag),
+				      tmp, sizeof(tmp)));
 		if (info.jited_prog_len)
 			fprintf(f, "jited ");
+		dump_ok = 1;
 	}
 
 	close(fd);
+	return dump_ok;
 }
 
 static int bpf_parse_string(char *arg, bool from_file, __u16 *bpf_len,

tc/f_bpf.c +4 -4

index 2f8d12a..4fb9209 100644
@@ -177,6 +177,7 @@ static int bpf_print_opt(struct filter_util *qu, FILE *f,
 			 struct rtattr *opt, __u32 handle)
 {
 	struct rtattr *tb[TCA_BPF_MAX + 1];
+	int dump_ok = 0;
 
 	if (opt == NULL)
 		return 0;

@@ -221,7 +222,9 @@ static int bpf_print_opt(struct filter_util *qu, FILE *f,
 		bpf_print_ops(f, tb[TCA_BPF_OPS],
 			      rta_getattr_u16(tb[TCA_BPF_OPS_LEN]));
 
-	if (tb[TCA_BPF_TAG]) {
+	if (tb[TCA_BPF_ID])
+		dump_ok = bpf_dump_prog_info(f, rta_getattr_u32(tb[TCA_BPF_ID]));
+	if (!dump_ok && tb[TCA_BPF_TAG]) {
 		SPRINT_BUF(b);
 
 		fprintf(f, "tag %s ",

@@ -230,9 +233,6 @@ static int bpf_print_opt(struct filter_util *qu, FILE *f,
 				      b, sizeof(b)));
 	}
 
-	if (tb[TCA_BPF_ID])
-		bpf_dump_prog_info(f, rta_getattr_u32(tb[TCA_BPF_ID]));
-
 	if (tb[TCA_BPF_POLICE]) {
 		fprintf(f, "\n");
 		tc_print_police(f, tb[TCA_BPF_POLICE]);

tc/m_bpf.c +4 -4

index df559bc..e3d0a2b 100644
@@ -154,6 +154,7 @@ static int bpf_print_opt(struct action_util *au, FILE *f, struct rtattr *arg)
 {
 	struct rtattr *tb[TCA_ACT_BPF_MAX + 1];
 	struct tc_act_bpf *parm;
+	int dump_ok = 0;
 
 	if (arg == NULL)
 		return -1;

@@ -177,7 +178,9 @@ static int bpf_print_opt(struct action_util *au, FILE *f, struct rtattr *arg)
 		fprintf(f, " ");
 	}
 
-	if (tb[TCA_ACT_BPF_TAG]) {
+	if (tb[TCA_ACT_BPF_ID])
+		dump_ok = bpf_dump_prog_info(f, rta_getattr_u32(tb[TCA_ACT_BPF_ID]));
+	if (!dump_ok && tb[TCA_ACT_BPF_TAG]) {
 		SPRINT_BUF(b);
 
 		fprintf(f, "tag %s ",

@@ -186,9 +189,6 @@ static int bpf_print_opt(struct action_util *au, FILE *f, struct rtattr *arg)
 				      b, sizeof(b)));
 	}
 
-        if (tb[TCA_ACT_BPF_ID])
-                bpf_dump_prog_info(f, rta_getattr_u32(tb[TCA_ACT_BPF_ID]));
-
 	print_action_control(f, "default-action ", parm->action, "\n");
 	fprintf(f, "\tindex %u ref %d bind %d", parm->index, parm->refcnt,
 		parm->bindcnt);

Signature · 1 line

1.9.3

Stephen Hemminger

Applied

2017-09-05 16:27 UTC #5 lore raw

In reply to Daniel Borkmann

Quoted 10 lines · On Tue, 5 Sep 2017 02:24:30 +0200 Daniel Borkmann <[email protected]> wrote:

Two minor updates including a small cleanup for dumping the trace pipe and one for consolidating prog dumps for tc and xdp to use bpf_prog_info_by_fd() when possible.
Thanks!
Daniel Borkmann (2):
bpf: minor cleanups for bpf_trace_pipe
bpf: consolidate dumps to use bpf_dump_prog_info

Quoted 9 lines

 include/bpf_util.h |  2 +-
 ip/ipaddress.c     |  6 ++++--
 ip/iplink_xdp.c    | 19 +++++++++++++++----
 ip/xdp.h           |  2 +-
 lib/bpf.c          | 31 ++++++++++++++++++-------------
 tc/f_bpf.c         |  8 ++++----
 tc/m_bpf.c         |  8 ++++----
 7 files changed, 47 insertions(+), 29 deletions(-)

Applied, thanks Daniel

Stephen Hemminger

2017-09-05 16:35 UTC #3 lore raw

Re: [PATCH iproute2 master 2/2] bpf: consolidate dumps to use bpf_dump_prog_info

In reply to Daniel Borkmann

Quoted 11 lines · On Tue, 5 Sep 2017 02:24:32 +0200 Daniel Borkmann <[email protected]> wrote:

Consolidate dump of prog info to use bpf_dump_prog_info() when possible. Moving forward, we want to have a consistent output for BPF progs when being dumped. E.g. in cls/act case we used to dump tag as a separate netlink attribute before we had BPF_OBJ_GET_INFO_BY_FD bpf(2) command.
Move dumping tag into bpf_dump_prog_info() as well, and only dump the netlink attribute for older kernels. Also, reuse bpf_dump_prog_info() for XDP case, so we can dump tag and whether program was jited, which we currently don't show.
Signed-off-by Daniel Borkmann <[email protected]>

I applied this to master, and resolved conflicts with net-next. But the dump with JSON of xdp is now incomplete.

Daniel Borkmann

author

2017-09-05 16:37 UTC #4 lore raw

Re: [PATCH iproute2 master 2/2] bpf: consolidate dumps to use bpf_dump_prog_info

On 09/05/2017 06:35 PM, Stephen Hemminger wrote:
[...]

Quoted 2 lines

I applied this to master, and resolved conflicts with net-next. But the dump with JSON of xdp is now incomplete.

Ok, I will check it out, and send a follow-up to make it complete again.