[PATCH bpf-next 0/5] samples: bpf: start effort to get rid of bpf_load

[PATCH bpf-next 0/5] samples: bpf: start effort to get rid of bpf_load

[Jakub Kicinski]

Hi!

This set is next part of a quest to get rid of the bpf_load
ELF loader.  It fixes some minor issues with the samples and
starts the conversion.

First patch fixes ping invocations, ping localhost defaults
to IPv6 on modern setups. Next load_sock_ops sample is removed
and users are directed towards using bpftool directly.

Patch 4 removes the use of bpf_load from samples which don't
need the auto-attachment functionality at all.

Patch 5 improves symbol counting in libbpf, it's not currently
an issue but it will be when anyone adds a symbol with a long
name. Let's make sure that person doesn't have to spend time
scratching their head and wondering why .a and .so symbol
counts don't match.

Jakub Kicinski (5):
  samples: bpf: force IPv4 in ping
  samples: bpf: remove load_sock_ops in favour of bpftool
  tools: libbpf: add a correctly named define for map iteration
  samples: bpf: use libbpf where easy
  tools: libbpf: make sure readelf shows full names in build checks

 samples/bpf/.gitignore                        |  1 -
 samples/bpf/Makefile                          |  8 +-
 samples/bpf/fds_example.c                     |  9 +-
 samples/bpf/load_sock_ops.c                   | 97 -------------------
 samples/bpf/sock_example.c                    |  2 +-
 samples/bpf/sockex1_user.c                    | 24 ++---
 samples/bpf/sockex2_user.c                    | 22 +++--
 samples/bpf/sockex3_user.c                    |  2 +-
 samples/bpf/tcp_basertt_kern.c                |  2 +-
 samples/bpf/tcp_bpf.readme                    | 14 +--
 samples/bpf/tcp_bufs_kern.c                   |  2 +-
 samples/bpf/tcp_clamp_kern.c                  |  2 +-
 samples/bpf/tcp_cong_kern.c                   |  2 +-
 samples/bpf/tcp_iw_kern.c                     |  2 +-
 samples/bpf/tcp_rwnd_kern.c                   |  2 +-
 samples/bpf/tcp_synrto_kern.c                 |  2 +-
 samples/bpf/tcp_tos_reflect_kern.c            |  2 +-
 samples/bpf/tracex2_user.c                    |  2 +-
 tools/bpf/bpftool/prog.c                      |  4 +-
 tools/lib/bpf/Makefile                        |  4 +-
 tools/lib/bpf/libbpf.c                        |  8 +-
 tools/lib/bpf/libbpf.h                        |  3 +-
 tools/perf/util/bpf-loader.c                  |  4 +-
 .../testing/selftests/bpf/test_libbpf_open.c  |  2 +-
 24 files changed, 66 insertions(+), 156 deletions(-)
 delete mode 100644 samples/bpf/load_sock_ops.c

-- 
2.19.2

[PATCH bpf-next 1/5] samples: bpf: force IPv4 in ping

[Jakub Kicinski]

ping localhost may default of IPv6 on modern systems, but
samples are trying to only parse IPv4.  Force IPv4.

samples/bpf/tracex1_user.c doesn't interpret the packet so
we don't care which IP version will be used there.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 samples/bpf/sock_example.c | 2 +-
 samples/bpf/sockex1_user.c | 2 +-
 samples/bpf/sockex2_user.c | 2 +-
 samples/bpf/sockex3_user.c | 2 +-
 samples/bpf/tracex2_user.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/bpf/sock_example.c b/samples/bpf/sock_example.c
index 60ec467c78ab..00aae1d33fca 100644
--- a/samples/bpf/sock_example.c
+++ b/samples/bpf/sock_example.c
@@ -99,7 +99,7 @@ int main(void)
 {
 	FILE *f;
 
-	f = popen("ping -c5 localhost", "r");
+	f = popen("ping -4 -c5 localhost", "r");
 	(void)f;
 
 	return test_sock();
diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
index 93ec01c56104..be8ba5686924 100644
--- a/samples/bpf/sockex1_user.c
+++ b/samples/bpf/sockex1_user.c
@@ -26,7 +26,7 @@ int main(int ac, char **argv)
 	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
 			  sizeof(prog_fd[0])) == 0);
 
-	f = popen("ping -c5 localhost", "r");
+	f = popen("ping -4 -c5 localhost", "r");
 	(void) f;
 
 	for (i = 0; i < 5; i++) {
diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
index 1d5c6e9a6d27..125ee6efc913 100644
--- a/samples/bpf/sockex2_user.c
+++ b/samples/bpf/sockex2_user.c
@@ -34,7 +34,7 @@ int main(int ac, char **argv)
 	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
 			  sizeof(prog_fd[0])) == 0);
 
-	f = popen("ping -c5 localhost", "r");
+	f = popen("ping -4 -c5 localhost", "r");
 	(void) f;
 
 	for (i = 0; i < 5; i++) {
diff --git a/samples/bpf/sockex3_user.c b/samples/bpf/sockex3_user.c
index 9d02e0404719..bbb1cd0666a9 100644
--- a/samples/bpf/sockex3_user.c
+++ b/samples/bpf/sockex3_user.c
@@ -58,7 +58,7 @@ int main(int argc, char **argv)
 			  sizeof(__u32)) == 0);
 
 	if (argc > 1)
-		f = popen("ping -c5 localhost", "r");
+		f = popen("ping -4 -c5 localhost", "r");
 	else
 		f = popen("netperf -l 4 localhost", "r");
 	(void) f;
diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
index 1a81e6a5c2ea..c9544a4ce61a 100644
--- a/samples/bpf/tracex2_user.c
+++ b/samples/bpf/tracex2_user.c
@@ -131,7 +131,7 @@ int main(int ac, char **argv)
 	signal(SIGTERM, int_exit);
 
 	/* start 'ping' in the background to have some kfree_skb events */
-	f = popen("ping -c5 localhost", "r");
+	f = popen("ping -4 -c5 localhost", "r");
 	(void) f;
 
 	/* start 'dd' in the background to have plenty of 'write' syscalls */
-- 
2.19.2

[PATCH bpf-next 2/5] samples: bpf: remove load_sock_ops in favour of bpftool

[Jakub Kicinski]

bpftool can do all the things load_sock_ops used to do, and more.
Point users to bpftool instead of maintaining this sample utility.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 samples/bpf/.gitignore             |  1 -
 samples/bpf/Makefile               |  2 -
 samples/bpf/load_sock_ops.c        | 97 ------------------------------
 samples/bpf/tcp_basertt_kern.c     |  2 +-
 samples/bpf/tcp_bpf.readme         | 14 +++--
 samples/bpf/tcp_bufs_kern.c        |  2 +-
 samples/bpf/tcp_clamp_kern.c       |  2 +-
 samples/bpf/tcp_cong_kern.c        |  2 +-
 samples/bpf/tcp_iw_kern.c          |  2 +-
 samples/bpf/tcp_rwnd_kern.c        |  2 +-
 samples/bpf/tcp_synrto_kern.c      |  2 +-
 samples/bpf/tcp_tos_reflect_kern.c |  2 +-
 12 files changed, 16 insertions(+), 114 deletions(-)
 delete mode 100644 samples/bpf/load_sock_ops.c

diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
index 8ae4940025f8..dbb817dbacfc 100644
--- a/samples/bpf/.gitignore
+++ b/samples/bpf/.gitignore
@@ -1,7 +1,6 @@
 cpustat
 fds_example
 lathist
-load_sock_ops
 lwt_len_hist
 map_perf_test
 offwaketime
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index a333e258f319..4dd98100678e 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -40,7 +40,6 @@ hostprogs-y += lwt_len_hist
 hostprogs-y += xdp_tx_iptunnel
 hostprogs-y += test_map_in_map
 hostprogs-y += per_socket_stats_example
-hostprogs-y += load_sock_ops
 hostprogs-y += xdp_redirect
 hostprogs-y += xdp_redirect_map
 hostprogs-y += xdp_redirect_cpu
@@ -71,7 +70,6 @@ tracex4-objs := bpf_load.o tracex4_user.o
 tracex5-objs := bpf_load.o tracex5_user.o
 tracex6-objs := bpf_load.o tracex6_user.o
 tracex7-objs := bpf_load.o tracex7_user.o
