* Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration
From: Alexei Starovoitov @ 2019-02-28 2:47 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Andrii Nakryiko, Daniel Borkmann, netdev, bpf, oss-drivers
In-Reply-To: <20190227155703.121514a2@cakuba.netronome.com>
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.
^ permalink raw reply
* [PATCH bpf-next v2 0/5] samples: bpf: continue effort to get rid of bpf_load
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, 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.
v2: - specify prog_type where possible (Andrii).
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 | 10 +-
samples/bpf/load_sock_ops.c | 97 -------------------
samples/bpf/sock_example.c | 2 +-
samples/bpf/sockex1_user.c | 25 ++---
samples/bpf/sockex2_user.c | 23 +++--
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, 69 insertions(+), 156 deletions(-)
delete mode 100644 samples/bpf/load_sock_ops.c
--
2.19.2
^ permalink raw reply
* [PATCH bpf-next v2 1/5] samples: bpf: force IPv4 in ping
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, Jakub Kicinski
In-Reply-To: <20190228030414.18973-1-jakub.kicinski@netronome.com>
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
^ permalink raw reply related
* [PATCH bpf-next v2 2/5] samples: bpf: remove load_sock_ops in favour of bpftool
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, Jakub Kicinski
In-Reply-To: <20190228030414.18973-1-jakub.kicinski@netronome.com>
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>
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
^ permalink raw reply related
* [PATCH bpf-next v2 3/5] tools: libbpf: add a correctly named define for map iteration
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, Jakub Kicinski
In-Reply-To: <20190228030414.18973-1-jakub.kicinski@netronome.com>
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
^ permalink raw reply related
* [PATCH bpf-next v2 4/5] samples: bpf: use libbpf where easy
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, Jakub Kicinski
In-Reply-To: <20190228030414.18973-1-jakub.kicinski@netronome.com>
Some samples don't really need the magic of bpf_load,
switch them to libbpf.
v2: - specify program types.
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 | 10 +++++++---
samples/bpf/sockex1_user.c | 23 +++++++++++++----------
samples/bpf/sockex2_user.c | 21 ++++++++++++---------
4 files changed, 35 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..e51eb060244e 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,14 @@ 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, BPF_PROG_TYPE_UNSPEC,
+ &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..7f90796ae15a 100644
--- a/samples/bpf/sockex1_user.c
+++ b/samples/bpf/sockex1_user.c
@@ -3,28 +3,31 @@
#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, BPF_PROG_TYPE_SOCKET_FILTER,
+ &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 +37,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..bc257333ad92 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,25 @@ 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, BPF_PROG_TYPE_SOCKET_FILTER,
+ &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 +44,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
^ permalink raw reply related
* [PATCH bpf-next v2 5/5] tools: libbpf: make sure readelf shows full names in build checks
From: Jakub Kicinski @ 2019-02-28 3:04 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: netdev, bpf, oss-drivers, Jakub Kicinski
In-Reply-To: <20190228030414.18973-1-jakub.kicinski@netronome.com>
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>
Acked-by: Andrii Nakryiko <andriin@fb.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
^ permalink raw reply related
* [PATCH] xfrm: policy: Fix possible user after free in __xfrm_policy_unlink
From: Yue Haibing @ 2019-02-28 3:16 UTC (permalink / raw)
To: steffen.klassert, herbert, davem; +Cc: linux-kernel, netdev, YueHaibing
From: YueHaibing <yuehaibing@huawei.com>
UBSAN report this:
UBSAN: Undefined behaviour in net/xfrm/xfrm_policy.c:1289:24
index 6 is out of range for type 'unsigned int [6]'
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.4.162-514.55.6.9.x86_64+ #13
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
0000000000000000 1466cf39b41b23c9 ffff8801f6b07a58 ffffffff81cb35f4
0000000041b58ab3 ffffffff83230f9c ffffffff81cb34e0 ffff8801f6b07a80
ffff8801f6b07a20 1466cf39b41b23c9 ffffffff851706e0 ffff8801f6b07ae8
Call Trace:
<IRQ> [<ffffffff81cb35f4>] __dump_stack lib/dump_stack.c:15 [inline]
<IRQ> [<ffffffff81cb35f4>] dump_stack+0x114/0x1a0 lib/dump_stack.c:51
[<ffffffff81d94225>] ubsan_epilogue+0x12/0x8f lib/ubsan.c:164
[<ffffffff81d954db>] __ubsan_handle_out_of_bounds+0x16e/0x1b2 lib/ubsan.c:382
[<ffffffff82a25acd>] __xfrm_policy_unlink+0x3dd/0x5b0 net/xfrm/xfrm_policy.c:1289
[<ffffffff82a2e572>] xfrm_policy_delete+0x52/0xb0 net/xfrm/xfrm_policy.c:1309
[<ffffffff82a3319b>] xfrm_policy_timer+0x30b/0x590 net/xfrm/xfrm_policy.c:243
[<ffffffff813d3927>] call_timer_fn+0x237/0x990 kernel/time/timer.c:1144
[<ffffffff813d8e7e>] __run_timers kernel/time/timer.c:1218 [inline]
[<ffffffff813d8e7e>] run_timer_softirq+0x6ce/0xb80 kernel/time/timer.c:1401
[<ffffffff8120d6f9>] __do_softirq+0x299/0xe10 kernel/softirq.c:273
[<ffffffff8120e676>] invoke_softirq kernel/softirq.c:350 [inline]
[<ffffffff8120e676>] irq_exit+0x216/0x2c0 kernel/softirq.c:391
[<ffffffff82c5edab>] exiting_irq arch/x86/include/asm/apic.h:652 [inline]
[<ffffffff82c5edab>] smp_apic_timer_interrupt+0x8b/0xc0 arch/x86/kernel/apic/apic.c:926
[<ffffffff82c5c985>] apic_timer_interrupt+0xa5/0xb0 arch/x86/entry/entry_64.S:735
<EOI> [<ffffffff81188096>] ? native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:52
[<ffffffff810834d7>] arch_safe_halt arch/x86/include/asm/paravirt.h:111 [inline]
[<ffffffff810834d7>] default_idle+0x27/0x430 arch/x86/kernel/process.c:446
[<ffffffff81085f05>] arch_cpu_idle+0x15/0x20 arch/x86/kernel/process.c:437
[<ffffffff8132abc3>] default_idle_call+0x53/0x90 kernel/sched/idle.c:92
[<ffffffff8132b32d>] cpuidle_idle_call kernel/sched/idle.c:156 [inline]
[<ffffffff8132b32d>] cpu_idle_loop kernel/sched/idle.c:251 [inline]
[<ffffffff8132b32d>] cpu_startup_entry+0x60d/0x9a0 kernel/sched/idle.c:299
[<ffffffff8113e119>] start_secondary+0x3c9/0x560 arch/x86/kernel/smpboot.c:245
The issue is triggered as this:
xfrm_add_policy
-->verify_newpolicy_info //check the index provided by user with XFRM_POLICY_MAX
//In my case, the index is 0x6E6BB6, so it pass the check.
-->xfrm_policy_construct //copy the user's policy and set xfrm_policy_timer
-->xfrm_policy_insert
--> __xfrm_policy_link //use the orgin dir, in my case is 2
--> xfrm_gen_index //generate policy index, there is 0x6E6BB6
then xfrm_policy_timer be fired
xfrm_policy_timer
--> xfrm_policy_id2dir //get dir from (policy index & 7), in my case is 6
--> xfrm_policy_delete
--> __xfrm_policy_unlink //access policy_count[dir], trigger out of range access
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: e682adf021be ("xfrm: Try to honor policy index if it's supplied by user")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/xfrm/xfrm_policy.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 8d1a898..b27eb742 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -316,6 +316,8 @@ static void xfrm_policy_timer(struct timer_list *t)
goto out;
dir = xfrm_policy_id2dir(xp->index);
+ if (dir >= XFRM_POLICY_MAX * 2)
+ dir = dir & XFRM_POLICY_MAX;
if (xp->lft.hard_add_expires_seconds) {
time64_t tmo = xp->lft.hard_add_expires_seconds +
--
2.7.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 4/5] samples: bpf: use libbpf where easy
From: Andrii Nakryiko @ 2019-02-28 3:35 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, bpf, oss-drivers
In-Reply-To: <20190228030414.18973-5-jakub.kicinski@netronome.com>
On Wed, Feb 27, 2019 at 7:04 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> Some samples don't really need the magic of bpf_load,
> switch them to libbpf.
>
> v2: - specify program types.
thanks!
Acked-by: Andrii Nakryiko <andriin@fb.com>
>
> 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 | 10 +++++++---
> samples/bpf/sockex1_user.c | 23 +++++++++++++----------
> samples/bpf/sockex2_user.c | 21 ++++++++++++---------
> 4 files changed, 35 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..e51eb060244e 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,14 @@ 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, BPF_PROG_TYPE_UNSPEC,
> + &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..7f90796ae15a 100644
> --- a/samples/bpf/sockex1_user.c
> +++ b/samples/bpf/sockex1_user.c
> @@ -3,28 +3,31 @@
> #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, BPF_PROG_TYPE_SOCKET_FILTER,
> + &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 +37,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..bc257333ad92 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,25 @@ 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, BPF_PROG_TYPE_SOCKET_FILTER,
> + &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 +44,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
>
^ permalink raw reply
* RE: [PATCH] can: flexcan: add TX support for variable payload size
From: Joakim Zhang @ 2019-02-28 3:43 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <cb0bf438-e3c3-5591-dcf7-8802346feefc@pengutronix.de>
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2019年2月27日 22:03
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH] can: flexcan: add TX support for variable payload size
>
> On 12/12/18 7:46 AM, Joakim Zhang wrote:
> > Now the FlexCAN driver always use last mailbox for TX, it will work
> > well when MB payload size is 8/16 bytes.
> > TX mailbox would change to 13 when MB payload size is 64 bytes to
> > support CANFD. So we may need to set iflag register to add support for
> > variable payload size.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> > drivers/net/can/flexcan.c | 42
> > +++++++++++++++++++++++++++++----------
> > 1 file changed, 32 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 0f36eafe3ac1..13fd085fcf84 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -141,7 +141,9 @@
> > #define FLEXCAN_TX_MB_RESERVED_OFF_FIFO 8
> > #define FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP 0
> > #define FLEXCAN_RX_MB_OFF_TIMESTAMP_FIRST
> (FLEXCAN_TX_MB_RESERVED_OFF_TIMESTAMP + 1)
> > -#define FLEXCAN_IFLAG_MB(x) BIT((x) & 0x1f)
> > +#define FLEXCAN_IFLAG1_MB_NUM 32
> > +#define FLEXCAN_IFLAG1_MB(x) BIT(x)
> > +#define FLEXCAN_IFLAG2_MB(x) BIT((x) & 0x1f)
> > #define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW BIT(7)
> > #define FLEXCAN_IFLAG_RX_FIFO_WARN BIT(6)
> > #define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE BIT(5)
> > @@ -822,9 +824,15 @@ static inline u64 flexcan_read_reg_iflag_rx(struct
> flexcan_priv *priv)
> > struct flexcan_regs __iomem *regs = priv->regs;
> > u32 iflag1, iflag2;
> >
> > - iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default &
> > - ~FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> > - iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default;
> > + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> > + iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default &
> > + ~FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> > + iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default;
> > + } else {
> > + iflag2 = priv->read(®s->iflag2) & priv->reg_imask2_default;
> > + iflag1 = priv->read(®s->iflag1) & priv->reg_imask1_default &
> > + ~FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> > + }
>
> I just noticed, that FLEXCAN_IFLAGx_MB(priv->tx_mb_idx) should already be
> part of the corresponding imaskx_default. See flexcan_open(). So we can
> remove it completely here, right?
Hi Marc,
flexcan_read_reg_iflag_rx() is the function to confirm the irq which RX mailbox generated, if we remove it completely here, how can we exclude that it is not a TX mailbox irq?
> >
> > return (u64)iflag2 << 32 | iflag1;
> > }
> > @@ -836,7 +844,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> > struct flexcan_priv *priv = netdev_priv(dev);
> > struct flexcan_regs __iomem *regs = priv->regs;
> > irqreturn_t handled = IRQ_NONE;
> > - u32 reg_iflag2, reg_esr;
> > + u32 reg_tx_iflag, tx_iflag_idx, reg_esr;
>
> "tx_iflag_idx" is not an index (going from 0...63) but a bit-mask.
>
> > + void __iomem *reg_iflag;
>
> "reg_iflag" is not a register but a pointer to a register, better rename it. There is
> a "u64 reg_iflag" in the same function already, but in a different scope. Why
> not make it a u32 instead of a void?
Of course, we can make it a u32.
Best Regards,
Joakim Zhang
> > enum can_state last_state = priv->can.state;
> >
> > /* reception interrupt */
> > @@ -870,10 +879,18 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> > }
> > }
> >
> > - reg_iflag2 = priv->read(®s->iflag2);
> > + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> > + reg_tx_iflag = priv->read(®s->iflag2);
> > + tx_iflag_idx = FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> > + reg_iflag = ®s->iflag2;
> > + } else {
> > + reg_tx_iflag = priv->read(®s->iflag1);
> > + tx_iflag_idx = FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> > + reg_iflag = ®s->iflag1;
> > + }
> >
> > /* transmission complete interrupt */
> > - if (reg_iflag2 & FLEXCAN_IFLAG_MB(priv->tx_mb_idx)) {
> > + if (reg_tx_iflag & tx_iflag_idx) {
> > u32 reg_ctrl = priv->read(&priv->tx_mb->can_ctrl);
> >
> > handled = IRQ_HANDLED;
> > @@ -885,7 +902,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
> > /* after sending a RTR frame MB is in RX mode */
> > priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
> > &priv->tx_mb->can_ctrl);
> > - priv->write(FLEXCAN_IFLAG_MB(priv->tx_mb_idx), ®s->iflag2);
> > + priv->write(tx_iflag_idx, reg_iflag);
> > netif_wake_queue(dev);
> > }
> >
> > @@ -1244,8 +1261,13 @@ static int flexcan_open(struct net_device *dev)
> > priv->tx_mb_idx = priv->mb_count - 1;
> > priv->tx_mb = flexcan_get_mb(priv, priv->tx_mb_idx);
> >
> > - priv->reg_imask1_default = 0;
> > - priv->reg_imask2_default = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
> > + if (priv->tx_mb_idx >= FLEXCAN_IFLAG1_MB_NUM) {
> > + priv->reg_imask1_default = 0;
> > + priv->reg_imask2_default = FLEXCAN_IFLAG2_MB(priv->tx_mb_idx);
> > + } else {
> > + priv->reg_imask1_default = FLEXCAN_IFLAG1_MB(priv->tx_mb_idx);
> > + priv->reg_imask2_default = 0;
> > + }
> >
> > priv->offload.mailbox_read = flexcan_mailbox_read;
> >
> >
>
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
^ permalink raw reply
* Re: [PATCH bpf-next 3/5] tools: libbpf: add a correctly named define for map iteration
From: Andrii Nakryiko @ 2019-02-28 3:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Jakub Kicinski, Daniel Borkmann, netdev, bpf, oss-drivers
In-Reply-To: <20190228024715.ijjki2fleuunxydn@ast-mbp>
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>
^ permalink raw reply
* Re: [PATCH] net: dsa: read mac address from DT for slave device
From: Florian Fainelli @ 2019-02-28 3:54 UTC (permalink / raw)
To: xiaofeis
Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
Andrew Lunn, Vivien Didelot, Niklas Cassel, netdev
In-Reply-To: <399272053c9e69386e709923544ba0d6@codeaurora.org>
On 2/27/2019 6:23 PM, xiaofeis@codeaurora.org wrote:
> On 2019-02-27 11:13, Florian Fainelli wrote:
>> On 2/26/2019 6:04 PM, xiaofeis@codeaurora.org wrote:
>>> On 2019-02-26 15:45, xiaofeis@codeaurora.org wrote:
>>>> On 2019-02-26 01:27, Florian Fainelli wrote:
>>>>> On 2/25/19 5:28 AM, xiaofeis@codeaurora.org wrote:
>>>>>> Hi Florian
>>>>>>
>>>>>> We have two slave DSA interfaces, wan0 and lan0, one is for wan port,
>>>>>> and the other is for lan port. Customer has it's mac address pool,
>>>>>> they
>>>>>> want
>>>>>> to assign the mac address from the pool on wan0, lan0, and other
>>>>>> interfaces like
>>>>>> wifi, bt. Coreboot/uboot will populate it to the DTS node, so the
>>>>>> driver
>>>>>> can
>>>>>> get it from it's node. For DSA slave interface, it already has
>>>>>> it's own
>>>>>> DTS node, it's
>>>>>> easy to just add one porperty "local-mac-address" there for the
>>>>>> usage in
>>>>>> DSA driver.
>>>>>>
>>>>>> If not use DSA framework, normally we will use eth0.x and eth0.y for
>>>>>> wan
>>>>>> and lan.
>>>>>> On this case, customer usually also assign the MAC address on these
>>>>>> logical interface
>>>>>> from it's pool.
>>>>>
>>>>> OK, but this is not necessary per my previous explanation: the CPU <=>
>>>>> WAN pipe is a separate broadcast domain (otherwise it is a security
>>>>> hole
>>>>> since you exposing LAN machines to the outside world), and so there is
>>>>> no need for a separate MAC address. It might be convenient to have
>>>>> one,
>>>>> especially for the provider, if they run a management software (e.g.:
>>>>> TR69), but it is not required per-se.
>>>>>
>>>>> Let me ask a secondary question here, how many Ethernet MACs
>>>>> connect to
>>>>> the switch in this configuration? Is there one that is supposed to be
>>>>> assigned all LAN traffic and one that is supposed to be assigned
>>>>> all WAN
>>>>> traffic? If so, then what you are doing makes even less
>>>>>
>>>>
>>>> Only one MAC connected to switch cpu port, both lan0 and wan0 are on
>>>> the top of
>>>> same interface(eth0).
>>>>
>>> Customer doesn't care about the MAC controller's MAC address, just leave
>>> it as the driver
>>> randomly generated. They just want to assign the MAC address on wan and
>>> lan DSA logical
>>> interface.
>>>
>>> Many customer doesn't use DSA, for example, they use eth0.1/eth0.2 for
>>> lan/wan with one MAC controller.
>>> They configure switch wan port in vlan2 group, and lan port in vlan1
>>> group, they usually assign mac address
>>> on the logical interface(eth0.1ð0.2), i think this is similar with
>>> DSA slave interfaces.
>>
>> Yes it is a similar use case, and in both cases there is no really a
>> functional need for a separate MAC address for lan/eth0.1 or wan/eth0.2
>> since the switch should be configured to perform IVL (Individual VLAN
>> Learning) and would determine the egress port just fine based on the MAC
>> DA. Because it is an established practice does not mean we should not
>> challenge it :).
>>
>> My issue with your change is that because DSA is meant to be a flexible
>> framework we do not know the type/nature of DSA master network device
>> that is going to be used. That DSA master network device may or may not
>> have it own MAC DA filtering capability. Having to filter its own MAC
>> address is fine and a well established behavior, having to filter for
>> more than one unicast address starts to be questionable and eats up
>> filter space that could be better used for filtering MC addresses
>> instead. Another possible concern is a station trying to spoof the MAC
>> address, some switches may support protecting only one UC/management MAC
>> address, so having more than one could create security attack surfaces.
>>
>> To give you an example, I work with 3 generations of DSA master network
>> controllers (bcmgenet and bcmsysport drivers).
>>
>> - GENET supports 17 perfect filters, but we must include its own MAC
>> address, the broadcast address and that leaves only 15 filters for MC
>>
>> - SYSTEMPORT is always attached to a switch but supports filtering the
>> MAC DA based on its own MAC and then it is in promiscuous mode
>>
>> So with your scheme, we would leave only 13 filters for MC on GENET and
>> we would putting the interface in promiscuous mode for SYSTEMPORT.
>>
>> Until we have a better switch-side filtering framework (and this is
>> being worked on right now), I would prefer that we defer accepting those
>> type of features. Andrew and Vivien might feel differently about that
>> though.
> This patch is just add one more option, if there is valid mac address
> populated
> in the DTS, then use it or else still inherti from master. I don't think
> it will
> break the DSA flexible framework. I think this change make DSA more
> flexible on
> MAC address setting.
> Many cusomter use some of our QCA chips, some direclty use DSA, some use
> internal similar
> mechanism(one netdevice for each switch port with swtichdev), we didn't
> see any limiation
> when they populiate the mac address for each port in DTS with only one
> HW mac controller.
> So my opinion is this patch is want to add a option which is already
> used in many
> products, this change does not break anything, developer/customer can
> chose use or not.
As I wrote, I am not totally opposed to it, I would prefer we had a
better infrastructure for UC/MC filtering in place before landing this
change but that is not there yet, and won't happen over night. So please
address Andrew's feedback to provide an update to the DSA binding
document, repost and I will certainly Ack it this time.
Please be considerate of people giving you feedback and do not try to
circle back to your use case and just explaining in a different way than
"works for me, accept it", because that's not going to work really well
in the long run.
Looking forward for more contributions on the qca8k driver, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/6] net: 8021q: vlan_dev: add vid tag to addresses of uc and mc lists
From: Florian Fainelli @ 2019-02-28 4:09 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem, grygorii.strashko
Cc: linux-omap, netdev, linux-kernel, jiri, ilias.apalodimas
In-Reply-To: <20190226184556.16082-3-ivan.khoronzhuk@linaro.org>
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> Update vlan mc and uc addresses with VID tag while propagating
> addresses to lower devices, do this only if address is not synced.
> It allows at end driver level to distinguish addresses belonging
> to vlan devices.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
[snip]
>
> +u16 vlan_dev_get_addr_vid(struct net_device *dev, const u8 *addr)
Having some kernel doc comment here would also be nice.
> +{
> + u16 vid = 0;
> +
> + if (dev->vid_len != NET_8021Q_VID_TSIZE)
> + return vid;
> +
> + vid = addr[dev->addr_len];
> + vid |= (addr[dev->addr_len + 1] & 0xf) << 8;
This uses knowledge of the maximum VLAN ID is 4095, which is fine, might
be a good idea to add a check on VID not exceeding the maximum VLAN ID
number instead of doing a silent truncation?
[snip]
> +static void vlan_dev_align_addr_vid(struct net_device *vlan_dev)
> +{
> + struct net_device *real_dev = vlan_dev_real_dev(vlan_dev);
> + struct netdev_hw_addr *ha;
> +
> + if (!real_dev->vid_len)
> + return;
Should not this check be moved to dev_{mc,uc}_sync? It does not seem to
me like this would scale really well across different stacked devices
(VLAN, bond, macvlan) as well as underlying drivers (cpsw, dsa, etc.).
Or maybe the check should be if vlan_dev->vid_len > real_dev->vid_len ->
error, right?
--
Florian
^ permalink raw reply
* Re: [PATCH] xfrm: policy: Fix possible user after free in __xfrm_policy_unlink
From: Herbert Xu @ 2019-02-28 4:12 UTC (permalink / raw)
To: Yue Haibing; +Cc: steffen.klassert, davem, linux-kernel, netdev, Fan Du
In-Reply-To: <20190228031623.22184-1-yuehaibing@huawei.com>
On Thu, Feb 28, 2019 at 11:16:23AM +0800, Yue Haibing wrote:
>
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 8d1a898..b27eb742 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -316,6 +316,8 @@ static void xfrm_policy_timer(struct timer_list *t)
> goto out;
>
> dir = xfrm_policy_id2dir(xp->index);
> + if (dir >= XFRM_POLICY_MAX * 2)
> + dir = dir & XFRM_POLICY_MAX;
This is still wrong. We shouldn't be allowing bogus policies
to be in the system at all.
I have digged deeper and the problem was introduced by:
commit e682adf021be796940be6cc10c07be7f7398c220
Author: Fan Du <fan.du@windriver.com>
Date: Thu Nov 7 17:47:48 2013 +0800
xfrm: Try to honor policy index if it's supplied by user
Where the check for the user-supplied index is simply wrong in
verify_newpolicy_info. So please fix it there.
Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH net-next 3/6] net: 8021q: vlan_dev: add vid tag for vlan device own address
From: Florian Fainelli @ 2019-02-28 4:13 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem, grygorii.strashko
Cc: linux-omap, netdev, linux-kernel, jiri, ilias.apalodimas
In-Reply-To: <20190226184556.16082-4-ivan.khoronzhuk@linaro.org>
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> The vlan device address is held separately from uc/mc lists and
> handled differently. The vlan dev address is bound with real device
> address only if it's inherited from init, in all other cases it's
> separate address entry in uc list. With vid set, the address becomes
> not inherited from real device after it's set manually as before, but
> is part of uc list any way, with appropriate vid tag set. If vid_len
> for real device is 0, the behaviour is the same as before this change,
> so shouldn't be any impact on systems w/o individual virtual device
> filtering (IVDF) enabled. This allows to control and sync vlan device
> address and disable concrete vlan packet income when vlan interface is
> down.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
[snip]
>
> +static int vlan_dev_add_addr(struct net_device *dev, u8 *addr)
> +{
> + struct net_device *real_dev = vlan_dev_real_dev(dev);
> + unsigned char naddr[ETH_ALEN + NET_8021Q_VID_TSIZE];
> +
> + if (real_dev->vid_len) {
Don't you need to check that real_dev->vid_len is >= NET_8021Q_VID_TSIZE
here?
> + memcpy(naddr, addr, dev->addr_len);
> + vlan_dev_set_addr_vid(dev, naddr);
> + return dev_vid_uc_add(real_dev, naddr);
> + }
> +
> + if (ether_addr_equal(addr, real_dev->dev_addr))
> + return 0;
> +
> + return dev_uc_add(real_dev, addr);
> +}
> +
> +static void vlan_dev_del_addr(struct net_device *dev, u8 *addr)
> +{
> + struct net_device *real_dev = vlan_dev_real_dev(dev);
> + unsigned char naddr[ETH_ALEN + NET_8021Q_VID_TSIZE];
> +
> + if (real_dev->vid_len) {
Same here.
> + memcpy(naddr, addr, dev->addr_len);
> + vlan_dev_set_addr_vid(dev, naddr);
> + dev_vid_uc_del(real_dev, naddr);
> + return;
> + }
> +
> + if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr))
> + dev_uc_del(real_dev, addr);
> +}
> +
> +static int vlan_dev_subs_addr(struct net_device *dev, u8 *addr)
> +{
> + int err;
> +
> + err = vlan_dev_add_addr(dev, addr);
> + if (err < 0)
> + return err;
> +
> + vlan_dev_del_addr(dev, dev->dev_addr);
> + return err;
> +}
> +
> bool vlan_dev_inherit_address(struct net_device *dev,
> struct net_device *real_dev)
> {
> if (dev->addr_assign_type != NET_ADDR_STOLEN)
> return false;
>
> + if (real_dev->vid_len)
> + if (vlan_dev_subs_addr(dev, real_dev->dev_addr))
> + return false;
The check on real_dev->vid_len can be absorbed into vlan_dev_subs_addr()?
> +
> ether_addr_copy(dev->dev_addr, real_dev->dev_addr);
> call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
> return true;
> @@ -278,9 +327,10 @@ static int vlan_dev_open(struct net_device *dev)
> !(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
> return -ENETDOWN;
>
> - if (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr) &&
> - !vlan_dev_inherit_address(dev, real_dev)) {
> - err = dev_uc_add(real_dev, dev->dev_addr);
> + if (ether_addr_equal(dev->dev_addr, real_dev->dev_addr) ||
> + (!ether_addr_equal(dev->dev_addr, real_dev->dev_addr) &&
> + !vlan_dev_inherit_address(dev, real_dev))) {
Should this condition simply become if !vlan_dev_inherit_address() now?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 5/6] net: ethernet: ti: cpsw: update mc filtering to use IVDF
From: Florian Fainelli @ 2019-02-28 4:17 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem, grygorii.strashko
Cc: linux-omap, netdev, linux-kernel, jiri, ilias.apalodimas
In-Reply-To: <20190226184556.16082-6-ivan.khoronzhuk@linaro.org>
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> The cpsw can filter multicast addresses only per vlan. Thus if mcast
> address is set for one of them or only for real device it must be
> added for every created vlan consuming ALE table w/o reason. In order to
> simplify dispatching vlan filters, the IVDF recently added is resused.
>
> In case IVDF is disabled - mc is updated only for real device as before.
> The previous method is harder to reuse and vlan filtering is limited
> only for vlans directly connected to real netdev, so drop it in flavor
> of IVDF decision.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
> drivers/net/ethernet/ti/Kconfig | 1 +
> drivers/net/ethernet/ti/cpsw.c | 113 ++++----------------------------
> 2 files changed, 13 insertions(+), 101 deletions(-)
Nice diffstat!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: core: dev_addr_lists: add VID to device address
From: Florian Fainelli @ 2019-02-28 4:24 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem, grygorii.strashko
Cc: linux-omap, netdev, linux-kernel, jiri, ilias.apalodimas
In-Reply-To: <20190226184556.16082-2-ivan.khoronzhuk@linaro.org>
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> Despite this is supposed to be used for Ethernet VLANs, not Ethernet
> addresses with space for VID also can reuse this, so VID is considered
> as virtual ID extension, not belonging strictly to Ethernet VLAN VIDs,
> and overall change can be named individual virtual device filtering
> (IVDF).
>
> This patch adds VID tag at the end of each address. The actual
> reserved address size is 32 bytes. For Ethernet addresses with 6 bytes
> long that's possible to add tag w/o increasing address size. Thus,
> each address for the case has 32 - 6 = 26 bytes to hold additional
> info, say VID for virtual device addresses.
>
> Therefore, when addresses are synced to the address list of parent
> device the address list of latter can contain separate addresses for
> virtual devices. It allows to track separate address tables for
> virtual devices if they present and the device can be placed on
> any place of device tree as the address is propagated to to the end
> real device thru *_sync()/ndo_set_rx_mode() APIs. Also it simplifies
> handling VID addresses at real device when it supports IVDF.
>
> If parent device doesn't want to have virtual addresses in its address
> space the vid_len has to be 0, thus its address space is "shrunk" to
> the state as before this patch. For now it's 0 for every device. It
> allows two devices with and w/o IVDF to be part of same bond device
> for instance.
>
> The end real device supporting IVDF can retrieve VID tag from an
> address and set it for a given virtual device only. By default, vid 0
> is used for real devices to distinguish it from virtual addresses.
>
> See next patches to see how it's used.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
[snip]
> @@ -1889,6 +1890,7 @@ struct net_device {
> unsigned char perm_addr[MAX_ADDR_LEN];
> unsigned char addr_assign_type;
> unsigned char addr_len;
> + unsigned char vid_len;
Have not compiled or tested this patch series yet, but did you check
that adding this member does not change the structure layout (you can
use pahole for that purpose).
> unsigned short neigh_priv_len;
> unsigned short dev_id;
> unsigned short dev_port;
> @@ -4141,8 +4143,10 @@ int dev_addr_init(struct net_device *dev);
>
> /* Functions used for unicast addresses handling */
> int dev_uc_add(struct net_device *dev, const unsigned char *addr);
> +int dev_vid_uc_add(struct net_device *dev, const unsigned char *addr);
> int dev_uc_add_excl(struct net_device *dev, const unsigned char *addr);
> int dev_uc_del(struct net_device *dev, const unsigned char *addr);
> +int dev_vid_uc_del(struct net_device *dev, const unsigned char *addr);
> int dev_uc_sync(struct net_device *to, struct net_device *from);
> int dev_uc_sync_multiple(struct net_device *to, struct net_device *from);
> void dev_uc_unsync(struct net_device *to, struct net_device *from);
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> index a6723b306717..e3c80e044b8c 100644
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
> @@ -545,6 +545,26 @@ int dev_addr_del(struct net_device *dev, const unsigned char *addr,
> }
> EXPORT_SYMBOL(dev_addr_del);
>
> +static int get_addr_len(struct net_device *dev)
> +{
> + return dev->addr_len + dev->vid_len;
> +}
> +
> +static int set_vid_addr(struct net_device *dev, const unsigned char *addr,
> + unsigned char *naddr)
Having some kernel doc comments here would be nice to indicate that the
return value is dev->addr_len, it was not obvious until I saw in the
next function how you used it.
> +{
> + int i;
> +
> + if (!dev->vid_len)
> + return dev->addr_len;
> +
> + memcpy(naddr, addr, dev->addr_len);
> + for (i = 0; i < dev->vid_len; i++)
> + naddr[dev->addr_len + i] = 0;
memset(naddr + dev->addr_len, 0, dev->vid_len) would be more compact and
maybe a little less error prone too?
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 4/6] ethernet: eth: add default vid len for all ehternet kind devices
From: Florian Fainelli @ 2019-02-28 4:29 UTC (permalink / raw)
To: Ivan Khoronzhuk, davem, grygorii.strashko
Cc: linux-omap, netdev, linux-kernel, jiri, ilias.apalodimas
In-Reply-To: <20190226184556.16082-5-ivan.khoronzhuk@linaro.org>
On 2/26/2019 10:45 AM, Ivan Khoronzhuk wrote:
> IVDF - individual virtual device filtering. Allows to set per vlan
> l2 address filters on end real network device (for unicast and for
> multicast) and drop redundant not expected packet income.
>
> If CONFIG_VLAN_8021Q_IVDF is enabled the following changes are
> applied, and only for ethernet network devices.
>
> By default every ethernet netdev needs vid len = 2 bytes to be able to
> hold up to 4096 vids. So set it for every eth device to be correct,
> except vlan devs.
>
> In order to shrink all addresses of devices above vlan, the vid_len
> for vlan dev = 0, as result all suckers sync their addresses to common
> base not taking in to account vid part (vid_len of "to" devices is
> important only). And only vlan device is the source of addresses with
> actual its vid set, propagating it to parent devices while rx_mode().
>
> Also, don't bother those ethernet devices that at this moment are not
> moved to vlan addressing scheme, so while end ethernet device is
> created - set vid_len to 0, thus, while syncing, its address space is
> concatenated to one dimensional like usual, and who needs IVDF - set
> it to NET_8021Q_VID_TSIZE.
>
> There is another decision - is to inherit vid_len or some feature flag
> from end root device in order to all upper devices have vlan extended
> address space only if exact end real device have such capability. But
> I didn't, because it requires more changes and probably I'm not
> familiar with all places where it should be inherited, I would
> appreciate if someone can guid where it's applicable, then it could
> become a little bit more limited.
I would think that a call to vlan_dev_ivdf_set() would be enough to
indicate that the underlying network device driver supports IVDF and
wants to make use of it. The infrastructure itself that you added costs
little memory, it is once the call to vlan_dev_ivdf_set() is made that
the memory consumption increases which is fine, since we want to make
use of that feature.
While I appreciate the thoughts given to making this a configurable
option, I feel that enabling it unconditionally and having the
underlying driver decide would be more manageable.
We have had that conversation before, but let me ask again when we call
dev_{uc,mc}_sync() and ultimately the network device's
ndo_set_rx_mode(), by the time the ndo_set_rx_mode() function is called,
we lost track of the call chain, like which virtual device was it
originating from. If we somehow added a notification information about
the network device stack (and we could use netdevice notifiers for
that), then maybe we don't really need to add all of this code and we
can just derive the necessary bits of information we want by checking:
is this a VLAN network device? It is, okay what's your VLAN ID, etc.?
Either approach would get us our cookie anyway :)
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
[snip]
> @@ -404,8 +405,13 @@ EXPORT_SYMBOL(ether_setup);
> struct net_device *alloc_etherdev_mqs(int sizeof_priv, unsigned int txqs,
> unsigned int rxqs)
> {
> - return alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
> - ether_setup, txqs, rxqs);
> + struct net_device *dev;
> +
> + dev = alloc_netdev_mqs(sizeof_priv, "eth%d", NET_NAME_UNKNOWN,
> + ether_setup, txqs, rxqs);
You need to check the return value of alloc_netdev_mqs() now, otherwise
you could be doing a NPD in the call right under. Since that is the
default though, do you really need to call vlan_dev_ivdf_set() below?
--
Florian
^ permalink raw reply
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-28 4:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: si-wei liu, Samudrala, Sridhar, Siwei Liu, Jiri Pirko,
Stephen Hemminger, David Miller, Netdev, virtualization,
virtio-dev, Brandeburg, Jesse, Alexander Duyck, Jason Wang,
liran.alon
In-Reply-To: <20190227175218.736e13b6@cakuba.netronome.com>
On Wed, Feb 27, 2019 at 05:52:18PM -0800, Jakub Kicinski wrote:
> On Wed, 27 Feb 2019 20:26:02 -0500, Michael S. Tsirkin wrote:
> > On Wed, Feb 27, 2019 at 04:52:05PM -0800, Jakub Kicinski wrote:
> > > On Wed, 27 Feb 2019 19:41:32 -0500, Michael S. Tsirkin wrote:
> > > > > As this scheme adds much complexity to the kernel naming convention
> > > > > (currently it's just ethX names) that no userspace can understand.
> > > >
> > > > Anything that pokes at slaves needs to be specially designed anyway.
> > > > Naming seems like a minor issue.
> > >
> > > Can the users who care about the naming put net_failover into
> > > "user space will do the bond enslavement" mode, and do the bond
> > > creation/management themselves from user space (in systemd/
> > > Network Manager) based on the failover flag?
> >
> > Putting issues of compatibility aside (userspace tends to be confused if
> > you give it two devices with same MAC), how would you have it work in
> > practice? Timer based hacks like netvsc where if userspace didn't
> > respond within X seconds we assume it won't and do everything ourselves?
>
> Well, what I'm saying is basically if user space knows how to deal with
> the auto-bonding, we can put aside net_failover for the most part. It
> can either be blacklisted or it can have some knob which will
> effectively disable the auto-enslavement.
OK I guess we could add a module parameter to skip this.
Is this what you mean?
> Auto-bonding capable user space can do the renames, spawn the bond,
> etc. all by itself. I'm basically going back to my initial proposal
> here :) There is a RedHat bugzilla for the NetworkManager team to do
> this, but we merged net_failover before those folks got around to
> implementing it.
In particular because there's no policy involved whatsoever
here so it's just mechanism being pushed up to userspace.
> IOW if NM/systemd is capable of doing the auto-bonding itself it can
> disable the kernel mechanism and take care of it all. If kernel is
> booted with an old user space which doesn't have capable NM/systemd -
> net_failover will kick in and do its best.
Sure - it's just 2 lines of code, see below.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
But I don't intend to bother until there's actual interest from
userspace developers to bother. In particular it is not just NM/systemd
even on Fedora - e.g. you will need to teach dracut to somehow detect
and handle this - right now it gets confused if there are two devices
with same MAC addresses.
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 955b3e76eb8d..dd2b2c370003 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -43,6 +43,7 @@ static bool csum = true, gso = true, napi_tx;
module_param(csum, bool, 0444);
module_param(gso, bool, 0444);
module_param(napi_tx, bool, 0644);
+module_param(disable_failover, bool, 0644);
/* FIXME: MTU in config. */
#define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
@@ -3163,6 +3164,7 @@ static int virtnet_probe(struct virtio_device *vdev)
virtnet_init_settings(dev);
- if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY)) {
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_STANDBY) &&
+ !disable_failover) {
vi->failover = net_failover_create(vi->dev);
if (IS_ERR(vi->failover)) {
err = PTR_ERR(vi->failover);
^ permalink raw reply related
* linux-next: manual merge of the staging tree with the net-next tree
From: Stephen Rothwell @ 2019-02-28 4:52 UTC (permalink / raw)
To: Greg KH, David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Florian Fainelli
[-- Attachment #1: Type: text/plain, Size: 805 bytes --]
Hi all,
Today's linux-next merge of the staging tree got a conflict in:
drivers/staging/fsl-dpaa2/ethsw/ethsw.c
between commit:
570b68c8ddde ("staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_SET")
from the net-next tree and commit:
11f27765f611 ("staging: fsl-dpaa2: ethsw: Add missing netdevice check")
from the staging tree.
I fixed it up (the former is a superset of the latter) and can carry the
fix as necessary. This is now fixed as far as linux-next is concerned,
but any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging. You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v5 perf,bpf 00/15] perf annotation of BPF programs
From: Song Liu @ 2019-02-28 5:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
Changes v4 to v5:
1. Rebase to latest bpf-next;
2. Add dependency of 94816add0005 from Arnaldo's tree;
3. More details in change logs;
4. Add perf_env__init() to init bpf related lock and rbtrees;
5. Small clean ups.
Changes v3 to v4:
1. Incorporate feedbacks from Jiri and Namhyung;
2. Fixed compilation error with different feature-disassembler-four-args;
3. Split some patches to smaller patches;
4. Improved error handleing in symbol__disassemble_bpf();
5. Made side band thread more generic;
6. Added comments as suggested.
Changes v2 to v3:
1. Remove unnecessary include in header files;
2. Improved error handling;
3. Better naming of functions, variables, etc.;
4. Enable bpf events by default for perf-top.
Changes v1 to v2:
1. Fix compilation error with different feature-disassembler-four-args;
2. Fix a segfault in perf-record;
3. Split patches 5/9 and 6/9 so that perf_env changes and perf.data changes
are in separate patches.
This series enables annotation of BPF programs in perf.
perf tool gathers information via sys_bpf and (optionally) stores them in
perf.data as headers.
Patch 1/15 fixes a minor issue in kernel;
Patch 2/15 to 4/15 introduce new helper functions and use them in perf and
bpftool;
Patch 5/15 to 9/15 saves information of bpf program in perf_env;
Patch 10/15 adds --bpf-event options to perf-top;
Patch 11/15 to 13/15 enables annotation of bpf progs based on information
gathered in 5/15 to 9/15;
Patch 14/15 introduces side band polling thread that gathers information
for special kernel events during perf-record or perf-top.
Patch 15/15 handles information of short living BPF program using the new
side band polling thread.
Commands tested during developments are perf-top, perf-record, perf-report,
and perf-annotate.
===================== Note on patch dependency ========================
This set has dependency in both bpf-next tree and tip/perf/core. Current
version is developed on bpf-next tree with the following commits
cherry-picked from tip/perf/core (or acme/perf/core):
(from 1/11 to 11/11)
commit 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
commit d764ac646491 ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
commit 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
commit df063c83aa2c ("tools headers uapi: Sync tools/include/uapi/linux/perf_event.h")
commit 9aa0bfa370b2 ("perf tools: Handle PERF_RECORD_KSYMBOL")
commit 45178a928a4b ("perf tools: Handle PERF_RECORD_BPF_EVENT")
commit 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
commit a40b95bcd30c ("perf top: Synthesize BPF events for pre-existing loaded BPF programs")
commit 6934058d9fb6 ("bpf: Add module name [bpf] to ksymbols for bpf programs")
commit 811184fb6977 ("perf bpf: Fix synthesized PERF_RECORD_KSYMBOL/BPF_EVENT")
comimt 94816add0005 ("perf tools: Add perf_exe() helper to find perf binary")
========================================================================
This set is also available at:
https://github.com/liu-song-6/linux/tree/bpf-annotation
Thanks!!
Song Liu (15):
perf, bpf: consider events with attr.bpf_event as side-band events
bpf: libbpf: introduce bpf_program__get_prog_info_linear()
bpf: bpftool: use bpf_program__get_prog_info_linear() in
prog.c:do_dump()
perf, bpf: synthesize bpf events with
bpf_program__get_prog_info_linear()
perf: change prototype of perf_event__synthesize_bpf_events()
perf, bpf: save bpf_prog_info in a rbtree in perf_env
perf, bpf: save bpf_prog_info information as headers to perf.data
perf, bpf: save btf in a rbtree in perf_env
perf, bpf: save btf information as headers to perf.data
perf-top: add option --no-bpf-event
perf: add -lopcodes to feature-libbfd
perf, bpf: enable annotation of bpf program
perf, bpf: process PERF_BPF_EVENT_PROG_LOAD for annotation
perf: introduce side band thread
perf, bpf: save bpf_prog_info and btf of short living bpf programs
kernel/events/core.c | 3 +-
tools/bpf/bpftool/prog.c | 266 +++++++------------------------
tools/build/Makefile.feature | 6 +-
tools/lib/bpf/libbpf.c | 251 +++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 63 ++++++++
tools/lib/bpf/libbpf.map | 3 +
tools/perf/Makefile.config | 6 +-
tools/perf/builtin-record.c | 12 +-
tools/perf/builtin-top.c | 15 +-
tools/perf/perf.c | 1 +
tools/perf/util/annotate.c | 150 ++++++++++++++++-
tools/perf/util/bpf-event.c | 301 +++++++++++++++++++++++++----------
tools/perf/util/bpf-event.h | 36 ++++-
tools/perf/util/dso.c | 1 +
tools/perf/util/dso.h | 33 ++--
tools/perf/util/env.c | 149 +++++++++++++++++
tools/perf/util/env.h | 22 +++
tools/perf/util/evlist.c | 105 ++++++++++++
tools/perf/util/evlist.h | 13 ++
tools/perf/util/header.c | 249 ++++++++++++++++++++++++++++-
tools/perf/util/header.h | 2 +
tools/perf/util/session.c | 1 +
tools/perf/util/symbol.c | 1 +
tools/perf/util/top.h | 1 +
24 files changed, 1379 insertions(+), 311 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v5 perf,bpf 01/15] perf, bpf: consider events with attr.bpf_event as side-band events
From: Song Liu @ 2019-02-28 5:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190228050643.958685-1-songliubraving@fb.com>
Events with bpf_event should be considered as side-band event, as they
carry information about BPF programs.
Fixes: 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
Signed-off-by: Song Liu <songliubraving@fb.com>
---
kernel/events/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 795bfab6f8a3..6a17584810ca 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4238,7 +4238,8 @@ static bool is_sb_event(struct perf_event *event)
if (attr->mmap || attr->mmap_data || attr->mmap2 ||
attr->comm || attr->comm_exec ||
attr->task || attr->ksymbol ||
- attr->context_switch)
+ attr->context_switch ||
+ attr->bpf_event)
return true;
return false;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v5 perf,bpf 05/15] perf: change prototype of perf_event__synthesize_bpf_events()
From: Song Liu @ 2019-02-28 5:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190228050643.958685-1-songliubraving@fb.com>
This patch changes the arguments of perf_event__synthesize_bpf_events()
to include perf_session* instead of perf_tool*. perf_session will be used
in the next patch.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/builtin-record.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/util/bpf-event.c | 8 +++++---
tools/perf/util/bpf-event.h | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 88ea11d57c6f..2355e0a9eda0 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1083,7 +1083,7 @@ static int record__synthesize(struct record *rec, bool tail)
return err;
}
- err = perf_event__synthesize_bpf_events(tool, process_synthesized_event,
+ err = perf_event__synthesize_bpf_events(session, process_synthesized_event,
machine, opts);
if (err < 0)
pr_warning("Couldn't synthesize bpf events.\n");
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5a486d4de56e..27d8d42e0a4d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1216,7 +1216,7 @@ static int __cmd_top(struct perf_top *top)
init_process_thread(top);
- ret = perf_event__synthesize_bpf_events(&top->tool, perf_event__process,
+ ret = perf_event__synthesize_bpf_events(top->session, perf_event__process,
&top->session->machines.host,
&top->record_opts);
if (ret < 0)
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index e6dfb95029e5..ff7ee149ec46 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -10,6 +10,7 @@
#include "debug.h"
#include "symbol.h"
#include "machine.h"
+#include "session.h"
#define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
@@ -42,7 +43,7 @@ int machine__process_bpf_event(struct machine *machine __maybe_unused,
* -1 for failures;
* -2 for lack of kernel support.
*/
-static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
+static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
int fd,
@@ -52,6 +53,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
struct ksymbol_event *ksymbol_event = &event->ksymbol_event;
struct bpf_event *bpf_event = &event->bpf_event;
struct bpf_prog_info_linear *info_linear;
+ struct perf_tool *tool = session->tool;
struct bpf_prog_info *info;
struct btf *btf = NULL;
bool has_btf = false;
@@ -175,7 +177,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
return err ? -1 : 0;
}
-int perf_event__synthesize_bpf_events(struct perf_tool *tool,
+int perf_event__synthesize_bpf_events(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
struct record_opts *opts)
@@ -209,7 +211,7 @@ int perf_event__synthesize_bpf_events(struct perf_tool *tool,
continue;
}
- err = perf_event__synthesize_one_bpf_prog(tool, process,
+ err = perf_event__synthesize_one_bpf_prog(session, process,
machine, fd,
event, opts);
close(fd);
diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
index 7890067e1a37..6698683612a7 100644
--- a/tools/perf/util/bpf-event.h
+++ b/tools/perf/util/bpf-event.h
@@ -15,7 +15,7 @@ struct record_opts;
int machine__process_bpf_event(struct machine *machine, union perf_event *event,
struct perf_sample *sample);
-int perf_event__synthesize_bpf_events(struct perf_tool *tool,
+int perf_event__synthesize_bpf_events(struct perf_session *session,
perf_event__handler_t process,
struct machine *machine,
struct record_opts *opts);
@@ -27,7 +27,7 @@ static inline int machine__process_bpf_event(struct machine *machine __maybe_unu
return 0;
}
-static inline int perf_event__synthesize_bpf_events(struct perf_tool *tool __maybe_unused,
+static inline int perf_event__synthesize_bpf_events(struct perf_session *session __maybe_unused,
perf_event__handler_t process __maybe_unused,
struct machine *machine __maybe_unused,
struct record_opts *opts __maybe_unused)
--
2.17.1
^ permalink raw reply related
* [PATCH v5 perf,bpf 07/15] perf, bpf: save bpf_prog_info information as headers to perf.data
From: Song Liu @ 2019-02-28 5:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190228050643.958685-1-songliubraving@fb.com>
This patch enables perf-record to save bpf_prog_info information as
headers to perf.data. A new header type HEADER_BPF_PROG_INFO is
introduced for this data.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
| 143 ++++++++++++++++++++++++++++++++++++++-
| 1 +
2 files changed, 143 insertions(+), 1 deletion(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4b88de5e9192..16f5bedb0b7d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -18,6 +18,7 @@
#include <sys/utsname.h>
#include <linux/time64.h>
#include <dirent.h>
+#include <bpf/libbpf.h>
#include "evlist.h"
#include "evsel.h"
@@ -39,6 +40,7 @@
#include "tool.h"
#include "time-utils.h"
#include "units.h"
+#include "bpf-event.h"
#include "sane_ctype.h"
@@ -1074,6 +1076,51 @@ static int write_clockid(struct feat_fd *ff,
sizeof(ff->ph->env.clockid_res_ns));
}
+static int write_bpf_prog_info(struct feat_fd *ff,
+ struct perf_evlist *evlist __maybe_unused)
+{
+ struct perf_env *env = &ff->ph->env;
+ struct rb_root *root;
+ struct rb_node *next;
+ u32 count = 0;
+ int ret;
+
+ down_read(&env->bpf_progs.lock);
+
+ root = &env->bpf_progs.infos;
+ next = rb_first(root);
+ while (next) {
+ ++count;
+ next = rb_next(next);
+ }
+
+ ret = do_write(ff, &count, sizeof(count));
+
+ if (ret < 0)
+ goto out;
+
+ next = rb_first(root);
+ while (next) {
+ struct bpf_prog_info_node *node;
+ size_t len;
+
+ node = rb_entry(next, struct bpf_prog_info_node, rb_node);
+ next = rb_next(&node->rb_node);
+ len = sizeof(struct bpf_prog_info_linear) +
+ node->info_linear->data_len;
+
+ /* before writing to file, translate address to offset */
+ bpf_program__bpil_addr_to_offs(node->info_linear);
+ ret = do_write(ff, node->info_linear, len);
+ bpf_program__bpil_offs_to_addr(node->info_linear);
+ if (ret < 0)
+ goto out;
+ }
+out:
+ up_read(&env->bpf_progs.lock);
+ return ret;
+}
+
static int cpu_cache_level__sort(const void *a, const void *b)
{
struct cpu_cache_level *cache_a = (struct cpu_cache_level *)a;
@@ -1554,6 +1601,29 @@ static void print_clockid(struct feat_fd *ff, FILE *fp)
ff->ph->env.clockid_res_ns * 1000);
}
+static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp)
+{
+ struct perf_env *env = &ff->ph->env;
+ struct rb_root *root;
+ struct rb_node *next;
+
+ down_read(&env->bpf_progs.lock);
+
+ root = &env->bpf_progs.infos;
+ next = rb_first(root);
+
+ while (next) {
+ struct bpf_prog_info_node *node;
+
+ node = rb_entry(next, struct bpf_prog_info_node, rb_node);
+ next = rb_next(&node->rb_node);
+ fprintf(fp, "# bpf_prog_info of id %u\n",
+ node->info_linear->info.id);
+ }
+
+ up_read(&env->bpf_progs.lock);
+}
+
static void free_event_desc(struct perf_evsel *events)
{
struct perf_evsel *evsel;
@@ -2586,6 +2656,76 @@ static int process_clockid(struct feat_fd *ff,
return 0;
}
+static int process_bpf_prog_info(struct feat_fd *ff,
+ void *data __maybe_unused)
+{
+ struct bpf_prog_info_linear *info_linear;
+ struct bpf_prog_info_node *info_node;
+ struct perf_env *env = &ff->ph->env;
+ u32 count, i;
+ int err = -1;
+
+ if (do_read_u32(ff, &count))
+ return -1;
+
+ down_write(&env->bpf_progs.lock);
+
+ for (i = 0; i < count; ++i) {
+ u32 info_len, data_len;
+
+ info_linear = NULL;
+ info_node = NULL;
+ if (do_read_u32(ff, &info_len))
+ goto out;
+ if (do_read_u32(ff, &data_len))
+ goto out;
+
+ if (info_len > sizeof(struct bpf_prog_info)) {
+ pr_warning("detected invalid bpf_prog_info\n");
+ goto out;
+ }
+
+ info_linear = malloc(sizeof(struct bpf_prog_info_linear) +
+ data_len);
+ if (!info_linear)
+ goto out;
+ info_linear->info_len = sizeof(struct bpf_prog_info);
+ info_linear->data_len = data_len;
+ if (do_read_u64(ff, (u64 *)(&info_linear->arrays)))
+ goto out;
+ if (__do_read(ff, &info_linear->info, info_len))
+ goto out;
+ if (info_len < sizeof(struct bpf_prog_info))
+ memset(((void *)(&info_linear->info)) + info_len, 0,
+ sizeof(struct bpf_prog_info) - info_len);
+
+ if (__do_read(ff, info_linear->data, data_len))
+ goto out;
+
+ /* endian mismatch, drop the info, continue */
+ if (ff->ph->needs_swap) {
+ free(info_linear);
+ continue;
+ }
+
+ info_node = malloc(sizeof(struct bpf_prog_info_node));
+ if (!info_node)
+ goto out;
+
+ /* after reading from file, translate offset to address */
+ bpf_program__bpil_offs_to_addr(info_linear);
+ info_node->info_linear = info_linear;
+ perf_env__insert_bpf_prog_info(env, info_node);
+ }
+
+ return 0;
+out:
+ free(info_linear);
+ free(info_node);
+ up_write(&env->bpf_progs.lock);
+ return err;
+}
+
struct feature_ops {
int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2645,7 +2785,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
FEAT_OPN(CACHE, cache, true),
FEAT_OPR(SAMPLE_TIME, sample_time, false),
FEAT_OPR(MEM_TOPOLOGY, mem_topology, true),
- FEAT_OPR(CLOCKID, clockid, false)
+ FEAT_OPR(CLOCKID, clockid, false),
+ FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false)
};
struct header_print_data {
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 0d553ddca0a3..0785c91b4c3a 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -39,6 +39,7 @@ enum {
HEADER_SAMPLE_TIME,
HEADER_MEM_TOPOLOGY,
HEADER_CLOCKID,
+ HEADER_BPF_PROG_INFO,
HEADER_LAST_FEATURE,
HEADER_FEAT_BITS = 256,
};
--
2.17.1
^ permalink raw reply related
* [PATCH v5 perf,bpf 11/15] perf: add -lopcodes to feature-libbfd
From: Song Liu @ 2019-02-28 5:06 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190228050643.958685-1-songliubraving@fb.com>
Both libbfd and libopcodes are distributed with binutil-dev/devel. When
libbfd presents, it is OK to assume libopcodes also presents. This has
been a safe assumption for bpftool.
This patch adds -lopcodes to perf/Makefile.config. libopcodes will be
used in the next commit for bpf annotation.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/Makefile.config | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index b441c88cafa1..e0bafbc273af 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -701,7 +701,7 @@ else
endif
ifeq ($(feature-libbfd), 1)
- EXTLIBS += -lbfd
+ EXTLIBS += -lbfd -lopcodes
else
# we are on a system that requires -liberty and (maybe) -lz
# to link against -lbfd; test each case individually here
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox