* [PATCH v4 perf,bpf 11/15] perf: add -lopcodes to feature-libbfd
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-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
* [PATCH net] netfilter: bridge: Don't sabotage nf_hook calls for an l3mdev slave
From: David Ahern @ 2019-02-26 0:21 UTC (permalink / raw)
To: pablo; +Cc: netdev, netfilter-devel, David Ahern
From: David Ahern <dsahern@gmail.com>
Followup to a173f066c7cf ("netfilter: bridge: Don't sabotage nf_hook
calls from an l3mdev"). Some packets (e.g., ndisc) do not have the skb
device flipped to the l3mdev (e.g., VRF) device. Update ip_sabotage_in
to not drop packets for slave devices too. Currently, neighbor
solicitation packets for 'dev -> bridge (addr) -> vrf' setups are getting
dropped. This patch enables IPv6 communications for bridges with an
address that are enslaved to a VRF.
Fixes: 73e20b761acf ("net: vrf: Add support for PREROUTING rules on vrf device")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/bridge/br_netfilter_hooks.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index c93c35bb73dd..8ab222babc98 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -831,7 +831,8 @@ static unsigned int ip_sabotage_in(void *priv,
struct nf_bridge_info *nf_bridge = nf_bridge_info_get(skb);
if (nf_bridge && !nf_bridge->in_prerouting &&
- !netif_is_l3_master(skb->dev)) {
+ !netif_is_l3_master(skb->dev) &&
+ !netif_is_l3_slave(skb->dev)) {
state->okfn(state->net, state->sk, skb);
return NF_STOLEN;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v4 perf,bpf 10/15] perf-top: add option --no-bpf-event
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
bpf events should be tracked by default for perf-top. This patch makes it
on by default, and adds option to disable bpf events.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/builtin-top.c | 3 +++
tools/perf/util/top.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 27d8d42e0a4d..ccdf5689452f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1492,6 +1492,7 @@ int cmd_top(int argc, const char **argv)
"Display raw encoding of assembly instructions (default)"),
OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
"Enable kernel symbol demangling"),
+ OPT_BOOLEAN(0, "no-bpf-event", &top.no_bpf_event, "do not record bpf events"),
OPT_STRING(0, "objdump", &top.annotation_opts.objdump_path, "path",
"objdump binary to use for disassembly and annotations"),
OPT_STRING('M', "disassembler-style", &top.annotation_opts.disassembler_style, "disassembler style",
@@ -1651,6 +1652,8 @@ int cmd_top(int argc, const char **argv)
signal(SIGWINCH, winch_sig);
}
+ top.record_opts.bpf_event = !top.no_bpf_event;
+
status = __cmd_top(&top);
out_delete_evlist:
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index 19f95eaf75c8..862a37bd27ea 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -32,6 +32,7 @@ struct perf_top {
bool use_tui, use_stdio;
bool vmlinux_warned;
bool dump_symtab;
+ bool no_bpf_event;
struct hist_entry *sym_filter_entry;
struct perf_evsel *sym_evsel;
struct perf_session *session;
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 12/15] perf, bpf: enable annotation of bpf program
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
This patch enables the annotation of bpf program.
A new dso type DSO_BINARY_TYPE__BPF_PROG_INFO is introduced to for BPF
programs. In symbol__disassemble(), DSO_BINARY_TYPE__BPF_PROG_INFO dso
calls into a new function symbol__disassemble_bpf(), where annotation
line information is filled based bpf_prog_info and btf saved in given
perf_env.
symbol__disassemble_bpf() uses libbfd to disassemble bpf programs.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/build/Makefile.feature | 6 +-
tools/perf/Makefile.config | 4 +
tools/perf/util/annotate.c | 149 ++++++++++++++++++++++++++++++++++-
tools/perf/util/dso.c | 1 +
tools/perf/util/dso.h | 33 +++++---
| 7 +-
tools/perf/util/symbol.c | 1 +
tools/perf/util/util.c | 10 +++
tools/perf/util/util.h | 2 +
9 files changed, 193 insertions(+), 20 deletions(-)
diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index 5467c6bf9ceb..4f35e9ff1e00 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -71,7 +71,8 @@ FEATURE_TESTS_BASIC := \
sdt \
setns \
libopencsd \
- libaio
+ libaio \
+ disassembler-four-args
# FEATURE_TESTS_BASIC + FEATURE_TESTS_EXTRA is the complete list
# of all feature tests
@@ -118,7 +119,8 @@ FEATURE_DISPLAY ?= \
lzma \
get_cpuid \
bpf \
- libaio
+ libaio \
+ disassembler-four-args
# Set FEATURE_CHECK_(C|LD)FLAGS-all for all FEATURE_TESTS features.
# If in the future we need per-feature checks/flags for features not
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index e0bafbc273af..ab223239f1fb 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -796,6 +796,10 @@ ifdef HAVE_KVM_STAT_SUPPORT
CFLAGS += -DHAVE_KVM_STAT_SUPPORT
endif
+ifeq ($(feature-disassembler-four-args), 1)
+ CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
+endif
+
ifeq (${IS_64_BIT}, 1)
ifndef NO_PERF_READ_VDSO32
$(call feature_check,compile-32)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 70de8f6b3aee..716fdb07f45e 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -9,6 +9,10 @@
#include <errno.h>
#include <inttypes.h>
+#include <bpf/bpf.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+#include <linux/btf.h>
#include "util.h"
#include "ui/ui.h"
#include "sort.h"
@@ -22,6 +26,7 @@
#include "annotate.h"
#include "evsel.h"
#include "evlist.h"
+#include "bpf-event.h"
#include "block-range.h"
#include "string2.h"
#include "arch/common.h"
@@ -29,6 +34,9 @@
#include <pthread.h>
#include <linux/bitops.h>
#include <linux/kernel.h>
+#include <bfd.h>
+#include <dis-asm.h>
+#include <bpf/libbpf.h>
/* FIXME: For the HE_COLORSET */
#include "ui/browser.h"
@@ -1672,6 +1680,143 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
return 0;
}
+static int symbol__disassemble_bpf(struct symbol *sym,
+ struct annotate_args *args)
+{
+ struct annotation *notes = symbol__annotation(sym);
+ struct annotation_options *opts = args->options;
+ struct bpf_prog_info_linear *info_linear;
+ struct bpf_prog_linfo *prog_linfo = NULL;
+ struct bpf_prog_info_node *info_node;
+ int len = sym->end - sym->start;
+ disassembler_ftype disassemble;
+ struct map *map = args->ms.map;
+ struct disassemble_info info;
+ struct dso *dso = map->dso;
+ int pc = 0, count, sub_id;
+ struct btf *btf = NULL;
+ char tpath[PATH_MAX];
+ size_t buf_size;
+ int nr_skip = 0;
+ int ret = -1;
+ char *buf;
+ bfd *bfdf;
+ FILE *s;
+
+ if (dso->binary_type != DSO_BINARY_TYPE__BPF_PROG_INFO)
+ return -1;
+
+ pr_debug("%s: handling sym %s addr %lx len %lx\n", __func__,
+ sym->name, sym->start, sym->end - sym->start);
+
+ memset(tpath, 0, sizeof(tpath));
+ get_exec_path(tpath, sizeof(tpath));
+
+ bfdf = bfd_openr(tpath, NULL);
+ assert(bfdf);
+ assert(bfd_check_format(bfdf, bfd_object));
+
+ s = open_memstream(&buf, &buf_size);
+ if (!s)
+ goto out;
+ init_disassemble_info(&info, s,
+ (fprintf_ftype) fprintf);
+
+ info.arch = bfd_get_arch(bfdf);
+ info.mach = bfd_get_mach(bfdf);
+
+ info_node = perf_env__find_bpf_prog_info(dso->bpf_prog.env,
+ dso->bpf_prog.id);
+ if (!info_node)
+ goto out;
+ info_linear = info_node->info_linear;
+ sub_id = dso->bpf_prog.sub_id;
+
+ info.buffer = (void *)(info_linear->info.jited_prog_insns);
+ info.buffer_length = info_linear->info.jited_prog_len;
+
+ if (info_linear->info.nr_line_info)
+ prog_linfo = bpf_prog_linfo__new(&info_linear->info);
+
+ if (info_linear->info.btf_id) {
+ struct btf_node *node;
+
+ node = perf_env__find_btf(dso->bpf_prog.env,
+ info_linear->info.btf_id);
+ if (node)
+ btf = btf__new((__u8 *)(node->data),
+ node->data_size);
+ }
+
+ disassemble_init_for_target(&info);
+
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+ disassemble = disassembler(info.arch,
+ bfd_big_endian(bfdf),
+ info.mach,
+ bfdf);
+#else
+ disassemble = disassembler(bfdf);
+#endif
+ assert(disassemble);
+
+ fflush(s);
+ do {
+ const struct bpf_line_info *linfo = NULL;
+ struct disasm_line *dl;
+ size_t prev_buf_size;
+ const char *srcline;
+ u64 addr;
+
+ addr = pc + ((u64 *)(info_linear->info.jited_ksyms))[sub_id];
+ count = disassemble(pc, &info);
+
+ if (prog_linfo)
+ linfo = bpf_prog_linfo__lfind_addr_func(prog_linfo,
+ addr, sub_id,
+ nr_skip);
+
+ if (linfo && btf) {
+ srcline = btf__name_by_offset(btf, linfo->line_off);
+ nr_skip++;
+ } else
+ srcline = NULL;
+
+ fprintf(s, "\n");
+ prev_buf_size = buf_size;
+ fflush(s);
+
+ if (!opts->hide_src_code && srcline) {
+ args->offset = -1;
+ args->line = strdup(srcline);
+ args->line_nr = 0;
+ args->ms.sym = sym;
+ dl = disasm_line__new(args);
+ if (dl)
+ annotation_line__add(&dl->al,
+ ¬es->src->source);
+ }
+
+ args->offset = pc;
+ args->line = buf + prev_buf_size;
+ args->line_nr = 0;
+ args->ms.sym = sym;
+ dl = disasm_line__new(args);
+ if (dl)
+ annotation_line__add(&dl->al, ¬es->src->source);
+
+ pc += count;
+ } while (count > 0 && pc < len);
+
+ ret = 0;
+out:
+ free(prog_linfo);
+ free(btf);
+ fclose(s);
+ bfd_close(bfdf);
+ return ret;
+}
+
static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
{
struct annotation_options *opts = args->options;
@@ -1699,7 +1844,9 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
pr_debug("annotating [%p] %30s : [%p] %30s\n",
dso, dso->long_name, sym, sym->name);
- if (dso__is_kcore(dso)) {
+ if (dso->binary_type == DSO_BINARY_TYPE__BPF_PROG_INFO) {
+ return symbol__disassemble_bpf(sym, args);
+ } else if (dso__is_kcore(dso)) {
kce.kcore_filename = symfs_filename;
kce.addr = map__rip_2objdump(map, sym->start);
kce.offs = sym->start;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 62c8cf622607..1798192bf0f9 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -181,6 +181,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
case DSO_BINARY_TYPE__KALLSYMS:
case DSO_BINARY_TYPE__GUEST_KALLSYMS:
case DSO_BINARY_TYPE__JAVA_JIT:
+ case DSO_BINARY_TYPE__BPF_PROG_INFO:
case DSO_BINARY_TYPE__NOT_FOUND:
ret = -1;
break;
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 8c8a7abe809d..f20d319463f1 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -14,6 +14,8 @@
#include "namespaces.h"
#include "build-id.h"
+struct perf_env;
+
enum dso_binary_type {
DSO_BINARY_TYPE__KALLSYMS = 0,
DSO_BINARY_TYPE__GUEST_KALLSYMS,
@@ -34,6 +36,7 @@ enum dso_binary_type {
DSO_BINARY_TYPE__KCORE,
DSO_BINARY_TYPE__GUEST_KCORE,
DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO,
+ DSO_BINARY_TYPE__BPF_PROG_INFO,
DSO_BINARY_TYPE__NOT_FOUND,
};
@@ -177,17 +180,25 @@ struct dso {
struct auxtrace_cache *auxtrace_cache;
int comp;
- /* dso data file */
- struct {
- struct rb_root cache;
- int fd;
- int status;
- u32 status_seen;
- size_t file_size;
- struct list_head open_entry;
- u64 debug_frame_offset;
- u64 eh_frame_hdr_offset;
- } data;
+ union {
+ /* dso data file */
+ struct {
+ struct rb_root cache;
+ int fd;
+ int status;
+ u32 status_seen;
+ size_t file_size;
+ struct list_head open_entry;
+ u64 debug_frame_offset;
+ u64 eh_frame_hdr_offset;
+ } data;
+ /* bpf prog information */
+ struct {
+ u32 id;
+ u32 sub_id;
+ struct perf_env *env;
+ } bpf_prog;
+ };
union { /* Tool specific area */
void *priv;
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ce5402101468..c39e107ac5b8 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -533,12 +533,7 @@ static int write_cmdline(struct feat_fd *ff,
int i, ret;
/* actual path to perf binary */
- ret = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
- if (ret <= 0)
- return -1;
-
- /* readlink() does not add null termination */
- buf[ret] = '\0';
+ get_exec_path(buf, sizeof(buf));
/* account for binary path */
n = perf_env.nr_cmdline + 1;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 48efad6d0f90..33ae59e89da2 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1441,6 +1441,7 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod,
case DSO_BINARY_TYPE__BUILD_ID_CACHE_DEBUGINFO:
return true;
+ case DSO_BINARY_TYPE__BPF_PROG_INFO:
case DSO_BINARY_TYPE__NOT_FOUND:
default:
return false;
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 093352e93d50..0bc9ec69f38a 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -506,3 +506,13 @@ const char *perf_tip(const char *dirpath)
return tip;
}
+
+void get_exec_path(char *tpath, size_t size)
+{
+ const char *path = "/proc/self/exe";
+ ssize_t len;
+
+ len = readlink(path, tpath, size - 1);
+ assert(len > 0);
+ tpath[len] = 0;
+}
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index ece040b799f6..8d0a88eb8685 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -76,6 +76,8 @@ extern bool perf_singlethreaded;
void perf_set_singlethreaded(void);
void perf_set_multithreaded(void);
+void get_exec_path(char *tpath, size_t size);
+
#ifndef O_CLOEXEC
#ifdef __sparc__
#define O_CLOEXEC 0x400000
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 13/15] perf, bpf: process PERF_BPF_EVENT_PROG_LOAD for annotation
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
This patch adds processing of PERF_BPF_EVENT_PROG_LOAD, which sets proper
DSO type/id/etc of memory regions mapped to BPF programs to
DSO_BINARY_TYPE__BPF_PROG_INFO
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/util/bpf-event.c | 53 +++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 370b830f2433..048ef00371ad 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -25,12 +25,65 @@ static int snprintf_hex(char *buf, size_t size, unsigned char *data, size_t len)
return ret;
}
+static int machine__process_bpf_event_load(struct machine *machine __maybe_unused,
+ union perf_event *event,
+ struct perf_sample *sample __maybe_unused)
+{
+ struct bpf_prog_info_linear *info_linear;
+ struct bpf_prog_info_node *info_node;
+ struct perf_env *env = machine->env;
+ int id = event->bpf_event.id;
+ unsigned int i;
+
+ /* perf-record, no need to handle bpf-event */
+ if (env == NULL)
+ return 0;
+
+ info_node = perf_env__find_bpf_prog_info(env, id);
+ if (!info_node)
+ return 0;
+ info_linear = info_node->info_linear;
+
+ for (i = 0; i < info_linear->info.nr_jited_ksyms; i++) {
+ u64 *addrs = (u64 *)(info_linear->info.jited_ksyms);
+ u64 addr = addrs[i];
+ struct map *map;
+
+ map = map_groups__find(&machine->kmaps, addr);
+
+ if (map) {
+ map->dso->binary_type = DSO_BINARY_TYPE__BPF_PROG_INFO;
+ map->dso->bpf_prog.id = id;
+ map->dso->bpf_prog.sub_id = i;
+ map->dso->bpf_prog.env = env;
+ }
+ }
+ return 0;
+}
+
int machine__process_bpf_event(struct machine *machine __maybe_unused,
union perf_event *event,
struct perf_sample *sample __maybe_unused)
{
if (dump_trace)
perf_event__fprintf_bpf_event(event, stdout);
+
+ switch (event->bpf_event.type) {
+ case PERF_BPF_EVENT_PROG_LOAD:
+ return machine__process_bpf_event_load(machine, event, sample);
+
+ case PERF_BPF_EVENT_PROG_UNLOAD:
+ /*
+ * Do not free bpf_prog_info and btf of the program here,
+ * as annotation still need them. They will be freed at
+ * the end of the session.
+ */
+ break;
+ default:
+ pr_debug("unexpected bpf_event type of %d\n",
+ event->bpf_event.type);
+ break;
+ }
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 09/15] perf, bpf: save btf information as headers to perf.data
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
This patch enables perf-record to save btf information as headers to
perf.data A new header type HEADER_BPF_BTF is introduced for this data.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
| 108 ++++++++++++++++++++++++++++++++++++++-
| 1 +
2 files changed, 108 insertions(+), 1 deletion(-)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index c89dce7942d1..ce5402101468 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1127,6 +1127,45 @@ static int write_bpf_prog_info(struct feat_fd *ff,
return ret;
}
+static int write_bpf_btf(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.btfs;
+ 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 btf_node *node;
+
+ node = rb_entry(next, struct btf_node, rb_node);
+ next = rb_next(&node->rb_node);
+ ret = do_write(ff, &node->id,
+ sizeof(u32) * 2 + node->data_size);
+ 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;
@@ -1630,6 +1669,28 @@ static void print_bpf_prog_info(struct feat_fd *ff, FILE *fp)
up_read(&env->bpf_progs.lock);
}
+static void print_bpf_btf(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.btfs;
+ next = rb_first(root);
+
+ while (next) {
+ struct btf_node *node;
+
+ node = rb_entry(next, struct btf_node, rb_node);
+ next = rb_next(&node->rb_node);
+ fprintf(fp, "# btf info of id %u\n", node->id);
+ }
+
+ up_read(&env->bpf_progs.lock);
+}
+
static void free_event_desc(struct perf_evsel *events)
{
struct perf_evsel *evsel;
@@ -2732,6 +2793,50 @@ static int process_bpf_prog_info(struct feat_fd *ff,
return err;
}
+static int process_bpf_btf(struct feat_fd *ff, void *data __maybe_unused)
+{
+ struct perf_env *env = &ff->ph->env;
+ u32 count, i;
+
+ if (do_read_u32(ff, &count))
+ return -1;
+
+ down_write(&env->bpf_progs.lock);
+
+ for (i = 0; i < count; ++i) {
+ struct btf_node *node;
+ u32 id, data_size;
+
+ if (do_read_u32(ff, &id))
+ return -1;
+ if (do_read_u32(ff, &data_size))
+ return -1;
+
+ node = malloc(sizeof(struct btf_node) + data_size);
+ if (!node)
+ return -1;
+
+ node->id = id;
+ node->data_size = data_size;
+
+ if (__do_read(ff, node->data, data_size)) {
+ free(node);
+ return -1;
+ }
+
+ /* endian mismatch, drop the btf, continue */
+ if (ff->ph->needs_swap) {
+ free(node);
+ continue;
+ }
+
+ perf_env__insert_btf(env, node);
+ }
+
+ up_write(&env->bpf_progs.lock);
+ return 0;
+}
+
struct feature_ops {
int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2792,7 +2897,8 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
FEAT_OPR(SAMPLE_TIME, sample_time, false),
FEAT_OPR(MEM_TOPOLOGY, mem_topology, true),
FEAT_OPR(CLOCKID, clockid, false),
- FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false)
+ FEAT_OPR(BPF_PROG_INFO, bpf_prog_info, false),
+ FEAT_OPR(BPF_BTF, bpf_btf, false)
};
struct header_print_data {
--git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 0785c91b4c3a..9e7d931f7c0d 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -40,6 +40,7 @@ enum {
HEADER_MEM_TOPOLOGY,
HEADER_CLOCKID,
HEADER_BPF_PROG_INFO,
+ HEADER_BPF_BTF,
HEADER_LAST_FEATURE,
HEADER_FEAT_BITS = 256,
};
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 05/15] perf: change prototype of perf_event__synthesize_bpf_events()
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-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 v4 perf,bpf 04/15] perf, bpf: synthesize bpf events with bpf_program__get_prog_info_linear()
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>
With bpf_program__get_prog_info_linear, we can simplify the logic that
synthesizes bpf events.
This patch doesn't change the behavior of the code.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/util/bpf-event.c | 118 ++++++++++++------------------------
1 file changed, 40 insertions(+), 78 deletions(-)
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 796ef793f4ce..e6dfb95029e5 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -3,7 +3,9 @@
#include <stdlib.h>
#include <bpf/bpf.h>
#include <bpf/btf.h>
+#include <bpf/libbpf.h>
#include <linux/btf.h>
+#include <linux/err.h>
#include "bpf-event.h"
#include "debug.h"
#include "symbol.h"
@@ -49,99 +51,62 @@ 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;
- u32 sub_prog_cnt, i, func_info_rec_size = 0;
- u8 (*prog_tags)[BPF_TAG_SIZE] = NULL;
- struct bpf_prog_info info = { .type = 0, };
- u32 info_len = sizeof(info);
- void *func_infos = NULL;
- u64 *prog_addrs = NULL;
+ struct bpf_prog_info_linear *info_linear;
+ struct bpf_prog_info *info;
struct btf *btf = NULL;
- u32 *prog_lens = NULL;
bool has_btf = false;
- char errbuf[512];
+ u32 sub_prog_cnt, i;
int err = 0;
+ u64 arrays;
- /* Call bpf_obj_get_info_by_fd() to get sizes of arrays */
- err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+ arrays = 1UL << BPF_PROG_INFO_JITED_KSYMS;
+ arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
+ arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
+ arrays |= 1UL << BPF_PROG_INFO_PROG_TAGS;
- if (err) {
- pr_debug("%s: failed to get BPF program info: %s, aborting\n",
- __func__, str_error_r(errno, errbuf, sizeof(errbuf)));
+ info_linear = bpf_program__get_prog_info_linear(fd, arrays);
+ if (IS_ERR_OR_NULL(info_linear)) {
+ info_linear = NULL;
+ pr_debug("%s: failed to get BPF program info. aborting\n", __func__);
return -1;
}
- if (info_len < offsetof(struct bpf_prog_info, prog_tags)) {
+
+ if (info_linear->info_len < offsetof(struct bpf_prog_info, prog_tags)) {
pr_debug("%s: the kernel is too old, aborting\n", __func__);
return -2;
}
+ info = &info_linear->info;
+
/* number of ksyms, func_lengths, and tags should match */
- sub_prog_cnt = info.nr_jited_ksyms;
- if (sub_prog_cnt != info.nr_prog_tags ||
- sub_prog_cnt != info.nr_jited_func_lens)
+ sub_prog_cnt = info->nr_jited_ksyms;
+ if (sub_prog_cnt != info->nr_prog_tags ||
+ sub_prog_cnt != info->nr_jited_func_lens)
return -1;
/* check BTF func info support */
- if (info.btf_id && info.nr_func_info && info.func_info_rec_size) {
+ if (info->btf_id && info->nr_func_info && info->func_info_rec_size) {
/* btf func info number should be same as sub_prog_cnt */
- if (sub_prog_cnt != info.nr_func_info) {
+ if (sub_prog_cnt != info->nr_func_info) {
pr_debug("%s: mismatch in BPF sub program count and BTF function info count, aborting\n", __func__);
- return -1;
- }
- if (btf__get_from_id(info.btf_id, &btf)) {
- pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info.btf_id);
- return -1;
+ err = -1;
+ goto out;
}
- func_info_rec_size = info.func_info_rec_size;
- func_infos = calloc(sub_prog_cnt, func_info_rec_size);
- if (!func_infos) {
- pr_debug("%s: failed to allocate memory for func_infos, aborting\n", __func__);
- return -1;
+ if (btf__get_from_id(info->btf_id, &btf)) {
+ pr_debug("%s: failed to get BTF of id %u, aborting\n", __func__, info->btf_id);
+ err = -1;
+ btf = NULL;
+ goto out;
}
has_btf = true;
}
- /*
- * We need address, length, and tag for each sub program.
- * Allocate memory and call bpf_obj_get_info_by_fd() again
- */
- prog_addrs = calloc(sub_prog_cnt, sizeof(u64));
- if (!prog_addrs) {
- pr_debug("%s: failed to allocate memory for prog_addrs, aborting\n", __func__);
- goto out;
- }
- prog_lens = calloc(sub_prog_cnt, sizeof(u32));
- if (!prog_lens) {
- pr_debug("%s: failed to allocate memory for prog_lens, aborting\n", __func__);
- goto out;
- }
- prog_tags = calloc(sub_prog_cnt, BPF_TAG_SIZE);
- if (!prog_tags) {
- pr_debug("%s: failed to allocate memory for prog_tags, aborting\n", __func__);
- goto out;
- }
-
- memset(&info, 0, sizeof(info));
- info.nr_jited_ksyms = sub_prog_cnt;
- info.nr_jited_func_lens = sub_prog_cnt;
- info.nr_prog_tags = sub_prog_cnt;
- info.jited_ksyms = ptr_to_u64(prog_addrs);
- info.jited_func_lens = ptr_to_u64(prog_lens);
- info.prog_tags = ptr_to_u64(prog_tags);
- info_len = sizeof(info);
- if (has_btf) {
- info.nr_func_info = sub_prog_cnt;
- info.func_info_rec_size = func_info_rec_size;
- info.func_info = ptr_to_u64(func_infos);
- }
-
- err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
- if (err) {
- pr_debug("%s: failed to get BPF program info, aborting\n", __func__);
- goto out;
- }
-
/* Synthesize PERF_RECORD_KSYMBOL */
for (i = 0; i < sub_prog_cnt; i++) {
+ u8 (*prog_tags)[BPF_TAG_SIZE] = (void *)(info->prog_tags);
+ __u32 *prog_lens = (__u32 *)(info->jited_func_lens);
+ __u64 *prog_addrs = (__u64 *)(info->jited_ksyms);
+ void *func_infos = (void *)(info->func_info);
const struct bpf_func_info *finfo;
const char *short_name = NULL;
const struct btf_type *t;
@@ -163,13 +128,13 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
KSYM_NAME_LEN - name_len,
prog_tags[i], BPF_TAG_SIZE);
if (has_btf) {
- finfo = func_infos + i * info.func_info_rec_size;
+ finfo = func_infos + i * info->func_info_rec_size;
t = btf__type_by_id(btf, finfo->type_id);
short_name = btf__name_by_offset(btf, t->name_off);
} else if (i == 0 && sub_prog_cnt == 1) {
/* no subprog */
- if (info.name[0])
- short_name = info.name;
+ if (info->name[0])
+ short_name = info->name;
} else
short_name = "F";
if (short_name)
@@ -195,9 +160,9 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
},
.type = PERF_BPF_EVENT_PROG_LOAD,
.flags = 0,
- .id = info.id,
+ .id = info->id,
};
- memcpy(bpf_event->tag, prog_tags[i], BPF_TAG_SIZE);
+ memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
memset((void *)event + event->header.size, 0, machine->id_hdr_size);
event->header.size += machine->id_hdr_size;
err = perf_tool__process_synth_event(tool, event,
@@ -205,10 +170,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
}
out:
- free(prog_tags);
- free(prog_lens);
- free(prog_addrs);
- free(func_infos);
+ free(info_linear);
free(btf);
return err ? -1 : 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 perf,bpf 01/15] perf, bpf: consider events with attr.bpf_event as side-band events
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-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 0a8dab322111..9403bdda5f8c 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 v4 perf,bpf 00/15] perf annotation of BPF programs
From: Song Liu @ 2019-02-26 0:20 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
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:
(from 1/10 to 10/10)
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")
========================================================================
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 information about 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/util/annotate.c | 149 ++++++++++++++++-
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 | 150 +++++++++++++++++
tools/perf/util/env.h | 21 +++
tools/perf/util/evlist.c | 105 ++++++++++++
tools/perf/util/evlist.h | 13 ++
tools/perf/util/header.c | 256 ++++++++++++++++++++++++++++-
tools/perf/util/header.h | 2 +
tools/perf/util/symbol.c | 1 +
tools/perf/util/top.h | 1 +
tools/perf/util/util.c | 10 ++
tools/perf/util/util.h | 2 +
24 files changed, 1389 insertions(+), 317 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH net-next] net: sched: don't release block->lock when dumping chains
From: Cong Wang @ 2019-02-26 0:15 UTC (permalink / raw)
To: Vlad Buslov
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
David Miller
In-Reply-To: <20190225154544.10453-1-vladbu@mellanox.com>
On Mon, Feb 25, 2019 at 7:45 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Function tc_dump_chain() obtains and releases block->lock on each iteration
> of its inner loop that dumps all chains on block. Outputting chain template
> info is fast operation so locking/unlocking mutex multiple times is an
> overhead when lock is highly contested. Modify tc_dump_chain() to only
> obtain block->lock once and dump all chains without releasing it.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Thanks for the followup!
Isn't it similar for __tcf_get_next_proto() in tcf_chain_dump()?
And for tc_dump_tfilter()?
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-26 0:09 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <5079b888-a62f-991a-6f0b-1248d23d0045@gmail.com>
On 2/25/19 11:31 PM, Eric Dumazet wrote:
> On 02/25/2019 03:21 PM, Dmitry Safonov wrote:
>> Well, sure - but it seems confusing that rtnl_unregister() will require
>> synchronize_rcu(), while rtnl_unregister_all() will not.
>
> rtnl_unregister_all() is a different beast, since it removes the whole rtnl_msg_handlers[protocol]
>
> rtnl_unregister() only removes a subset, with different usages.
>
>> And I thought no one would care about another synchronize_rcu() in exit
>> path.
>
> We definitely care about things be done properly.
>
> If synchronize_rcu() is needed there, be it, but kfree_rcu() seems to be fine.
>
> In any case, I believe you need to more carefully explain what is the problem here,
> because I could not really see it.
Ugh, sorry - it seems that I haven't had enough coffee today.
I've read again rtnetlink_rcv_msg(), who is the only user of
rtnl_msg_handlers[] under read_lock, and it calls try_module_get(owner).
Sorry for the noise,
Dmitry
^ permalink raw reply
* [PATCH] netlabel: fix out-of-bounds memory accesses
From: Paul Moore @ 2019-02-26 0:06 UTC (permalink / raw)
To: netdev; +Cc: linux-security-module, selinux
There are two array out-of-bounds memory accesses, one in
cipso_v4_map_lvl_valid(), the other in netlbl_bitmap_walk(). Both
errors are embarassingly simple, and the fixes are straightforward.
As a FYI for anyone backporting this patch to kernels prior to v4.8,
you'll want to apply the netlbl_bitmap_walk() patch to
cipso_v4_bitmap_walk() as netlbl_bitmap_walk() doesn't exist before
Linux v4.8.
Reported-by: Jann Horn <jannh@google.com>
Fixes: 446fda4f2682 ("[NetLabel]: CIPSOv4 engine")
Fixes: 3faa8f982f95 ("netlabel: Move bitmap manipulation functions to the NetLabel core.")
Signed-off-by: Paul Moore <paul@paul-moore.com>
---
net/ipv4/cipso_ipv4.c | 3 ++-
net/netlabel/netlabel_kapi.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b7fb13..f4b83de2263e 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -667,7 +667,8 @@ static int cipso_v4_map_lvl_valid(const struct cipso_v4_doi *doi_def, u8 level)
case CIPSO_V4_MAP_PASS:
return 0;
case CIPSO_V4_MAP_TRANS:
- if (doi_def->map.std->lvl.cipso[level] < CIPSO_V4_INV_LVL)
+ if ((level < doi_def->map.std->lvl.cipso_size) &&
+ (doi_def->map.std->lvl.cipso[level] < CIPSO_V4_INV_LVL))
return 0;
break;
}
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index ea7c67050792..ee3e5b6471a6 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -903,7 +903,8 @@ int netlbl_bitmap_walk(const unsigned char *bitmap, u32 bitmap_len,
(state == 0 && (byte & bitmask) == 0))
return bit_spot;
- bit_spot++;
+ if (++bit_spot >= bitmap_len)
+ return -1;
bitmask >>= 1;
if (bitmask == 0) {
byte = bitmap[++byte_offset];
^ permalink raw reply related
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Yafang Shao @ 2019-02-25 23:54 UTC (permalink / raw)
To: Cong Wang
Cc: David Miller, Daniel Borkmann, Eric Dumazet, Joe Perches,
Linux Kernel Network Developers, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
On Tue, Feb 26, 2019 at 6:58 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Mon, Feb 25, 2019 at 2:29 PM David Miller <davem@davemloft.net> wrote:
> >
> > From: Cong Wang <xiyou.wangcong@gmail.com>
> > Date: Mon, 25 Feb 2019 14:00:09 -0800
> >
> > > Just to clarify, I have been suggesting to completely remove
> > > this unused macro, never suggest to just undefine it in-tree.
> > >
> > > There is no reason to keep it in-tree, whether defined or undefined,
> > > just for downstream users.
> >
> > And this is where you and I fundamentally disagree.
>
> So you agree that I can add debugging printk's only for my own use?
> I can claim that I only use them downstream and you can't force me
> to carry local changes?
>
> If not, what is your criteria for accepting debugging printk's? Whose
> can be accepted and whose can't?
>
> Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
>
Per my personal view, I agree with you that we should remove it completely.
Clean up such kind of legacy code can make the kernel more clean.
Thanks
Yafang
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-25 23:52 UTC (permalink / raw)
To: Stanislav Fomichev, Alexei Starovoitov
Cc: davem@davemloft.net, daniel@iogearbox.net, edumazet@google.com,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <20190225230703.GB32115@mini-arch>
On 2/25/19 3:07 PM, Stanislav Fomichev wrote:
>> +#define BPF_PROG_RUN(prog, ctx) ({ \
>> + u32 ret; \
>> + cant_sleep(); \
>> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
>> + struct bpf_prog_stats *stats; \
>> + u64 start = sched_clock(); \
> QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
> in the bpf_test_run()? Or even why not local_clock?
> I'm just wondering what king of trade off we are doing here
> regarding precision vs run time cost.
I'm making this decision based on documentation:
Documentation/timers/timekeeping.txt
"Compared to clock sources, sched_clock() has to be very fast: it is
called much more often, especially by the scheduler. If you have to do
trade-offs between accuracy compared to the clock source, you may
sacrifice accuracy for speed in sched_clock()."
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Eric Dumazet @ 2019-02-25 23:31 UTC (permalink / raw)
To: Dmitry Safonov, Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <d20c023b-33d5-4c48-a404-e2ba05f98a99@arista.com>
On 02/25/2019 03:21 PM, Dmitry Safonov wrote:
> Hi Eric,
>
> On 2/25/19 11:09 PM, Eric Dumazet wrote:
>> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>>> While it's possible to document that rtnl_unregister() requires
>>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>>> the module exit is very much slow-path.
>>
>> rtnl_unregister_all() needs the sychronize_rcu() at this moment
>> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> I may be wrong here, but shouldn't we wait for grace period to elapse by
> the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
> Like, without synchronize_net() in rtnl_unregister() - what prevents
> module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
>
>
>>> --- a/net/core/rtnetlink.c
>>> +++ b/net/core/rtnetlink.c
>>> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
>>> rcu_assign_pointer(tab[msgindex], NULL);
>>> rtnl_unlock();
>>>
>>> - kfree_rcu(link, rcu);
>>> + synchronize_net();
>>> +
>>> + kfree(link);
>>
>>
>> I really do not see a difference here (other than this being much slower of course)
>>
>> If the caller needs rcu_barrier(), then add it in the caller ?
>
> Well, sure - but it seems confusing that rtnl_unregister() will require
> synchronize_rcu(), while rtnl_unregister_all() will not.
rtnl_unregister_all() is a different beast, since it removes the whole rtnl_msg_handlers[protocol]
rtnl_unregister() only removes a subset, with different usages.
> And I thought no one would care about another synchronize_rcu() in exit
> path.
We definitely care about things be done properly.
If synchronize_rcu() is needed there, be it, but kfree_rcu() seems to be fine.
In any case, I believe you need to more carefully explain what is the problem here,
because I could not really see it.
^ permalink raw reply
* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Paul Moore @ 2019-02-25 23:30 UTC (permalink / raw)
To: David Miller; +Cc: s-nazarov, netdev, linux-security-module, kuznet, yoshfuji
In-Reply-To: <20190225.143355.245256286926542053.davem@davemloft.net>
On Mon, Feb 25, 2019 at 5:33 PM David Miller <davem@davemloft.net> wrote:
> From: Nazarov Sergey <s-nazarov@yandex.ru>
> Date: Mon, 25 Feb 2019 19:24:15 +0300
>
> > Add __icmp_send function having ip_options struct parameter
> >
> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
>
> Applied with Subject line fixes up. This commit doesn't even make
> changes to cipse_v4_error().
>
> Anyone who ACK'd this change or added their Reviewed-by did not read
> this email and are just rubber stamping crap.
I should have checked the subject line closer that's my fault (Gmail
does interesting things to threads sometimes, and obscured the subject
line), however, I did look at the content of patch before giving it my
thumbs up. Claiming the email wasn't read isn't correct (although you
could rightly argue I didn't read the subject line), or I'm "rubber
stamping crap" isn't correct.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-25 23:27 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <d20c023b-33d5-4c48-a404-e2ba05f98a99@arista.com>
On 2/25/19 11:21 PM, Dmitry Safonov wrote:
> Hi Eric,
>
> On 2/25/19 11:09 PM, Eric Dumazet wrote:
>> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>>> While it's possible to document that rtnl_unregister() requires
>>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>>> the module exit is very much slow-path.
>>
>> rtnl_unregister_all() needs the sychronize_rcu() at this moment
>> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> I may be wrong here, but shouldn't we wait for grace period to elapse by
> the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
> Like, without synchronize_net() in rtnl_unregister() - what prevents
> module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
There is synchronize_rcu() in free_module() - but I believe it's a bit
too far in unloading. Maybe, I'm missing another call on the way.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Dmitry Safonov @ 2019-02-25 23:21 UTC (permalink / raw)
To: Eric Dumazet, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <eb7bca34-2b6a-1140-b230-71f333b42807@gmail.com>
Hi Eric,
On 2/25/19 11:09 PM, Eric Dumazet wrote:
> On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
>> While it's possible to document that rtnl_unregister() requires
>> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
>> the module exit is very much slow-path.
>
> rtnl_unregister_all() needs the sychronize_rcu() at this moment
> because of the kfree(tab), not because of the kfree_rcu(link, rcu);
I may be wrong here, but shouldn't we wait for grace period to elapse by
the reason that rtnl_msg_handlers are protected by RCU, not only by rtnl?
Like, without synchronize_net() in rtnl_unregister() - what prevents
module exit race to say, rtnetlink_rcv_msg()=>rtnl_get_link()?
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
>> rcu_assign_pointer(tab[msgindex], NULL);
>> rtnl_unlock();
>>
>> - kfree_rcu(link, rcu);
>> + synchronize_net();
>> +
>> + kfree(link);
>
>
> I really do not see a difference here (other than this being much slower of course)
>
> If the caller needs rcu_barrier(), then add it in the caller ?
Well, sure - but it seems confusing that rtnl_unregister() will require
synchronize_rcu(), while rtnl_unregister_all() will not.
And I thought no one would care about another synchronize_rcu() in exit
path.
Thanks,
Dmitry
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: Cong Wang @ 2019-02-25 23:18 UTC (permalink / raw)
To: David Miller, LKML
Cc: Yafang Shao, Daniel Borkmann, Eric Dumazet, Joe Perches,
Linux Kernel Network Developers, shaoyafang
In-Reply-To: <20190225.151141.792446098735157842.davem@davemloft.net>
(Cc'ing LKML)
On Mon, Feb 25, 2019 at 3:11 PM David Miller <davem@davemloft.net> wrote:
>
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Mon, 25 Feb 2019 14:58:36 -0800
>
> > Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
>
> And this isn't happening either Cong, I reserve the right to apply my
> judgment as networking maintainer on a case by case basis as I so see
> fit.
Thanks for rephrasing non-transparency in such a wonderful way.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Stanislav Fomichev @ 2019-02-25 23:14 UTC (permalink / raw)
To: brakmo
Cc: netdev, Martin Lau, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Kernel Team
In-Reply-To: <20190223010703.678070-3-brakmo@fb.com>
On 02/22, brakmo wrote:
> From: Martin KaFai Lau <kafai@fb.com>
>
> This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr
> "int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)".
> It is added to BPF_PROG_TYPE_CGROUP_SKB which can be attached
> to the egress path where the bpf prog is called by
> ip_finish_output() or ip6_finish_output(). The verifier
> ensures that the parameter must be a tcp_sock.
>
> This helper makes a tcp_sock enter CWR state. It can be used
> by a bpf_prog to manage egress network bandwidth limit per
> cgroupv2. A later patch will have a sample program to
> show how it can be used to limit bandwidth usage per cgroupv2.
>
> To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
> attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
> during load time if the prog uses this new helper.
> The newly added prog->enforce_expected_attach_type bit will also be set
> if this new helper is used. This bit is for backward compatibility reason
> because currently prog->expected_attach_type has been ignored in
> BPF_PROG_TYPE_CGROUP_SKB. During attach time,
> prog->expected_attach_type is only enforced if the
> prog->enforce_expected_attach_type bit is set.
> i.e. prog->expected_attach_type is only enforced if this new helper
> is used by the prog.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> include/linux/bpf.h | 1 +
> include/linux/filter.h | 3 ++-
> include/uapi/linux/bpf.h | 9 ++++++++-
> kernel/bpf/syscall.c | 12 ++++++++++++
> kernel/bpf/verifier.c | 4 ++++
> net/core/filter.c | 25 +++++++++++++++++++++++++
> 6 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index d5ba2fc01af3..2d54ba7cf9dd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -195,6 +195,7 @@ enum bpf_arg_type {
> ARG_PTR_TO_SOCKET, /* pointer to bpf_sock */
> ARG_PTR_TO_SPIN_LOCK, /* pointer to bpf_spin_lock */
> ARG_PTR_TO_SOCK_COMMON, /* pointer to sock_common */
> + ARG_PTR_TO_TCP_SOCK, /* pointer to tcp_sock */
> };
>
> /* type of values returned from helper functions */
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3eca5a04..c6e878bdc5a6 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -510,7 +510,8 @@ struct bpf_prog {
> blinded:1, /* Was blinded */
> is_func:1, /* program is a bpf function */
> kprobe_override:1, /* Do we override a kprobe? */
> - has_callchain_buf:1; /* callchain buffer allocated? */
> + has_callchain_buf:1, /* callchain buffer allocated? */
> + enforce_expected_attach_type:1; /* Enforce expected_attach_type checking at attach time */
> enum bpf_prog_type type; /* Type of BPF program */
> enum bpf_attach_type expected_attach_type; /* For some prog types */
> u32 len; /* Number of filter blocks */
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..95b5058fa945 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2359,6 +2359,12 @@ union bpf_attr {
> * Return
> * A **struct bpf_tcp_sock** pointer on success, or NULL in
> * case of failure.
> + *
> + * int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)
> + * Description
> + * Make a tcp_sock enter CWR state.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2457,7 +2463,8 @@ union bpf_attr {
> FN(spin_lock), \
> FN(spin_unlock), \
> FN(sk_fullsock), \
> - FN(tcp_sock),
> + FN(tcp_sock), \
> + FN(tcp_enter_cwr),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ec7c552af76b..9a478f2875cd 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1482,6 +1482,14 @@ bpf_prog_load_check_attach_type(enum bpf_prog_type prog_type,
> default:
> return -EINVAL;
> }
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + switch (expected_attach_type) {
> + case BPF_CGROUP_INET_INGRESS:
> + case BPF_CGROUP_INET_EGRESS:
> + return 0;
> + default:
> + return -EINVAL;
> + }
> default:
> return 0;
> }
> @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> case BPF_PROG_TYPE_CGROUP_SOCK:
> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + return prog->enforce_expected_attach_type &&
> + prog->expected_attach_type != attach_type ?
> + -EINVAL : 0;
> default:
> return 0;
> }
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b9496c41383..95fb385c6f3c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2424,6 +2424,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> return -EFAULT;
> }
> meta->ptr_id = reg->id;
> + } else if (arg_type == ARG_PTR_TO_TCP_SOCK) {
> + expected_type = PTR_TO_TCP_SOCK;
> + if (type != expected_type)
> + goto err_type;
> } else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
> if (meta->func_id == BPF_FUNC_spin_lock) {
> if (process_spin_lock(env, regno, true))
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 97916eedfe69..ca57ef25279c 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> .arg1_type = ARG_PTR_TO_SOCK_COMMON,
> };
>
> +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
> +{
> + struct sock *sk = (struct sock *)tp;
> +
> + if (sk->sk_state == TCP_ESTABLISHED) {
> + tcp_enter_cwr(sk);
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +
> +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> + .func = bpf_tcp_enter_cwr,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_TCP_SOCK,
> +};
> #endif /* CONFIG_INET */
>
> bool bpf_helper_changes_pkt_data(void *func)
> @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> #ifdef CONFIG_INET
> case BPF_FUNC_tcp_sock:
> return &bpf_tcp_sock_proto;
[...]
> + case BPF_FUNC_tcp_enter_cwr:
> + if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> + prog->enforce_expected_attach_type = 1;
> + return &bpf_tcp_enter_cwr_proto;
Instead of this back and forth with enforce_expected_attach_type, can we
just do here:
if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
return &bpf_tcp_enter_cwr_proto;
else
return null;
Wouldn't it have the same effect?
> + } else {
> + return NULL;
> + }
> #endif
> default:
> return sk_filter_func_proto(func_id, prog);
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: David Miller @ 2019-02-25 23:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: laoar.shao, daniel, edumazet, joe, netdev, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 25 Feb 2019 14:58:36 -0800
> Please be specific, and ideally make it a formal doc in netdev-FAQ.txt.
And this isn't happening either Cong, I reserve the right to apply my
judgment as networking maintainer on a case by case basis as I so see
fit.
^ permalink raw reply
* Re: [PATCH v3 net-next 2/2] net: sock: undefine SOCK_DEBUGGING
From: David Miller @ 2019-02-25 23:11 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: laoar.shao, daniel, edumazet, joe, netdev, shaoyafang
In-Reply-To: <CAM_iQpUknnab0bwJgp3-F864-02LsH+J07c8SzMZyYK84yoezw@mail.gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 25 Feb 2019 14:58:36 -0800
> So you agree that I can add debugging printk's only for my own use?
Let's put it this way.
If I ignore the fact that some of the data centers with the furtest
reach on the entire planet use something that is in the tree already,
then I am being unreasonable and insensitive.
This means no adding, but keeping things we already have.
Don't try to weasel word what I'm trying to say here Cong.
Have a nice day.
^ permalink raw reply
* Re: [PATCH] rtnetlink: Synchronze net in rtnl_unregister()
From: Eric Dumazet @ 2019-02-25 23:09 UTC (permalink / raw)
To: Dmitry Safonov, linux-kernel
Cc: 0x7f454c46, David S. Miller, Florian Westphal,
Hannes Frederic Sowa, netdev
In-Reply-To: <20190225212721.2908-1-dima@arista.com>
On 02/25/2019 01:27 PM, Dmitry Safonov wrote:
> rtnl_unregister() unsets handler from table, which is protected
> by rtnl_lock or RCU. At this moment only dump handlers access the table
> with rcu_lock(). Every other user accesses under rtnl.
>
> Callers may expect that rtnl_unregister() prevents any further handlers
> calls, alike rtnl_unregister_all(). And they do expect it.
>
> I've looked on in-tree caller uses:
> br_mdb: safe, but in err-path br_netlink_init()
> fib_rules: safe - err-path is very early in __init
> ip6mr: safe - following unregister_pernet_subsys() calls internally rcu_barrier()
> qrtr: safe - following sock_unregister() calls internally synchronize_rcu()
If rcu_barrier() was needed, then all callers should use it.
If synchronize_rcu() was needed, then all callers should use it.
But mixing is probably wrong.
>
> While it's possible to document that rtnl_unregister() requires
> synchronize_net() afterwards - unlike rtnl_unregister_all(), I believe
> the module exit is very much slow-path.
rtnl_unregister_all() needs the sychronize_rcu() at this moment
because of the kfree(tab), not because of the kfree_rcu(link, rcu);
>
> Issue seems to be very theoretical and unlikely, so I'm not Cc'ing
> stable tree.
>
> Fixes: 6853dd488119 ("rtnetlink: protect handler table with rcu")
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Florian Westphal <fw@strlen.de>
> Cc: "Hannes Frederic Sowa" <hannes@stressinduktion.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
> net/core/rtnetlink.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5ea1bed08ede..3db70da4f951 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -308,7 +308,9 @@ int rtnl_unregister(int protocol, int msgtype)
> rcu_assign_pointer(tab[msgindex], NULL);
> rtnl_unlock();
>
> - kfree_rcu(link, rcu);
> + synchronize_net();
> +
> + kfree(link);
I really do not see a difference here (other than this being much slower of course)
If the caller needs rcu_barrier(), then add it in the caller ?
^ permalink raw reply
* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Stanislav Fomichev @ 2019-02-25 23:07 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, daniel, edumazet, netdev, bpf, kernel-team
In-Reply-To: <20190225222842.2031962-2-ast@kernel.org>
On 02/25, Alexei Starovoitov wrote:
> JITed BPF programs are indistinguishable from kernel functions, but unlike
> kernel code BPF code can be changed often.
> Typical approach of "perf record" + "perf report" profiling and tuning of
> kernel code works just as well for BPF programs, but kernel code doesn't
> need to be monitored whereas BPF programs do.
> Users load and run large amount of BPF programs.
> These BPF stats allow tools monitor the usage of BPF on the server.
> The monitoring tools will turn sysctl kernel.bpf_stats_enabled
> on and off for few seconds to sample average cost of the programs.
> Aggregated data over hours and days will provide an insight into cost of BPF
> and alarms can trigger in case given program suddenly gets more expensive.
>
> The cost of two sched_clock() per program invocation adds ~20 nsec.
> Fast BPF progs (like selftests/bpf/progs/test_pkt_access.c) will slow down
> from ~10 nsec to ~30 nsec.
> static_key minimizes the cost of the stats collection.
> There is no measurable difference before/after this patch
> with kernel.bpf_stats_enabled=0
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> include/linux/bpf.h | 9 +++++++++
> include/linux/filter.h | 20 +++++++++++++++++++-
> kernel/bpf/core.c | 31 +++++++++++++++++++++++++++++--
> kernel/bpf/syscall.c | 34 ++++++++++++++++++++++++++++++++--
> kernel/bpf/verifier.c | 7 ++++++-
> kernel/sysctl.c | 34 ++++++++++++++++++++++++++++++++++
> 6 files changed, 129 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index de18227b3d95..a2132e09dc1c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -16,6 +16,7 @@
> #include <linux/rbtree_latch.h>
> #include <linux/numa.h>
> #include <linux/wait.h>
> +#include <linux/u64_stats_sync.h>
>
> struct bpf_verifier_env;
> struct perf_event;
> @@ -340,6 +341,12 @@ enum bpf_cgroup_storage_type {
>
> #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
>
> +struct bpf_prog_stats {
> + u64 cnt;
> + u64 nsecs;
> + struct u64_stats_sync syncp;
> +};
> +
> struct bpf_prog_aux {
> atomic_t refcnt;
> u32 used_map_cnt;
> @@ -389,6 +396,7 @@ struct bpf_prog_aux {
> * main prog always has linfo_idx == 0
> */
> u32 linfo_idx;
> + struct bpf_prog_stats __percpu *stats;
> union {
> struct work_struct work;
> struct rcu_head rcu;
> @@ -559,6 +567,7 @@ void bpf_map_area_free(void *base);
> void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
>
> extern int sysctl_unprivileged_bpf_disabled;
> +extern int sysctl_bpf_stats_enabled;
>
> int bpf_map_new_fd(struct bpf_map *map, int flags);
> int bpf_prog_new_fd(struct bpf_prog *prog);
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3eca5a04..7e5e3db11106 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -533,7 +533,24 @@ struct sk_filter {
> struct bpf_prog *prog;
> };
>
> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> +
> +#define BPF_PROG_RUN(prog, ctx) ({ \
> + u32 ret; \
> + cant_sleep(); \
> + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> + struct bpf_prog_stats *stats; \
> + u64 start = sched_clock(); \
QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
in the bpf_test_run()? Or even why not local_clock?
I'm just wondering what king of trade off we are doing here
regarding precision vs run time cost.
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + stats = this_cpu_ptr(prog->aux->stats); \
> + u64_stats_update_begin(&stats->syncp); \
> + stats->cnt++; \
> + stats->nsecs += sched_clock() - start; \
> + u64_stats_update_end(&stats->syncp); \
> + } else { \
> + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> + } \
> + ret; })
>
> #define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
>
> @@ -764,6 +781,7 @@ void bpf_prog_free_jited_linfo(struct bpf_prog *prog);
> void bpf_prog_free_unused_jited_linfo(struct bpf_prog *prog);
>
> struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags);
> +struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags);
> struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
> gfp_t gfp_extra_flags);
> void __bpf_prog_free(struct bpf_prog *fp);
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ef88b167959d..1c14c347f3cf 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -78,7 +78,7 @@ void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, uns
> return NULL;
> }
>
> -struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
> +struct bpf_prog *bpf_prog_alloc_no_stats(unsigned int size, gfp_t gfp_extra_flags)
> {
> gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
> struct bpf_prog_aux *aux;
> @@ -104,6 +104,26 @@ struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
>
> return fp;
> }
> +
> +struct bpf_prog *bpf_prog_alloc(unsigned int size, gfp_t gfp_extra_flags)
> +{
> + gfp_t gfp_flags = GFP_KERNEL | __GFP_ZERO | gfp_extra_flags;
> + struct bpf_prog *prog;
> +
> + prog = bpf_prog_alloc_no_stats(size, gfp_extra_flags);
> + if (!prog)
> + return NULL;
> +
> + prog->aux->stats = alloc_percpu_gfp(struct bpf_prog_stats, gfp_flags);
> + if (!prog->aux->stats) {
> + kfree(prog->aux);
> + vfree(prog);
> + return NULL;
> + }
> +
> + u64_stats_init(&prog->aux->stats->syncp);
> + return prog;
> +}
> EXPORT_SYMBOL_GPL(bpf_prog_alloc);
>
> int bpf_prog_alloc_jited_linfo(struct bpf_prog *prog)
> @@ -231,7 +251,10 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, unsigned int size,
>
> void __bpf_prog_free(struct bpf_prog *fp)
> {
> - kfree(fp->aux);
> + if (fp->aux) {
> + free_percpu(fp->aux->stats);
> + kfree(fp->aux);
> + }
> vfree(fp);
> }
>
> @@ -2069,6 +2092,10 @@ int __weak skb_copy_bits(const struct sk_buff *skb, int offset, void *to,
> return -EFAULT;
> }
>
> +DEFINE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> +EXPORT_SYMBOL(bpf_stats_enabled_key);
> +int sysctl_bpf_stats_enabled __read_mostly;
> +
> /* All definitions of tracepoints related to BPF. */
> #define CREATE_TRACE_POINTS
> #include <linux/bpf_trace.h>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index ec7c552af76b..31cf66fc3f5c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1283,24 +1283,54 @@ static int bpf_prog_release(struct inode *inode, struct file *filp)
> return 0;
> }
>
> +static void bpf_prog_get_stats(const struct bpf_prog *prog,
> + struct bpf_prog_stats *stats)
> +{
> + u64 nsecs = 0, cnt = 0;
> + int cpu;
> +
> + for_each_possible_cpu(cpu) {
> + const struct bpf_prog_stats *st;
> + unsigned int start;
> + u64 tnsecs, tcnt;
> +
> + st = per_cpu_ptr(prog->aux->stats, cpu);
> + do {
> + start = u64_stats_fetch_begin_irq(&st->syncp);
> + tnsecs = st->nsecs;
> + tcnt = st->cnt;
> + } while (u64_stats_fetch_retry_irq(&st->syncp, start));
> + nsecs += tnsecs;
> + cnt += tcnt;
> + }
> + stats->nsecs = nsecs;
> + stats->cnt = cnt;
> +}
> +
> #ifdef CONFIG_PROC_FS
> static void bpf_prog_show_fdinfo(struct seq_file *m, struct file *filp)
> {
> const struct bpf_prog *prog = filp->private_data;
> char prog_tag[sizeof(prog->tag) * 2 + 1] = { };
> + struct bpf_prog_stats stats;
>
> + bpf_prog_get_stats(prog, &stats);
> bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> seq_printf(m,
> "prog_type:\t%u\n"
> "prog_jited:\t%u\n"
> "prog_tag:\t%s\n"
> "memlock:\t%llu\n"
> - "prog_id:\t%u\n",
> + "prog_id:\t%u\n"
> + "run_time_ns:\t%llu\n"
> + "run_cnt:\t%llu\n",
> prog->type,
> prog->jited,
> prog_tag,
> prog->pages * 1ULL << PAGE_SHIFT,
> - prog->aux->id);
> + prog->aux->id,
> + stats.nsecs,
> + stats.cnt);
> }
> #endif
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1b9496c41383..0e4edd7e3c5f 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -7320,7 +7320,12 @@ static int jit_subprogs(struct bpf_verifier_env *env)
> subprog_end = env->subprog_info[i + 1].start;
>
> len = subprog_end - subprog_start;
> - func[i] = bpf_prog_alloc(bpf_prog_size(len), GFP_USER);
> + /* BPF_PROG_RUN doesn't call subprogs directly,
> + * hence main prog stats include the runtime of subprogs.
> + * subprogs don't have IDs and not reachable via prog_get_next_id
> + * func[i]->aux->stats will never be accessed and stays NULL
> + */
> + func[i] = bpf_prog_alloc_no_stats(bpf_prog_size(len), GFP_USER);
> if (!func[i])
> goto out_free;
> memcpy(func[i]->insnsi, &prog->insnsi[subprog_start],
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index ba4d9e85feb8..86e0771352f2 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -224,6 +224,9 @@ static int proc_dostring_coredump(struct ctl_table *table, int write,
> #endif
> static int proc_dopipe_max_size(struct ctl_table *table, int write,
> void __user *buffer, size_t *lenp, loff_t *ppos);
> +static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
> + void __user *buffer, size_t *lenp,
> + loff_t *ppos);
>
> #ifdef CONFIG_MAGIC_SYSRQ
> /* Note: sysrq code uses its own private copy */
> @@ -1230,6 +1233,15 @@ static struct ctl_table kern_table[] = {
> .extra2 = &one,
> },
> #endif
> + {
> + .procname = "bpf_stats_enabled",
> + .data = &sysctl_bpf_stats_enabled,
> + .maxlen = sizeof(sysctl_bpf_stats_enabled),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax_bpf_stats,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> #if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU)
> {
> .procname = "panic_on_rcu_stall",
> @@ -3260,6 +3272,28 @@ int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int write,
>
> #endif /* CONFIG_PROC_SYSCTL */
>
> +static int proc_dointvec_minmax_bpf_stats(struct ctl_table *table, int write,
> + void __user *buffer, size_t *lenp,
> + loff_t *ppos)
> +{
> + int ret, bpf_stats = *(int *)table->data;
> + struct ctl_table tmp = *table;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + tmp.data = &bpf_stats;
> + ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
> + if (write && !ret) {
> + *(int *)table->data = bpf_stats;
> + if (bpf_stats)
> + static_branch_enable(&bpf_stats_enabled_key);
> + else
> + static_branch_disable(&bpf_stats_enabled_key);
> + }
> + return ret;
> +}
> +
> /*
> * No sense putting this after each symbol definition, twice,
> * exception granted :-)
> --
> 2.20.0
>
^ permalink raw reply
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