-load_sock_ops-objs := bpf_load.o load_sock_ops.o
 test_probe_write_user-objs := bpf_load.o test_probe_write_user_user.o
 trace_output-objs := bpf_load.o trace_output_user.o $(TRACE_HELPERS)
 lathist-objs := bpf_load.o lathist_user.o
diff --git a/samples/bpf/load_sock_ops.c b/samples/bpf/load_sock_ops.c
deleted file mode 100644
index 8ecb41ea0c03..000000000000
--- a/samples/bpf/load_sock_ops.c
+++ /dev/null
@@ -1,97 +0,0 @@
-/* Copyright (c) 2017 Facebook
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- */
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <linux/bpf.h>
-#include <bpf/bpf.h>
-#include "bpf_load.h"
-#include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <linux/unistd.h>
-
-static void usage(char *pname)
-{
-	printf("USAGE:\n  %s [-l] <cg-path> <prog filename>\n", pname);
-	printf("\tLoad and attach a sock_ops program to the specified "
-	       "cgroup\n");
-	printf("\tIf \"-l\" is used, the program will continue to run\n");
-	printf("\tprinting the BPF log buffer\n");
-	printf("\tIf the specified filename does not end in \".o\", it\n");
-	printf("\tappends \"_kern.o\" to the name\n");
-	printf("\n");
-	printf("  %s -r <cg-path>\n", pname);
-	printf("\tDetaches the currently attached sock_ops program\n");
-	printf("\tfrom the specified cgroup\n");
-	printf("\n");
-	exit(1);
-}
-
-int main(int argc, char **argv)
-{
-	int logFlag = 0;
-	int error = 0;
-	char *cg_path;
-	char fn[500];
-	char *prog;
-	int cg_fd;
-
-	if (argc < 3)
-		usage(argv[0]);
-
-	if (!strcmp(argv[1], "-r")) {
-		cg_path = argv[2];
-		cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
-		error = bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS);
-		if (error) {
-			printf("ERROR: bpf_prog_detach: %d (%s)\n",
-			       error, strerror(errno));
-			return 2;
-		}
-		return 0;
-	} else if (!strcmp(argv[1], "-h")) {
-		usage(argv[0]);
-	} else if (!strcmp(argv[1], "-l")) {
-		logFlag = 1;
-		if (argc < 4)
-			usage(argv[0]);
-	}
-
-	prog = argv[argc - 1];
-	cg_path = argv[argc - 2];
-	if (strlen(prog) > 480) {
-		fprintf(stderr, "ERROR: program name too long (> 480 chars)\n");
-		return 3;
-	}
-	cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
-
-	if (!strcmp(prog + strlen(prog)-2, ".o"))
-		strcpy(fn, prog);
-	else
-		sprintf(fn, "%s_kern.o", prog);
-	if (logFlag)
-		printf("loading bpf file:%s\n", fn);
-	if (load_bpf_file(fn)) {
-		printf("ERROR: load_bpf_file failed for: %s\n", fn);
-		printf("%s", bpf_log_buf);
-		return 4;
-	}
-	if (logFlag)
-		printf("TCP BPF Loaded %s\n", fn);
-
-	error = bpf_prog_attach(prog_fd[0], cg_fd, BPF_CGROUP_SOCK_OPS, 0);
-	if (error) {
-		printf("ERROR: bpf_prog_attach: %d (%s)\n",
-		       error, strerror(errno));
-		return 5;
-	} else if (logFlag) {
-		read_trace_pipe();
-	}
-
-	return error;
-}
diff --git a/samples/bpf/tcp_basertt_kern.c b/samples/bpf/tcp_basertt_kern.c
index 4bf4fc597db9..6ef1625e8b2c 100644
--- a/samples/bpf/tcp_basertt_kern.c
+++ b/samples/bpf/tcp_basertt_kern.c
@@ -7,7 +7,7 @@
  * BPF program to set base_rtt to 80us when host is running TCP-NV and
  * both hosts are in the same datacenter (as determined by IPv6 prefix).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_bpf.readme b/samples/bpf/tcp_bpf.readme
index 831fb601e3c9..fee746621aec 100644
--- a/samples/bpf/tcp_bpf.readme
+++ b/samples/bpf/tcp_bpf.readme
@@ -8,14 +8,16 @@ a cgroupv2 and attach a bash shell to the group.
   bash
   echo $$ >> /tmp/cgroupv2/foo/cgroup.procs
 
-Anything that runs under this shell belongs to the foo cgroupv2 To load
+Anything that runs under this shell belongs to the foo cgroupv2. To load
 (attach) one of the tcp_*_kern.o programs:
 
-  ./load_sock_ops -l /tmp/cgroupv2/foo tcp_basertt_kern.o
+  bpftool prog load tcp_basertt_kern.o /sys/fs/bpf/tcp_prog
+  bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
+  bpftool prog tracelog
 
-If the "-l" flag is used, the load_sock_ops program will continue to run
-printing the BPF log buffer. The tcp_*_kern.o programs use special print
-functions to print logging information (if enabled by the ifdef).
+"bpftool prog tracelog" will continue to run printing the BPF log buffer.
+The tcp_*_kern.o programs use special print functions to print logging
+information (if enabled by the ifdef).
 
 If using netperf/netserver to create traffic, you need to run them under the
 cgroupv2 to which the BPF programs are attached (i.e. under bash shell
@@ -23,4 +25,4 @@ attached to the cgroupv2).
 
 To remove (unattach) a socket_ops BPF program from a cgroupv2:
 
-  ./load_sock_ops -r /tmp/cgroupv2/foo
+  bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
diff --git a/samples/bpf/tcp_bufs_kern.c b/samples/bpf/tcp_bufs_kern.c
index 0566b7fa38a1..e03e204739fa 100644
--- a/samples/bpf/tcp_bufs_kern.c
+++ b/samples/bpf/tcp_bufs_kern.c
@@ -9,7 +9,7 @@
  * doing appropriate checks that indicate the hosts are far enough
  * away (i.e. large RTT).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_clamp_kern.c b/samples/bpf/tcp_clamp_kern.c
index f4225c9d2c0c..a0dc2d254aca 100644
--- a/samples/bpf/tcp_clamp_kern.c
+++ b/samples/bpf/tcp_clamp_kern.c
@@ -9,7 +9,7 @@
  * the same datacenter. For his example, we assume they are within the same
  * datacenter when the first 5.5 bytes of their IPv6 addresses are the same.
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_cong_kern.c b/samples/bpf/tcp_cong_kern.c
index ad0f1ba8206a..4fd3ca979a06 100644
--- a/samples/bpf/tcp_cong_kern.c
+++ b/samples/bpf/tcp_cong_kern.c
@@ -7,7 +7,7 @@
  * BPF program to set congestion control to dctcp when both hosts are
  * in the same datacenter (as deteremined by IPv6 prefix).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_iw_kern.c b/samples/bpf/tcp_iw_kern.c
index 4ca5ecc9f580..9b139ec69560 100644
--- a/samples/bpf/tcp_iw_kern.c
+++ b/samples/bpf/tcp_iw_kern.c
@@ -9,7 +9,7 @@
  * would usually be done after doing appropriate checks that indicate
  * the hosts are far enough away (i.e. large RTT).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_rwnd_kern.c b/samples/bpf/tcp_rwnd_kern.c
index 09ff65b40b31..cc71ee96e044 100644
--- a/samples/bpf/tcp_rwnd_kern.c
+++ b/samples/bpf/tcp_rwnd_kern.c
@@ -8,7 +8,7 @@
  * and the first 5.5 bytes of the IPv6 addresses are not the same (in this
  * example that means both hosts are not the same datacenter).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_synrto_kern.c b/samples/bpf/tcp_synrto_kern.c
