* [RFC perf,bpf 4/5] perf util: introduce bpf_prog_info_event
From: Song Liu @ 2018-11-06 20:52 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: kernel-team, Song Liu, ast, daniel, peterz, acme
In-Reply-To: <20181106205246.567448-1-songliubraving@fb.com>
This patch introduces struct bpf_prog_info_event to union perf_event.
struct bpf_prog_info_event {
struct perf_event_header header;
u32 prog_info_len;
u32 ksym_table_len;
u64 ksym_table;
struct bpf_prog_info prog_info;
char data[];
};
struct bpf_prog_info_event contains information about a bpf program.
These events are written to perf.data by perf-record, and processed by
perf-report.
struct bpf_prog_info_event uses arrays for some data (ksym_table, and
arrays in struct bpf_prog_info). To make these arrays easy to serialize,
we allocate continuous memory (data). These array pointers are translated
to offset in bpf_prog_info_event before written to file. And vice-versa
when the event is read from file.
This patch enables synthesizing these events at the beginning of
perf-record run. Next patch will process short living bpf programs that
are created during perf-record.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/perf/builtin-record.c | 5 +
tools/perf/builtin-report.c | 2 +
tools/perf/util/Build | 2 +
tools/perf/util/bpf-info.c | 287 ++++++++++++++++++++++++++++++++++++
tools/perf/util/bpf-info.h | 29 ++++
tools/perf/util/event.c | 1 +
tools/perf/util/event.h | 14 ++
tools/perf/util/session.c | 4 +
tools/perf/util/tool.h | 3 +-
9 files changed, 346 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/util/bpf-info.c
create mode 100644 tools/perf/util/bpf-info.h
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 0980dfe3396b..73b02bde1ebc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -41,6 +41,7 @@
#include "util/perf-hooks.h"
#include "util/time-utils.h"
#include "util/units.h"
+#include "util/bpf-info.h"
#include "asm/bug.h"
#include <errno.h>
@@ -850,6 +851,9 @@ static int record__synthesize(struct record *rec, bool tail)
err = __machine__synthesize_threads(machine, tool, &opts->target, rec->evlist->threads,
process_synthesized_event, opts->sample_address,
opts->proc_map_timeout, 1);
+
+ err = perf_event__synthesize_bpf_prog_info(
+ &rec->tool, process_synthesized_event, machine);
out:
return err;
}
@@ -1531,6 +1535,7 @@ static struct record record = {
.namespaces = perf_event__process_namespaces,
.mmap = perf_event__process_mmap,
.mmap2 = perf_event__process_mmap2,
+ .bpf_prog_info = perf_event__process_bpf_prog_info,
.ordered_events = true,
},
};
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c0703979c51d..4a9a3e8da4e0 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -41,6 +41,7 @@
#include "util/auxtrace.h"
#include "util/units.h"
#include "util/branch.h"
+#include "util/bpf-info.h"
#include <dlfcn.h>
#include <errno.h>
@@ -981,6 +982,7 @@ int cmd_report(int argc, const char **argv)
.auxtrace_info = perf_event__process_auxtrace_info,
.auxtrace = perf_event__process_auxtrace,
.feature = process_feature_event,
+ .bpf_prog_info = perf_event__process_bpf_prog_info,
.ordered_events = true,
.ordering_requires_timestamps = true,
},
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index ecd9f9ceda77..624c7281217c 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -150,6 +150,8 @@ endif
libperf-y += perf-hooks.o
+libperf-$(CONFIG_LIBBPF) += bpf-info.o
+
libperf-$(CONFIG_CXX) += c++/
CFLAGS_config.o += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
diff --git a/tools/perf/util/bpf-info.c b/tools/perf/util/bpf-info.c
new file mode 100644
index 000000000000..fa598c4328be
--- /dev/null
+++ b/tools/perf/util/bpf-info.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Facebook
+ */
+#include <errno.h>
+#include <stdio.h>
+#include <bpf/bpf.h>
+#include "bpf-info.h"
+#include "debug.h"
+#include "session.h"
+
+#define KSYM_NAME_LEN 128
+#define BPF_PROG_INFO_MIN_SIZE 128 /* minimal require jited_func_lens */
+
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+ return (__u64) (unsigned long) ptr;
+}
+
+/* fetch information of the bpf program via bpf syscall. */
+struct bpf_prog_info_event *perf_bpf_info__get_bpf_prog_info_event(u32 prog_id)
+{
+ struct bpf_prog_info_event *prog_info_event = NULL;
+ struct bpf_prog_info info = {};
+ u32 info_len = sizeof(info);
+ u32 event_len, i;
+ int fd, err;
+ void *ptr;
+
+ fd = bpf_prog_get_fd_by_id(prog_id);
+ if (fd < 0) {
+ pr_debug("Failed to get fd for prog_id %u\n", prog_id);
+ return NULL;
+ }
+
+ err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+ if (err) {
+ pr_debug("can't get prog info: %s", strerror(errno));
+ goto close_fd;
+ }
+ if (info_len < BPF_PROG_INFO_MIN_SIZE) {
+ pr_debug("kernel is too old to support proper prog info\n");
+ goto close_fd;
+ }
+
+ /* calculate size of bpf_prog_info_event */
+ event_len = sizeof(struct bpf_prog_info_event);
+ event_len += info_len;
+ event_len -= sizeof(info);
+ event_len += info.jited_prog_len;
+ event_len += info.xlated_prog_len;
+ event_len += info.nr_map_ids * sizeof(u32);
+ event_len += info.nr_jited_ksyms * sizeof(u64);
+ event_len += info.nr_jited_func_lens * sizeof(u32);
+ event_len += info.nr_jited_ksyms * KSYM_NAME_LEN;
+
+ prog_info_event = (struct bpf_prog_info_event *) malloc(event_len);
+ if (!prog_info_event)
+ goto close_fd;
+
+ /* assign pointers for map_ids, jited_prog_insns, etc. */
+ ptr = prog_info_event->data;
+ info.map_ids = ptr_to_u64(ptr);
+ ptr += info.nr_map_ids * sizeof(u32);
+ info.jited_prog_insns = ptr_to_u64(ptr);
+ ptr += info.jited_prog_len;
+ info.xlated_prog_insns = ptr_to_u64(ptr);
+ ptr += info.xlated_prog_len;
+ info.jited_ksyms = ptr_to_u64(ptr);
+ ptr += info.nr_jited_ksyms * sizeof(u64);
+ info.jited_func_lens = ptr_to_u64(ptr);
+ ptr += info.nr_jited_func_lens * sizeof(u32);
+
+ err = bpf_obj_get_info_by_fd(fd, &info, &info_len);
+ if (err) {
+ pr_err("can't get prog info: %s\n", strerror(errno));
+ free(prog_info_event);
+ prog_info_event = NULL;
+ goto close_fd;
+ }
+
+ /* fill data in prog_info_event */
+ prog_info_event->header.type = PERF_RECORD_BPF_PROG_INFO;
+ prog_info_event->header.misc = 0;
+ prog_info_event->prog_info_len = info_len;
+
+ memcpy(&prog_info_event->prog_info, &info, info_len);
+
+ prog_info_event->ksym_table_len = 0;
+ prog_info_event->ksym_table = ptr_to_u64(ptr);
+
+ /* fill in fake symbol name for now, add real name after BTF */
+ if (info.nr_jited_func_lens == 1 && info.name) { /* only main prog */
+ size_t l;
+
+ assert(info.nr_jited_ksyms == 1);
+ l = snprintf(ptr, KSYM_NAME_LEN, "bpf_prog_%s", info.name);
+ prog_info_event->ksym_table_len += l + 1;
+ ptr += l + 1;
+
+ } else {
+ assert(info.nr_jited_ksyms == info.nr_jited_func_lens);
+
+ for (i = 0; i < info.nr_jited_ksyms; i++) {
+ size_t l;
+
+ l = snprintf(ptr, KSYM_NAME_LEN, "bpf_prog_%d_%d",
+ info.id, i);
+ prog_info_event->ksym_table_len += l + 1;
+ ptr += l + 1;
+ }
+ }
+
+ prog_info_event->header.size = ptr - (void *)prog_info_event;
+
+close_fd:
+ close(fd);
+ return prog_info_event;
+}
+
+static size_t fprintf_bpf_prog_info(
+ struct bpf_prog_info_event *prog_info_event, FILE *fp)
+{
+ struct bpf_prog_info *info = &prog_info_event->prog_info;
+ unsigned long *jited_ksyms = (unsigned long *)(info->jited_ksyms);
+ char *name_ptr = (char *) prog_info_event->ksym_table;
+ unsigned int i;
+ size_t ret;
+
+ ret = fprintf(fp, "bpf_prog: type: %u id: %u ", info->type, info->id);
+ ret += fprintf(fp, "nr_jited_ksyms: %u\n", info->nr_jited_ksyms);
+
+ for (i = 0; i < info->nr_jited_ksyms; i++) {
+ ret += fprintf(fp, "jited_ksyms[%u]: %lx %s\n",
+ i, jited_ksyms[i], name_ptr);
+ name_ptr += strlen(name_ptr);
+ }
+ return ret;
+}
+
+size_t perf_event__fprintf_bpf_prog_info(union perf_event *event, FILE *fp)
+{
+ return fprintf_bpf_prog_info(&event->bpf_prog_info, fp);
+}
+
+/*
+ * translate all array ptr to offset from base address, called before
+ * writing the event to file
+ */
+void perf_bpf_info__ptr_to_offset(
+ struct bpf_prog_info_event *prog_info_event)
+{
+ u64 base = ptr_to_u64(prog_info_event);
+
+ prog_info_event->ksym_table -= base;
+ prog_info_event->prog_info.jited_prog_insns -= base;
+ prog_info_event->prog_info.xlated_prog_insns -= base;
+ prog_info_event->prog_info.map_ids -= base;
+ prog_info_event->prog_info.jited_ksyms -= base;
+ prog_info_event->prog_info.jited_func_lens -= base;
+}
+
+/*
+ * translate offset from base address to array pointer, called after
+ * reading the event from file
+ */
+void perf_bpf_info__offset_to_ptr(
+ struct bpf_prog_info_event *prog_info_event)
+{
+ u64 base = ptr_to_u64(prog_info_event);
+
+ prog_info_event->ksym_table += base;
+ prog_info_event->prog_info.jited_prog_insns += base;
+ prog_info_event->prog_info.xlated_prog_insns += base;
+ prog_info_event->prog_info.map_ids += base;
+ prog_info_event->prog_info.jited_ksyms += base;
+ prog_info_event->prog_info.jited_func_lens += base;
+}
+
+int perf_event__synthesize_one_bpf_prog_info(struct perf_tool *tool,
+ perf_event__handler_t process,
+ struct machine *machine,
+ __u32 id)
+{
+ struct bpf_prog_info_event *prog_info_event;
+
+ prog_info_event = perf_bpf_info__get_bpf_prog_info_event(id);
+
+ if (!prog_info_event) {
+ pr_err("Failed to get prog_info_event\n");
+ return -1;
+ }
+ perf_bpf_info__ptr_to_offset(prog_info_event);
+
+ if (perf_tool__process_synth_event(
+ tool, (union perf_event *)prog_info_event,
+ machine, process) != 0) {
+ free(prog_info_event);
+ return -1;
+ }
+
+ free(prog_info_event);
+ return 0;
+}
+
+int perf_event__synthesize_bpf_prog_info(struct perf_tool *tool,
+ perf_event__handler_t process,
+ struct machine *machine)
+{
+ __u32 id = 0;
+ int err = 0;
+
+ while (true) {
+ err = bpf_prog_get_next_id(id, &id);
+ if (err) {
+ if (errno == ENOENT) {
+ err = 0;
+ break;
+ }
+ fprintf(stderr, "can't get next program: %s%s",
+ strerror(errno),
+ errno == EINVAL ? " -- kernel too old?" : "");
+ err = -1;
+ break;
+ }
+ err = perf_event__synthesize_one_bpf_prog_info(
+ tool, process, machine, id);
+ }
+ return err;
+}
+
+int perf_event__process_bpf_prog_info(struct perf_session *session,
+ union perf_event *event)
+{
+ struct machine *machine = &session->machines.host;
+ struct bpf_prog_info_event *prog_info_event;
+ struct bpf_prog_info *info;
+ struct symbol *sym;
+ struct map *map;
+ char *name_ptr;
+ int ret = 0;
+ u64 *addrs;
+ u32 *lens;
+ u32 i;
+
+ prog_info_event = (struct bpf_prog_info_event *)
+ malloc(event->header.size);
+ if (!prog_info_event)
+ return -ENOMEM;
+
+ /* copy the data to rw memeory so we can modify it */
+ memcpy(prog_info_event, &event->bpf_prog_info, event->header.size);
+ info = &prog_info_event->prog_info;
+
+ perf_bpf_info__offset_to_ptr(prog_info_event);
+ name_ptr = (char *) prog_info_event->ksym_table;
+ addrs = (u64 *)info->jited_ksyms;
+ lens = (u32 *)info->jited_func_lens;
+ for (i = 0; i < info->nr_jited_ksyms; i++) {
+ u32 len = info->nr_jited_func_lens == 1 ?
+ len = info->jited_prog_len : lens[i];
+
+ map = map_groups__find(&machine->kmaps, addrs[i]);
+ if (!map) {
+ map = dso__new_map("bpf_prog");
+ if (!map) {
+ ret = -ENOMEM;
+ break;
+ }
+ map->start = addrs[i];
+ map->pgoff = map->start;
+ map->end = map->start + len;
+ map_groups__insert(&machine->kmaps, map);
+ }
+
+ sym = symbol__new(addrs[i], len, 0, 0, name_ptr);
+ if (!sym) {
+ ret = -ENOMEM;
+ break;
+ }
+ dso__insert_symbol(map->dso, sym);
+ name_ptr += strlen(name_ptr) + 1;
+ }
+
+ free(prog_info_event);
+ return ret;
+}
diff --git a/tools/perf/util/bpf-info.h b/tools/perf/util/bpf-info.h
new file mode 100644
index 000000000000..813cad07bacb
--- /dev/null
+++ b/tools/perf/util/bpf-info.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __PERF_BPF_INFO_H
+#define __PERF_BPF_INFO_H
+
+#include "event.h"
+#include "machine.h"
+#include "tool.h"
+#include "symbol.h"
+
+struct bpf_prog_info_event *perf_bpf_info__get_bpf_prog_info_event(u32 prog_id);
+
+size_t perf_event__fprintf_bpf_prog_info(union perf_event *event, FILE *fp);
+
+int perf_event__synthesize_one_bpf_prog_info(struct perf_tool *tool,
+ perf_event__handler_t process,
+ struct machine *machine,
+ __u32 id);
+
+int perf_event__synthesize_bpf_prog_info(struct perf_tool *tool,
+ perf_event__handler_t process,
+ struct machine *machine);
+
+void perf_bpf_info__ptr_to_offset(struct bpf_prog_info_event *prog_info_event);
+void perf_bpf_info__offset_to_ptr(struct bpf_prog_info_event *prog_info_event);
+
+int perf_event__process_bpf_prog_info(struct perf_session *session,
+ union perf_event *event);
+
+#endif /* __PERF_BPF_INFO_H */
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 601432afbfb2..33b1c168b83e 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -61,6 +61,7 @@ static const char *perf_event__names[] = {
[PERF_RECORD_EVENT_UPDATE] = "EVENT_UPDATE",
[PERF_RECORD_TIME_CONV] = "TIME_CONV",
[PERF_RECORD_HEADER_FEATURE] = "FEATURE",
+ [PERF_RECORD_BPF_PROG_INFO] = "BPF_PROG_INFO",
};
static const char *perf_ns__names[] = {
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 13a0c64dd0ed..dc64d800eaa6 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -5,6 +5,7 @@
#include <limits.h>
#include <stdio.h>
#include <linux/kernel.h>
+#include <linux/bpf.h>
#include "../perf.h"
#include "build-id.h"
@@ -258,6 +259,7 @@ enum perf_user_event_type { /* above any possible kernel type */
PERF_RECORD_EVENT_UPDATE = 78,
PERF_RECORD_TIME_CONV = 79,
PERF_RECORD_HEADER_FEATURE = 80,
+ PERF_RECORD_BPF_PROG_INFO = 81,
PERF_RECORD_HEADER_MAX
};
@@ -629,6 +631,17 @@ struct feature_event {
char data[];
};
+#define KSYM_NAME_LEN 128
+
+struct bpf_prog_info_event {
+ struct perf_event_header header;
+ u32 prog_info_len;
+ u32 ksym_table_len;
+ u64 ksym_table;
+ struct bpf_prog_info prog_info;
+ char data[];
+};
+
union perf_event {
struct perf_event_header header;
struct mmap_event mmap;
@@ -661,6 +674,7 @@ union perf_event {
struct time_conv_event time_conv;
struct feature_event feat;
struct bpf_event bpf_event;
+ struct bpf_prog_info_event bpf_prog_info;
};
void perf_event__print_totals(void);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index dffe5120d2d3..5365ee1dfbec 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -415,6 +415,8 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
tool->time_conv = process_event_op2_stub;
if (tool->feature == NULL)
tool->feature = process_event_op2_stub;
+ if (tool->bpf_prog_info == NULL)
+ tool->bpf_prog_info = process_event_op2_stub;
}
static void swap_sample_id_all(union perf_event *event, void *data)
@@ -1397,6 +1399,8 @@ static s64 perf_session__process_user_event(struct perf_session *session,
return tool->time_conv(session, event);
case PERF_RECORD_HEADER_FEATURE:
return tool->feature(session, event);
+ case PERF_RECORD_BPF_PROG_INFO:
+ return tool->bpf_prog_info(session, event);
default:
return -EINVAL;
}
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index 69ae898ca024..739a4b1188f7 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -70,7 +70,8 @@ struct perf_tool {
stat_config,
stat,
stat_round,
- feature;
+ feature,
+ bpf_prog_info;
event_op3 auxtrace;
bool ordered_events;
bool ordering_requires_timestamps;
--
2.17.1
^ permalink raw reply related
* [RFC perf,bpf 2/5] perf: sync tools/include/uapi/linux/perf_event.h
From: Song Liu @ 2018-11-06 20:52 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: kernel-team, Song Liu, ast, daniel, peterz, acme
In-Reply-To: <20181106205246.567448-1-songliubraving@fb.com>
Sync changes for PERF_RECORD_BPF_EVENT.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/include/uapi/linux/perf_event.h | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index f35eb72739c0..d51cacb3077a 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -372,7 +372,8 @@ struct perf_event_attr {
context_switch : 1, /* context switch data */
write_backward : 1, /* Write ring buffer from end to beginning */
namespaces : 1, /* include namespaces data */
- __reserved_1 : 35;
+ bpf_event : 1, /* include bpf events */
+ __reserved_1 : 34;
union {
__u32 wakeup_events; /* wakeup every n events */
@@ -963,9 +964,33 @@ enum perf_event_type {
*/
PERF_RECORD_NAMESPACES = 16,
+ /*
+ * Record different types of bpf events:
+ * enum perf_bpf_event_type {
+ * PERF_BPF_EVENT_UNKNOWN = 0,
+ * PERF_BPF_EVENT_PROG_LOAD = 1,
+ * PERF_BPF_EVENT_PROG_UNLOAD = 2,
+ * };
+ *
+ * struct {
+ * struct perf_event_header header;
+ * u16 type;
+ * u16 flags;
+ * u32 id; // prog_id or map_id
+ * };
+ */
+ PERF_RECORD_BPF_EVENT = 17,
+
PERF_RECORD_MAX, /* non-ABI */
};
+enum perf_bpf_event_type {
+ PERF_BPF_EVENT_UNKNOWN = 0,
+ PERF_BPF_EVENT_PROG_LOAD = 1,
+ PERF_BPF_EVENT_PROG_UNLOAD = 2,
+ PERF_BPF_EVENT_MAX, /* non-ABI */
+};
+
#define PERF_MAX_STACK_DEPTH 127
#define PERF_MAX_CONTEXTS_PER_STACK 8
--
2.17.1
^ permalink raw reply related
* [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT
From: Song Liu @ 2018-11-06 20:52 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: kernel-team, Song Liu, ast, daniel, peterz, acme
In-Reply-To: <20181106205246.567448-1-songliubraving@fb.com>
For better performance analysis of BPF programs, this patch introduces
PERF_RECORD_BPF_EVENT, a new perf_event_type that exposes BPF program
load/unload information to user space.
/*
* Record different types of bpf events:
* enum perf_bpf_event_type {
* PERF_BPF_EVENT_UNKNOWN = 0,
* PERF_BPF_EVENT_PROG_LOAD = 1,
* PERF_BPF_EVENT_PROG_UNLOAD = 2,
* };
*
* struct {
* struct perf_event_header header;
* u16 type;
* u16 flags;
* u32 id; // prog_id or map_id
* };
*/
PERF_RECORD_BPF_EVENT = 17,
PERF_RECORD_BPF_EVENT contains minimal information about the BPF program.
Perf utility (or other user space tools) should listen to this event and
fetch more details about the event via BPF syscalls
(BPF_PROG_GET_FD_BY_ID, BPF_OBJ_GET_INFO_BY_FD, etc.).
Currently, PERF_RECORD_BPF_EVENT only support two events:
PERF_BPF_EVENT_PROG_LOAD and PERF_BPF_EVENT_PROG_UNLOAD. But it can be
easily extended to support more events.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
include/linux/perf_event.h | 5 ++
include/uapi/linux/perf_event.h | 27 ++++++++++-
kernel/bpf/syscall.c | 4 ++
kernel/events/core.c | 82 ++++++++++++++++++++++++++++++++-
4 files changed, 116 insertions(+), 2 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 53c500f0ca79..a3126fd5b7f1 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1113,6 +1113,9 @@ static inline void perf_event_task_sched_out(struct task_struct *prev,
}
extern void perf_event_mmap(struct vm_area_struct *vma);
+extern void perf_event_bpf_event(enum perf_bpf_event_type type,
+ u16 flags, u32 id);
+
extern struct perf_guest_info_callbacks *perf_guest_cbs;
extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
@@ -1333,6 +1336,8 @@ static inline int perf_unregister_guest_info_callbacks
(struct perf_guest_info_callbacks *callbacks) { return 0; }
static inline void perf_event_mmap(struct vm_area_struct *vma) { }
+static inline void perf_event_bpf_event(enum perf_bpf_event_type type,
+ u16 flags, u32 id) { }
static inline void perf_event_exec(void) { }
static inline void perf_event_comm(struct task_struct *tsk, bool exec) { }
static inline void perf_event_namespaces(struct task_struct *tsk) { }
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index f35eb72739c0..d51cacb3077a 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -372,7 +372,8 @@ struct perf_event_attr {
context_switch : 1, /* context switch data */
write_backward : 1, /* Write ring buffer from end to beginning */
namespaces : 1, /* include namespaces data */
- __reserved_1 : 35;
+ bpf_event : 1, /* include bpf events */
+ __reserved_1 : 34;
union {
__u32 wakeup_events; /* wakeup every n events */
@@ -963,9 +964,33 @@ enum perf_event_type {
*/
PERF_RECORD_NAMESPACES = 16,
+ /*
+ * Record different types of bpf events:
+ * enum perf_bpf_event_type {
+ * PERF_BPF_EVENT_UNKNOWN = 0,
+ * PERF_BPF_EVENT_PROG_LOAD = 1,
+ * PERF_BPF_EVENT_PROG_UNLOAD = 2,
+ * };
+ *
+ * struct {
+ * struct perf_event_header header;
+ * u16 type;
+ * u16 flags;
+ * u32 id; // prog_id or map_id
+ * };
+ */
+ PERF_RECORD_BPF_EVENT = 17,
+
PERF_RECORD_MAX, /* non-ABI */
};
+enum perf_bpf_event_type {
+ PERF_BPF_EVENT_UNKNOWN = 0,
+ PERF_BPF_EVENT_PROG_LOAD = 1,
+ PERF_BPF_EVENT_PROG_UNLOAD = 2,
+ PERF_BPF_EVENT_MAX, /* non-ABI */
+};
+
#define PERF_MAX_STACK_DEPTH 127
#define PERF_MAX_CONTEXTS_PER_STACK 8
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 18e3be193a05..b37051a13be6 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1101,9 +1101,12 @@ static void __bpf_prog_put_rcu(struct rcu_head *rcu)
static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
{
if (atomic_dec_and_test(&prog->aux->refcnt)) {
+ int prog_id = prog->aux->id;
+
/* bpf_prog_free_id() must be called first */
bpf_prog_free_id(prog, do_idr_lock);
bpf_prog_kallsyms_del_all(prog);
+ perf_event_bpf_event(PERF_BPF_EVENT_PROG_UNLOAD, 0, prog_id);
call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu);
}
@@ -1441,6 +1444,7 @@ static int bpf_prog_load(union bpf_attr *attr)
}
bpf_prog_kallsyms_add(prog);
+ perf_event_bpf_event(PERF_BPF_EVENT_PROG_LOAD, 0, prog->aux->id);
return err;
free_used_maps:
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 5a97f34bc14c..54667be6669b 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -385,6 +385,7 @@ static atomic_t nr_namespaces_events __read_mostly;
static atomic_t nr_task_events __read_mostly;
static atomic_t nr_freq_events __read_mostly;
static atomic_t nr_switch_events __read_mostly;
+static atomic_t nr_bpf_events __read_mostly;
static LIST_HEAD(pmus);
static DEFINE_MUTEX(pmus_lock);
@@ -4235,7 +4236,7 @@ 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->task || attr->bpf_event ||
attr->context_switch)
return true;
return false;
@@ -4305,6 +4306,8 @@ static void unaccount_event(struct perf_event *event)
dec = true;
if (has_branch_stack(event))
dec = true;
+ if (event->attr.bpf_event)
+ atomic_dec(&nr_bpf_events);
if (dec) {
if (!atomic_add_unless(&perf_sched_count, -1, 1))
@@ -7650,6 +7653,81 @@ static void perf_log_throttle(struct perf_event *event, int enable)
perf_output_end(&handle);
}
+/*
+ * bpf load/unload tracking
+ */
+
+struct perf_bpf_event {
+ struct {
+ struct perf_event_header header;
+ u16 type;
+ u16 flags;
+ u32 id;
+ } event_id;
+};
+
+static int perf_event_bpf_match(struct perf_event *event)
+{
+ return event->attr.bpf_event;
+}
+
+static void perf_event_bpf_output(struct perf_event *event,
+ void *data)
+{
+ struct perf_bpf_event *bpf_event = data;
+ struct perf_output_handle handle;
+ struct perf_sample_data sample;
+ int size = bpf_event->event_id.header.size;
+ int ret;
+
+ if (!perf_event_bpf_match(event))
+ return;
+
+ perf_event_header__init_id(&bpf_event->event_id.header, &sample, event);
+ ret = perf_output_begin(&handle, event,
+ bpf_event->event_id.header.size);
+ if (ret)
+ goto out;
+
+ perf_output_put(&handle, bpf_event->event_id);
+ perf_event__output_id_sample(event, &handle, &sample);
+
+ perf_output_end(&handle);
+out:
+ bpf_event->event_id.header.size = size;
+}
+
+static void perf_event_bpf(struct perf_bpf_event *bpf_event)
+{
+ perf_iterate_sb(perf_event_bpf_output,
+ bpf_event,
+ NULL);
+}
+
+void perf_event_bpf_event(enum perf_bpf_event_type type, u16 flags, u32 id)
+{
+ struct perf_bpf_event bpf_event;
+
+ if (!atomic_read(&nr_bpf_events))
+ return;
+
+ if (type <= PERF_BPF_EVENT_UNKNOWN || type >= PERF_BPF_EVENT_MAX)
+ return;
+
+ bpf_event = (struct perf_bpf_event){
+ .event_id = {
+ .header = {
+ .type = PERF_RECORD_BPF_EVENT,
+ .size = sizeof(bpf_event.event_id),
+ },
+ .type = type,
+ .flags = flags,
+ .id = id,
+ },
+ };
+ perf_event_bpf(&bpf_event);
+}
+
void perf_event_itrace_started(struct perf_event *event)
{
event->attach_state |= PERF_ATTACH_ITRACE;
@@ -9871,6 +9949,8 @@ static void account_event(struct perf_event *event)
inc = true;
if (is_cgroup_event(event))
inc = true;
+ if (event->attr.bpf_event)
+ atomic_inc(&nr_bpf_events);
if (inc) {
/*
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] net: phy: Allow BCM54616S PHY to setup internal TX/RX clock delay
From: Tao Ren @ 2018-11-06 19:42 UTC (permalink / raw)
To: David Miller
Cc: andrew@lunn.ch, f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, openbmc@lists.ozlabs.org
In-Reply-To: <20181106.111736.1149054212290410715.davem@davemloft.net>
On 11/6/18 11:17 AM, David Miller wrote:
> From: Tao Ren <taoren@fb.com>
> Date: Mon, 5 Nov 2018 14:35:40 -0800
>
>> This patch allows users to enable/disable internal TX and/or RX clock
>> delay for BCM54616S PHYs so as to satisfy RGMII timing specifications.
>>
>> On a particular platform, whether TX and/or RX clock delay is required
>> depends on how PHY connected to the MAC IP. This requirement can be
>> specified through "phy-mode" property in the platform device tree.
>>
>> The patch is inspired by commit 733336262b28 ("net: phy: Allow BCM5481x
>> PHYs to setup internal TX/RX clock delay").
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>
> This is fine for 'net', applied, thanks.
Thanks David for the quick action.
- Tao Ren
^ permalink raw reply
* Re: [PATCH] net: skbuff.h: remove unnecessary unlikely()
From: David Miller @ 2018-11-06 19:22 UTC (permalink / raw)
To: tiny.windzz
Cc: edumazet, dja, willemb, ast, sbrivio, posk, pabeni, borisp,
linux-kernel, netdev
In-Reply-To: <20181106154536.8789-1-tiny.windzz@gmail.com>
From: Yangtao Li <tiny.windzz@gmail.com>
Date: Tue, 6 Nov 2018 10:45:36 -0500
> WARN_ON() already contains an unlikely(), so it's not necessary to use
> unlikely.
>
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: phy: Allow BCM54616S PHY to setup internal TX/RX clock delay
From: David Miller @ 2018-11-06 19:17 UTC (permalink / raw)
To: taoren; +Cc: andrew, f.fainelli, netdev, linux-kernel, openbmc
In-Reply-To: <20181105223540.1897084-1-taoren@fb.com>
From: Tao Ren <taoren@fb.com>
Date: Mon, 5 Nov 2018 14:35:40 -0800
> This patch allows users to enable/disable internal TX and/or RX clock
> delay for BCM54616S PHYs so as to satisfy RGMII timing specifications.
>
> On a particular platform, whether TX and/or RX clock delay is required
> depends on how PHY connected to the MAC IP. This requirement can be
> specified through "phy-mode" property in the platform device tree.
>
> The patch is inspired by commit 733336262b28 ("net: phy: Allow BCM5481x
> PHYs to setup internal TX/RX clock delay").
>
> Signed-off-by: Tao Ren <taoren@fb.com>
This is fine for 'net', applied, thanks.
^ permalink raw reply
* Re: [PATCH] staging: net: ipv4: tcp_westwood: fixed warnings and checks
From: David Miller @ 2018-11-06 19:15 UTC (permalink / raw)
To: suraj1998; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1541425985-31869-1-git-send-email-suraj1998@gmail.com>
From: Suraj Singh <suraj1998@gmail.com>
Date: Mon, 5 Nov 2018 19:23:05 +0530
> Fixed warnings and checks for TCP Westwood
>
> Signed-off-by: Suraj Singh <suraj1998@gmail.com>
I asked you yesterday why "staging: " appears in your subject line
and you have failed to respond and explain.
There are also functional issues with your patch:
> - tp->snd_cwnd = tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
> + tp->snd_cwnd = tcp_westwood_bw_rttmin(sk);
> + tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
This is bogus, now tcp_westwood_bw_rttmin(sk) will potentially be called
two times instead of once.
The existing code is fine, please do not modify it.
^ permalink raw reply
* Re: [PATCH v2] ISDN: eicon: Remove driver
From: David Miller @ 2018-11-06 19:04 UTC (permalink / raw)
To: olof; +Cc: isdn, netdev, linux-kernel, isdn4linux, mac
In-Reply-To: <20181102220026.6387-1-olof@lixom.net>
From: Olof Johansson <olof@lixom.net>
Date: Fri, 2 Nov 2018 15:00:26 -0700
> I started looking at the history of this driver, and last time the
> maintainer was active on the mailing list was when discussing how to
> remove it. This was in 2012:
>
> https://lore.kernel.org/lkml/4F4DE175.30002@melware.de/
>
> It looks to me like this has in practice been an orphan for quite a while.
> It's throwing warnings about stack size in a function that is in dire
> need of refactoring, and it's probably a case of "it's time to call it".
>
> Cc: Armin Schindler <mac@melware.de>
> Cc: Karsten Keil <isdn@linux-pingi.de>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>
> v2:
> Missed a git add of drivers/isdn/hardware/Kconfig
Applied to net-next.
^ permalink raw reply
* [PATCH net] igb: fix uninitialized variables
From: wangyunjian @ 2018-11-06 8:27 UTC (permalink / raw)
To: netdev, intel-wired-lan; +Cc: stone.zhou, Yunjian Wang
From: Yunjian Wang <wangyunjian@huawei.com>
This patch fixes the variable 'phy_word' may be used uninitialized.
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
drivers/net/ethernet/intel/igb/e1000_i210.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/intel/igb/e1000_i210.c b/drivers/net/ethernet/intel/igb/e1000_i210.c
index c54ebed..c393cb2 100644
--- a/drivers/net/ethernet/intel/igb/e1000_i210.c
+++ b/drivers/net/ethernet/intel/igb/e1000_i210.c
@@ -842,6 +842,7 @@ s32 igb_pll_workaround_i210(struct e1000_hw *hw)
nvm_word = E1000_INVM_DEFAULT_AL;
tmp_nvm = nvm_word | E1000_INVM_PLL_WO_VAL;
igb_write_phy_reg_82580(hw, I347AT4_PAGE_SELECT, E1000_PHY_PLL_FREQ_PAGE);
+ phy_word = E1000_PHY_PLL_UNCONF;
for (i = 0; i < E1000_MAX_PLL_TRIES; i++) {
/* check current state directly from internal PHY */
igb_read_phy_reg_82580(hw, E1000_PHY_PLL_FREQ_REG, &phy_word);
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH net-next 5/6] net/ncsi: Reset channel state in ncsi_start_dev()
From: Justin.Lee1 @ 2018-11-06 17:27 UTC (permalink / raw)
To: sam, netdev; +Cc: davem, linux-kernel, openbmc
In-Reply-To: <de9816e6c9cb31fdae1bb3d4a38da65b8f3a7694.camel@mendozajonas.com>
> On Mon, 2018-11-05 at 18:01 +0000, Justin.Lee1@Dell.com wrote:
> > > On Tue, 2018-10-30 at 21:26 +0000, Justin.Lee1@Dell.com wrote:
> > > > > +int ncsi_reset_dev(struct ncsi_dev *nd)
> > > > > +{
> > > > > + struct ncsi_dev_priv *ndp = TO_NCSI_DEV_PRIV(nd);
> > > > > + struct ncsi_channel *nc, *active;
> > > > > + struct ncsi_package *np;
> > > > > + unsigned long flags;
> > > > > + bool enabled;
> > > > > + int state;
> > > > > +
> > > > > + active = NULL;
> > > > > + NCSI_FOR_EACH_PACKAGE(ndp, np) {
> > > > > + NCSI_FOR_EACH_CHANNEL(np, nc) {
> > > > > + spin_lock_irqsave(&nc->lock, flags);
> > > > > + enabled = nc->monitor.enabled;
> > > > > + state = nc->state;
> > > > > + spin_unlock_irqrestore(&nc->lock, flags);
> > > > > +
> > > > > + if (enabled)
> > > > > + ncsi_stop_channel_monitor(nc);
> > > > > + if (state == NCSI_CHANNEL_ACTIVE) {
> > > > > + active = nc;
> > > > > + break;
> > > >
> > > > Is the original intention to process the channel one by one?
> > > > If it is the case, there are two loops and we might need to use
> > > > "goto found" instead.
> > >
> > > Yes we'll need to break out of the package loop here as well.
> > >
> > > > > + }
> > > > > + }
> > > > > + }
> > > > > +
> > > >
> > > > found: ?
> > > >
> > > > > + if (!active) {
> > > > > + /* Done */
> > > > > + spin_lock_irqsave(&ndp->lock, flags);
> > > > > + ndp->flags &= ~NCSI_DEV_RESET;
> > > > > + spin_unlock_irqrestore(&ndp->lock, flags);
> > > > > + return ncsi_choose_active_channel(ndp);
> > > > > + }
> > > > > +
> > > > > + spin_lock_irqsave(&ndp->lock, flags);
> > > > > + ndp->flags |= NCSI_DEV_RESET;
> > > > > + ndp->active_channel = active;
> > > > > + ndp->active_package = active->package;
> > > > > + spin_unlock_irqrestore(&ndp->lock, flags);
> > > > > +
> > > > > + nd->state = ncsi_dev_state_suspend;
> > > > > + schedule_work(&ndp->work);
> > > > > + return 0;
> > > > > +}
> > > >
> > > > Also similar issue in ncsi_choose_active_channel() function below.
> > > >
> > > > > @@ -916,32 +1045,49 @@ static int ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
> > > > >
> > > > > ncm = &nc->modes[NCSI_MODE_LINK];
> > > > > if (ncm->data[2] & 0x1) {
> > > > > - spin_unlock_irqrestore(&nc->lock, flags);
> > > > > found = nc;
> > > > > - goto out;
> > > > > + with_link = true;
> > > > > }
> > > > >
> > > > > - spin_unlock_irqrestore(&nc->lock, flags);
> > > > > + /* If multi_channel is enabled configure all valid
> > > > > + * channels whether or not they currently have link
> > > > > + * so they will have AENs enabled.
> > > > > + */
> > > > > + if (with_link || np->multi_channel) {
> > > >
> > > > I notice that there is a case that we will misconfigure the interface.
> > > > For example below, multi-channel is not enable for package 1.
> > > > But we enable the channel for ncsi2 below (package 1 channel 0) as that interface is the first
> > > > channel for that package with link.
> > >
> > > I don't think I see the issue here; multi-channel is not set on package
> > > 1, but both channels are in the channel whitelist. Channel 0 is
> > > configured since it's the first found on package 1, and channel 1 is not
> > > since channel 0 is already found. Are you expecting something different?
> > >
> >
> > The setting is that multi-package is enable for both package 0 and 1.
> > Multi-channel is only enabled for package 0.
> >
> > > > cat /sys/kernel/debug/ncsi_protocol/ncsi_device_
> > > > IFIDX IFNAME NAME PID CID RX TX MP MC WP WC PC CS PS LS RU CR NQ HA
> > > > =====================================================================
> > > > 2 eth2 ncsi0 000 000 1 1 1 1 1 1 0 2 1 1 1 1 0 1
> > > > 2 eth2 ncsi1 000 001 1 0 1 1 1 1 0 2 1 1 1 1 0 1
> > > > 2 eth2 ncsi2 001 000 1 0 1 0 1 1 0 2 1 1 1 1 0 1
> >
> > I was replying to the wrong old email and it might cause a bit confusion.
> > The first 1 meaning channel is enabled for package 1 channel 0 (ncsi2).
> > For eth2, we already has ncsi0 as the active channel with TX enable.
> > I would think that package doesn't have the multi-channel enabled and
> > we should not enable the channel for ncsi2. The problem is that package 1 doesn't
> > enable the multi-channel and it believes it needs to enable one channel for its package
> > but it doesn't aware that the other package already has one active channel.
>
> Ah, maybe the confusion here is that multi_channel is a per-package
> setting; it determines what a package does with its own channels.
>
> So you have package 0 with multi-channel enabled so it enables channels 0
> & 1.
> Then you have package 1 without multi-channel so it enables only channel
> 0.
> There is still only one Tx channel (package 0, channel 0).
>
> Does that sound right, or have I missed something?
Yes, you are right. There is only one TX enabled.
If we can hold off a few seconds before applying, then we will not see
these configuration changes in between the back to back netlink commands.
Thanks,
Justin
^ permalink raw reply
* Re: [PATCH] cw1200: fix small typo
From: Kalle Valo @ 2018-11-06 17:04 UTC (permalink / raw)
To: Yangtao Li; +Cc: pizza, davem, linux-wireless, netdev, linux-kernel, Yangtao Li
In-Reply-To: <20181101153319.22830-1-tiny.windzz@gmail.com>
Yangtao Li <tiny.windzz@gmail.com> wrote:
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
f4bd758f3f20 cw1200: fix small typo
--
https://patchwork.kernel.org/patch/10664149/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: Remove same duplicated includes
From: Kalle Valo @ 2018-11-06 17:02 UTC (permalink / raw)
To: zhong jiang; +Cc: pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <1540361256-678-1-git-send-email-zhongjiang@huawei.com>
zhong jiang <zhongjiang@huawei.com> wrote:
> Just remove same duplicated includes.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
963b307361bd rtlwifi: Remove same duplicated includes
--
https://patchwork.kernel.org/patch/10654183/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCHv3] rtlwifi: rtl8723ae: Remove set but not used variables and #defines
From: Kalle Valo @ 2018-11-06 17:01 UTC (permalink / raw)
To: zhong jiang; +Cc: pkshih, davem, linux-wireless, joe, netdev, linux-kernel
In-Reply-To: <1540360329-65013-1-git-send-email-zhongjiang@huawei.com>
zhong jiang <zhongjiang@huawei.com> wrote:
> radiob_array_table' and 'radiob_arraylen' are not used after setting its value.
> It is safe to remove the unused variable. Meanwhile, radio B array should be
> removed as well. because it will no longer be referenced.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
Patch applied to wireless-drivers-next.git, thanks.
90e3243d16ad rtlwifi: rtl8723ae: Remove set but not used variables and #defines
--
https://patchwork.kernel.org/patch/10654181/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 07/20] iwlegacy: 4965-mac: mark expected switch fall-through
From: Kalle Valo @ 2018-11-06 17:00 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Stanislaw Gruszka, linux-wireless, David S. Miller, netdev,
linux-kernel, Gustavo A. R. Silva
In-Reply-To: <8d72b8a6f1529906672ac458e96d272bc55a1410.1540239684.git.gustavo@embeddedor.com>
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
14 patches applied to wireless-drivers-next.git, thanks.
e9904084dd1b iwlegacy: 4965-mac: mark expected switch fall-through
af71f8fef45c iwlegacy: common: mark expected switch fall-throughs
d56b26801e1d orinoco_usb: mark expected switch fall-through
d22b8fadd08e prism54: isl_38xx: Mark expected switch fall-through
3d238b9d5048 prism54: isl_ioctl: mark expected switch fall-through
38a0792d08e9 prism54: islpci_dev: mark expected switch fall-through
63fdc952df36 mwifiex: Mark expected switch fall-through
6eba8fd22352 rt2x00: rt2400pci: mark expected switch fall-through
10bb92217747 rt2x00: rt2500pci: mark expected switch fall-through
916e6bbcfcff rt2x00: rt2800lib: mark expected switch fall-throughs
641dd8068ecb rt2x00: rt61pci: mark expected switch fall-through
d22d2492a35d ray_cs: mark expected switch fall-throughs
89e54fa4562e rtlwifi: rtl8821ae: phy: Mark expected switch fall-through
7cbbe1597e44 zd1201: mark expected switch fall-through
--
https://patchwork.kernel.org/patch/10652563/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/2] rtl8xxxu: Mark expected switch fall-throughs
From: Kalle Valo @ 2018-11-06 16:59 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: linux-kernel, Jes Sorensen, linux-wireless, David S. Miller,
netdev, Gustavo A. R. Silva
In-Reply-To: <08817d137b32d5d091eacc6fee0a3eca68d49d94.1540208577.git.gustavo@embeddedor.com>
"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> Addresses-Coverity-ID: 1357355 ("Missing break in switch")
> Addresses-Coverity-ID: 1357378 ("Missing break in switch")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
2 patches applied to wireless-drivers-next.git, thanks.
e20c50cdca19 rtl8xxxu: Mark expected switch fall-throughs
307b00c5e695 rtl8xxxu: Fix missing break in switch
--
https://patchwork.kernel.org/patch/10651953/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: remove set but not used variable 'ppsc'
From: Kalle Valo @ 2018-11-06 16:58 UTC (permalink / raw)
To: YueHaibing
Cc: Ping-Ke Shih, Larry Finger, Colin Ian King, Nathan Chancellor,
YueHaibing, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1540194675-65562-1-git-send-email-yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function 'halbtc_leave_lps':
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:295:21: warning:
> variable 'ppsc' set but not used [-Wunused-but-set-variable]
>
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function 'halbtc_enter_lps':
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:318:21: warning:
> variable 'ppsc' set but not used [-Wunused-but-set-variable]
>
> It never used since introduction in
> commit aa45a673b291 ("rtlwifi: btcoexist: Add new mini driver")
>
> Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Acked-by: Ping-Ke Shih <pkshih-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
Patch applied to wireless-drivers-next.git, thanks.
9198f460ec9d rtlwifi: btcoex: remove set but not used variable 'ppsc'
--
https://patchwork.kernel.org/patch/10651825/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] brcmfmac: fix spelling mistake "Retreiving" -> "Retrieving"
From: Kalle Valo @ 2018-11-06 16:57 UTC (permalink / raw)
To: Colin King
Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, David S . Miller, Pieter-Paul Giesberts,
linux-wireless, brcm80211-dev-list.pdl, brcm80211-dev-list,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20181016174342.1867-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in brcmf_err error message.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Patch applied to wireless-drivers-next.git, thanks.
e966a79c2f76 brcmfmac: fix spelling mistake "Retreiving" -> "Retrieving"
--
https://patchwork.kernel.org/patch/10643927/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] wlcore: Fix the return value in case of error in 'wlcore_vendor_cmd_smart_config_start()'
From: Kalle Valo @ 2018-11-06 16:53 UTC (permalink / raw)
To: Christophe JAILLET
Cc: davem, tony, linux-wireless, netdev, linux-kernel,
kernel-janitors, Christophe JAILLET
In-Reply-To: <20181016073940.26797-1-christophe.jaillet@wanadoo.fr>
Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> We return 0 unconditionally at the end of
> 'wlcore_vendor_cmd_smart_config_start()'.
> However, 'ret' is set to some error codes in several error handling paths
> and we already return some error codes at the beginning of the function.
>
> Return 'ret' instead to propagate the error code.
>
> Fixes: 80ff8063e87c ("wlcore: handle smart config vendor commands")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Patch applied to wireless-drivers-next.git, thanks.
3419348a97bc wlcore: Fix the return value in case of error in 'wlcore_vendor_cmd_smart_config_start()'
--
https://patchwork.kernel.org/patch/10643195/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] libertas: remove set but not used variable 'int_type'
From: Kalle Valo @ 2018-11-06 16:52 UTC (permalink / raw)
To: YueHaibing
Cc: Lubomir Rintel, YueHaibing,
libertas-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1539569795-176889-1-git-send-email-yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/marvell/libertas/if_spi.c: In function 'if_spi_h2c':
> drivers/net/wireless/marvell/libertas/if_spi.c:799:6: warning:
> variable 'int_type' set but not used [-Wunused-but-set-variable]
>
> It never used since introduction in
> commit d2b21f191753 ("libertas: if_spi, driver for libertas GSPI devices")
>
> Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Tested-by: Lubomir Rintel <lkundrak-NGH9Lh4a5iE@public.gmane.org>
Patch applied to wireless-drivers-next.git, thanks.
937a13091cbd libertas: remove set but not used variable 'int_type'
--
https://patchwork.kernel.org/patch/10641005/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rsi: fix spelling mistake "Initialzing" -> "Initializing"
From: Kalle Valo @ 2018-11-06 16:52 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, Amitkumar Karwar, Prameela Rani Garnepudi,
linux-wireless, netdev, kernel-janitors, linux-kernel
In-Reply-To: <20181013153730.5761-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake in rsi_dbg debug message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
55930d2bf79b rsi: fix spelling mistake "Initialzing" -> "Initializing"
--
https://patchwork.kernel.org/patch/10640377/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 3/5] VSOCK: support receive mergeable rx buffer in guest
From: jiangyiwen @ 2018-11-06 6:41 UTC (permalink / raw)
To: Jason Wang, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <791d67e7-ad95-38e4-0d38-2b7c54d68040@redhat.com>
On 2018/11/6 12:00, Jason Wang wrote:
>
> On 2018/11/5 下午3:47, jiangyiwen wrote:
>> Guest receive mergeable rx buffer, it can merge
>> scatter rx buffer into a big buffer and then copy
>> to user space.
>>
>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
>> ---
>> include/linux/virtio_vsock.h | 9 ++++
>> net/vmw_vsock/virtio_transport.c | 75 +++++++++++++++++++++++++++++----
>> net/vmw_vsock/virtio_transport_common.c | 59 ++++++++++++++++++++++----
>> 3 files changed, 127 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index da9e1fe..6be3cd7 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -13,6 +13,8 @@
>> #define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
>> #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
>> #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
>> +/* virtio_vsock_pkt + max_pkt_len(default MAX_PKT_BUF_SIZE) */
>> +#define VIRTIO_VSOCK_MAX_MRG_BUF_NUM ((VIRTIO_VSOCK_MAX_PKT_BUF_SIZE / PAGE_SIZE) + 1)
>>
>> /* Virtio-vsock feature */
>> #define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */
>> @@ -48,6 +50,11 @@ struct virtio_vsock_sock {
>> struct list_head rx_queue;
>> };
>>
>> +struct virtio_vsock_mrg_rxbuf {
>> + void *buf;
>> + u32 len;
>> +};
>> +
>> struct virtio_vsock_pkt {
>> struct virtio_vsock_hdr hdr;
>> struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
>> @@ -59,6 +66,8 @@ struct virtio_vsock_pkt {
>> u32 len;
>> u32 off;
>> bool reply;
>> + bool mergeable;
>> + struct virtio_vsock_mrg_rxbuf mrg_rxbuf[VIRTIO_VSOCK_MAX_MRG_BUF_NUM];
>> };
>
>
> It's better to use iov here I think, and drop buf completely.
>
> And this is better to be done in an independent patch.
>
You're right, I can use kvec instead of customized structure,
in addition, I don't understand about drop buf completely and
an independent patch.
Thanks.
>
>>
>> struct virtio_vsock_pkt_info {
>> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
>> index 2040a9e..3557ad3 100644
>> --- a/net/vmw_vsock/virtio_transport.c
>> +++ b/net/vmw_vsock/virtio_transport.c
>> @@ -359,11 +359,62 @@ static bool virtio_transport_more_replies(struct virtio_vsock *vsock)
>> return val < virtqueue_get_vring_size(vq);
>> }
>>
>> +static struct virtio_vsock_pkt *receive_mergeable(struct virtqueue *vq,
>> + struct virtio_vsock *vsock, unsigned int *total_len)
>> +{
>> + struct virtio_vsock_pkt *pkt;
>> + u16 num_buf;
>> + void *page;
>> + unsigned int len;
>> + int i = 0;
>> +
>> + page = virtqueue_get_buf(vq, &len);
>> + if (!page)
>> + return NULL;
>> +
>> + *total_len = len;
>> + vsock->rx_buf_nr--;
>> +
>> + pkt = page;
>> + num_buf = le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers);
>> + if (!num_buf || num_buf > VIRTIO_VSOCK_MAX_MRG_BUF_NUM)
>> + goto err;
>> +
>> + pkt->mergeable = true;
>> + if (!le32_to_cpu(pkt->hdr.len))
>> + return pkt;
>> +
>> + len -= sizeof(struct virtio_vsock_pkt);
>> + pkt->mrg_rxbuf[i].buf = page + sizeof(struct virtio_vsock_pkt);
>> + pkt->mrg_rxbuf[i].len = len;
>> + i++;
>> +
>> + while (--num_buf) {
>> + page = virtqueue_get_buf(vq, &len);
>> + if (!page)
>> + goto err;
>> +
>> + *total_len += len;
>> + vsock->rx_buf_nr--;
>> +
>> + pkt->mrg_rxbuf[i].buf = page;
>> + pkt->mrg_rxbuf[i].len = len;
>> + i++;
>> + }
>> +
>> + return pkt;
>> +err:
>> + virtio_transport_free_pkt(pkt);
>> + return NULL;
>> +}
>
>
> Similar logic could be found at virtio-net driver.
>
> Haven't thought this deeply, but it looks to me use virtio-net driver is also possible, e.g for data-path, just register vsock specific callbacks.
>
>
>> +
>> static void virtio_transport_rx_work(struct work_struct *work)
>> {
>> struct virtio_vsock *vsock =
>> container_of(work, struct virtio_vsock, rx_work);
>> struct virtqueue *vq;
>> + size_t vsock_hlen = vsock->mergeable ? sizeof(struct virtio_vsock_pkt) :
>> + sizeof(struct virtio_vsock_hdr);
>>
>> vq = vsock->vqs[VSOCK_VQ_RX];
>>
>> @@ -383,21 +434,29 @@ static void virtio_transport_rx_work(struct work_struct *work)
>> goto out;
>> }
>>
>> - pkt = virtqueue_get_buf(vq, &len);
>> - if (!pkt) {
>> - break;
>> - }
>> + if (likely(vsock->mergeable)) {
>> + pkt = receive_mergeable(vq, vsock, &len);
>> + if (!pkt)
>> + break;
>>
>> - vsock->rx_buf_nr--;
>> + pkt->len = le32_to_cpu(pkt->hdr.len);
>> + } else {
>> + pkt = virtqueue_get_buf(vq, &len);
>> + if (!pkt) {
>> + break;
>> + }
>> +
>> + vsock->rx_buf_nr--;
>> + }
>>
>> /* Drop short/long packets */
>> - if (unlikely(len < sizeof(pkt->hdr) ||
>> - len > sizeof(pkt->hdr) + pkt->len)) {
>> + if (unlikely(len < vsock_hlen ||
>> + len > vsock_hlen + pkt->len)) {
>> virtio_transport_free_pkt(pkt);
>> continue;
>> }
>>
>> - pkt->len = len - sizeof(pkt->hdr);
>> + pkt->len = len - vsock_hlen;
>> virtio_transport_deliver_tap_pkt(pkt);
>> virtio_transport_recv_pkt(pkt);
>> }
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 3ae3a33..7bef1d5 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -272,14 +272,49 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
>> */
>> spin_unlock_bh(&vvs->rx_lock);
>>
>> - err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
>> - if (err)
>> - goto out;
>> + if (pkt->mergeable) {
>> + struct virtio_vsock_mrg_rxbuf *buf = pkt->mrg_rxbuf;
>> + size_t mrg_copy_bytes, last_buf_total = 0, rxbuf_off;
>> + size_t tmp_bytes = bytes;
>> + int i;
>> +
>> + for (i = 0; i < le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers); i++) {
>> + if (pkt->off > last_buf_total + buf[i].len) {
>> + last_buf_total += buf[i].len;
>> + continue;
>> + }
>> +
>> + rxbuf_off = pkt->off - last_buf_total;
>> + mrg_copy_bytes = min(buf[i].len - rxbuf_off, tmp_bytes);
>> + err = memcpy_to_msg(msg, buf[i].buf + rxbuf_off, mrg_copy_bytes);
>> + if (err)
>> + goto out;
>> +
>> + tmp_bytes -= mrg_copy_bytes;
>> + pkt->off += mrg_copy_bytes;
>> + last_buf_total += buf[i].len;
>> + if (!tmp_bytes)
>> + break;
>> + }
>
>
> After switch to use iov, you can user iov_iter helper to avoid the above open-coding I believe.
>
> And you can also drop the if (mergeable) condition.
>
> Thanks
>
Thanks, I will try to use iov instead.
>
>> +
>> + if (tmp_bytes) {
>> + printk(KERN_WARNING "WARNING! bytes = %llu, "
>> + "bytes = %llu\n",
>> + (unsigned long long)bytes,
>> + (unsigned long long)tmp_bytes);
>> + }
>> +
>> + total += (bytes - tmp_bytes);
>> + } else {
>> + err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
>> + if (err)
>> + goto out;
>> +
>> + total += bytes;
>> + pkt->off += bytes;
>> + }
>>
>> spin_lock_bh(&vvs->rx_lock);
>> -
>> - total += bytes;
>> - pkt->off += bytes;
>> if (pkt->off == pkt->len) {
>> virtio_transport_dec_rx_pkt(vvs, pkt);
>> list_del(&pkt->list);
>> @@ -1050,8 +1085,16 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt)
>>
>> void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt)
>> {
>> - kfree(pkt->buf);
>> - kfree(pkt);
>> + int i;
>> +
>> + if (pkt->mergeable) {
>> + for (i = 1; i < le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers); i++)
>> + free_page((unsigned long)pkt->mrg_rxbuf[i].buf);
>> + free_page((unsigned long)(void *)pkt);
>> + } else {
>> + kfree(pkt->buf);
>> + kfree(pkt);
>> + }
>> }
>> EXPORT_SYMBOL_GPL(virtio_transport_free_pkt);
>>
>
> .
>
^ permalink raw reply
* Re: [PATCH] ath10k: fix some typo
From: Kalle Valo @ 2018-11-06 16:01 UTC (permalink / raw)
To: Yangtao Li
Cc: Yangtao Li, netdev, linux-wireless, linux-kernel, ath10k, davem
In-Reply-To: <20181101152932.22684-1-tiny.windzz@gmail.com>
Yangtao Li <tiny.windzz@gmail.com> wrote:
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
c8cb09644c6c ath10k: fix some typo
--
https://patchwork.kernel.org/patch/10664139/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath10k: fix some typo
From: Kalle Valo @ 2018-11-06 16:01 UTC (permalink / raw)
To: Yangtao Li
Cc: davem, ath10k, linux-wireless, netdev, linux-kernel, Yangtao Li
In-Reply-To: <20181101152932.22684-1-tiny.windzz@gmail.com>
Yangtao Li <tiny.windzz@gmail.com> wrote:
> Signed-off-by: Yangtao Li <tiny.windzz@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
c8cb09644c6c ath10k: fix some typo
--
https://patchwork.kernel.org/patch/10664139/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 2/5] VSOCK: support fill data to mergeable rx buffer in host
From: jiangyiwen @ 2018-11-06 6:30 UTC (permalink / raw)
To: Jason Wang, stefanha; +Cc: netdev, kvm, virtualization
In-Reply-To: <485c2c5d-d73e-e679-9549-aad3de02f0ab@redhat.com>
On 2018/11/6 11:43, Jason Wang wrote:
>
> On 2018/11/5 下午3:45, jiangyiwen wrote:
>> When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature,
>> it will merge big packet into rx vq.
>>
>> Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
>> ---
>> drivers/vhost/vsock.c | 117 +++++++++++++++++++++++++++++++-------
>> include/linux/virtio_vsock.h | 1 +
>> include/uapi/linux/virtio_vsock.h | 5 ++
>> 3 files changed, 102 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
>> index 34bc3ab..648be39 100644
>> --- a/drivers/vhost/vsock.c
>> +++ b/drivers/vhost/vsock.c
>> @@ -22,7 +22,8 @@
>> #define VHOST_VSOCK_DEFAULT_HOST_CID 2
>>
>> enum {
>> - VHOST_VSOCK_FEATURES = VHOST_FEATURES,
>> + VHOST_VSOCK_FEATURES = VHOST_FEATURES |
>> + (1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
>> };
>>
>> /* Used to track all the vhost_vsock instances on the system. */
>> @@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> return vsock;
>> }
>>
>> +static int get_rx_bufs(struct vhost_virtqueue *vq,
>> + struct vring_used_elem *heads, int datalen,
>> + unsigned *iovcount, unsigned int quota)
>> +{
>> + unsigned int out, in;
>> + int seg = 0;
>> + int headcount = 0;
>> + unsigned d;
>> + int ret;
>> + /*
>> + * len is always initialized before use since we are always called with
>> + * datalen > 0.
>> + */
>> + u32 uninitialized_var(len);
>> +
>> + while (datalen > 0 && headcount < quota) {
>> + if (unlikely(seg >= UIO_MAXIOV)) {
>> + ret = -ENOBUFS;
>> + goto err;
>> + }
>> +
>> + ret = vhost_get_vq_desc(vq, vq->iov + seg,
>> + ARRAY_SIZE(vq->iov) - seg, &out,
>> + &in, NULL, NULL);
>> + if (unlikely(ret < 0))
>> + goto err;
>> +
>> + d = ret;
>> + if (d == vq->num) {
>> + ret = 0;
>> + goto err;
>> + }
>> +
>> + if (unlikely(out || in <= 0)) {
>> + vq_err(vq, "unexpected descriptor format for RX: "
>> + "out %d, in %d\n", out, in);
>> + ret = -EINVAL;
>> + goto err;
>> + }
>> +
>> + heads[headcount].id = cpu_to_vhost32(vq, d);
>> + len = iov_length(vq->iov + seg, in);
>> + heads[headcount].len = cpu_to_vhost32(vq, len);
>> + datalen -= len;
>> + ++headcount;
>> + seg += in;
>> + }
>> +
>> + heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
>> + *iovcount = seg;
>> +
>> + /* Detect overrun */
>> + if (unlikely(datalen > 0)) {
>> + ret = UIO_MAXIOV + 1;
>> + goto err;
>> + }
>> + return headcount;
>> +err:
>> + vhost_discard_vq_desc(vq, headcount);
>> + return ret;
>> +}
>
>
> Seems duplicated with the one used by vhost-net.
>
> In packed virtqueue implementation, I plan to move this to vhost.c.
>
Yes, this code is full copied from vhost-net, if it can be packed into
vhost.c, it would be great.
>
>> +
>> static void
>> vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>> struct vhost_virtqueue *vq)
>> @@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
>> bool added = false;
>> bool restart_tx = false;
>> + int mergeable;
>> + size_t vsock_hlen;
>>
>> mutex_lock(&vq->mutex);
>>
>> if (!vq->private_data)
>> goto out;
>>
>> + mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
>> + /*
>> + * Guest fill page for rx vq in mergeable case, so it will not
>> + * allocate pkt structure, we should reserve size of pkt in advance.
>> + */
>> + if (likely(mergeable))
>> + vsock_hlen = sizeof(struct virtio_vsock_pkt);
>> + else
>> + vsock_hlen = sizeof(struct virtio_vsock_hdr);
>> +
>> /* Avoid further vmexits, we're already processing the virtqueue */
>> vhost_disable_notify(&vsock->dev, vq);
>>
>> for (;;) {
>> struct virtio_vsock_pkt *pkt;
>> struct iov_iter iov_iter;
>> - unsigned out, in;
>> + unsigned out = 0, in = 0;
>> size_t nbytes;
>> size_t len;
>> - int head;
>> + s16 headcount;
>>
>> spin_lock_bh(&vsock->send_pkt_list_lock);
>> if (list_empty(&vsock->send_pkt_list)) {
>> @@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> list_del_init(&pkt->list);
>> spin_unlock_bh(&vsock->send_pkt_list_lock);
>>
>> - head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
>> - &out, &in, NULL, NULL);
>> - if (head < 0) {
>> - spin_lock_bh(&vsock->send_pkt_list_lock);
>> - list_add(&pkt->list, &vsock->send_pkt_list);
>> - spin_unlock_bh(&vsock->send_pkt_list_lock);
>> - break;
>> - }
>> -
>> - if (head == vq->num) {
>> + headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
>> + &in, likely(mergeable) ? UIO_MAXIOV : 1);
>> + if (headcount <= 0) {
>> spin_lock_bh(&vsock->send_pkt_list_lock);
>> list_add(&pkt->list, &vsock->send_pkt_list);
>> spin_unlock_bh(&vsock->send_pkt_list_lock);
>> @@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> /* We cannot finish yet if more buffers snuck in while
>> * re-enabling notify.
>> */
>> - if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
>> + if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
>> vhost_disable_notify(&vsock->dev, vq);
>> continue;
>> }
>> break;
>> }
>>
>> - if (out) {
>> - virtio_transport_free_pkt(pkt);
>> - vq_err(vq, "Expected 0 output buffers, got %u\n", out);
>> - break;
>> - }
>> -
>> len = iov_length(&vq->iov[out], in);
>> iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
>>
>> @@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> break;
>> }
>>
>> + if (likely(mergeable)) {
>> + pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
>> + nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
>> + sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
>> + if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
>> + virtio_transport_free_pkt(pkt);
>> + vq_err(vq, "Faulted on copying rxbuf hdr\n");
>> + break;
>> + }
>> + iov_iter_advance(&iov_iter, (vsock_hlen -
>> + sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
>> + }
>> +
>> nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
>> if (nbytes != pkt->len) {
>> virtio_transport_free_pkt(pkt);
>> @@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>> break;
>> }
>>
>> - vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
>> + vhost_add_used_n(vq, vq->heads, headcount);
>> added = true;
>
>
> Looks rather similar to vhost-net mergeable rx buffer implementation. Another proof of using vhost-net.
>
> Thanks
Yes.
>
>
>>
>> if (pkt->reply) {
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index bf84418..da9e1fe 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -50,6 +50,7 @@ struct virtio_vsock_sock {
>>
>> struct virtio_vsock_pkt {
>> struct virtio_vsock_hdr hdr;
>> + struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
>> struct work_struct work;
>> struct list_head list;
>> /* socket refcnt not held, only use for cancellation */
>> diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
>> index 1d57ed3..2292f30 100644
>> --- a/include/uapi/linux/virtio_vsock.h
>> +++ b/include/uapi/linux/virtio_vsock.h
>> @@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
>> __le32 fwd_cnt;
>> } __attribute__((packed));
>>
>> +/* It add mergeable rx buffers feature */
>> +struct virtio_vsock_mrg_rxbuf_hdr {
>> + __le16 num_buffers; /* number of mergeable rx buffers */
>> +} __attribute__((packed));
>> +
>> enum virtio_vsock_type {
>> VIRTIO_VSOCK_TYPE_STREAM = 1,
>> };
>
> .
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
From: Alexei Starovoitov @ 2018-11-06 6:29 UTC (permalink / raw)
To: Edward Cree
Cc: Martin Lau, Yonghong Song, Alexei Starovoitov,
daniel@iogearbox.net, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <dffb0db2-521c-0adb-ff3d-68a379d17b26@solarflare.com>
On Thu, Nov 01, 2018 at 09:08:37PM +0000, Edward Cree wrote:
> I've spent a bit more time thinking about / sleeping on this, and I
> still think there's a major disagreement here. Basically it seems
> like I'm saying "the design of BTF is wrong" and you're saying "but
> it's the design" (with the possible implication — I'm not entirely
> sure — of "but that's what DWARF does").
> So let's back away from the details about FUNC/PROTO, and talk in
> more general terms about what a BTF record means.
> There are two classes of things we might want to put in debug-info:
> * There exists a type T
> * I have an instance X (variable, subprogram, etc.) of type T
> Both of these may need to reference other types, and have the same
> space of possible things T could be, but there the similarity ends:
> they are semantically different things.
> Indeed, the only reason for any record of the first class is to
> define types referenced by records of the second class. Some
> concrete examples of records of the second class are:
> 1) I have a map named "foo" with key-type T1 and value-type T2
> 2) I have a subprogram named "bar" with prototype T3
> 3) I am using stack slot fp-8 to store a value of type T4
> 4) I am casting ctx+8 to a pointer type T5 before dereferencing it
> Currently we have (1) and this patch series adds (2), both done
> through records that look like they are just defining a type (i.e.
> the first class of record) but have 'magic' semantics (in the case
> of (1), special names of the form ____btf_map_foo. How anyone
> thought that was a clean and tasteful design is beyond me.)
> What IMHO the design *should* be, is that we have a 'types'
> subsection that *only* contains records of the first class, and
> then other subsections to hold records of the second class that
> reference records of the first class by ID.
Such pure type approach wouldn't be practical.
BTF is not pure type information. BTF is everything that verifier needs
to know to make safety decisions that bpf instruction set doesn't have.
For example two anonymous structs:
struct {
int a;
int b;
} var1;
struct {
int c;
int d;
} var2;
from C point of view have the same type, but from BTF point of view
they are different. Names of the fields are essential part of the BTF
because the purpose of BTF is to provide information about bpf objects
for debugging and safety reasons.
Similarly
int (*) (void *src, void *dst, int len);
and
int (*) (void *dst, void *src, int len);
are the same from C and compiler point of view,
but they are different in BTF, because names carry information
that needs to be preserved.
Same goes for function declarations. The function name and argument
names are part of the 'type description'.
We shouldn't be using word 'type' in pure form otherwise
it will cause confusion like this thread demonstrated.
Beyond prog names expressed in BTF we're adding global variables support.
They will be expressed in BTF as a new KIND.
Think of all global variables in a single .c file as fields of
anonymous structure. They have offsets from beginning of .bss,
sizes, further references into btf_type_ids and most importantly names.
Another thing we're working on is spin_lock support.
There we also have to rely on BTF to make sure that the certain
bytes of map's value or cgroup local storage that belong to spin_lock
will be masked for lookup/update calls.
typedef u32 bpf_spin_lock;
will be recognized by the verifier by its name.
May be we will introduce new KIND_ for spin_lock too and convert name
into KIND in btf writer. That is TBD. The main point that
names of types, variables, functions has to be expressed in BTF
as one coherent graph of information.
Splitting pure types into one section, variables into another,
functions into yet another is not practical, since the same
modifiers (like const or volatile) need to be applied to
variables and functions. At the end all sections will have
the same style of encoding, hence no need to duplicate
the encoding three times and instead it's cleaner to encode
all of them BTF-style via different KINDs.
vmlinux's BTF will include all global variables and functions,
so that tracing scripts can reference to particular variable
or function argument by name making kernel debuging easier
and less error prone.
Consider that right now typical bcc's trace.py script looks like:
trace.py -I linux/skbuff.h -I net/sock.h \
'skb_recv_datagram(struct sock *sk, unsigned int flags, int noblock, int *err) \
"sk_recv_q:%llx next:%llx prev:%llx" \
&sk->sk_receive_queue,sk->sk_receive_queue.next,sk->sk_receive_queue.prev'
where func declaration is copy pasted from the C source
and there are no guarantees that argument names and their order match
what vmlinux actually has.
With fully named BTF of vmlinux the tracing scripts will become more robust.
Note that BTF_KIND_FUNC is pretty much the same as BTF_KIND_STRUCT with
an addition of return type.
Kernel specific things like per-cpu attribute of the variable will
be BTF KIND too. This is information that tooling and kernel needs
and current BTF graph design is perfectly suited to carry such info.
^ 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