[PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample
@ 2019-01-17  1:01 Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 1/6] libbpf: Add a helper for retrieving a map fd for a given name Maciej Fijalkowski
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Hi!
This patchset tries to address the situation where:
* user loads a particular xdp sample application that does stats polling
* user loads another sample application on the same interface
* then, user sends SIGINT/SIGTERM to the app that was attached as a first one
* second application ends up with an unloaded xdp program

1st patch contains a helper libbpf function for getting the map fd by a
given map name.
2nd patch updates a bunch of xdp samples to make the use of libbpf.
Patch 3 adjusts RLIMIT_MEMLOCK for two samples touched in this patchset.
Patch 4 makes the samples behavior similar to what iproute2 does when loading
xdp prog - the "force" flag is introduced.
Patch 5 introduces the libbpf function that will query the driver from
userspace about the currently attached xdp prog id.

Use it in samples that do polling by checking the prog id in signal handler
and comparing it with previously stored one which is the scope of 6th patch.

Thanks!

Maciej Fijalkowski (6):
  libbpf: Add a helper for retrieving a map fd for a given name
  samples: bpf: Convert XDP samples to libbpf usage
  samples: bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4}
  samples: bpf: Add a "force" flag to XDP samples
  libbpf: Add a support for getting xdp prog id on ifindex
  samples: bpf: Check the prog id before exiting

 samples/bpf/Makefile                |   8 +-
 samples/bpf/xdp1_user.c             |  29 +++++-
 samples/bpf/xdp_adjust_tail_user.c  |  33 +++++--
 samples/bpf/xdp_redirect_map_user.c |  94 ++++++++++++++++----
 samples/bpf/xdp_redirect_user.c     |  92 +++++++++++++++----
 samples/bpf/xdp_router_ipv4_user.c  | 171 +++++++++++++++++++++++++-----------
 samples/bpf/xdp_rxq_info_user.c     |  36 ++++++--
 samples/bpf/xdp_sample_pkts_user.c  |  76 +++++++++++++---
 samples/bpf/xdp_tx_iptunnel_user.c  |  66 ++++++++++----
 samples/bpf/xdpsock_user.c          |  25 +++++-
 tools/lib/bpf/libbpf.c              |  12 +++
 tools/lib/bpf/libbpf.h              |   4 +
 tools/lib/bpf/libbpf.map            |   5 ++
 tools/lib/bpf/netlink.c             |  84 ++++++++++++++++++
 14 files changed, 598 insertions(+), 137 deletions(-)

-- 
2.16.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 1/6] libbpf: Add a helper for retrieving a map fd for a given name
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Maciej Fijalkowski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

XDP samples are mostly cooperating with eBPF maps through their file
descriptors. In case of a eBPF program that contains multiple maps it
might be tiresome to iterate through them and call bpf_map__fd for each
one. Add a helper mostly based on bpf_object__find_map_by_name, but
instead of returning the struct bpf_map pointer, return map fd.

Bump libbpf ABI version to 0.0.2.

Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/lib/bpf/libbpf.c   | 12 ++++++++++++
 tools/lib/bpf/libbpf.h   |  3 +++
 tools/lib/bpf/libbpf.map |  4 ++++
 3 files changed, 19 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 169e347c76f6..dc838bea403f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2840,6 +2840,18 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
 	return NULL;
 }
 
+int
+bpf_object__find_map_fd_by_name(struct bpf_object *obj, const char *name)
+{
+	struct bpf_map *pos;
+
+	bpf_map__for_each(pos, obj) {
+		if (pos->name && !strcmp(pos->name, name))
+			return bpf_map__fd(pos);
+	}
+	return -ENOENT;
+}
+
 struct bpf_map *
 bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset)
 {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5f68d7b75215..7f10d36abdde 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -264,6 +264,9 @@ struct bpf_map;
 LIBBPF_API struct bpf_map *
 bpf_object__find_map_by_name(struct bpf_object *obj, const char *name);
 
+LIBBPF_API int
+bpf_object__find_map_fd_by_name(struct bpf_object *obj, const char *name);
+
 /*
  * Get bpf_map through the offset of corresponding struct bpf_map_def
  * in the BPF object file.
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index cd02cd4e2cc3..7c59e4f64082 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -124,3 +124,7 @@ LIBBPF_0.0.1 {
 	local:
 		*;
 };
+LIBBPF_0.0.2 {
+	global:
+		bpf_object__find_map_fd_by_name;
+} LIBBPF_0.0.1;
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 1/6] libbpf: Add a helper for retrieving a map fd for a given name Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17 11:19   ` Jesper Dangaard Brouer
  2019-01-25  8:22   ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Jesper Dangaard Brouer
  2019-01-17  1:01 ` [PATCH bpf-next 3/6] samples: bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4} Maciej Fijalkowski
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Some of XDP samples that are attaching the bpf program to the interface
via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
loading and manipulating the ebpf program and maps. Convert them to do
this through libbpf usage and remove bpf_load from the picture.

While at it remove what looks like debug leftover in
xdp_redirect_map_user.c

xdp_redirect_cpu is omitted because of read_trace_pipe() usage, which
doesn't seem to be handled in libbpf ATM.

Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/Makefile                |  8 ++--
 samples/bpf/xdp_redirect_map_user.c | 47 ++++++++++++++++-------
 samples/bpf/xdp_redirect_user.c     | 44 +++++++++++++++++-----
 samples/bpf/xdp_router_ipv4_user.c  | 75 ++++++++++++++++++++++++++-----------
 samples/bpf/xdp_tx_iptunnel_user.c  | 37 ++++++++++++------
 5 files changed, 151 insertions(+), 60 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 66ae15f27c70..4486fedaf09a 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -87,18 +87,18 @@ test_cgrp2_sock2-objs := bpf_load.o test_cgrp2_sock2.o
 xdp1-objs := xdp1_user.o
 # reuse xdp1 source intentionally
 xdp2-objs := xdp1_user.o
-xdp_router_ipv4-objs := bpf_load.o xdp_router_ipv4_user.o
+xdp_router_ipv4-objs := xdp_router_ipv4_user.o
 test_current_task_under_cgroup-objs := bpf_load.o $(CGROUP_HELPERS) \
 				       test_current_task_under_cgroup_user.o
 trace_event-objs := bpf_load.o trace_event_user.o $(TRACE_HELPERS)
 sampleip-objs := bpf_load.o sampleip_user.o $(TRACE_HELPERS)
 tc_l2_redirect-objs := bpf_load.o tc_l2_redirect_user.o
 lwt_len_hist-objs := bpf_load.o lwt_len_hist_user.o
-xdp_tx_iptunnel-objs := bpf_load.o xdp_tx_iptunnel_user.o
+xdp_tx_iptunnel-objs := xdp_tx_iptunnel_user.o
 test_map_in_map-objs := bpf_load.o test_map_in_map_user.o
 per_socket_stats_example-objs := cookie_uid_helper_example.o
-xdp_redirect-objs := bpf_load.o xdp_redirect_user.o
-xdp_redirect_map-objs := bpf_load.o xdp_redirect_map_user.o
+xdp_redirect-objs := xdp_redirect_user.o
+xdp_redirect_map-objs := xdp_redirect_map_user.o
 xdp_redirect_cpu-objs := bpf_load.o xdp_redirect_cpu_user.o
 xdp_monitor-objs := bpf_load.o xdp_monitor_user.o
 xdp_rxq_info-objs := xdp_rxq_info_user.o
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 4445e76854b5..60d46eea225b 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -22,15 +22,16 @@
 #include <libgen.h>
 #include <sys/resource.h>
 
-#include "bpf_load.h"
 #include "bpf_util.h"
 #include <bpf/bpf.h>
+#include "bpf/libbpf.h"
 
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
 static __u32 xdp_flags;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,7 @@ static void poll_stats(int interval, int ifindex)
 		int i;
 
 		sleep(interval);
-		assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+		assert(bpf_map_lookup_elem(rxcnt_map_fd, &key, values) == 0);
 		for (i = 0; i < nr_cpus; i++)
 			sum += (values[i] - prev[i]);
 		if (sum)
@@ -76,9 +77,16 @@ static void usage(const char *prog)
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_program *prog, *dummy_prog;
+	int prog_fd, dummy_prog_fd;
 	const char *optstr = "SN";
-	char filename[256];
+	struct bpf_object *obj;
 	int ret, opt, key = 0;
+	char filename[256];
+	int tx_port_map_fd;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -109,24 +117,40 @@ int main(int argc, char **argv)
 	printf("input: %d output: %d\n", ifindex_in, ifindex_out);
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
+
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
+		return 1;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	prog = bpf_program__next(NULL, obj);
+	dummy_prog = bpf_program__next(prog, obj);
+	if (!prog || !dummy_prog) {
+		printf("finding a prog in obj file failed\n");
+		return 1;
+	}
+	/* bpf_prog_load_xattr gives us the pointer to first prog's fd,
+	 * so we're missing only the fd for dummy prog
+	 */
+	dummy_prog_fd = bpf_program__fd(dummy_prog);
+	if (prog_fd < 0 || dummy_prog_fd < 0) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
 
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	if (tx_port_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
 		return 1;
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd, xdp_flags) < 0) {
 		printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
 		return 1;
 	}
 
 	/* Loading dummy XDP prog on out-device */