index 232bb242823e..ca87ed34f896 100644
--- a/samples/bpf/tcp_synrto_kern.c
+++ b/samples/bpf/tcp_synrto_kern.c
@@ -8,7 +8,7 @@
  * and the first 5.5 bytes of the IPv6 addresses are the same (in this example
  * that means both hosts are in the same datacenter).
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
diff --git a/samples/bpf/tcp_tos_reflect_kern.c b/samples/bpf/tcp_tos_reflect_kern.c
index d51dab19eca6..de788be6f862 100644
--- a/samples/bpf/tcp_tos_reflect_kern.c
+++ b/samples/bpf/tcp_tos_reflect_kern.c
@@ -4,7 +4,7 @@
  *
  * BPF program to automatically reflect TOS option from received syn packet
  *
- * Use load_sock_ops to load this BPF program.
+ * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
  */
 
 #include <uapi/linux/bpf.h>
-- 
2.19.2

[PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Jakub Kicinski]

For historical reasons the helper to loop over maps in an object
is called bpf_map__for_each while it really should be called
bpf_object__for_each_map.  Rename and add a correctly named
define for backward compatibility.

Switch all in-tree users to the correct name (Quentin).

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/bpf/bpftool/prog.c                       | 4 ++--
 tools/lib/bpf/libbpf.c                         | 8 ++++----
 tools/lib/bpf/libbpf.h                         | 3 ++-
 tools/perf/util/bpf-loader.c                   | 4 ++--
 tools/testing/selftests/bpf/test_libbpf_open.c | 2 +-
 5 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 0c35dd543d49..8ef80d65a474 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1053,7 +1053,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 	j = 0;
 	while (j < old_map_fds && map_replace[j].name) {
 		i = 0;
-		bpf_map__for_each(map, obj) {
+		bpf_object__for_each_map(map, obj) {
 			if (!strcmp(bpf_map__name(map), map_replace[j].name)) {
 				map_replace[j].idx = i;
 				break;
@@ -1074,7 +1074,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
 	/* Set ifindex and name reuse */
 	j = 0;
 	idx = 0;
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		if (!bpf_map__is_offload_neutral(map))
 			bpf_map__set_ifindex(map, ifindex);
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index b38dcbe7460a..f5eb60379c8d 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2100,7 +2100,7 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
 	if (err)
 		return err;
 
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		char buf[PATH_MAX];
 		int len;
 
@@ -2147,7 +2147,7 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
 	if (!obj)
 		return -ENOENT;
 
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		char buf[PATH_MAX];
 		int len;
 
@@ -2835,7 +2835,7 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
 {
 	struct bpf_map *pos;
 
-	bpf_map__for_each(pos, obj) {
+	bpf_object__for_each_map(pos, obj) {
 		if (pos->name && !strcmp(pos->name, name))
 			return pos;
 	}
@@ -2928,7 +2928,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 			first_prog = prog;
 	}
 
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		if (!bpf_map__is_offload_neutral(map))
 			map->map_ifindex = attr->ifindex;
 	}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 6c0168f8bba5..b4652aa1a58a 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
 
 LIBBPF_API struct bpf_map *
 bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
-#define bpf_map__for_each(pos, obj)		\
+#define bpf_object__for_each_map(pos, obj)		\
 	for ((pos) = bpf_map__next(NULL, (obj));	\
 	     (pos) != NULL;				\
 	     (pos) = bpf_map__next((pos), (obj)))
+#define bpf_map__for_each bpf_object__for_each_map
 
 LIBBPF_API struct bpf_map *
 bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 037d8ff6a634..31b7e5a1453b 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -1489,7 +1489,7 @@ apply_obj_config_object(struct bpf_object *obj)
 	struct bpf_map *map;
 	int err;
 
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		err = apply_obj_config_map(map);
 		if (err)
 			return err;
@@ -1513,7 +1513,7 @@ int bpf__apply_obj_config(void)
 
 #define bpf__for_each_map(pos, obj, objtmp)	\
 	bpf_object__for_each_safe(obj, objtmp)	\
-		bpf_map__for_each(pos, obj)
+		bpf_object__for_each_map(pos, obj)
 
 #define bpf__for_each_map_named(pos, obj, objtmp, name)	\
 	bpf__for_each_map(pos, obj, objtmp) 		\
diff --git a/tools/testing/selftests/bpf/test_libbpf_open.c b/tools/testing/selftests/bpf/test_libbpf_open.c
index 1909ecf4d999..65cbd30704b5 100644
--- a/tools/testing/selftests/bpf/test_libbpf_open.c
+++ b/tools/testing/selftests/bpf/test_libbpf_open.c
@@ -67,7 +67,7 @@ int test_walk_maps(struct bpf_object *obj, bool verbose)
 	struct bpf_map *map;
 	int cnt = 0;
 
-	bpf_map__for_each(map, obj) {
+	bpf_object__for_each_map(map, obj) {
 		cnt++;
 		if (verbose)
 			printf("Map (count:%d) name: %s\n", cnt,
-- 
2.19.2

[PATCH bpf-next 4/5] samples: bpf: use libbpf where easy

[Jakub Kicinski]

Some samples don't really need the magic of bpf_load,
switch them to libbpf.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 samples/bpf/Makefile       |  6 +++---
 samples/bpf/fds_example.c  |  9 ++++++---
 samples/bpf/sockex1_user.c | 22 ++++++++++++----------
 samples/bpf/sockex2_user.c | 20 +++++++++++---------
 4 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 4dd98100678e..0c62ac39c697 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -59,9 +59,9 @@ LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
 CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
 TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
 
-fds_example-objs := bpf_load.o fds_example.o
-sockex1-objs := bpf_load.o sockex1_user.o
-sockex2-objs := bpf_load.o sockex2_user.o
+fds_example-objs := fds_example.o
+sockex1-objs := sockex1_user.o
+sockex2-objs := sockex2_user.o
 sockex3-objs := bpf_load.o sockex3_user.o
 tracex1-objs := bpf_load.o tracex1_user.o
 tracex2-objs := bpf_load.o tracex2_user.o
diff --git a/samples/bpf/fds_example.c b/samples/bpf/fds_example.c
index 9854854f05d1..36f1f18aae3c 100644
--- a/samples/bpf/fds_example.c
+++ b/samples/bpf/fds_example.c
@@ -14,8 +14,8 @@
 
 #include <bpf/bpf.h>
 
+#include "bpf/libbpf.h"
 #include "bpf_insn.h"
-#include "bpf_load.h"
 #include "sock_example.h"
 
 #define BPF_F_PIN	(1 << 0)
@@ -57,10 +57,13 @@ static int bpf_prog_create(const char *object)
 		BPF_EXIT_INSN(),
 	};
 	size_t insns_cnt = sizeof(insns) / sizeof(struct bpf_insn);
+	char bpf_log_buf[BPF_LOG_BUF_SIZE];
+	struct bpf_object *obj;
+	int prog_fd;
 
 	if (object) {
-		assert(!load_bpf_file((char *)object));
-		return prog_fd[0];
+		assert(!bpf_prog_load(object, 0, &obj, &prog_fd));
+		return prog_fd;
 	} else {
 		return bpf_load_program(BPF_PROG_TYPE_SOCKET_FILTER,
 					insns, insns_cnt, "GPL", 0,
diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
index be8ba5686924..5e7c4be3e645 100644
--- a/samples/bpf/sockex1_user.c
+++ b/samples/bpf/sockex1_user.c
@@ -3,28 +3,30 @@
 #include <assert.h>
 #include <linux/bpf.h>
 #include <bpf/bpf.h>
-#include "bpf_load.h"
+#include "bpf/libbpf.h"
 #include "sock_example.h"
 #include <unistd.h>
 #include <arpa/inet.h>
 
 int main(int ac, char **argv)
 {
+	struct bpf_object *obj;
+	int map_fd, prog_fd;
 	char filename[256];
-	FILE *f;
 	int i, sock;
+	FILE *f;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load(filename, 0, &obj, &prog_fd))
 		return 1;
-	}
+
+	map_fd = bpf_object__find_map_fd_by_name(obj, "my_map");
 
 	sock = open_raw_sock("lo");
 
-	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
-			  sizeof(prog_fd[0])) == 0);
+	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
+			  sizeof(prog_fd)) == 0);
 
 	f = popen("ping -4 -c5 localhost", "r");
 	(void) f;
@@ -34,13 +36,13 @@ int main(int ac, char **argv)
 		int key;
 
 		key = IPPROTO_TCP;
-		assert(bpf_map_lookup_elem(map_fd[0], &key, &tcp_cnt) == 0);
+		assert(bpf_map_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
 
 		key = IPPROTO_UDP;
-		assert(bpf_map_lookup_elem(map_fd[0], &key, &udp_cnt) == 0);
+		assert(bpf_map_lookup_elem(map_fd, &key, &udp_cnt) == 0);
 
 		key = IPPROTO_ICMP;
-		assert(bpf_map_lookup_elem(map_fd[0], &key, &icmp_cnt) == 0);
+		assert(bpf_map_lookup_elem(map_fd, &key, &icmp_cnt) == 0);
 
 		printf("TCP %lld UDP %lld ICMP %lld bytes\n",
 		       tcp_cnt, udp_cnt, icmp_cnt);
diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
index 125ee6efc913..e3611dbfce97 100644
--- a/samples/bpf/sockex2_user.c
+++ b/samples/bpf/sockex2_user.c
@@ -3,7 +3,7 @@
 #include <assert.h>
 #include <linux/bpf.h>
 #include <bpf/bpf.h>
-#include "bpf_load.h"
+#include "bpf/libbpf.h"
 #include "sock_example.h"
 #include <unistd.h>
 #include <arpa/inet.h>
@@ -17,22 +17,24 @@ struct pair {
 int main(int ac, char **argv)
 {
 	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+	struct bpf_object *obj;
+	int map_fd, prog_fd;
 	char filename[256];
-	FILE *f;
 	int i, sock;
+	FILE *f;
 
 	snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
 	setrlimit(RLIMIT_MEMLOCK, &r);
 
-	if (load_bpf_file(filename)) {
-		printf("%s", bpf_log_buf);
+	if (bpf_prog_load(filename, 0, &obj, &prog_fd))
 		return 1;
-	}
+
+	map_fd = bpf_object__find_map_fd_by_name(obj, "hash_map");
 
 	sock = open_raw_sock("lo");
 
-	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
-			  sizeof(prog_fd[0])) == 0);
+	assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
+			  sizeof(prog_fd)) == 0);
 
 	f = popen("ping -4 -c5 localhost", "r");
 	(void) f;
@@ -41,8 +43,8 @@ int main(int ac, char **argv)
 		int key = 0, next_key;
 		struct pair value;
 
-		while (bpf_map_get_next_key(map_fd[0], &key, &next_key) == 0) {
-			bpf_map_lookup_elem(map_fd[0], &next_key, &value);
+		while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
+			bpf_map_lookup_elem(map_fd, &next_key, &value);
 			printf("ip %s bytes %lld packets %lld\n",
 			       inet_ntoa((struct in_addr){htonl(next_key)}),
 			       value.bytes, value.packets);
-- 
2.19.2

[PATCH bpf-next 5/5] tools: libbpf: make sure readelf shows full names in build checks

[Jakub Kicinski]

readelf truncates its output by default to attempt to make it more
readable.  This can lead to function names getting aliased if they
differ late in the string.  Use --wide parameter to avoid
truncation.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
 tools/lib/bpf/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index 761691bd72ad..a05c43468bd0 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -132,9 +132,9 @@ BPF_IN    := $(OUTPUT)libbpf-in.o
 LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
 VERSION_SCRIPT := libbpf.map
 
-GLOBAL_SYM_COUNT = $(shell readelf -s $(BPF_IN) | \
+GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \
 			   awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {s++} END{print s}')
-VERSIONED_SYM_COUNT = $(shell readelf -s $(OUTPUT)libbpf.so | \
+VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \
 			      grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
 
 CMD_TARGETS = $(LIB_FILE)
-- 
2.19.2

Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> For historical reasons the helper to loop over maps in an object
> is called bpf_map__for_each while it really should be called
> bpf_object__for_each_map.  Rename and add a correctly named
> define for backward compatibility.

Seems like there are at least 3 more functions that are not named correctly:
- __bpf_map__iter (__bpf_object__iter_map?)
- bpf_map__next (=> bpf_object__next_map?)
- bpf_map__prev (=> bpf_object__prev_map?)

Let's rename them as well?


>
> Switch all in-tree users to the correct name (Quentin).
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  tools/bpf/bpftool/prog.c                       | 4 ++--
>  tools/lib/bpf/libbpf.c                         | 8 ++++----
>  tools/lib/bpf/libbpf.h                         | 3 ++-
>  tools/perf/util/bpf-loader.c                   | 4 ++--
>  tools/testing/selftests/bpf/test_libbpf_open.c | 2 +-
>  5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 0c35dd543d49..8ef80d65a474 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1053,7 +1053,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
>         j = 0;
>         while (j < old_map_fds && map_replace[j].name) {
>                 i = 0;
> -               bpf_map__for_each(map, obj) {
> +               bpf_object__for_each_map(map, obj) {
>                         if (!strcmp(bpf_map__name(map), map_replace[j].name)) {
>                                 map_replace[j].idx = i;
>                                 break;
> @@ -1074,7 +1074,7 @@ static int load_with_options(int argc, char **argv, bool first_prog_only)
>         /* Set ifindex and name reuse */
>         j = 0;
>         idx = 0;
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 if (!bpf_map__is_offload_neutral(map))
>                         bpf_map__set_ifindex(map, ifindex);
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b38dcbe7460a..f5eb60379c8d 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -2100,7 +2100,7 @@ int bpf_object__pin_maps(struct bpf_object *obj, const char *path)
>         if (err)
>                 return err;
>
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 char buf[PATH_MAX];
>                 int len;
>
> @@ -2147,7 +2147,7 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
>         if (!obj)
>                 return -ENOENT;
>
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 char buf[PATH_MAX];
>                 int len;
>
> @@ -2835,7 +2835,7 @@ bpf_object__find_map_by_name(struct bpf_object *obj, const char *name)
>  {
>         struct bpf_map *pos;
>
> -       bpf_map__for_each(pos, obj) {
> +       bpf_object__for_each_map(pos, obj) {
>                 if (pos->name && !strcmp(pos->name, name))
>                         return pos;
>         }
> @@ -2928,7 +2928,7 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>                         first_prog = prog;
>         }
>
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 if (!bpf_map__is_offload_neutral(map))
>                         map->map_ifindex = attr->ifindex;
>         }
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 6c0168f8bba5..b4652aa1a58a 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
>
>  LIBBPF_API struct bpf_map *
>  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> -#define bpf_map__for_each(pos, obj)            \
> +#define bpf_object__for_each_map(pos, obj)             \
>         for ((pos) = bpf_map__next(NULL, (obj));        \
>              (pos) != NULL;                             \
>              (pos) = bpf_map__next((pos), (obj)))
> +#define bpf_map__for_each bpf_object__for_each_map

Should we get rid of this as well, instead of accumulating cruft?

>
>  LIBBPF_API struct bpf_map *
>  bpf_map__prev(struct bpf_map *map, struct bpf_object *obj);
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 037d8ff6a634..31b7e5a1453b 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -1489,7 +1489,7 @@ apply_obj_config_object(struct bpf_object *obj)
>         struct bpf_map *map;
>         int err;
>
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 err = apply_obj_config_map(map);
>                 if (err)
>                         return err;
> @@ -1513,7 +1513,7 @@ int bpf__apply_obj_config(void)
>
>  #define bpf__for_each_map(pos, obj, objtmp)    \
>         bpf_object__for_each_safe(obj, objtmp)  \
> -               bpf_map__for_each(pos, obj)
> +               bpf_object__for_each_map(pos, obj)
>
>  #define bpf__for_each_map_named(pos, obj, objtmp, name)        \
>         bpf__for_each_map(pos, obj, objtmp)             \
> diff --git a/tools/testing/selftests/bpf/test_libbpf_open.c b/tools/testing/selftests/bpf/test_libbpf_open.c
> index 1909ecf4d999..65cbd30704b5 100644
> --- a/tools/testing/selftests/bpf/test_libbpf_open.c
> +++ b/tools/testing/selftests/bpf/test_libbpf_open.c
> @@ -67,7 +67,7 @@ int test_walk_maps(struct bpf_object *obj, bool verbose)
>         struct bpf_map *map;
>         int cnt = 0;
>
> -       bpf_map__for_each(map, obj) {
> +       bpf_object__for_each_map(map, obj) {
>                 cnt++;
>                 if (verbose)
>                         printf("Map (count:%d) name: %s\n", cnt,
> --
> 2.19.2
>

Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Jakub Kicinski]

On Wed, 27 Feb 2019 15:47:56 -0800, Andrii Nakryiko wrote:
> On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
> <jakub.kicinski@netronome.com> wrote:
> >
> > For historical reasons the helper to loop over maps in an object
> > is called bpf_map__for_each while it really should be called
> > bpf_object__for_each_map.  Rename and add a correctly named
> > define for backward compatibility.  
> 
> Seems like there are at least 3 more functions that are not named correctly:
> - __bpf_map__iter (__bpf_object__iter_map?)
> - bpf_map__next (=> bpf_object__next_map?)
> - bpf_map__prev (=> bpf_object__prev_map?)
> 
> Let's rename them as well?

At least those are consistently named between programs and maps.
I'm happy to do the rename if we don't need backward compat, seems 
a little much to add aliases?

> > Switch all in-tree users to the correct name (Quentin).
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 6c0168f8bba5..b4652aa1a58a 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
> >
> >  LIBBPF_API struct bpf_map *
> >  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> > -#define bpf_map__for_each(pos, obj)            \
> > +#define bpf_object__for_each_map(pos, obj)             \
> >         for ((pos) = bpf_map__next(NULL, (obj));        \
> >              (pos) != NULL;                             \
> >              (pos) = bpf_map__next((pos), (obj)))
> > +#define bpf_map__for_each bpf_object__for_each_map  
> 
> Should we get rid of this as well, instead of accumulating cruft?

Well, we did some gymnastics in the past to maintain backward compat, 
I thought we do need it..?

Re: [PATCH bpf-next 4/5] samples: bpf: use libbpf where easy

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> Some samples don't really need the magic of bpf_load,
> switch them to libbpf.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  samples/bpf/Makefile       |  6 +++---
>  samples/bpf/fds_example.c  |  9 ++++++---
>  samples/bpf/sockex1_user.c | 22 ++++++++++++----------
>  samples/bpf/sockex2_user.c | 20 +++++++++++---------
>  4 files changed, 32 insertions(+), 25 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 4dd98100678e..0c62ac39c697 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -59,9 +59,9 @@ LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
>  CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
>  TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
>
> -fds_example-objs := bpf_load.o fds_example.o
> -sockex1-objs := bpf_load.o sockex1_user.o
> -sockex2-objs := bpf_load.o sockex2_user.o
> +fds_example-objs := fds_example.o
> +sockex1-objs := sockex1_user.o
> +sockex2-objs := sockex2_user.o
>  sockex3-objs := bpf_load.o sockex3_user.o
>  tracex1-objs := bpf_load.o tracex1_user.o
>  tracex2-objs := bpf_load.o tracex2_user.o
> diff --git a/samples/bpf/fds_example.c b/samples/bpf/fds_example.c
> index 9854854f05d1..36f1f18aae3c 100644
> --- a/samples/bpf/fds_example.c
> +++ b/samples/bpf/fds_example.c
> @@ -14,8 +14,8 @@
>
>  #include <bpf/bpf.h>
>
> +#include "bpf/libbpf.h"
>  #include "bpf_insn.h"
> -#include "bpf_load.h"
>  #include "sock_example.h"
>
>  #define BPF_F_PIN      (1 << 0)
> @@ -57,10 +57,13 @@ static int bpf_prog_create(const char *object)
>                 BPF_EXIT_INSN(),
>         };
>         size_t insns_cnt = sizeof(insns) / sizeof(struct bpf_insn);
> +       char bpf_log_buf[BPF_LOG_BUF_SIZE];
> +       struct bpf_object *obj;
> +       int prog_fd;
>
>         if (object) {
> -               assert(!load_bpf_file((char *)object));
> -               return prog_fd[0];
> +               assert(!bpf_prog_load(object, 0, &obj, &prog_fd));

Here and in few more places below: is it possible to specify correct
bpf_prog_type instead of 0?

> +               return prog_fd;
>         } else {
>                 return bpf_load_program(BPF_PROG_TYPE_SOCKET_FILTER,
>                                         insns, insns_cnt, "GPL", 0,
> diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
> index be8ba5686924..5e7c4be3e645 100644
> --- a/samples/bpf/sockex1_user.c
> +++ b/samples/bpf/sockex1_user.c
> @@ -3,28 +3,30 @@
>  #include <assert.h>
>  #include <linux/bpf.h>
>  #include <bpf/bpf.h>
> -#include "bpf_load.h"
> +#include "bpf/libbpf.h"
>  #include "sock_example.h"
>  #include <unistd.h>
>  #include <arpa/inet.h>
>
>  int main(int ac, char **argv)
>  {
> +       struct bpf_object *obj;
> +       int map_fd, prog_fd;
>         char filename[256];
> -       FILE *f;
>         int i, sock;
> +       FILE *f;
>
>         snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
>
> -       if (load_bpf_file(filename)) {
> -               printf("%s", bpf_log_buf);
> +       if (bpf_prog_load(filename, 0, &obj, &prog_fd))
>                 return 1;
> -       }
> +
> +       map_fd = bpf_object__find_map_fd_by_name(obj, "my_map");
>
>         sock = open_raw_sock("lo");
>
> -       assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
> -                         sizeof(prog_fd[0])) == 0);
> +       assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
> +                         sizeof(prog_fd)) == 0);
>
>         f = popen("ping -4 -c5 localhost", "r");
>         (void) f;
> @@ -34,13 +36,13 @@ int main(int ac, char **argv)
>                 int key;
>
>                 key = IPPROTO_TCP;
> -               assert(bpf_map_lookup_elem(map_fd[0], &key, &tcp_cnt) == 0);
> +               assert(bpf_map_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
>
>                 key = IPPROTO_UDP;
> -               assert(bpf_map_lookup_elem(map_fd[0], &key, &udp_cnt) == 0);
> +               assert(bpf_map_lookup_elem(map_fd, &key, &udp_cnt) == 0);
>
>                 key = IPPROTO_ICMP;
> -               assert(bpf_map_lookup_elem(map_fd[0], &key, &icmp_cnt) == 0);
> +               assert(bpf_map_lookup_elem(map_fd, &key, &icmp_cnt) == 0);
>
>                 printf("TCP %lld UDP %lld ICMP %lld bytes\n",
>                        tcp_cnt, udp_cnt, icmp_cnt);
> diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
> index 125ee6efc913..e3611dbfce97 100644
> --- a/samples/bpf/sockex2_user.c
> +++ b/samples/bpf/sockex2_user.c
> @@ -3,7 +3,7 @@
>  #include <assert.h>
>  #include <linux/bpf.h>
>  #include <bpf/bpf.h>
> -#include "bpf_load.h"
> +#include "bpf/libbpf.h"
>  #include "sock_example.h"
>  #include <unistd.h>
>  #include <arpa/inet.h>
> @@ -17,22 +17,24 @@ struct pair {
>  int main(int ac, char **argv)
>  {
>         struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> +       struct bpf_object *obj;
> +       int map_fd, prog_fd;
>         char filename[256];
> -       FILE *f;
>         int i, sock;
> +       FILE *f;
>
>         snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
>         setrlimit(RLIMIT_MEMLOCK, &r);
>
> -       if (load_bpf_file(filename)) {
> -               printf("%s", bpf_log_buf);
> +       if (bpf_prog_load(filename, 0, &obj, &prog_fd))
>                 return 1;
> -       }
> +
> +       map_fd = bpf_object__find_map_fd_by_name(obj, "hash_map");
>
>         sock = open_raw_sock("lo");
>
> -       assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
> -                         sizeof(prog_fd[0])) == 0);
> +       assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
> +                         sizeof(prog_fd)) == 0);
>
>         f = popen("ping -4 -c5 localhost", "r");
>         (void) f;
> @@ -41,8 +43,8 @@ int main(int ac, char **argv)
>                 int key = 0, next_key;
>                 struct pair value;
>
> -               while (bpf_map_get_next_key(map_fd[0], &key, &next_key) == 0) {
> -                       bpf_map_lookup_elem(map_fd[0], &next_key, &value);
> +               while (bpf_map_get_next_key(map_fd, &key, &next_key) == 0) {
> +                       bpf_map_lookup_elem(map_fd, &next_key, &value);
>                         printf("ip %s bytes %lld packets %lld\n",
>                                inet_ntoa((struct in_addr){htonl(next_key)}),
>                                value.bytes, value.packets);
> --
> 2.19.2
>

Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:57 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Wed, 27 Feb 2019 15:47:56 -0800, Andrii Nakryiko wrote:
> > On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
> > <jakub.kicinski@netronome.com> wrote:
> > >
> > > For historical reasons the helper to loop over maps in an object
> > > is called bpf_map__for_each while it really should be called
> > > bpf_object__for_each_map.  Rename and add a correctly named
> > > define for backward compatibility.
> >
> > Seems like there are at least 3 more functions that are not named correctly:
> > - __bpf_map__iter (__bpf_object__iter_map?)
> > - bpf_map__next (=> bpf_object__next_map?)
> > - bpf_map__prev (=> bpf_object__prev_map?)
> >
> > Let's rename them as well?
>
> At least those are consistently named between programs and maps.
> I'm happy to do the rename if we don't need backward compat, seems
> a little much to add aliases?
>
> > > Switch all in-tree users to the correct name (Quentin).
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
>
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index 6c0168f8bba5..b4652aa1a58a 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
> > >
> > >  LIBBPF_API struct bpf_map *
> > >  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> > > -#define bpf_map__for_each(pos, obj)            \
> > > +#define bpf_object__for_each_map(pos, obj)             \
> > >         for ((pos) = bpf_map__next(NULL, (obj));        \
> > >              (pos) != NULL;                             \
> > >              (pos) = bpf_map__next((pos), (obj)))
> > > +#define bpf_map__for_each bpf_object__for_each_map
> >
> > Should we get rid of this as well, instead of accumulating cruft?
>
> Well, we did some gymnastics in the past to maintain backward compat,
> I thought we do need it..?

I'll let others chime in, but, imo, it feels like while we are at
0.0.2 it might be worthwhile to streamline libbpf early on to reduce
confusion later at the expense of early adopters having to do
straightforward conversion right now.

Re: [PATCH bpf-next 5/5] tools: libbpf: make sure readelf shows full names in build checks

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> readelf truncates its output by default to attempt to make it more
> readable.  This can lead to function names getting aliased if they
> differ late in the string.  Use --wide parameter to avoid
> truncation.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> ---
>  tools/lib/bpf/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 761691bd72ad..a05c43468bd0 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -132,9 +132,9 @@ BPF_IN    := $(OUTPUT)libbpf-in.o
>  LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
>  VERSION_SCRIPT := libbpf.map
>
> -GLOBAL_SYM_COUNT = $(shell readelf -s $(BPF_IN) | \
> +GLOBAL_SYM_COUNT = $(shell readelf -s --wide $(BPF_IN) | \
>                            awk '/GLOBAL/ && /DEFAULT/ && !/UND/ {s++} END{print s}')
> -VERSIONED_SYM_COUNT = $(shell readelf -s $(OUTPUT)libbpf.so | \
> +VERSIONED_SYM_COUNT = $(shell readelf -s --wide $(OUTPUT)libbpf.so | \
>                               grep -Eo '[^ ]+@LIBBPF_' | cut -d@ -f1 | sort -u | wc -l)
>
>  CMD_TARGETS = $(LIB_FILE)
> --
> 2.19.2
>

Looks good.

Acked-by: Andrii Nakryiko <andriin@fb.com>

Re: [PATCH bpf-next 1/5] samples: bpf: force IPv4 in ping

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> ping localhost may default of IPv6 on modern systems, but
> samples are trying to only parse IPv4.  Force IPv4.
>
> samples/bpf/tracex1_user.c doesn't interpret the packet so
> we don't care which IP version will be used there.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Acked-by: Andrii Nakryiko <andriin@fb.com>

> ---
>  samples/bpf/sock_example.c | 2 +-
>  samples/bpf/sockex1_user.c | 2 +-
>  samples/bpf/sockex2_user.c | 2 +-
>  samples/bpf/sockex3_user.c | 2 +-
>  samples/bpf/tracex2_user.c | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/samples/bpf/sock_example.c b/samples/bpf/sock_example.c
> index 60ec467c78ab..00aae1d33fca 100644
> --- a/samples/bpf/sock_example.c
> +++ b/samples/bpf/sock_example.c
> @@ -99,7 +99,7 @@ int main(void)
>  {
>         FILE *f;
>
> -       f = popen("ping -c5 localhost", "r");
> +       f = popen("ping -4 -c5 localhost", "r");
>         (void)f;
>
>         return test_sock();
> diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
> index 93ec01c56104..be8ba5686924 100644
> --- a/samples/bpf/sockex1_user.c
> +++ b/samples/bpf/sockex1_user.c
> @@ -26,7 +26,7 @@ int main(int ac, char **argv)
>         assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
>                           sizeof(prog_fd[0])) == 0);
>
> -       f = popen("ping -c5 localhost", "r");
> +       f = popen("ping -4 -c5 localhost", "r");
>         (void) f;
>
>         for (i = 0; i < 5; i++) {
> diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
> index 1d5c6e9a6d27..125ee6efc913 100644
> --- a/samples/bpf/sockex2_user.c
> +++ b/samples/bpf/sockex2_user.c
> @@ -34,7 +34,7 @@ int main(int ac, char **argv)
>         assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
>                           sizeof(prog_fd[0])) == 0);
>
> -       f = popen("ping -c5 localhost", "r");
> +       f = popen("ping -4 -c5 localhost", "r");
>         (void) f;
>
>         for (i = 0; i < 5; i++) {
> diff --git a/samples/bpf/sockex3_user.c b/samples/bpf/sockex3_user.c
> index 9d02e0404719..bbb1cd0666a9 100644
> --- a/samples/bpf/sockex3_user.c
> +++ b/samples/bpf/sockex3_user.c
> @@ -58,7 +58,7 @@ int main(int argc, char **argv)
>                           sizeof(__u32)) == 0);
>
>         if (argc > 1)
> -               f = popen("ping -c5 localhost", "r");
> +               f = popen("ping -4 -c5 localhost", "r");
>         else
>                 f = popen("netperf -l 4 localhost", "r");
>         (void) f;
> diff --git a/samples/bpf/tracex2_user.c b/samples/bpf/tracex2_user.c
> index 1a81e6a5c2ea..c9544a4ce61a 100644
> --- a/samples/bpf/tracex2_user.c
> +++ b/samples/bpf/tracex2_user.c
> @@ -131,7 +131,7 @@ int main(int ac, char **argv)
>         signal(SIGTERM, int_exit);
>
>         /* start 'ping' in the background to have some kfree_skb events */
> -       f = popen("ping -c5 localhost", "r");
> +       f = popen("ping -4 -c5 localhost", "r");
>         (void) f;
>
>         /* start 'dd' in the background to have plenty of 'write' syscalls */
> --
> 2.19.2
>

Re: [PATCH bpf-next 2/5] samples: bpf: remove load_sock_ops in favour of bpftool

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> bpftool can do all the things load_sock_ops used to do, and more.
> Point users to bpftool instead of maintaining this sample utility.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>

> ---
>  samples/bpf/.gitignore             |  1 -
>  samples/bpf/Makefile               |  2 -
>  samples/bpf/load_sock_ops.c        | 97 ------------------------------
>  samples/bpf/tcp_basertt_kern.c     |  2 +-
>  samples/bpf/tcp_bpf.readme         | 14 +++--
>  samples/bpf/tcp_bufs_kern.c        |  2 +-
>  samples/bpf/tcp_clamp_kern.c       |  2 +-
>  samples/bpf/tcp_cong_kern.c        |  2 +-
>  samples/bpf/tcp_iw_kern.c          |  2 +-
>  samples/bpf/tcp_rwnd_kern.c        |  2 +-
>  samples/bpf/tcp_synrto_kern.c      |  2 +-
>  samples/bpf/tcp_tos_reflect_kern.c |  2 +-
>  12 files changed, 16 insertions(+), 114 deletions(-)
>  delete mode 100644 samples/bpf/load_sock_ops.c
>
> diff --git a/samples/bpf/.gitignore b/samples/bpf/.gitignore
> index 8ae4940025f8..dbb817dbacfc 100644
> --- a/samples/bpf/.gitignore
> +++ b/samples/bpf/.gitignore
> @@ -1,7 +1,6 @@
>  cpustat
>  fds_example
>  lathist
> -load_sock_ops
>  lwt_len_hist
>  map_perf_test
>  offwaketime
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index a333e258f319..4dd98100678e 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -40,7 +40,6 @@ hostprogs-y += lwt_len_hist
>  hostprogs-y += xdp_tx_iptunnel
>  hostprogs-y += test_map_in_map
>  hostprogs-y += per_socket_stats_example
> -hostprogs-y += load_sock_ops
>  hostprogs-y += xdp_redirect
>  hostprogs-y += xdp_redirect_map
>  hostprogs-y += xdp_redirect_cpu
> @@ -71,7 +70,6 @@ tracex4-objs := bpf_load.o tracex4_user.o
>  tracex5-objs := bpf_load.o tracex5_user.o
>  tracex6-objs := bpf_load.o tracex6_user.o
>  tracex7-objs := bpf_load.o tracex7_user.o
> -load_sock_ops-objs := bpf_load.o load_sock_ops.o
>  test_probe_write_user-objs := bpf_load.o test_probe_write_user_user.o
>  trace_output-objs := bpf_load.o trace_output_user.o $(TRACE_HELPERS)
>  lathist-objs := bpf_load.o lathist_user.o
> diff --git a/samples/bpf/load_sock_ops.c b/samples/bpf/load_sock_ops.c
> deleted file mode 100644
> index 8ecb41ea0c03..000000000000
> --- a/samples/bpf/load_sock_ops.c
> +++ /dev/null
> @@ -1,97 +0,0 @@
> -/* Copyright (c) 2017 Facebook
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of version 2 of the GNU General Public
> - * License as published by the Free Software Foundation.
> - */
> -#include <stdio.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <linux/bpf.h>
> -#include <bpf/bpf.h>
> -#include "bpf_load.h"
> -#include <unistd.h>
> -#include <errno.h>
> -#include <fcntl.h>
> -#include <linux/unistd.h>
> -
> -static void usage(char *pname)
> -{
> -       printf("USAGE:\n  %s [-l] <cg-path> <prog filename>\n", pname);
> -       printf("\tLoad and attach a sock_ops program to the specified "
> -              "cgroup\n");
> -       printf("\tIf \"-l\" is used, the program will continue to run\n");
> -       printf("\tprinting the BPF log buffer\n");
> -       printf("\tIf the specified filename does not end in \".o\", it\n");
> -       printf("\tappends \"_kern.o\" to the name\n");
> -       printf("\n");
> -       printf("  %s -r <cg-path>\n", pname);
> -       printf("\tDetaches the currently attached sock_ops program\n");
> -       printf("\tfrom the specified cgroup\n");
> -       printf("\n");
> -       exit(1);
> -}
> -
> -int main(int argc, char **argv)
> -{
> -       int logFlag = 0;
> -       int error = 0;
> -       char *cg_path;
> -       char fn[500];
> -       char *prog;
> -       int cg_fd;
> -
> -       if (argc < 3)
> -               usage(argv[0]);
> -
> -       if (!strcmp(argv[1], "-r")) {
> -               cg_path = argv[2];
> -               cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
> -               error = bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS);
> -               if (error) {
> -                       printf("ERROR: bpf_prog_detach: %d (%s)\n",
> -                              error, strerror(errno));
> -                       return 2;
> -               }
> -               return 0;
> -       } else if (!strcmp(argv[1], "-h")) {
> -               usage(argv[0]);
> -       } else if (!strcmp(argv[1], "-l")) {
> -               logFlag = 1;
> -               if (argc < 4)
> -                       usage(argv[0]);
> -       }
> -
> -       prog = argv[argc - 1];
> -       cg_path = argv[argc - 2];
> -       if (strlen(prog) > 480) {
> -               fprintf(stderr, "ERROR: program name too long (> 480 chars)\n");
> -               return 3;
> -       }
> -       cg_fd = open(cg_path, O_DIRECTORY, O_RDONLY);
> -
> -       if (!strcmp(prog + strlen(prog)-2, ".o"))
> -               strcpy(fn, prog);
> -       else
> -               sprintf(fn, "%s_kern.o", prog);
> -       if (logFlag)
> -               printf("loading bpf file:%s\n", fn);
> -       if (load_bpf_file(fn)) {
> -               printf("ERROR: load_bpf_file failed for: %s\n", fn);
> -               printf("%s", bpf_log_buf);
> -               return 4;
> -       }
> -       if (logFlag)
> -               printf("TCP BPF Loaded %s\n", fn);
> -
> -       error = bpf_prog_attach(prog_fd[0], cg_fd, BPF_CGROUP_SOCK_OPS, 0);
> -       if (error) {
> -               printf("ERROR: bpf_prog_attach: %d (%s)\n",
> -                      error, strerror(errno));
> -               return 5;
> -       } else if (logFlag) {
> -               read_trace_pipe();
> -       }
> -
> -       return error;
> -}
> diff --git a/samples/bpf/tcp_basertt_kern.c b/samples/bpf/tcp_basertt_kern.c
> index 4bf4fc597db9..6ef1625e8b2c 100644
> --- a/samples/bpf/tcp_basertt_kern.c
> +++ b/samples/bpf/tcp_basertt_kern.c
> @@ -7,7 +7,7 @@
>   * BPF program to set base_rtt to 80us when host is running TCP-NV and
>   * both hosts are in the same datacenter (as determined by IPv6 prefix).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_bpf.readme b/samples/bpf/tcp_bpf.readme
> index 831fb601e3c9..fee746621aec 100644
> --- a/samples/bpf/tcp_bpf.readme
> +++ b/samples/bpf/tcp_bpf.readme
> @@ -8,14 +8,16 @@ a cgroupv2 and attach a bash shell to the group.
>    bash
>    echo $$ >> /tmp/cgroupv2/foo/cgroup.procs
>
> -Anything that runs under this shell belongs to the foo cgroupv2 To load
> +Anything that runs under this shell belongs to the foo cgroupv2. To load
>  (attach) one of the tcp_*_kern.o programs:
>
> -  ./load_sock_ops -l /tmp/cgroupv2/foo tcp_basertt_kern.o
> +  bpftool prog load tcp_basertt_kern.o /sys/fs/bpf/tcp_prog
> +  bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
> +  bpftool prog tracelog
>
> -If the "-l" flag is used, the load_sock_ops program will continue to run
> -printing the BPF log buffer. The tcp_*_kern.o programs use special print
> -functions to print logging information (if enabled by the ifdef).
> +"bpftool prog tracelog" will continue to run printing the BPF log buffer.
> +The tcp_*_kern.o programs use special print functions to print logging
> +information (if enabled by the ifdef).
>
>  If using netperf/netserver to create traffic, you need to run them under the
>  cgroupv2 to which the BPF programs are attached (i.e. under bash shell
> @@ -23,4 +25,4 @@ attached to the cgroupv2).
>
>  To remove (unattach) a socket_ops BPF program from a cgroupv2:
>
> -  ./load_sock_ops -r /tmp/cgroupv2/foo
> +  bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
> diff --git a/samples/bpf/tcp_bufs_kern.c b/samples/bpf/tcp_bufs_kern.c
> index 0566b7fa38a1..e03e204739fa 100644
> --- a/samples/bpf/tcp_bufs_kern.c
> +++ b/samples/bpf/tcp_bufs_kern.c
> @@ -9,7 +9,7 @@
>   * doing appropriate checks that indicate the hosts are far enough
>   * away (i.e. large RTT).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_clamp_kern.c b/samples/bpf/tcp_clamp_kern.c
> index f4225c9d2c0c..a0dc2d254aca 100644
> --- a/samples/bpf/tcp_clamp_kern.c
> +++ b/samples/bpf/tcp_clamp_kern.c
> @@ -9,7 +9,7 @@
>   * the same datacenter. For his example, we assume they are within the same
>   * datacenter when the first 5.5 bytes of their IPv6 addresses are the same.
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_cong_kern.c b/samples/bpf/tcp_cong_kern.c
> index ad0f1ba8206a..4fd3ca979a06 100644
> --- a/samples/bpf/tcp_cong_kern.c
> +++ b/samples/bpf/tcp_cong_kern.c
> @@ -7,7 +7,7 @@
>   * BPF program to set congestion control to dctcp when both hosts are
>   * in the same datacenter (as deteremined by IPv6 prefix).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_iw_kern.c b/samples/bpf/tcp_iw_kern.c
> index 4ca5ecc9f580..9b139ec69560 100644
> --- a/samples/bpf/tcp_iw_kern.c
> +++ b/samples/bpf/tcp_iw_kern.c
> @@ -9,7 +9,7 @@
>   * would usually be done after doing appropriate checks that indicate
>   * the hosts are far enough away (i.e. large RTT).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_rwnd_kern.c b/samples/bpf/tcp_rwnd_kern.c
> index 09ff65b40b31..cc71ee96e044 100644
> --- a/samples/bpf/tcp_rwnd_kern.c
> +++ b/samples/bpf/tcp_rwnd_kern.c
> @@ -8,7 +8,7 @@
>   * and the first 5.5 bytes of the IPv6 addresses are not the same (in this
>   * example that means both hosts are not the same datacenter).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_synrto_kern.c b/samples/bpf/tcp_synrto_kern.c
> index 232bb242823e..ca87ed34f896 100644
> --- a/samples/bpf/tcp_synrto_kern.c
> +++ b/samples/bpf/tcp_synrto_kern.c
> @@ -8,7 +8,7 @@
>   * and the first 5.5 bytes of the IPv6 addresses are the same (in this example
>   * that means both hosts are in the same datacenter).
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> diff --git a/samples/bpf/tcp_tos_reflect_kern.c b/samples/bpf/tcp_tos_reflect_kern.c
> index d51dab19eca6..de788be6f862 100644
> --- a/samples/bpf/tcp_tos_reflect_kern.c
> +++ b/samples/bpf/tcp_tos_reflect_kern.c
> @@ -4,7 +4,7 @@
>   *
>   * BPF program to automatically reflect TOS option from received syn packet
>   *
> - * Use load_sock_ops to load this BPF program.
> + * Use "bpftool cgroup attach $cg sock_ops $prog" to load this BPF program.
>   */
>
>  #include <uapi/linux/bpf.h>
> --
> 2.19.2
>

Re: [PATCH bpf-next 4/5] samples: bpf: use libbpf where easy

[Jakub Kicinski]

On Wed, 27 Feb 2019 16:05:45 -0800, Andrii Nakryiko wrote:
> >         if (object) {
> > -               assert(!load_bpf_file((char *)object));
> > -               return prog_fd[0];
> > +               assert(!bpf_prog_load(object, 0, &obj, &prog_fd));  
> 
> Here and in few more places below: is it possible to specify correct
> bpf_prog_type instead of 0?

Some of them yes, some of them no.  There are objects with multiple
program types IIRC.  This program type argument is only used for
checking if kernel versions are needed, and they are provided in
samples so it doesn't matter.

I'll try to add where possible.

Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Alexei Starovoitov]

On Wed, Feb 27, 2019 at 03:57:03PM -0800, Jakub Kicinski wrote:
> On Wed, 27 Feb 2019 15:47:56 -0800, Andrii Nakryiko wrote:
> > On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
> > <jakub.kicinski@netronome.com> wrote:
> > >
> > > For historical reasons the helper to loop over maps in an object
> > > is called bpf_map__for_each while it really should be called
> > > bpf_object__for_each_map.  Rename and add a correctly named
> > > define for backward compatibility.  
> > 
> > Seems like there are at least 3 more functions that are not named correctly:
> > - __bpf_map__iter (__bpf_object__iter_map?)
> > - bpf_map__next (=> bpf_object__next_map?)
> > - bpf_map__prev (=> bpf_object__prev_map?)
> > 
> > Let's rename them as well?
> 
> At least those are consistently named between programs and maps.

I think this patch makes naming consistent enough.

> I'm happy to do the rename if we don't need backward compat, seems 
> a little much to add aliases?
> 
> > > Switch all in-tree users to the correct name (Quentin).
> > >
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> 
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index 6c0168f8bba5..b4652aa1a58a 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
> > >
> > >  LIBBPF_API struct bpf_map *
> > >  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> > > -#define bpf_map__for_each(pos, obj)            \
> > > +#define bpf_object__for_each_map(pos, obj)             \
> > >         for ((pos) = bpf_map__next(NULL, (obj));        \
> > >              (pos) != NULL;                             \
> > >              (pos) = bpf_map__next((pos), (obj)))
> > > +#define bpf_map__for_each bpf_object__for_each_map  
> > 
> > Should we get rid of this as well, instead of accumulating cruft?
> 
> Well, we did some gymnastics in the past to maintain backward compat, 
> I thought we do need it..?

We do need to keep backward compat.
This line is necessary.

imo this set looks good to me as-is.

Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration

[Andrii Nakryiko]

On Wed, Feb 27, 2019 at 6:47 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Feb 27, 2019 at 03:57:03PM -0800, Jakub Kicinski wrote:
> > On Wed, 27 Feb 2019 15:47:56 -0800, Andrii Nakryiko wrote:
> > > On Wed, Feb 27, 2019 at 3:31 PM Jakub Kicinski
> > > <jakub.kicinski@netronome.com> wrote:
> > > >
> > > > For historical reasons the helper to loop over maps in an object
> > > > is called bpf_map__for_each while it really should be called
> > > > bpf_object__for_each_map.  Rename and add a correctly named
> > > > define for backward compatibility.
> > >
> > > Seems like there are at least 3 more functions that are not named correctly:
> > > - __bpf_map__iter (__bpf_object__iter_map?)
> > > - bpf_map__next (=> bpf_object__next_map?)
> > > - bpf_map__prev (=> bpf_object__prev_map?)
> > >
> > > Let's rename them as well?
> >
> > At least those are consistently named between programs and maps.
>
> I think this patch makes naming consistent enough.
>
> > I'm happy to do the rename if we don't need backward compat, seems
> > a little much to add aliases?
> >
> > > > Switch all in-tree users to the correct name (Quentin).
> > > >
> > > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > > Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
> >
> > > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > > index 6c0168f8bba5..b4652aa1a58a 100644
> > > > --- a/tools/lib/bpf/libbpf.h
> > > > +++ b/tools/lib/bpf/libbpf.h
> > > > @@ -278,10 +278,11 @@ bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset);
> > > >
> > > >  LIBBPF_API struct bpf_map *
> > > >  bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
> > > > -#define bpf_map__for_each(pos, obj)            \
> > > > +#define bpf_object__for_each_map(pos, obj)             \
> > > >         for ((pos) = bpf_map__next(NULL, (obj));        \
> > > >              (pos) != NULL;                             \
> > > >              (pos) = bpf_map__next((pos), (obj)))
> > > > +#define bpf_map__for_each bpf_object__for_each_map
> > >
> > > Should we get rid of this as well, instead of accumulating cruft?
> >
> > Well, we did some gymnastics in the past to maintain backward compat,
> > I thought we do need it..?
>
> We do need to keep backward compat.
> This line is necessary.
>
> imo this set looks good to me as-is.
>

bpf_map__next/prev and bpf_program__next/prev convention feels a bit
weird, they fill more like methods of bpf_object rather than methods
of bpf_map and bpf_program, but I can understand why we might want to
preserve them.

Would it make sense to still add bpf_object__{next,prev}_{map,program}
(with struct bpf_object as a first arg) and just call them from
bpf_{map,program}__{next,prev}? That way we'll have consistent naming
that follows libbpf's own naming guidelines and we'll preserve
backwards compatibility? We can do that in follow up patches, if at
all, so I'll ack this patch.


Acked-by: Andrii Nakryiko <andriin@fb.com>