-	if (bpf_set_link_xdp_fd(ifindex_out, prog_fd[1],
+	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
 		printf("WARN: link set xdp fd failed on %d\n", ifindex_out);
 		ifindex_out_xdp_dummy_attached = false;
@@ -135,11 +159,8 @@ int main(int argc, char **argv)
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
-	printf("map[0] (vports) = %i, map[1] (map) = %i, map[2] (count) = %i\n",
-		map_fd[0], map_fd[1], map_fd[2]);
-
 	/* populate virtual to physical port map */
-	ret = bpf_map_update_elem(map_fd[0], &key, &ifindex_out, 0);
+	ret = bpf_map_update_elem(tx_port_map_fd, &key, &ifindex_out, 0);
 	if (ret) {
 		perror("bpf_update_elem");
 		goto out;
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index 81a69e36cb78..93404820df68 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -22,15 +22,16 @@
 #include <libgen.h>
 #include <sys/resource.h>
 
-#include "bpf_load.h"
 #include "bpf_util.h"
 #include <bpf/bpf.h>
+#include "bpf/libbpf.h"
 
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
 static __u32 xdp_flags;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,7 @@ static void poll_stats(int interval, int ifindex)
 		int i;
 
 		sleep(interval);
-		assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+		assert(bpf_map_lookup_elem(rxcnt_map_fd, &key, values) == 0);
 		for (i = 0; i < nr_cpus; i++)
 			sum += (values[i] - prev[i]);
 		if (sum)
@@ -77,9 +78,16 @@ static void usage(const char *prog)
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_program *prog, *dummy_prog;
+	int prog_fd, tx_port_map_fd, opt;
 	const char *optstr = "SN";
+	struct bpf_object *obj;
 	char filename[256];
-	int ret, opt, key = 0;
+	int dummy_prog_fd;
+	int ret, key = 0;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -110,24 +118,40 @@ int main(int argc, char **argv)
 	printf("input: %d output: %d\n", ifindex_in, ifindex_out);
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
+		return 1;
+
+	prog = bpf_program__next(NULL, obj);
+	dummy_prog = bpf_program__next(prog, obj);
+	if (!prog || !dummy_prog) {
+		printf("finding a prog in obj file failed\n");
+		return 1;
+	}
+	/* bpf_prog_load_xattr gives us the pointer to first prog's fd,
+	 * so we're missing only the fd for dummy prog
+	 */
+	dummy_prog_fd = bpf_program__fd(dummy_prog);
+	if (prog_fd < 0 || dummy_prog_fd < 0) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
 
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	if (tx_port_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
 		return 1;
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex_in, prog_fd, xdp_flags) < 0) {
 		printf("ERROR: link set xdp fd failed on %d\n", ifindex_in);
 		return 1;
 	}
 
 	/* Loading dummy XDP prog on out-device */
-	if (bpf_set_link_xdp_fd(ifindex_out, prog_fd[1],
+	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
 		printf("WARN: link set xdp fd failed on %d\n", ifindex_out);
 		ifindex_out_xdp_dummy_attached = false;
@@ -137,7 +161,7 @@ int main(int argc, char **argv)
 	signal(SIGTERM, int_exit);
 
 	/* bpf redirect port */
-	ret = bpf_map_update_elem(map_fd[0], &key, &ifindex_out, 0);
+	ret = bpf_map_update_elem(tx_port_map_fd, &key, &ifindex_out, 0);
 	if (ret) {
 		perror("bpf_update_elem");
 		goto out;
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index b2b4dfa776c8..cea2306f5ab7 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -15,7 +15,6 @@
 #include <string.h>
 #include <sys/socket.h>
 #include <unistd.h>
-#include "bpf_load.h"
 #include <bpf/bpf.h>
 #include <arpa/inet.h>
 #include <fcntl.h>
@@ -25,11 +24,17 @@
 #include <sys/ioctl.h>
 #include <sys/syscall.h>
 #include "bpf_util.h"
+#include "bpf/libbpf.h"
 
 int sock, sock_arp, flags = 0;
 static int total_ifindex;
 int *ifindex_list;
 char buf[8192];
+static int lpm_map_fd;
+static int rxcnt_map_fd;
+static int arp_table_map_fd;
+static int exact_match_map_fd;
+static int tx_port_map_fd;
 
 static int get_route_table(int rtm_family);
 static void int_exit(int sig)
@@ -186,7 +191,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
 				bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
 			exit(0);
 		}
-		assert(bpf_map_update_elem(map_fd[4], &route.iface, &route.iface, 0) == 0);
+		assert(bpf_map_update_elem(tx_port_map_fd,
+					   &route.iface, &route.iface, 0) == 0);
 		if (rtm_family == AF_INET) {
 			struct trie_value {
 				__u8 prefix[4];
@@ -207,11 +213,16 @@ static void read_route(struct nlmsghdr *nh, int nll)
 			direct_entry.arp.dst = 0;
 			if (route.dst_len == 32) {
 				if (nh->nlmsg_type == RTM_DELROUTE) {
-					assert(bpf_map_delete_elem(map_fd[3], &route.dst) == 0);
+					assert(bpf_map_delete_elem(exact_match_map_fd,
+								   &route.dst) == 0);
 				} else {
-					if (bpf_map_lookup_elem(map_fd[2], &route.dst, &direct_entry.arp.mac) == 0)
+					if (bpf_map_lookup_elem(arp_table_map_fd,
+								&route.dst,
+								&direct_entry.arp.mac) == 0)
 						direct_entry.arp.dst = route.dst;
-					assert(bpf_map_update_elem(map_fd[3], &route.dst, &direct_entry, 0) == 0);
+					assert(bpf_map_update_elem(exact_match_map_fd,
+								   &route.dst,
+								   &direct_entry, 0) == 0);
 				}
 			}
 			for (i = 0; i < 4; i++)
@@ -225,7 +236,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 			       route.gw, route.dst_len,
 			       route.metric,
 			       route.iface_name);
-			if (bpf_map_lookup_elem(map_fd[0], prefix_key,
+			if (bpf_map_lookup_elem(lpm_map_fd, prefix_key,
 						prefix_value) < 0) {
 				for (i = 0; i < 4; i++)
 					prefix_value->prefix[i] = prefix_key->data[i];
@@ -234,7 +245,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 				prefix_value->gw = route.gw;
 				prefix_value->metric = route.metric;
 
-				assert(bpf_map_update_elem(map_fd[0],
+				assert(bpf_map_update_elem(lpm_map_fd,
 							   prefix_key,
 							   prefix_value, 0
 							   ) == 0);
@@ -247,7 +258,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 					       prefix_key->data[2],
 					       prefix_key->data[3],
 					       prefix_key->prefixlen);
-					assert(bpf_map_delete_elem(map_fd[0],
+					assert(bpf_map_delete_elem(lpm_map_fd,
 								   prefix_key
 								   ) == 0);
 					/* Rereading the route table to check if
@@ -275,8 +286,7 @@ static void read_route(struct nlmsghdr *nh, int nll)
 					prefix_value->ifindex = route.iface;
 					prefix_value->gw = route.gw;
 					prefix_value->metric = route.metric;
-					assert(bpf_map_update_elem(
-								   map_fd[0],
+					assert(bpf_map_update_elem(lpm_map_fd,
 								   prefix_key,
 								   prefix_value,
 								   0) == 0);
@@ -401,7 +411,8 @@ static void read_arp(struct nlmsghdr *nh, int nll)
 		arp_entry.mac = atol(mac);
 		printf("%x\t\t%llx\n", arp_entry.dst, arp_entry.mac);
 		if (ndm_family == AF_INET) {
-			if (bpf_map_lookup_elem(map_fd[3], &arp_entry.dst,
+			if (bpf_map_lookup_elem(exact_match_map_fd,
+						&arp_entry.dst,
 						&direct_entry) == 0) {
 				if (nh->nlmsg_type == RTM_DELNEIGH) {
 					direct_entry.arp.dst = 0;
@@ -410,16 +421,17 @@ static void read_arp(struct nlmsghdr *nh, int nll)
 					direct_entry.arp.dst = arp_entry.dst;
 					direct_entry.arp.mac = arp_entry.mac;
 				}
-				assert(bpf_map_update_elem(map_fd[3],
+				assert(bpf_map_update_elem(exact_match_map_fd,
 							   &arp_entry.dst,
 							   &direct_entry, 0
 							   ) == 0);
 				memset(&direct_entry, 0, sizeof(direct_entry));
 			}
 			if (nh->nlmsg_type == RTM_DELNEIGH) {
-				assert(bpf_map_delete_elem(map_fd[2], &arp_entry.dst) == 0);
+				assert(bpf_map_delete_elem(arp_table_map_fd,
+							   &arp_entry.dst) == 0);
 			} else if (nh->nlmsg_type == RTM_NEWNEIGH) {
-				assert(bpf_map_update_elem(map_fd[2],
+				assert(bpf_map_update_elem(arp_table_map_fd,
 							   &arp_entry.dst,
 							   &arp_entry.mac, 0
 							   ) == 0);
@@ -553,7 +565,8 @@ static int monitor_route(void)
 		for (key = 0; key < nr_keys; key++) {
 			__u64 sum = 0;
 
-			assert(bpf_map_lookup_elem(map_fd[1], &key, values) == 0);
+			assert(bpf_map_lookup_elem(rxcnt_map_fd,
+						   &key, values) == 0);
 			for (i = 0; i < nr_cpus; i++)
 				sum += (values[i] - prev[key][i]);
 			if (sum)
@@ -596,11 +609,18 @@ static int monitor_route(void)
 
 int main(int ac, char **argv)
 {
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
+	int prog_fd;
 	int i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
+
 	if (ac < 2) {
 		printf("usage: %s [-S] Interface name list\n", argv[0]);
 		return 1;
@@ -614,15 +634,28 @@ int main(int ac, char **argv)
 		total_ifindex = ac - 1;
 		ifname_list = (argv + 1);
 	}
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
-	}
+
 	printf("\n**************loading bpf file*********************\n\n\n");
-	if (!prog_fd[0]) {
-		printf("load_bpf_file: %s\n", strerror(errno));
+	if (!prog_fd) {
+		printf("bpf_prog_load_xattr: %s\n", strerror(errno));
 		return 1;
 	}
+
+	lpm_map_fd = bpf_object__find_map_fd_by_name(obj, "lpm_map");
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	arp_table_map_fd = bpf_object__find_map_fd_by_name(obj, "arp_table");
+	exact_match_map_fd = bpf_object__find_map_fd_by_name(obj,
+							     "exact_match");
+	tx_port_map_fd = bpf_object__find_map_fd_by_name(obj, "tx_port");
+	if (lpm_map_fd < 0 || rxcnt_map_fd < 0 || arp_table_map_fd < 0 ||
+	    exact_match_map_fd < 0 || tx_port_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
+		return 1;
+	}
+
 	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
 	for (i = 0; i < total_ifindex; i++) {
 		ifindex_list[i] = if_nametoindex(ifname_list[i]);
@@ -633,7 +666,7 @@ int main(int ac, char **argv)
 		}
 	}
 	for (i = 0; i < total_ifindex; i++) {
-		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd[0], flags) < 0) {
+		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) < 0) {
 			printf("link set xdp fd failed\n");
 			int recovery_index = i;
 
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index a4ccc33adac0..5093d8220da5 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -17,7 +17,7 @@
 #include <netinet/ether.h>
 #include <unistd.h>
 #include <time.h>
-#include "bpf_load.h"
+#include "bpf/libbpf.h"
 #include <bpf/bpf.h>
 #include "bpf_util.h"
 #include "xdp_tx_iptunnel_common.h"
@@ -26,6 +26,7 @@
 
 static int ifindex = -1;
 static __u32 xdp_flags = 0;
+static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
@@ -53,7 +54,8 @@ static void poll_stats(unsigned int kill_after_s)
 		for (proto = 0; proto < nr_protos; proto++) {
 			__u64 sum = 0;
 
-			assert(bpf_map_lookup_elem(map_fd[0], &proto, values) == 0);
+			assert(bpf_map_lookup_elem(rxcnt_map_fd, &proto,
+						   values) == 0);
 			for (i = 0; i < nr_cpus; i++)
 				sum += (values[i] - prev[proto][i]);
 
@@ -138,15 +140,19 @@ static int parse_ports(const char *port_str, int *min_port, int *max_port)
 
 int main(int argc, char **argv)
 {
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type	= BPF_PROG_TYPE_XDP,
+	};
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	int min_port = 0, max_port = 0, vip2tnl_map_fd;
+	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
 	unsigned char opt_flags[256] = {};
 	unsigned int kill_after_s = 0;
-	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
-	int min_port = 0, max_port = 0;
 	struct iptnl_info tnl = {};
-	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_object *obj;
 	struct vip vip = {};
 	char filename[256];
-	int opt;
+	int opt, prog_fd;
 	int i;
 
 	tnl.family = AF_UNSPEC;
@@ -232,29 +238,36 @@ int main(int argc, char **argv)
 	}
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+	prog_load_attr.file = filename;
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
-	}
 
-	if (!prog_fd[0]) {
+	if (!prog_fd) {
 		printf("load_bpf_file: %s\n", strerror(errno));
 		return 1;
 	}
 
+	rxcnt_map_fd = bpf_object__find_map_fd_by_name(obj, "rxcnt");
+	vip2tnl_map_fd = bpf_object__find_map_fd_by_name(obj, "vip2tnl");
+	if (vip2tnl_map_fd < 0 || rxcnt_map_fd < 0) {
+		printf("bpf_object__find_map_fd_by_name failed\n");
+		return 1;
+	}
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
 	while (min_port <= max_port) {
 		vip.dport = htons(min_port++);
-		if (bpf_map_update_elem(map_fd[1], &vip, &tnl, BPF_NOEXIST)) {
+		if (bpf_map_update_elem(vip2tnl_map_fd, &vip, &tnl,
+					BPF_NOEXIST)) {
 			perror("bpf_map_update_elem(&vip2tnl)");
 			return 1;
 		}
 	}
 
-	if (bpf_set_link_xdp_fd(ifindex, prog_fd[0], xdp_flags) < 0) {
+	if (bpf_set_link_xdp_fd(ifindex, prog_fd, xdp_flags) < 0) {
 		printf("link set xdp fd failed\n");
 		return 1;
 	}
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage
  2019-01-17  1:01 ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Maciej Fijalkowski
@ 2019-01-17 11:19   ` Jesper Dangaard Brouer
  2019-01-17 11:26     ` [RFC bpf-next PATCH] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe Jesper Dangaard Brouer
  2019-01-25  8:22   ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Jesper Dangaard Brouer
  1 sibling, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2019-01-17 11:19 UTC (permalink / raw)
  To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer

On Thu, 17 Jan 2019 02:01:11 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:

> xdp_redirect_cpu is omitted because of read_trace_pipe() usage, which
> doesn't seem to be handled in libbpf ATM.

The sample xdp_redirect_cpu, doesn't need to use read_trace_pipe().
I'll send a patch to remove it (that you can include in your patchset).

It must be a left-over when I was developing this... as it doesn't do
any bpf_trace_printk calls.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC bpf-next PATCH] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe
  2019-01-17 11:19   ` Jesper Dangaard Brouer
@ 2019-01-17 11:26     ` Jesper Dangaard Brouer
  2019-01-21  8:39       ` Maciej Fijałkowski
  0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2019-01-17 11:26 UTC (permalink / raw)
  To: netdev, maciejromanfijalkowski
  Cc: Daniel Borkmann, jakub.kicinski, Alexei Starovoitov,
	Jesper Dangaard Brouer

The sample xdp_redirect_cpu is not using helper bpf_trace_printk.
Thus it makes no sense that the --debug option us reading
from /sys/kernel/debug/tracing/trace_pipe via read_trace_pipe.
Simply remove it.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
I request that Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
will take and integrate this patch in his patchset, such that
we can also complete the conversion of xdp_redirect_cpu to libbpf.

 samples/bpf/xdp_redirect_cpu_user.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 2d23054aaccf..f141e752ca0a 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -51,7 +51,6 @@ static const struct option long_options[] = {
 	{"help",	no_argument,		NULL, 'h' },
 	{"dev",		required_argument,	NULL, 'd' },
 	{"skb-mode",	no_argument,		NULL, 'S' },
-	{"debug",	no_argument,		NULL, 'D' },
 	{"sec",		required_argument,	NULL, 's' },
 	{"prognum",	required_argument,	NULL, 'p' },
 	{"qsize",	required_argument,	NULL, 'q' },
@@ -563,7 +562,6 @@ int main(int argc, char **argv)
 	bool use_separators = true;
 	bool stress_mode = false;
 	char filename[256];
-	bool debug = false;
 	int added_cpus = 0;
 	int longindex = 0;
 	int interval = 2;
@@ -624,9 +622,6 @@ int main(int argc, char **argv)
 		case 'S':
 			xdp_flags |= XDP_FLAGS_SKB_MODE;
 			break;
-		case 'D':
-			debug = true;
-			break;
 		case 'x':
 			stress_mode = true;
 			break;
@@ -688,11 +683,6 @@ int main(int argc, char **argv)
 		return EXIT_FAIL_XDP;
 	}
 
-	if (debug) {
-		printf("Debug-mode reading trace pipe (fix #define DEBUG)\n");
-		read_trace_pipe();
-	}
-
 	stats_poll(interval, use_separators, prog_num, stress_mode);
 	return EXIT_OK;
 }


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC bpf-next PATCH] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe
  2019-01-17 11:26     ` [RFC bpf-next PATCH] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe Jesper Dangaard Brouer
@ 2019-01-21  8:39       ` Maciej Fijałkowski
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijałkowski @ 2019-01-21  8:39 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski, netdev

Dnia 2019-01-17, o godz. 12:26:47
Jesper Dangaard Brouer <brouer@redhat.com> napisał(a):

> The sample xdp_redirect_cpu is not using helper bpf_trace_printk.
> Thus it makes no sense that the --debug option us reading
> from /sys/kernel/debug/tracing/trace_pipe via read_trace_pipe.
> Simply remove it.
> 
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> I request that Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> will take and integrate this patch in his patchset, such that
> we can also complete the conversion of xdp_redirect_cpu to libbpf.
>
Let me include your patch and send a v2 :)

>  samples/bpf/xdp_redirect_cpu_user.c |   10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/samples/bpf/xdp_redirect_cpu_user.c
> b/samples/bpf/xdp_redirect_cpu_user.c index 2d23054aaccf..f141e752ca0a 100644
> --- a/samples/bpf/xdp_redirect_cpu_user.c
> +++ b/samples/bpf/xdp_redirect_cpu_user.c
> @@ -51,7 +51,6 @@ static const struct option long_options[] = {
>  	{"help",	no_argument,		NULL, 'h' },
>  	{"dev",		required_argument,	NULL, 'd' },
>  	{"skb-mode",	no_argument,		NULL, 'S' },
> -	{"debug",	no_argument,		NULL, 'D' },
>  	{"sec",		required_argument,	NULL, 's' },
>  	{"prognum",	required_argument,	NULL, 'p' },
>  	{"qsize",	required_argument,	NULL, 'q' },
> @@ -563,7 +562,6 @@ int main(int argc, char **argv)
>  	bool use_separators = true;
>  	bool stress_mode = false;
>  	char filename[256];
> -	bool debug = false;
>  	int added_cpus = 0;
>  	int longindex = 0;
>  	int interval = 2;
> @@ -624,9 +622,6 @@ int main(int argc, char **argv)
>  		case 'S':
>  			xdp_flags |= XDP_FLAGS_SKB_MODE;
>  			break;
> -		case 'D':
> -			debug = true;
> -			break;
>  		case 'x':
>  			stress_mode = true;
>  			break;
> @@ -688,11 +683,6 @@ int main(int argc, char **argv)
>  		return EXIT_FAIL_XDP;
>  	}
>  
> -	if (debug) {
> -		printf("Debug-mode reading trace pipe (fix #define
> DEBUG)\n");
> -		read_trace_pipe();
> -	}
> -
>  	stats_poll(interval, use_separators, prog_num, stress_mode);
>  	return EXIT_OK;
>  }
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage
  2019-01-17  1:01 ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Maciej Fijalkowski
  2019-01-17 11:19   ` Jesper Dangaard Brouer
@ 2019-01-25  8:22   ` Jesper Dangaard Brouer
  1 sibling, 0 replies; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2019-01-25  8:22 UTC (permalink / raw)
  To: Maciej Fijalkowski; +Cc: daniel, ast, netdev, jakub.kicinski, brouer

On Thu, 17 Jan 2019 02:01:11 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:

> Some of XDP samples that are attaching the bpf program to the interface
> via libbpf's bpf_set_link_xdp_fd are still using the bpf_load.c for
> loading and manipulating the ebpf program and maps. Convert them to do
> this through libbpf usage and remove bpf_load from the picture.
> 
> While at it remove what looks like debug leftover in
> xdp_redirect_map_user.c
> 
> xdp_redirect_cpu is omitted because of read_trace_pipe() usage, which
> doesn't seem to be handled in libbpf ATM.
> 
> Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  samples/bpf/Makefile                |  8 ++--
>  samples/bpf/xdp_redirect_map_user.c | 47 ++++++++++++++++-------
>  samples/bpf/xdp_redirect_user.c     | 44 +++++++++++++++++-----
>  samples/bpf/xdp_router_ipv4_user.c  | 75 ++++++++++++++++++++++++++-----------
>  samples/bpf/xdp_tx_iptunnel_user.c  | 37 ++++++++++++------
>  5 files changed, 151 insertions(+), 60 deletions(-)

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

Thanks a lot for converting sample/bpf programs to use libbpf.  It have
been on my todo list for a very long time.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 3/6] samples: bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4}
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 1/6] libbpf: Add a helper for retrieving a map fd for a given name Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 4/6] samples: bpf: Add a "force" flag to XDP samples Maciej Fijalkowski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

There is a common problem with xdp samples that happens when user wants
to run a particular sample and some bpf program is already loaded. The
default 64kb RLIMIT_MEMLOCK resource limit will cause a following error
(assuming that xdp sample that is failing was converted to libbpf
usage):

libbpf: Error in bpf_object__probe_name():Operation not permitted(1).
Couldn't load basic 'r0 = 0' BPF program.
libbpf: failed to load object './xdp_sample_pkts_kern.o'

Fix it in xdp_sample_pkts and xdp_router_ipv4 by setting RLIMIT_MEMLOCK
to RLIM_INFINITY.

Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp_router_ipv4_user.c | 7 +++++++
 samples/bpf/xdp_sample_pkts_user.c | 7 +++++++
 2 files changed, 14 insertions(+)

diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index cea2306f5ab7..c63c6beec7d6 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -25,6 +25,7 @@
 #include <sys/syscall.h>
 #include "bpf_util.h"
 #include "bpf/libbpf.h"
+#include <sys/resource.h>
 
 int sock, sock_arp, flags = 0;
 static int total_ifindex;
@@ -609,6 +610,7 @@ static int monitor_route(void)
 
 int main(int ac, char **argv)
 {
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
@@ -635,6 +637,11 @@ int main(int ac, char **argv)
 		ifname_list = (argv + 1);
 	}
 
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
 	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &prog_fd))
 		return 1;
 
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 8dd87c1eb560..5f5828ee0761 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -12,6 +12,7 @@
 #include <signal.h>
 #include <libbpf.h>
 #include <bpf/bpf.h>
+#include <sys/resource.h>
 
 #include "perf-sys.h"
 #include "trace_helpers.h"
@@ -99,6 +100,7 @@ static void sig_handler(int signo)
 
 int main(int argc, char **argv)
 {
+	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
@@ -114,6 +116,11 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
+		perror("setrlimit(RLIMIT_MEMLOCK)");
+		return 1;
+	}
+
 	numcpus = get_nprocs();
 	if (numcpus > MAX_CPUS)
 		numcpus = MAX_CPUS;
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 4/6] samples: bpf: Add a "force" flag to XDP samples
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
                   ` (2 preceding siblings ...)
  2019-01-17  1:01 ` [PATCH bpf-next 3/6] samples: bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4} Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 5/6] libbpf: Add a support for getting xdp prog id on ifindex Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting Maciej Fijalkowski
  5 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Make xdp samples consistent with iproute2 behavior and set the
XDP_FLAGS_UPDATE_IF_NOEXIST by default when setting the xdp program on
interface. Provide an option for user to force the program loading,
which as a result will not include the mentioned flag in
bpf_set_link_xdp_fd call.

Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp1_user.c             | 10 +++++---
 samples/bpf/xdp_adjust_tail_user.c  |  8 ++++--
 samples/bpf/xdp_redirect_map_user.c | 10 +++++---
 samples/bpf/xdp_redirect_user.c     | 10 +++++---
 samples/bpf/xdp_router_ipv4_user.c  | 50 +++++++++++++++++++++++++++----------
 samples/bpf/xdp_rxq_info_user.c     |  8 ++++--
 samples/bpf/xdp_sample_pkts_user.c  | 40 +++++++++++++++++++++++------
 samples/bpf/xdp_tx_iptunnel_user.c  |  8 ++++--
 samples/bpf/xdpsock_user.c          |  7 ++++--
 9 files changed, 113 insertions(+), 38 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 8bfda95c77ad..505bce207165 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -22,7 +22,7 @@
 #include "bpf/libbpf.h"
 
 static int ifindex;
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static void int_exit(int sig)
 {
@@ -63,7 +63,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFACE\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -73,7 +74,7 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
@@ -87,6 +88,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index 3042ce37dae8..049bddf7778b 100644
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -24,7 +24,7 @@
 #define STATS_INTERVAL_S 2U
 
 static int ifindex = -1;
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static void int_exit(int sig)
 {
@@ -60,6 +60,7 @@ static void usage(const char *cmd)
 	printf("    -T <stop-after-X-seconds> Default: 0 (forever)\n");
 	printf("    -S use skb-mode\n");
 	printf("    -N enforce native mode\n");
+	printf("    -F force loading prog\n");
 	printf("    -h Display this help\n");
 }
 
@@ -70,8 +71,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	unsigned char opt_flags[256] = {};
+	const char *optstr = "i:T:SNFh";
 	unsigned int kill_after_s = 0;
-	const char *optstr = "i:T:SNh";
 	int i, prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
@@ -96,6 +97,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(argv[0]);
 			return 1;
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 60d46eea225b..470e1a7e8810 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -30,7 +30,7 @@ static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -70,7 +70,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFINDEX_IN IFINDEX_OUT\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -82,7 +83,7 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, dummy_prog_fd;
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	int ret, opt, key = 0;
 	char filename[256];
@@ -96,6 +97,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index 93404820df68..be6058cda97c 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -30,7 +30,7 @@ static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -70,7 +70,8 @@ static void usage(const char *prog)
 		"usage: %s [OPTS] IFINDEX_IN IFINDEX_OUT\n\n"
 		"OPTS:\n"
 		"    -S    use skb-mode\n"
-		"    -N    enforce native mode\n",
+		"    -N    enforce native mode\n"
+		"    -F    force loading prog\n",
 		prog);
 }
 
@@ -83,7 +84,7 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, tx_port_map_fd, opt;
-	const char *optstr = "SN";
+	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	char filename[256];
 	int dummy_prog_fd;
@@ -97,6 +98,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 			return 1;
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index c63c6beec7d6..208d6a996478 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -26,8 +26,9 @@
 #include "bpf_util.h"
 #include "bpf/libbpf.h"
 #include <sys/resource.h>
+#include <libgen.h>
 
-int sock, sock_arp, flags = 0;
+int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int total_ifindex;
 int *ifindex_list;
 char buf[8192];
@@ -608,33 +609,56 @@ static int monitor_route(void)
 	return ret;
 }
 
+static void usage(const char *prog)
+{
+	fprintf(stderr,
+		"%s: %s [OPTS] interface name list\n\n"
+		"OPTS:\n"
+		"    -S    use skb-mode\n"
+		"    -F    force loading prog\n",
+		__func__, prog);
+}
+
 int main(int ac, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	const char *optstr = "SF";
 	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
-	int prog_fd;
+	int prog_fd, opt;
 	int i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	prog_load_attr.file = filename;
 
-	if (ac < 2) {
-		printf("usage: %s [-S] Interface name list\n", argv[0]);
-		return 1;
+	total_ifindex = ac - 1;
+	ifname_list = (argv + 1);
+
+	while ((opt = getopt(ac, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'S':
+			flags |= XDP_FLAGS_SKB_MODE;
+			total_ifindex--;
+			ifname_list++;
+			break;
+		case 'F':
+			flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			total_ifindex--;
+			ifname_list++;
+			break;
+		default:
+			usage(basename(argv[0]));
+			return 1;
+		}
 	}
-	if (!strcmp(argv[1], "-S")) {
-		flags = XDP_FLAGS_SKB_MODE;
-		total_ifindex = ac - 2;
-		ifname_list = (argv + 2);
-	} else {
-		flags = 0;
-		total_ifindex = ac - 1;
-		ifname_list = (argv + 1);
+
+	if (optind == ac) {
+		usage(basename(argv[0]));
+		return 1;
 	}
 
 	if (setrlimit(RLIMIT_MEMLOCK, &r)) {
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index ef26f882f92f..e7a98c2a440f 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -30,7 +30,7 @@ static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
 
-static __u32 xdp_flags;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static struct bpf_map *stats_global_map;
 static struct bpf_map *rx_queue_index_map;
@@ -52,6 +52,7 @@ static const struct option long_options[] = {
 	{"action",	required_argument,	NULL, 'a' },
 	{"readmem", 	no_argument,		NULL, 'r' },
 	{"swapmac", 	no_argument,		NULL, 'm' },
+	{"force",	no_argument,		NULL, 'F' },
 	{0, 0, NULL,  0 }
 };
 
@@ -487,7 +488,7 @@ int main(int argc, char **argv)
 	}
 
 	/* Parse commands line args */
-	while ((opt = getopt_long(argc, argv, "hSd:",
+	while ((opt = getopt_long(argc, argv, "FhSrmzd:s:a:",
 				  long_options, &longindex)) != -1) {
 		switch (opt) {
 		case 'd':
@@ -524,6 +525,9 @@ int main(int argc, char **argv)
 		case 'm':
 			cfg_options |= SWAP_MAC;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		case 'h':
 		error:
 		default:
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 5f5828ee0761..362ad35b524d 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -13,6 +13,8 @@
 #include <libbpf.h>
 #include <bpf/bpf.h>
 #include <sys/resource.h>
+#include <libgen.h>
+#include <linux/if_link.h>
 
 #include "perf-sys.h"
 #include "trace_helpers.h"
@@ -21,12 +23,13 @@
 static int pmu_fds[MAX_CPUS], if_idx;
 static struct perf_event_mmap_page *headers[MAX_CPUS];
 static char *if_name;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
 static int do_attach(int idx, int fd, const char *name)
 {
 	int err;
 
-	err = bpf_set_link_xdp_fd(idx, fd, 0);
+	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
 	if (err < 0)
 		printf("ERROR: failed to attach program to %s\n", name);
 
@@ -98,21 +101,42 @@ static void sig_handler(int signo)
 	exit(0);
 }
 
+static void usage(const char *prog)
+{
+	fprintf(stderr,
+		"%s: %s [OPTS] IFINDEX\n\n"
+		"OPTS:\n"
+		"    -F    force loading prog\n",
+		__func__, prog);
+}
+
 int main(int argc, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	const char *optstr = "F";
+	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
-	int prog_fd, map_fd;
 	char filename[256];
 	int ret, err, i;
 	int numcpus;
 
-	if (argc < 2) {
-		printf("Usage: %s <ifname>\n", argv[0]);
+	while ((opt = getopt(argc, argv, optstr)) != -1) {
+		switch (opt) {
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
+		default:
+			usage(basename(argv[0]));
+			return 1;
+		}
+	}
+
+	if (optind == argc) {
+		usage(basename(argv[0]));
 		return 1;
 	}
 
@@ -143,16 +167,16 @@ int main(int argc, char **argv)
 	}
 	map_fd = bpf_map__fd(map);
 
-	if_idx = if_nametoindex(argv[1]);
+	if_idx = if_nametoindex(argv[optind]);
 	if (!if_idx)
-		if_idx = strtoul(argv[1], NULL, 0);
+		if_idx = strtoul(argv[optind], NULL, 0);
 
 	if (!if_idx) {
 		fprintf(stderr, "Invalid ifname\n");
 		return 1;
 	}
-	if_name = argv[1];
-	err = do_attach(if_idx, prog_fd, argv[1]);
+	if_name = argv[optind];
+	err = do_attach(if_idx, prog_fd, if_name);
 	if (err)
 		return err;
 
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index 5093d8220da5..e3de60930d27 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -25,7 +25,7 @@
 #define STATS_INTERVAL_S 2U
 
 static int ifindex = -1;
-static __u32 xdp_flags = 0;
+static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
@@ -83,6 +83,7 @@ static void usage(const char *cmd)
 	printf("    -P <IP-Protocol> Default is TCP\n");
 	printf("    -S use skb-mode\n");
 	printf("    -N enforce native mode\n");
+	printf("    -F Force loading the XDP prog\n");
 	printf("    -h Display this help\n");
 }
 
@@ -145,7 +146,7 @@ int main(int argc, char **argv)
 	};
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
 	int min_port = 0, max_port = 0, vip2tnl_map_fd;
-	const char *optstr = "i:a:p:s:d:m:T:P:SNh";
+	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
 	unsigned char opt_flags[256] = {};
 	unsigned int kill_after_s = 0;
 	struct iptnl_info tnl = {};
@@ -217,6 +218,9 @@ int main(int argc, char **argv)
 		case 'N':
 			xdp_flags |= XDP_FLAGS_DRV_MODE;
 			break;
+		case 'F':
+			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(argv[0]);
 			return 1;
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 57ecadc58403..188723784768 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -68,7 +68,7 @@ enum benchmark_type {
 };
 
 static enum benchmark_type opt_bench = BENCH_RXDROP;
-static u32 opt_xdp_flags;
+static u32 opt_xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static const char *opt_if = "";
 static int opt_ifindex;
 static int opt_queue;
@@ -682,7 +682,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "rtli:q:psSNn:cz", long_options,
+		c = getopt_long(argc, argv, "Frtli:q:psSNn:cz", long_options,
 				&option_index);
 		if (c == -1)
 			break;
@@ -725,6 +725,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'c':
 			opt_xdp_bind_flags |= XDP_COPY;
 			break;
+		case 'F':
+			opt_xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
+			break;
 		default:
 			usage(basename(argv[0]));
 		}
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 5/6] libbpf: Add a support for getting xdp prog id on ifindex
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
                   ` (3 preceding siblings ...)
  2019-01-17  1:01 ` [PATCH bpf-next 4/6] samples: bpf: Add a "force" flag to XDP samples Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17  1:01 ` [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting Maciej Fijalkowski
  5 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Since we have a dedicated netlink attributes for xdp setup on a
particular interface, it is now possible to retrieve the program id that
is currently attached to the interface. The use case is targeted for
sample xdp programs, which will store the program id just after loading
bpf program onto iface. On shutdown, the sample will make sure that it
can unload the program by querying again the iface and verifying that
both program id's matches.

Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 tools/lib/bpf/libbpf.h   |  1 +
 tools/lib/bpf/libbpf.map |  1 +
 tools/lib/bpf/netlink.c  | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 7f10d36abdde..8e55705e9a41 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -317,6 +317,7 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
 			     struct bpf_object **pobj, int *prog_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);
 
 enum bpf_perf_event_ret {
 	LIBBPF_PERF_EVENT_DONE	= 0,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 7c59e4f64082..5b73bf886d05 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -127,4 +127,5 @@ LIBBPF_0.0.1 {
 LIBBPF_0.0.2 {
 	global:
 		bpf_object__find_map_fd_by_name;
+		bpf_get_link_xdp_id;
 } LIBBPF_0.0.1;
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 0ce67aea8f3b..e44a6ef25678 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -21,6 +21,12 @@
 typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, libbpf_dump_nlmsg_t,
 			      void *cookie);
 
+struct xdp_id_md {
+	int ifindex;
+	__u32 flags;
+	__u32 id;
+};
+
 int libbpf_netlink_open(__u32 *nl_pid)
 {
 	struct sockaddr_nl sa;
@@ -196,6 +202,84 @@ static int __dump_link_nlmsg(struct nlmsghdr *nlh,
 	return dump_link_nlmsg(cookie, ifi, tb);
 }
 
+static unsigned char get_xdp_id_attr(unsigned char mode, __u32 flags)
+{
+	if (mode != XDP_ATTACHED_MULTI)
+		return IFLA_XDP_PROG_ID;
+	if (flags & XDP_FLAGS_DRV_MODE)
+		return IFLA_XDP_DRV_PROG_ID;
+	if (flags & XDP_FLAGS_HW_MODE)
+		return IFLA_XDP_HW_PROG_ID;
+	if (flags & XDP_FLAGS_SKB_MODE)
+		return IFLA_XDP_SKB_PROG_ID;
+
+	return IFLA_XDP_UNSPEC;
+}
+
+static int get_xdp_id(void *cookie, void *msg, struct nlattr **tb)
+{
+	struct nlattr *xdp_tb[IFLA_XDP_MAX + 1];
+	struct xdp_id_md *xdp_id = cookie;
+	struct ifinfomsg *ifinfo = msg;
+	unsigned char mode, xdp_attr;
+	int ret;
+
+	if (xdp_id->ifindex && xdp_id->ifindex != ifinfo->ifi_index)
+		return 0;
+
+	if (!tb[IFLA_XDP])
+		return 0;
+
+	ret = libbpf_nla_parse_nested(xdp_tb, IFLA_XDP_MAX, tb[IFLA_XDP], NULL);
+	if (ret)
+		return ret;
+
+	if (!xdp_tb[IFLA_XDP_ATTACHED])
+		return 0;
+
+	mode = libbpf_nla_getattr_u8(xdp_tb[IFLA_XDP_ATTACHED]);
+	if (mode == XDP_ATTACHED_NONE)
+		return 0;
+
+	xdp_attr = get_xdp_id_attr(mode, xdp_id->flags);
+	if (!xdp_attr || !xdp_tb[xdp_attr])
+		return -ENOENT;
+
+	xdp_id->id = libbpf_nla_getattr_u32(xdp_tb[xdp_attr]);
+
+	return 0;
+}
+
+int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags)
+{
+	struct xdp_id_md xdp_id = {};
+	int sock, ret;
+	__u32 nl_pid;
+	__u32 mask;
+
+	if (flags & ~XDP_FLAGS_MASK)
+		return -EINVAL;
+
+	/* Check whether the single {HW,DRV,SKB} mode is set */
+	flags &= XDP_FLAGS_MODES;
+	mask = flags - 1;
+	if (flags && flags & mask)
+		return -EINVAL;
+
+	sock = libbpf_netlink_open(&nl_pid);
+	if (sock < 0)
+		return sock;
+
+	xdp_id.ifindex = ifindex;
+	xdp_id.flags = flags;
+
+	ret = libbpf_nl_get_link(sock, nl_pid, get_xdp_id, &xdp_id);
+	*prog_id = xdp_id.id;
+
+	close(sock);
+	return ret;
+}
+
 int libbpf_nl_get_link(int sock, unsigned int nl_pid,
 		       libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
 {
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting
  2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
                   ` (4 preceding siblings ...)
  2019-01-17  1:01 ` [PATCH bpf-next 5/6] libbpf: Add a support for getting xdp prog id on ifindex Maciej Fijalkowski
@ 2019-01-17  1:01 ` Maciej Fijalkowski
  2019-01-17 11:36   ` Jesper Dangaard Brouer
  5 siblings, 1 reply; 13+ messages in thread
From: Maciej Fijalkowski @ 2019-01-17  1:01 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, jakub.kicinski, brouer

Check the program id within the signal handler on polling xdp samples
that were previously converted to libbpf usage. Avoid the situation of
unloading the program that was not attached by sample that is exiting.

Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 samples/bpf/xdp1_user.c             | 19 +++++++++++++++-
 samples/bpf/xdp_adjust_tail_user.c  | 25 +++++++++++++++++----
 samples/bpf/xdp_redirect_map_user.c | 37 ++++++++++++++++++++++++++++---
 samples/bpf/xdp_redirect_user.c     | 38 +++++++++++++++++++++++++++++---
 samples/bpf/xdp_router_ipv4_user.c  | 43 ++++++++++++++++++++++---------------
 samples/bpf/xdp_rxq_info_user.c     | 28 +++++++++++++++++++-----
 samples/bpf/xdp_sample_pkts_user.c  | 29 ++++++++++++++++++++-----
 samples/bpf/xdp_tx_iptunnel_user.c  | 23 +++++++++++++++++---
 samples/bpf/xdpsock_user.c          | 18 +++++++++++++++-
 9 files changed, 218 insertions(+), 42 deletions(-)

diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
index 505bce207165..3acc0e1d589a 100644
--- a/samples/bpf/xdp1_user.c
+++ b/samples/bpf/xdp1_user.c
@@ -23,10 +23,17 @@
 
 static int ifindex;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	else
+		printf("program on interface changed, not removing\n");
 	exit(0);
 }
 
@@ -74,11 +81,14 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "FSN";
 	int prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
+	int err;
 
 	while ((opt = getopt(argc, argv, optstr)) != -1) {
 		switch (opt) {
@@ -139,6 +149,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	poll_stats(map_fd, 2);
 
 	return 0;
diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
index 049bddf7778b..01fc700d6a0c 100644
--- a/samples/bpf/xdp_adjust_tail_user.c
+++ b/samples/bpf/xdp_adjust_tail_user.c
@@ -25,11 +25,19 @@
 
 static int ifindex = -1;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		else
+			printf("program on interface changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -72,11 +80,14 @@ int main(int argc, char **argv)
 	};
 	unsigned char opt_flags[256] = {};
 	const char *optstr = "i:T:SNFh";
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	unsigned int kill_after_s = 0;
 	int i, prog_fd, map_fd, opt;
 	struct bpf_object *obj;
 	struct bpf_map *map;
 	char filename[256];
+	int err;
 
 	for (i = 0; i < strlen(optstr); i++)
 		if (optstr[i] != 'h' && 'a' <= optstr[i] && optstr[i] <= 'z')
@@ -146,9 +157,15 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	poll_stats(map_fd, kill_after_s);
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return 1;
+	}
+	prog_id = info.id;
 
-	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	poll_stats(map_fd, kill_after_s);
+	int_exit(0);
 
 	return 0;
 }
diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
index 470e1a7e8810..cae7b9cead74 100644
--- a/samples/bpf/xdp_redirect_map_user.c
+++ b/samples/bpf/xdp_redirect_map_user.c
@@ -29,15 +29,29 @@
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
+static __u32 prog_id;
+static __u32 dummy_prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
-	if (ifindex_out_xdp_dummy_attached)
-		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
+	else
+		printf("program on iface IN changed, not removing\n");
+
+	if (ifindex_out_xdp_dummy_attached) {
+		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id, xdp_flags);
+		if (dummy_prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+		else
+			printf("program on iface OUT changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -82,6 +96,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	struct bpf_program *prog, *dummy_prog;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int prog_fd, dummy_prog_fd;
 	const char *optstr = "FSN";
 	struct bpf_object *obj;
@@ -153,6 +169,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	prog_id = info.id;
+
 	/* Loading dummy XDP prog on out-device */
 	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
@@ -160,6 +183,14 @@ int main(int argc, char **argv)
 		ifindex_out_xdp_dummy_attached = false;
 	}
 
+	memset(&info, 0, sizeof(info));
+	ret = bpf_obj_get_info_by_fd(dummy_prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	dummy_prog_id = info.id;
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
index be6058cda97c..230b1e5e7f61 100644
--- a/samples/bpf/xdp_redirect_user.c
+++ b/samples/bpf/xdp_redirect_user.c
@@ -29,15 +29,30 @@
 static int ifindex_in;
 static int ifindex_out;
 static bool ifindex_out_xdp_dummy_attached = true;
+static __u32 prog_id;
+static __u32 dummy_prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
 
 static void int_exit(int sig)
 {
-	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
-	if (ifindex_out_xdp_dummy_attached)
-		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
+	else
+		printf("program on iface IN changed, not removing\n");
+
+	if (ifindex_out_xdp_dummy_attached) {
+		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id,
+				    xdp_flags);
+		if (dummy_prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
+		else
+			printf("program on iface OUT changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -84,6 +99,8 @@ int main(int argc, char **argv)
 	};
 	struct bpf_program *prog, *dummy_prog;
 	int prog_fd, tx_port_map_fd, opt;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "FSN";
 	struct bpf_object *obj;
 	char filename[256];
@@ -154,6 +171,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	prog_id = info.id;
+
 	/* Loading dummy XDP prog on out-device */
 	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
 			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
@@ -161,6 +185,14 @@ int main(int argc, char **argv)
 		ifindex_out_xdp_dummy_attached = false;
 	}
 
+	memset(&info, 0, sizeof(info));
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return ret;
+	}
+	dummy_prog_id = info.id;
+
 	signal(SIGINT, int_exit);
 	signal(SIGTERM, int_exit);
 
diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
index 208d6a996478..3991bd42b20c 100644
--- a/samples/bpf/xdp_router_ipv4_user.c
+++ b/samples/bpf/xdp_router_ipv4_user.c
@@ -30,7 +30,8 @@
 
 int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int total_ifindex;
-int *ifindex_list;
+static int *ifindex_list;
+static __u32 *prog_id_list;
 char buf[8192];
 static int lpm_map_fd;
 static int rxcnt_map_fd;
@@ -41,23 +42,26 @@ static int tx_port_map_fd;
 static int get_route_table(int rtm_family);
 static void int_exit(int sig)
 {
+	__u32 prog_id;
 	int i = 0;
 
-	for (i = 0; i < total_ifindex; i++)
-		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
+	for (i = 0; i < total_ifindex; i++) {
+		bpf_get_link_xdp_id(ifindex_list[i], &prog_id, flags);
+		if (prog_id_list[i] == prog_id)
+			bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
+		else
+			printf("program on iface %d changed, not removing\n",
+			       ifindex_list[i]);
+	}
 	exit(0);
 }
 
 static void close_and_exit(int sig)
 {
-	int i = 0;
-
 	close(sock);
 	close(sock_arp);
 
-	for (i = 0; i < total_ifindex; i++)
-		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
-	exit(0);
+	int_exit(0);
 }
 
 /* Get the mac address of the interface given interface name */
@@ -186,13 +190,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
 		route.iface_name = alloca(sizeof(char *) * IFNAMSIZ);
 		route.iface_name = if_indextoname(route.iface, route.iface_name);
 		route.mac = getmac(route.iface_name);
-		if (route.mac == -1) {
-			int i = 0;
-
-			for (i = 0; i < total_ifindex; i++)
-				bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
-			exit(0);
-		}
+		if (route.mac == -1)
+			int_exit(0);
 		assert(bpf_map_update_elem(tx_port_map_fd,
 					   &route.iface, &route.iface, 0) == 0);
 		if (rtm_family == AF_INET) {
@@ -625,12 +624,14 @@ int main(int ac, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	const char *optstr = "SF";
 	struct bpf_object *obj;
 	char filename[256];
 	char **ifname_list;
 	int prog_fd, opt;
-	int i = 1;
+	int err, i = 1;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	prog_load_attr.file = filename;
@@ -687,7 +688,7 @@ int main(int ac, char **argv)
 		return 1;
 	}
 
-	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
+	ifindex_list = (int *)calloc(total_ifindex, sizeof(int *));
 	for (i = 0; i < total_ifindex; i++) {
 		ifindex_list[i] = if_nametoindex(ifname_list[i]);
 		if (!ifindex_list[i]) {
@@ -696,6 +697,7 @@ int main(int ac, char **argv)
 			return 1;
 		}
 	}
+	prog_id_list = (__u32 *)calloc(total_ifindex, sizeof(__u32 *));
 	for (i = 0; i < total_ifindex; i++) {
 		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) < 0) {
 			printf("link set xdp fd failed\n");
@@ -706,6 +708,13 @@ int main(int ac, char **argv)
 
 			return 1;
 		}
+		err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+		if (err) {
+			printf("can't get prog info - %s\n", strerror(errno));
+			return err;
+		}
+		prog_id_list[i] = info.id;
+		memset(&info, 0, sizeof(info));
 		printf("Attached to %d\n", ifindex_list[i]);
 	}
 	signal(SIGINT, int_exit);
diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
index e7a98c2a440f..7602a54eeba6 100644
--- a/samples/bpf/xdp_rxq_info_user.c
+++ b/samples/bpf/xdp_rxq_info_user.c
@@ -29,6 +29,7 @@ static const char *__doc__ = " XDP RX-queue info extract example\n\n"
 static int ifindex = -1;
 static char ifname_buf[IF_NAMESIZE];
 static char *ifname;
+static __u32 prog_id;
 
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 
@@ -58,11 +59,19 @@ static const struct option long_options[] = {
 
 static void int_exit(int sig)
 {
-	fprintf(stderr,
-		"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
-		ifindex, ifname);
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id) {
+			fprintf(stderr,
+				"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
+				ifindex, ifname);
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		} else {
+			printf("program on interface changed, not removing\n");
+		}
+	}
 	exit(EXIT_OK);
 }
 
@@ -447,6 +456,8 @@ int main(int argc, char **argv)
 	struct bpf_prog_load_attr prog_load_attr = {
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int prog_fd, map_fd, opt, err;
 	bool use_separators = true;
 	struct config cfg = { 0 };
@@ -580,6 +591,13 @@ int main(int argc, char **argv)
 		return EXIT_FAIL_XDP;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	stats_poll(interval, action, cfg_options);
 	return EXIT_OK;
 }
diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
index 362ad35b524d..dcf78fbc371e 100644
--- a/samples/bpf/xdp_sample_pkts_user.c
+++ b/samples/bpf/xdp_sample_pkts_user.c
@@ -24,25 +24,44 @@ static int pmu_fds[MAX_CPUS], if_idx;
 static struct perf_event_mmap_page *headers[MAX_CPUS];
 static char *if_name;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static __u32 prog_id;
 
 static int do_attach(int idx, int fd, const char *name)
 {
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	int err;
 
 	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
-	if (err < 0)
+	if (err < 0) {
 		printf("ERROR: failed to attach program to %s\n", name);
+		return err;
+	}
+
+	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
 
 	return err;
 }
 
 static int do_detach(int idx, const char *name)
 {
-	int err;
+	__u32 curr_prog_id;
+	int err = 0;
 
-	err = bpf_set_link_xdp_fd(idx, -1, 0);
-	if (err < 0)
-		printf("ERROR: failed to detach program from %s\n", name);
+	bpf_get_link_xdp_id(idx, &curr_prog_id, 0);
+
+	if (prog_id == curr_prog_id) {
+		err = bpf_set_link_xdp_fd(idx, -1, 0);
+		if (err < 0)
+			printf("ERROR: failed to detach prog from %s\n", name);
+	} else {
+		printf("program on interface changed, not removing\n");
+	}
 
 	return err;
 }
diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
index e3de60930d27..4c1b9b14aa79 100644
--- a/samples/bpf/xdp_tx_iptunnel_user.c
+++ b/samples/bpf/xdp_tx_iptunnel_user.c
@@ -27,11 +27,19 @@
 static int ifindex = -1;
 static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int rxcnt_map_fd;
+static __u32 prog_id;
 
 static void int_exit(int sig)
 {
-	if (ifindex > -1)
-		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+	__u32 curr_prog_id;
+
+	if (ifindex > -1) {
+		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
+		if (prog_id == curr_prog_id)
+			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
+		else
+			printf("program on interface changed, not removing\n");
+	}
 	exit(0);
 }
 
@@ -148,13 +156,15 @@ int main(int argc, char **argv)
 	int min_port = 0, max_port = 0, vip2tnl_map_fd;
 	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
 	unsigned char opt_flags[256] = {};
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	unsigned int kill_after_s = 0;
 	struct iptnl_info tnl = {};
 	struct bpf_object *obj;
 	struct vip vip = {};
 	char filename[256];
 	int opt, prog_fd;
-	int i;
+	int i, err;
 
 	tnl.family = AF_UNSPEC;
 	vip.protocol = IPPROTO_TCP;
@@ -276,6 +286,13 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return err;
+	}
+	prog_id = info.id;
+
 	poll_stats(kill_after_s);
 
 	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 188723784768..d7fb74d9a223 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -76,6 +76,7 @@ static int opt_poll;
 static int opt_shared_packet_buffer;
 static int opt_interval = 1;
 static u32 opt_xdp_bind_flags;
+static __u32 prog_id;
 
 struct xdp_umem_uqueue {
 	u32 cached_prod;
@@ -631,9 +632,15 @@ static void *poller(void *arg)
 
 static void int_exit(int sig)
 {
+	__u32 curr_prog_id;
+
 	(void)sig;
 	dump_stats();
-	bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
+	bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags);
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
+	else
+		printf("program on interface changed, not removing\n");
 	exit(EXIT_SUCCESS);
 }
 
@@ -907,6 +914,8 @@ int main(int argc, char **argv)
 		.prog_type	= BPF_PROG_TYPE_XDP,
 	};
 	int prog_fd, qidconf_map, xsks_map;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	struct bpf_object *obj;
 	char xdp_filename[256];
 	struct bpf_map *map;
@@ -953,6 +962,13 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (ret) {
+		printf("can't get prog info - %s\n", strerror(errno));
+		return 1;
+	}
+	prog_id = info.id;
+
 	ret = bpf_map_update_elem(qidconf_map, &key, &opt_queue, 0);
 	if (ret) {
 		fprintf(stderr, "ERROR: bpf_map_update_elem qidconf\n");
-- 
2.16.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting
  2019-01-17  1:01 ` [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting Maciej Fijalkowski
@ 2019-01-17 11:36   ` Jesper Dangaard Brouer
  2019-01-21  8:46     ` Maciej Fijałkowski
  0 siblings, 1 reply; 13+ messages in thread
From: Jesper Dangaard Brouer @ 2019-01-17 11:36 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: daniel, ast, netdev, jakub.kicinski, brouer,
	Björn Töpel, Jean Hsiao

On Thu, 17 Jan 2019 02:01:15 +0100
Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:

> Check the program id within the signal handler on polling xdp samples
> that were previously converted to libbpf usage. Avoid the situation of
> unloading the program that was not attached by sample that is exiting.

I love that you are doing this work!  QA have already hit these kind of
issues, and I've had to deal with figuring out why certain combination
of QA testing was failing.

Bjørn and I are working on hashing out XDP programs per RXQ see[1].  I
do think that this API:
  bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);

Will be will be compatible with our proposed semantic[1], as this reads
the "global-prog" (and later if match unloads the "global-prog").

[1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org#interface-semantics

> Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  samples/bpf/xdp1_user.c             | 19 +++++++++++++++-
>  samples/bpf/xdp_adjust_tail_user.c  | 25 +++++++++++++++++----
>  samples/bpf/xdp_redirect_map_user.c | 37 ++++++++++++++++++++++++++++---
>  samples/bpf/xdp_redirect_user.c     | 38 +++++++++++++++++++++++++++++---
>  samples/bpf/xdp_router_ipv4_user.c  | 43 ++++++++++++++++++++++---------------
>  samples/bpf/xdp_rxq_info_user.c     | 28 +++++++++++++++++++-----
>  samples/bpf/xdp_sample_pkts_user.c  | 29 ++++++++++++++++++++-----
>  samples/bpf/xdp_tx_iptunnel_user.c  | 23 +++++++++++++++++---
>  samples/bpf/xdpsock_user.c          | 18 +++++++++++++++-
>  9 files changed, 218 insertions(+), 42 deletions(-)
> 
> diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
> index 505bce207165..3acc0e1d589a 100644
> --- a/samples/bpf/xdp1_user.c
> +++ b/samples/bpf/xdp1_user.c
> @@ -23,10 +23,17 @@
>  
>  static int ifindex;
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> +static __u32 prog_id;
>  
>  static void int_exit(int sig)
>  {
> -	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> +	if (prog_id == curr_prog_id)
> +		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	else
> +		printf("program on interface changed, not removing\n");
>  	exit(0);
>  }
>  
> @@ -74,11 +81,14 @@ int main(int argc, char **argv)
>  	struct bpf_prog_load_attr prog_load_attr = {
>  		.prog_type	= BPF_PROG_TYPE_XDP,
>  	};
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	const char *optstr = "FSN";
>  	int prog_fd, map_fd, opt;
>  	struct bpf_object *obj;
>  	struct bpf_map *map;
>  	char filename[256];
> +	int err;
>  
>  	while ((opt = getopt(argc, argv, optstr)) != -1) {
>  		switch (opt) {
> @@ -139,6 +149,13 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (err) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return err;
> +	}
> +	prog_id = info.id;
> +
>  	poll_stats(map_fd, 2);
>  
>  	return 0;
> diff --git a/samples/bpf/xdp_adjust_tail_user.c b/samples/bpf/xdp_adjust_tail_user.c
> index 049bddf7778b..01fc700d6a0c 100644
> --- a/samples/bpf/xdp_adjust_tail_user.c
> +++ b/samples/bpf/xdp_adjust_tail_user.c
> @@ -25,11 +25,19 @@
>  
>  static int ifindex = -1;
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> +static __u32 prog_id;
>  
>  static void int_exit(int sig)
>  {
> -	if (ifindex > -1)
> -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	if (ifindex > -1) {
> +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> +		if (prog_id == curr_prog_id)
> +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +		else
> +			printf("program on interface changed, not removing\n");
> +	}
>  	exit(0);
>  }
>  
> @@ -72,11 +80,14 @@ int main(int argc, char **argv)
>  	};
>  	unsigned char opt_flags[256] = {};
>  	const char *optstr = "i:T:SNFh";
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	unsigned int kill_after_s = 0;
>  	int i, prog_fd, map_fd, opt;
>  	struct bpf_object *obj;
>  	struct bpf_map *map;
>  	char filename[256];
> +	int err;
>  
>  	for (i = 0; i < strlen(optstr); i++)
>  		if (optstr[i] != 'h' && 'a' <= optstr[i] && optstr[i] <= 'z')
> @@ -146,9 +157,15 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> -	poll_stats(map_fd, kill_after_s);
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (err) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return 1;
> +	}
> +	prog_id = info.id;
>  
> -	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	poll_stats(map_fd, kill_after_s);
> +	int_exit(0);
>  
>  	return 0;
>  }
> diff --git a/samples/bpf/xdp_redirect_map_user.c b/samples/bpf/xdp_redirect_map_user.c
> index 470e1a7e8810..cae7b9cead74 100644
> --- a/samples/bpf/xdp_redirect_map_user.c
> +++ b/samples/bpf/xdp_redirect_map_user.c
> @@ -29,15 +29,29 @@
>  static int ifindex_in;
>  static int ifindex_out;
>  static bool ifindex_out_xdp_dummy_attached = true;
> +static __u32 prog_id;
> +static __u32 dummy_prog_id;
>  
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>  static int rxcnt_map_fd;
>  
>  static void int_exit(int sig)
>  {
> -	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> -	if (ifindex_out_xdp_dummy_attached)
> -		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
> +	if (prog_id == curr_prog_id)
> +		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> +	else
> +		printf("program on iface IN changed, not removing\n");
> +
> +	if (ifindex_out_xdp_dummy_attached) {
> +		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id, xdp_flags);
> +		if (dummy_prog_id == curr_prog_id)
> +			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> +		else
> +			printf("program on iface OUT changed, not removing\n");
> +	}
>  	exit(0);
>  }
>  
> @@ -82,6 +96,8 @@ int main(int argc, char **argv)
>  		.prog_type	= BPF_PROG_TYPE_XDP,
>  	};
>  	struct bpf_program *prog, *dummy_prog;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	int prog_fd, dummy_prog_fd;
>  	const char *optstr = "FSN";
>  	struct bpf_object *obj;
> @@ -153,6 +169,13 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (ret) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return ret;
> +	}
> +	prog_id = info.id;
> +
>  	/* Loading dummy XDP prog on out-device */
>  	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
>  			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
> @@ -160,6 +183,14 @@ int main(int argc, char **argv)
>  		ifindex_out_xdp_dummy_attached = false;
>  	}
>  
> +	memset(&info, 0, sizeof(info));
> +	ret = bpf_obj_get_info_by_fd(dummy_prog_fd, &info, &info_len);
> +	if (ret) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return ret;
> +	}
> +	dummy_prog_id = info.id;
> +
>  	signal(SIGINT, int_exit);
>  	signal(SIGTERM, int_exit);
>  
> diff --git a/samples/bpf/xdp_redirect_user.c b/samples/bpf/xdp_redirect_user.c
> index be6058cda97c..230b1e5e7f61 100644
> --- a/samples/bpf/xdp_redirect_user.c
> +++ b/samples/bpf/xdp_redirect_user.c
> @@ -29,15 +29,30 @@
>  static int ifindex_in;
>  static int ifindex_out;
>  static bool ifindex_out_xdp_dummy_attached = true;
> +static __u32 prog_id;
> +static __u32 dummy_prog_id;
>  
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>  static int rxcnt_map_fd;
>  
>  static void int_exit(int sig)
>  {
> -	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> -	if (ifindex_out_xdp_dummy_attached)
> -		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
> +	if (prog_id == curr_prog_id)
> +		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> +	else
> +		printf("program on iface IN changed, not removing\n");
> +
> +	if (ifindex_out_xdp_dummy_attached) {
> +		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id,
> +				    xdp_flags);
> +		if (dummy_prog_id == curr_prog_id)
> +			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> +		else
> +			printf("program on iface OUT changed, not removing\n");
> +	}
>  	exit(0);
>  }
>  
> @@ -84,6 +99,8 @@ int main(int argc, char **argv)
>  	};
>  	struct bpf_program *prog, *dummy_prog;
>  	int prog_fd, tx_port_map_fd, opt;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	const char *optstr = "FSN";
>  	struct bpf_object *obj;
>  	char filename[256];
> @@ -154,6 +171,13 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (ret) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return ret;
> +	}
> +	prog_id = info.id;
> +
>  	/* Loading dummy XDP prog on out-device */
>  	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
>  			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) < 0) {
> @@ -161,6 +185,14 @@ int main(int argc, char **argv)
>  		ifindex_out_xdp_dummy_attached = false;
>  	}
>  
> +	memset(&info, 0, sizeof(info));
> +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (ret) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return ret;
> +	}
> +	dummy_prog_id = info.id;
> +
>  	signal(SIGINT, int_exit);
>  	signal(SIGTERM, int_exit);
>  
> diff --git a/samples/bpf/xdp_router_ipv4_user.c b/samples/bpf/xdp_router_ipv4_user.c
> index 208d6a996478..3991bd42b20c 100644
> --- a/samples/bpf/xdp_router_ipv4_user.c
> +++ b/samples/bpf/xdp_router_ipv4_user.c
> @@ -30,7 +30,8 @@
>  
>  int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>  static int total_ifindex;
> -int *ifindex_list;
> +static int *ifindex_list;
> +static __u32 *prog_id_list;
>  char buf[8192];
>  static int lpm_map_fd;
>  static int rxcnt_map_fd;
> @@ -41,23 +42,26 @@ static int tx_port_map_fd;
>  static int get_route_table(int rtm_family);
>  static void int_exit(int sig)
>  {
> +	__u32 prog_id;
>  	int i = 0;
>  
> -	for (i = 0; i < total_ifindex; i++)
> -		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> +	for (i = 0; i < total_ifindex; i++) {
> +		bpf_get_link_xdp_id(ifindex_list[i], &prog_id, flags);
> +		if (prog_id_list[i] == prog_id)
> +			bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> +		else
> +			printf("program on iface %d changed, not removing\n",
> +			       ifindex_list[i]);
> +	}
>  	exit(0);
>  }
>  
>  static void close_and_exit(int sig)
>  {
> -	int i = 0;
> -
>  	close(sock);
>  	close(sock_arp);
>  
> -	for (i = 0; i < total_ifindex; i++)
> -		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> -	exit(0);
> +	int_exit(0);
>  }
>  
>  /* Get the mac address of the interface given interface name */
> @@ -186,13 +190,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
>  		route.iface_name = alloca(sizeof(char *) * IFNAMSIZ);
>  		route.iface_name = if_indextoname(route.iface, route.iface_name);
>  		route.mac = getmac(route.iface_name);
> -		if (route.mac == -1) {
> -			int i = 0;
> -
> -			for (i = 0; i < total_ifindex; i++)
> -				bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> -			exit(0);
> -		}
> +		if (route.mac == -1)
> +			int_exit(0);
>  		assert(bpf_map_update_elem(tx_port_map_fd,
>  					   &route.iface, &route.iface, 0) == 0);
>  		if (rtm_family == AF_INET) {
> @@ -625,12 +624,14 @@ int main(int ac, char **argv)
>  	struct bpf_prog_load_attr prog_load_attr = {
>  		.prog_type	= BPF_PROG_TYPE_XDP,
>  	};
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	const char *optstr = "SF";
>  	struct bpf_object *obj;
>  	char filename[256];
>  	char **ifname_list;
>  	int prog_fd, opt;
> -	int i = 1;
> +	int err, i = 1;
>  
>  	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
>  	prog_load_attr.file = filename;
> @@ -687,7 +688,7 @@ int main(int ac, char **argv)
>  		return 1;
>  	}
>  
> -	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
> +	ifindex_list = (int *)calloc(total_ifindex, sizeof(int *));
>  	for (i = 0; i < total_ifindex; i++) {
>  		ifindex_list[i] = if_nametoindex(ifname_list[i]);
>  		if (!ifindex_list[i]) {
> @@ -696,6 +697,7 @@ int main(int ac, char **argv)
>  			return 1;
>  		}
>  	}
> +	prog_id_list = (__u32 *)calloc(total_ifindex, sizeof(__u32 *));
>  	for (i = 0; i < total_ifindex; i++) {
>  		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) < 0) {
>  			printf("link set xdp fd failed\n");
> @@ -706,6 +708,13 @@ int main(int ac, char **argv)
>  
>  			return 1;
>  		}
> +		err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +		if (err) {
> +			printf("can't get prog info - %s\n", strerror(errno));
> +			return err;
> +		}
> +		prog_id_list[i] = info.id;
> +		memset(&info, 0, sizeof(info));
>  		printf("Attached to %d\n", ifindex_list[i]);
>  	}
>  	signal(SIGINT, int_exit);
> diff --git a/samples/bpf/xdp_rxq_info_user.c b/samples/bpf/xdp_rxq_info_user.c
> index e7a98c2a440f..7602a54eeba6 100644
> --- a/samples/bpf/xdp_rxq_info_user.c
> +++ b/samples/bpf/xdp_rxq_info_user.c
> @@ -29,6 +29,7 @@ static const char *__doc__ = " XDP RX-queue info extract example\n\n"
>  static int ifindex = -1;
>  static char ifname_buf[IF_NAMESIZE];
>  static char *ifname;
> +static __u32 prog_id;
>  
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>  
> @@ -58,11 +59,19 @@ static const struct option long_options[] = {
>  
>  static void int_exit(int sig)
>  {
> -	fprintf(stderr,
> -		"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
> -		ifindex, ifname);
> -	if (ifindex > -1)
> -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	if (ifindex > -1) {
> +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> +		if (prog_id == curr_prog_id) {
> +			fprintf(stderr,
> +				"Interrupted: Removing XDP program on ifindex:%d device:%s\n",
> +				ifindex, ifname);
> +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +		} else {
> +			printf("program on interface changed, not removing\n");
> +		}
> +	}
>  	exit(EXIT_OK);
>  }
>  
> @@ -447,6 +456,8 @@ int main(int argc, char **argv)
>  	struct bpf_prog_load_attr prog_load_attr = {
>  		.prog_type	= BPF_PROG_TYPE_XDP,
>  	};
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	int prog_fd, map_fd, opt, err;
>  	bool use_separators = true;
>  	struct config cfg = { 0 };
> @@ -580,6 +591,13 @@ int main(int argc, char **argv)
>  		return EXIT_FAIL_XDP;
>  	}
>  
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (err) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return err;
> +	}
> +	prog_id = info.id;
> +
>  	stats_poll(interval, action, cfg_options);
>  	return EXIT_OK;
>  }
> diff --git a/samples/bpf/xdp_sample_pkts_user.c b/samples/bpf/xdp_sample_pkts_user.c
> index 362ad35b524d..dcf78fbc371e 100644
> --- a/samples/bpf/xdp_sample_pkts_user.c
> +++ b/samples/bpf/xdp_sample_pkts_user.c
> @@ -24,25 +24,44 @@ static int pmu_fds[MAX_CPUS], if_idx;
>  static struct perf_event_mmap_page *headers[MAX_CPUS];
>  static char *if_name;
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> +static __u32 prog_id;
>  
>  static int do_attach(int idx, int fd, const char *name)
>  {
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	int err;
>  
>  	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
> -	if (err < 0)
> +	if (err < 0) {
>  		printf("ERROR: failed to attach program to %s\n", name);
> +		return err;
> +	}
> +
> +	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
> +	if (err) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return err;
> +	}
> +	prog_id = info.id;
>  
>  	return err;
>  }
>  
>  static int do_detach(int idx, const char *name)
>  {
> -	int err;
> +	__u32 curr_prog_id;
> +	int err = 0;
>  
> -	err = bpf_set_link_xdp_fd(idx, -1, 0);
> -	if (err < 0)
> -		printf("ERROR: failed to detach program from %s\n", name);
> +	bpf_get_link_xdp_id(idx, &curr_prog_id, 0);
> +
> +	if (prog_id == curr_prog_id) {
> +		err = bpf_set_link_xdp_fd(idx, -1, 0);
> +		if (err < 0)
> +			printf("ERROR: failed to detach prog from %s\n", name);
> +	} else {
> +		printf("program on interface changed, not removing\n");
> +	}
>  
>  	return err;
>  }
> diff --git a/samples/bpf/xdp_tx_iptunnel_user.c b/samples/bpf/xdp_tx_iptunnel_user.c
> index e3de60930d27..4c1b9b14aa79 100644
> --- a/samples/bpf/xdp_tx_iptunnel_user.c
> +++ b/samples/bpf/xdp_tx_iptunnel_user.c
> @@ -27,11 +27,19 @@
>  static int ifindex = -1;
>  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
>  static int rxcnt_map_fd;
> +static __u32 prog_id;
>  
>  static void int_exit(int sig)
>  {
> -	if (ifindex > -1)
> -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +	__u32 curr_prog_id;
> +
> +	if (ifindex > -1) {
> +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> +		if (prog_id == curr_prog_id)
> +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> +		else
> +			printf("program on interface changed, not removing\n");
> +	}
>  	exit(0);
>  }
>  
> @@ -148,13 +156,15 @@ int main(int argc, char **argv)
>  	int min_port = 0, max_port = 0, vip2tnl_map_fd;
>  	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
>  	unsigned char opt_flags[256] = {};
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	unsigned int kill_after_s = 0;
>  	struct iptnl_info tnl = {};
>  	struct bpf_object *obj;
>  	struct vip vip = {};
>  	char filename[256];
>  	int opt, prog_fd;
> -	int i;
> +	int i, err;
>  
>  	tnl.family = AF_UNSPEC;
>  	vip.protocol = IPPROTO_TCP;
> @@ -276,6 +286,13 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (err) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return err;
> +	}
> +	prog_id = info.id;
> +
>  	poll_stats(kill_after_s);
>  
>  	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index 188723784768..d7fb74d9a223 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -76,6 +76,7 @@ static int opt_poll;
>  static int opt_shared_packet_buffer;
>  static int opt_interval = 1;
>  static u32 opt_xdp_bind_flags;
> +static __u32 prog_id;
>  
>  struct xdp_umem_uqueue {
>  	u32 cached_prod;
> @@ -631,9 +632,15 @@ static void *poller(void *arg)
>  
>  static void int_exit(int sig)
>  {
> +	__u32 curr_prog_id;
> +
>  	(void)sig;
>  	dump_stats();
> -	bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> +	bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags);
> +	if (prog_id == curr_prog_id)
> +		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> +	else
> +		printf("program on interface changed, not removing\n");
>  	exit(EXIT_SUCCESS);
>  }
>  
> @@ -907,6 +914,8 @@ int main(int argc, char **argv)
>  		.prog_type	= BPF_PROG_TYPE_XDP,
>  	};
>  	int prog_fd, qidconf_map, xsks_map;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>  	struct bpf_object *obj;
>  	char xdp_filename[256];
>  	struct bpf_map *map;
> @@ -953,6 +962,13 @@ int main(int argc, char **argv)
>  		exit(EXIT_FAILURE);
>  	}
>  
> +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (ret) {
> +		printf("can't get prog info - %s\n", strerror(errno));
> +		return 1;
> +	}
> +	prog_id = info.id;
> +
>  	ret = bpf_map_update_elem(qidconf_map, &key, &opt_queue, 0);
>  	if (ret) {
>  		fprintf(stderr, "ERROR: bpf_map_update_elem qidconf\n");



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting
  2019-01-17 11:36   ` Jesper Dangaard Brouer
@ 2019-01-21  8:46     ` Maciej Fijałkowski
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijałkowski @ 2019-01-21  8:46 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Daniel Borkmann, Björn Töpel, Alexei Starovoitov,
	Jakub Kicinski, Jean Hsiao, netdev

Dnia 2019-01-17, o godz. 12:36:50
Jesper Dangaard Brouer <brouer@redhat.com> napisał(a):

> On Thu, 17 Jan 2019 02:01:15 +0100
> Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> 
> > Check the program id within the signal handler on polling xdp samples
> > that were previously converted to libbpf usage. Avoid the situation of
> > unloading the program that was not attached by sample that is exiting.
> 
> I love that you are doing this work!  QA have already hit these kind of
> issues, and I've had to deal with figuring out why certain combination
> of QA testing was failing.
> 

Glad to hear that! This actually also came from our validation engineers, so it
seems this is a common issue. I have also seen some old email threads where you
were mentioning this problem.

> Bjørn and I are working on hashing out XDP programs per RXQ see[1].  I
> do think that this API:
>   bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> 
> Will be will be compatible with our proposed semantic[1], as this reads
> the "global-prog" (and later if match unloads the "global-prog").
>
> [1]
> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org#interface-semantics
> 
> > Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> >  samples/bpf/xdp1_user.c             | 19 +++++++++++++++-
> >  samples/bpf/xdp_adjust_tail_user.c  | 25 +++++++++++++++++----
> >  samples/bpf/xdp_redirect_map_user.c | 37 ++++++++++++++++++++++++++++---
> >  samples/bpf/xdp_redirect_user.c     | 38 +++++++++++++++++++++++++++++---
> >  samples/bpf/xdp_router_ipv4_user.c  | 43
> > ++++++++++++++++++++++--------------- samples/bpf/xdp_rxq_info_user.c     |
> > 28 +++++++++++++++++++----- samples/bpf/xdp_sample_pkts_user.c  | 29
> > ++++++++++++++++++++----- samples/bpf/xdp_tx_iptunnel_user.c  | 23
> > +++++++++++++++++--- samples/bpf/xdpsock_user.c          | 18
> > +++++++++++++++- 9 files changed, 218 insertions(+), 42 deletions(-)
> > 
> > diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c
> > index 505bce207165..3acc0e1d589a 100644
> > --- a/samples/bpf/xdp1_user.c
> > +++ b/samples/bpf/xdp1_user.c
> > @@ -23,10 +23,17 @@
> >  
> >  static int ifindex;
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > +static __u32 prog_id;
> >  
> >  static void int_exit(int sig)
> >  {
> > -	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> > +	if (prog_id == curr_prog_id)
> > +		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	else
> > +		printf("program on interface changed, not removing\n");
> >  	exit(0);
> >  }
> >  
> > @@ -74,11 +81,14 @@ int main(int argc, char **argv)
> >  	struct bpf_prog_load_attr prog_load_attr = {
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	const char *optstr = "FSN";
> >  	int prog_fd, map_fd, opt;
> >  	struct bpf_object *obj;
> >  	struct bpf_map *map;
> >  	char filename[256];
> > +	int err;
> >  
> >  	while ((opt = getopt(argc, argv, optstr)) != -1) {
> >  		switch (opt) {
> > @@ -139,6 +149,13 @@ int main(int argc, char **argv)
> >  		return 1;
> >  	}
> >  
> > +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (err) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return err;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	poll_stats(map_fd, 2);
> >  
> >  	return 0;
> > diff --git a/samples/bpf/xdp_adjust_tail_user.c
> > b/samples/bpf/xdp_adjust_tail_user.c index 049bddf7778b..01fc700d6a0c 100644
> > --- a/samples/bpf/xdp_adjust_tail_user.c
> > +++ b/samples/bpf/xdp_adjust_tail_user.c
> > @@ -25,11 +25,19 @@
> >  
> >  static int ifindex = -1;
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > +static __u32 prog_id;
> >  
> >  static void int_exit(int sig)
> >  {
> > -	if (ifindex > -1)
> > -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	if (ifindex > -1) {
> > +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> > +		if (prog_id == curr_prog_id)
> > +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +		else
> > +			printf("program on interface changed, not
> > removing\n");
> > +	}
> >  	exit(0);
> >  }
> >  
> > @@ -72,11 +80,14 @@ int main(int argc, char **argv)
> >  	};
> >  	unsigned char opt_flags[256] = {};
> >  	const char *optstr = "i:T:SNFh";
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	unsigned int kill_after_s = 0;
> >  	int i, prog_fd, map_fd, opt;
> >  	struct bpf_object *obj;
> >  	struct bpf_map *map;
> >  	char filename[256];
> > +	int err;
> >  
> >  	for (i = 0; i < strlen(optstr); i++)
> >  		if (optstr[i] != 'h' && 'a' <= optstr[i] && optstr[i] <=
> > 'z') @@ -146,9 +157,15 @@ int main(int argc, char **argv)
> >  		return 1;
> >  	}
> >  
> > -	poll_stats(map_fd, kill_after_s);
> > +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (err) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return 1;
> > +	}
> > +	prog_id = info.id;
> >  
> > -	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	poll_stats(map_fd, kill_after_s);
> > +	int_exit(0);
> >  
> >  	return 0;
> >  }
> > diff --git a/samples/bpf/xdp_redirect_map_user.c
> > b/samples/bpf/xdp_redirect_map_user.c index 470e1a7e8810..cae7b9cead74
> > 100644 --- a/samples/bpf/xdp_redirect_map_user.c
> > +++ b/samples/bpf/xdp_redirect_map_user.c
> > @@ -29,15 +29,29 @@
> >  static int ifindex_in;
> >  static int ifindex_out;
> >  static bool ifindex_out_xdp_dummy_attached = true;
> > +static __u32 prog_id;
> > +static __u32 dummy_prog_id;
> >  
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  static int rxcnt_map_fd;
> >  
> >  static void int_exit(int sig)
> >  {
> > -	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> > -	if (ifindex_out_xdp_dummy_attached)
> > -		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
> > +	if (prog_id == curr_prog_id)
> > +		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> > +	else
> > +		printf("program on iface IN changed, not removing\n");
> > +
> > +	if (ifindex_out_xdp_dummy_attached) {
> > +		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id, xdp_flags);
> > +		if (dummy_prog_id == curr_prog_id)
> > +			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> > +		else
> > +			printf("program on iface OUT changed, not
> > removing\n");
> > +	}
> >  	exit(0);
> >  }
> >  
> > @@ -82,6 +96,8 @@ int main(int argc, char **argv)
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> >  	struct bpf_program *prog, *dummy_prog;
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	int prog_fd, dummy_prog_fd;
> >  	const char *optstr = "FSN";
> >  	struct bpf_object *obj;
> > @@ -153,6 +169,13 @@ int main(int argc, char **argv)
> >  		return 1;
> >  	}
> >  
> > +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (ret) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return ret;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	/* Loading dummy XDP prog on out-device */
> >  	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
> >  			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) <
> > 0) { @@ -160,6 +183,14 @@ int main(int argc, char **argv)
> >  		ifindex_out_xdp_dummy_attached = false;
> >  	}
> >  
> > +	memset(&info, 0, sizeof(info));
> > +	ret = bpf_obj_get_info_by_fd(dummy_prog_fd, &info, &info_len);
> > +	if (ret) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return ret;
> > +	}
> > +	dummy_prog_id = info.id;
> > +
> >  	signal(SIGINT, int_exit);
> >  	signal(SIGTERM, int_exit);
> >  
> > diff --git a/samples/bpf/xdp_redirect_user.c
> > b/samples/bpf/xdp_redirect_user.c index be6058cda97c..230b1e5e7f61 100644
> > --- a/samples/bpf/xdp_redirect_user.c
> > +++ b/samples/bpf/xdp_redirect_user.c
> > @@ -29,15 +29,30 @@
> >  static int ifindex_in;
> >  static int ifindex_out;
> >  static bool ifindex_out_xdp_dummy_attached = true;
> > +static __u32 prog_id;
> > +static __u32 dummy_prog_id;
> >  
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  static int rxcnt_map_fd;
> >  
> >  static void int_exit(int sig)
> >  {
> > -	bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> > -	if (ifindex_out_xdp_dummy_attached)
> > -		bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	bpf_get_link_xdp_id(ifindex_in, &curr_prog_id, xdp_flags);
> > +	if (prog_id == curr_prog_id)
> > +		bpf_set_link_xdp_fd(ifindex_in, -1, xdp_flags);
> > +	else
> > +		printf("program on iface IN changed, not removing\n");
> > +
> > +	if (ifindex_out_xdp_dummy_attached) {
> > +		bpf_get_link_xdp_id(ifindex_out, &curr_prog_id,
> > +				    xdp_flags);
> > +		if (dummy_prog_id == curr_prog_id)
> > +			bpf_set_link_xdp_fd(ifindex_out, -1, xdp_flags);
> > +		else
> > +			printf("program on iface OUT changed, not
> > removing\n");
> > +	}
> >  	exit(0);
> >  }
> >  
> > @@ -84,6 +99,8 @@ int main(int argc, char **argv)
> >  	};
> >  	struct bpf_program *prog, *dummy_prog;
> >  	int prog_fd, tx_port_map_fd, opt;
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	const char *optstr = "FSN";
> >  	struct bpf_object *obj;
> >  	char filename[256];
> > @@ -154,6 +171,13 @@ int main(int argc, char **argv)
> >  		return 1;
> >  	}
> >  
> > +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (ret) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return ret;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	/* Loading dummy XDP prog on out-device */
> >  	if (bpf_set_link_xdp_fd(ifindex_out, dummy_prog_fd,
> >  			    (xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST)) <
> > 0) { @@ -161,6 +185,14 @@ int main(int argc, char **argv)
> >  		ifindex_out_xdp_dummy_attached = false;
> >  	}
> >  
> > +	memset(&info, 0, sizeof(info));
> > +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (ret) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return ret;
> > +	}
> > +	dummy_prog_id = info.id;
> > +
> >  	signal(SIGINT, int_exit);
> >  	signal(SIGTERM, int_exit);
> >  
> > diff --git a/samples/bpf/xdp_router_ipv4_user.c
> > b/samples/bpf/xdp_router_ipv4_user.c index 208d6a996478..3991bd42b20c 100644
> > --- a/samples/bpf/xdp_router_ipv4_user.c
> > +++ b/samples/bpf/xdp_router_ipv4_user.c
> > @@ -30,7 +30,8 @@
> >  
> >  int sock, sock_arp, flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  static int total_ifindex;
> > -int *ifindex_list;
> > +static int *ifindex_list;
> > +static __u32 *prog_id_list;
> >  char buf[8192];
> >  static int lpm_map_fd;
> >  static int rxcnt_map_fd;
> > @@ -41,23 +42,26 @@ static int tx_port_map_fd;
> >  static int get_route_table(int rtm_family);
> >  static void int_exit(int sig)
> >  {
> > +	__u32 prog_id;
> >  	int i = 0;
> >  
> > -	for (i = 0; i < total_ifindex; i++)
> > -		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> > +	for (i = 0; i < total_ifindex; i++) {
> > +		bpf_get_link_xdp_id(ifindex_list[i], &prog_id, flags);
> > +		if (prog_id_list[i] == prog_id)
> > +			bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> > +		else
> > +			printf("program on iface %d changed, not
> > removing\n",
> > +			       ifindex_list[i]);
> > +	}
> >  	exit(0);
> >  }
> >  
> >  static void close_and_exit(int sig)
> >  {
> > -	int i = 0;
> > -
> >  	close(sock);
> >  	close(sock_arp);
> >  
> > -	for (i = 0; i < total_ifindex; i++)
> > -		bpf_set_link_xdp_fd(ifindex_list[i], -1, flags);
> > -	exit(0);
> > +	int_exit(0);
> >  }
> >  
> >  /* Get the mac address of the interface given interface name */
> > @@ -186,13 +190,8 @@ static void read_route(struct nlmsghdr *nh, int nll)
> >  		route.iface_name = alloca(sizeof(char *) * IFNAMSIZ);
> >  		route.iface_name = if_indextoname(route.iface,
> > route.iface_name); route.mac = getmac(route.iface_name);
> > -		if (route.mac == -1) {
> > -			int i = 0;
> > -
> > -			for (i = 0; i < total_ifindex; i++)
> > -				bpf_set_link_xdp_fd(ifindex_list[i], -1,
> > flags);
> > -			exit(0);
> > -		}
> > +		if (route.mac == -1)
> > +			int_exit(0);
> >  		assert(bpf_map_update_elem(tx_port_map_fd,
> >  					   &route.iface, &route.iface, 0)
> > == 0); if (rtm_family == AF_INET) {
> > @@ -625,12 +624,14 @@ int main(int ac, char **argv)
> >  	struct bpf_prog_load_attr prog_load_attr = {
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	const char *optstr = "SF";
> >  	struct bpf_object *obj;
> >  	char filename[256];
> >  	char **ifname_list;
> >  	int prog_fd, opt;
> > -	int i = 1;
> > +	int err, i = 1;
> >  
> >  	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
> >  	prog_load_attr.file = filename;
> > @@ -687,7 +688,7 @@ int main(int ac, char **argv)
> >  		return 1;
> >  	}
> >  
> > -	ifindex_list = (int *)malloc(total_ifindex * sizeof(int *));
> > +	ifindex_list = (int *)calloc(total_ifindex, sizeof(int *));
> >  	for (i = 0; i < total_ifindex; i++) {
> >  		ifindex_list[i] = if_nametoindex(ifname_list[i]);
> >  		if (!ifindex_list[i]) {
> > @@ -696,6 +697,7 @@ int main(int ac, char **argv)
> >  			return 1;
> >  		}
> >  	}
> > +	prog_id_list = (__u32 *)calloc(total_ifindex, sizeof(__u32 *));
> >  	for (i = 0; i < total_ifindex; i++) {
> >  		if (bpf_set_link_xdp_fd(ifindex_list[i], prog_fd, flags) <
> > 0) { printf("link set xdp fd failed\n");
> > @@ -706,6 +708,13 @@ int main(int ac, char **argv)
> >  
> >  			return 1;
> >  		}
> > +		err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +		if (err) {
> > +			printf("can't get prog info - %s\n",
> > strerror(errno));
> > +			return err;
> > +		}
> > +		prog_id_list[i] = info.id;
> > +		memset(&info, 0, sizeof(info));
> >  		printf("Attached to %d\n", ifindex_list[i]);
> >  	}
> >  	signal(SIGINT, int_exit);
> > diff --git a/samples/bpf/xdp_rxq_info_user.c
> > b/samples/bpf/xdp_rxq_info_user.c index e7a98c2a440f..7602a54eeba6 100644
> > --- a/samples/bpf/xdp_rxq_info_user.c
> > +++ b/samples/bpf/xdp_rxq_info_user.c
> > @@ -29,6 +29,7 @@ static const char *__doc__ = " XDP RX-queue info extract
> > example\n\n" static int ifindex = -1;
> >  static char ifname_buf[IF_NAMESIZE];
> >  static char *ifname;
> > +static __u32 prog_id;
> >  
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  
> > @@ -58,11 +59,19 @@ static const struct option long_options[] = {
> >  
> >  static void int_exit(int sig)
> >  {
> > -	fprintf(stderr,
> > -		"Interrupted: Removing XDP program on ifindex:%d
> > device:%s\n",
> > -		ifindex, ifname);
> > -	if (ifindex > -1)
> > -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	if (ifindex > -1) {
> > +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> > +		if (prog_id == curr_prog_id) {
> > +			fprintf(stderr,
> > +				"Interrupted: Removing XDP program on
> > ifindex:%d device:%s\n",
> > +				ifindex, ifname);
> > +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +		} else {
> > +			printf("program on interface changed, not
> > removing\n");
> > +		}
> > +	}
> >  	exit(EXIT_OK);
> >  }
> >  
> > @@ -447,6 +456,8 @@ int main(int argc, char **argv)
> >  	struct bpf_prog_load_attr prog_load_attr = {
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	int prog_fd, map_fd, opt, err;
> >  	bool use_separators = true;
> >  	struct config cfg = { 0 };
> > @@ -580,6 +591,13 @@ int main(int argc, char **argv)
> >  		return EXIT_FAIL_XDP;
> >  	}
> >  
> > +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (err) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return err;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	stats_poll(interval, action, cfg_options);
> >  	return EXIT_OK;
> >  }
> > diff --git a/samples/bpf/xdp_sample_pkts_user.c
> > b/samples/bpf/xdp_sample_pkts_user.c index 362ad35b524d..dcf78fbc371e 100644
> > --- a/samples/bpf/xdp_sample_pkts_user.c
> > +++ b/samples/bpf/xdp_sample_pkts_user.c
> > @@ -24,25 +24,44 @@ static int pmu_fds[MAX_CPUS], if_idx;
> >  static struct perf_event_mmap_page *headers[MAX_CPUS];
> >  static char *if_name;
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > +static __u32 prog_id;
> >  
> >  static int do_attach(int idx, int fd, const char *name)
> >  {
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	int err;
> >  
> >  	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
> > -	if (err < 0)
> > +	if (err < 0) {
> >  		printf("ERROR: failed to attach program to %s\n", name);
> > +		return err;
> > +	}
> > +
> > +	err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
> > +	if (err) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return err;
> > +	}
> > +	prog_id = info.id;
> >  
> >  	return err;
> >  }
> >  
> >  static int do_detach(int idx, const char *name)
> >  {
> > -	int err;
> > +	__u32 curr_prog_id;
> > +	int err = 0;
> >  
> > -	err = bpf_set_link_xdp_fd(idx, -1, 0);
> > -	if (err < 0)
> > -		printf("ERROR: failed to detach program from %s\n", name);
> > +	bpf_get_link_xdp_id(idx, &curr_prog_id, 0);
> > +
> > +	if (prog_id == curr_prog_id) {
> > +		err = bpf_set_link_xdp_fd(idx, -1, 0);
> > +		if (err < 0)
> > +			printf("ERROR: failed to detach prog from %s\n",
> > name);
> > +	} else {
> > +		printf("program on interface changed, not removing\n");
> > +	}
> >  
> >  	return err;
> >  }
> > diff --git a/samples/bpf/xdp_tx_iptunnel_user.c
> > b/samples/bpf/xdp_tx_iptunnel_user.c index e3de60930d27..4c1b9b14aa79 100644
> > --- a/samples/bpf/xdp_tx_iptunnel_user.c
> > +++ b/samples/bpf/xdp_tx_iptunnel_user.c
> > @@ -27,11 +27,19 @@
> >  static int ifindex = -1;
> >  static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  static int rxcnt_map_fd;
> > +static __u32 prog_id;
> >  
> >  static void int_exit(int sig)
> >  {
> > -	if (ifindex > -1)
> > -		bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +	__u32 curr_prog_id;
> > +
> > +	if (ifindex > -1) {
> > +		bpf_get_link_xdp_id(ifindex, &curr_prog_id, xdp_flags);
> > +		if (prog_id == curr_prog_id)
> > +			bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > +		else
> > +			printf("program on interface changed, not
> > removing\n");
> > +	}
> >  	exit(0);
> >  }
> >  
> > @@ -148,13 +156,15 @@ int main(int argc, char **argv)
> >  	int min_port = 0, max_port = 0, vip2tnl_map_fd;
> >  	const char *optstr = "i:a:p:s:d:m:T:P:FSNh";
> >  	unsigned char opt_flags[256] = {};
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	unsigned int kill_after_s = 0;
> >  	struct iptnl_info tnl = {};
> >  	struct bpf_object *obj;
> >  	struct vip vip = {};
> >  	char filename[256];
> >  	int opt, prog_fd;
> > -	int i;
> > +	int i, err;
> >  
> >  	tnl.family = AF_UNSPEC;
> >  	vip.protocol = IPPROTO_TCP;
> > @@ -276,6 +286,13 @@ int main(int argc, char **argv)
> >  		return 1;
> >  	}
> >  
> > +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (err) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return err;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	poll_stats(kill_after_s);
> >  
> >  	bpf_set_link_xdp_fd(ifindex, -1, xdp_flags);
> > diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> > index 188723784768..d7fb74d9a223 100644
> > --- a/samples/bpf/xdpsock_user.c
> > +++ b/samples/bpf/xdpsock_user.c
> > @@ -76,6 +76,7 @@ static int opt_poll;
> >  static int opt_shared_packet_buffer;
> >  static int opt_interval = 1;
> >  static u32 opt_xdp_bind_flags;
> > +static __u32 prog_id;
> >  
> >  struct xdp_umem_uqueue {
> >  	u32 cached_prod;
> > @@ -631,9 +632,15 @@ static void *poller(void *arg)
> >  
> >  static void int_exit(int sig)
> >  {
> > +	__u32 curr_prog_id;
> > +
> >  	(void)sig;
> >  	dump_stats();
> > -	bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> > +	bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags);
> > +	if (prog_id == curr_prog_id)
> > +		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> > +	else
> > +		printf("program on interface changed, not removing\n");
> >  	exit(EXIT_SUCCESS);
> >  }
> >  
> > @@ -907,6 +914,8 @@ int main(int argc, char **argv)
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> >  	int prog_fd, qidconf_map, xsks_map;
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >  	struct bpf_object *obj;
> >  	char xdp_filename[256];
> >  	struct bpf_map *map;
> > @@ -953,6 +962,13 @@ int main(int argc, char **argv)
> >  		exit(EXIT_FAILURE);
> >  	}
> >  
> > +	ret = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (ret) {
> > +		printf("can't get prog info - %s\n", strerror(errno));
> > +		return 1;
> > +	}
> > +	prog_id = info.id;
> > +
> >  	ret = bpf_map_update_elem(qidconf_map, &key, &opt_queue, 0);
> >  	if (ret) {
> >  		fprintf(stderr, "ERROR: bpf_map_update_elem qidconf\n");
> 
> 
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-01-25  8:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-17  1:01 [PATCH bpf-next 0/6] xdp: Avoid unloading xdp prog not attached by sample Maciej Fijalkowski
2019-01-17  1:01 ` [PATCH bpf-next 1/6] libbpf: Add a helper for retrieving a map fd for a given name Maciej Fijalkowski
2019-01-17  1:01 ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Maciej Fijalkowski
2019-01-17 11:19   ` Jesper Dangaard Brouer
2019-01-17 11:26     ` [RFC bpf-next PATCH] samples/bpf: xdp_redirect_cpu have not need for read_trace_pipe Jesper Dangaard Brouer
2019-01-21  8:39       ` Maciej Fijałkowski
2019-01-25  8:22   ` [PATCH bpf-next 2/6] samples: bpf: Convert XDP samples to libbpf usage Jesper Dangaard Brouer
2019-01-17  1:01 ` [PATCH bpf-next 3/6] samples: bpf: Extend RLIMIT_MEMLOCK for xdp_{sample_pkts, router_ipv4} Maciej Fijalkowski
2019-01-17  1:01 ` [PATCH bpf-next 4/6] samples: bpf: Add a "force" flag to XDP samples Maciej Fijalkowski
2019-01-17  1:01 ` [PATCH bpf-next 5/6] libbpf: Add a support for getting xdp prog id on ifindex Maciej Fijalkowski
2019-01-17  1:01 ` [PATCH bpf-next 6/6] samples: bpf: Check the prog id before exiting Maciej Fijalkowski
2019-01-17 11:36   ` Jesper Dangaard Brouer
2019-01-21  8:46     ` Maciej Fijałkowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.