* [PATCH v7 bpf-next 1/5] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
to user space for additional processing. libbpf already has very low-level API
to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
use and requires a lot of code to set everything up. This patch adds
perf_buffer abstraction on top of it, abstracting setting up and polling
per-CPU logic into simple and convenient API, similar to what BCC provides.
perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
map entries. It accepts two user-provided callbacks: one for handling raw
samples and one for get notifications of lost samples due to buffer overflow.
perf_buffer__new_raw() is similar, but provides more control over how
perf events are set up (by accepting user-provided perf_event_attr), how
they are handled (perf_event_header pointer is passed directly to
user-provided callback), and on which CPUs ring buffers are created
(it's possible to provide a list of CPUs and corresponding map keys to
update). This API allows advanced users fuller control.
perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
utilizing epoll instance.
perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
All APIs are not thread-safe. User should ensure proper locking/coordination if
used in multi-threaded set up.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/lib/bpf/libbpf.c | 366 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 49 ++++++
tools/lib/bpf/libbpf.map | 4 +
3 files changed, 419 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2a08eb106221..ae569b50e2e0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,7 +32,9 @@
#include <linux/limits.h>
#include <linux/perf_event.h>
#include <linux/ring_buffer.h>
+#include <sys/epoll.h>
#include <sys/ioctl.h>
+#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
@@ -4354,6 +4356,370 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
return ret;
}
+struct perf_buffer;
+
+struct perf_buffer_params {
+ struct perf_event_attr *attr;
+ /* if event_cb is specified, it takes precendence */
+ perf_buffer_event_fn event_cb;
+ /* sample_cb and lost_cb are higher-level common-case callbacks */
+ perf_buffer_sample_fn sample_cb;
+ perf_buffer_lost_fn lost_cb;
+ void *ctx;
+ int cpu_cnt;
+ int *cpus;
+ int *map_keys;
+};
+
+struct perf_cpu_buf {
+ struct perf_buffer *pb;
+ void *base; /* mmap()'ed memory */
+ void *buf; /* for reconstructing segmented data */
+ size_t buf_size;
+ int fd;
+ int cpu;
+ int map_key;
+};
+
+struct perf_buffer {
+ perf_buffer_event_fn event_cb;
+ perf_buffer_sample_fn sample_cb;
+ perf_buffer_lost_fn lost_cb;
+ void *ctx; /* passed into callbacks */
+
+ size_t page_size;
+ size_t mmap_size;
+ struct perf_cpu_buf **cpu_bufs;
+ struct epoll_event *events;
+ int cpu_cnt;
+ int epoll_fd; /* perf event FD */
+ int map_fd; /* BPF_MAP_TYPE_PERF_EVENT_ARRAY BPF map FD */
+};
+
+static void perf_buffer__free_cpu_buf(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf)
+{
+ if (!cpu_buf)
+ return;
+ if (cpu_buf->base &&
+ munmap(cpu_buf->base, pb->mmap_size + pb->page_size))
+ pr_warning("failed to munmap cpu_buf #%d\n", cpu_buf->cpu);
+ if (cpu_buf->fd >= 0) {
+ ioctl(cpu_buf->fd, PERF_EVENT_IOC_DISABLE, 0);
+ close(cpu_buf->fd);
+ }
+ free(cpu_buf->buf);
+ free(cpu_buf);
+}
+
+void perf_buffer__free(struct perf_buffer *pb)
+{
+ int i;
+
+ if (!pb)
+ return;
+ if (pb->cpu_bufs) {
+ for (i = 0; i < pb->cpu_cnt && pb->cpu_bufs[i]; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->cpu_bufs[i];
+
+ bpf_map_delete_elem(pb->map_fd, &cpu_buf->map_key);
+ perf_buffer__free_cpu_buf(pb, cpu_buf);
+ }
+ free(pb->cpu_bufs);
+ }
+ if (pb->epoll_fd >= 0)
+ close(pb->epoll_fd);
+ free(pb->events);
+ free(pb);
+}
+
+static struct perf_cpu_buf *
+perf_buffer__open_cpu_buf(struct perf_buffer *pb, struct perf_event_attr *attr,
+ int cpu, int map_key)
+{
+ struct perf_cpu_buf *cpu_buf;
+ char msg[STRERR_BUFSIZE];
+ int err;
+
+ cpu_buf = calloc(1, sizeof(*cpu_buf));
+ if (!cpu_buf)
+ return ERR_PTR(-ENOMEM);
+
+ cpu_buf->pb = pb;
+ cpu_buf->cpu = cpu;
+ cpu_buf->map_key = map_key;
+
+ cpu_buf->fd = syscall(__NR_perf_event_open, attr, -1 /* pid */, cpu,
+ -1, PERF_FLAG_FD_CLOEXEC);
+ if (cpu_buf->fd < 0) {
+ err = -errno;
+ pr_warning("failed to open perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ cpu_buf->base = mmap(NULL, pb->mmap_size + pb->page_size,
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ cpu_buf->fd, 0);
+ if (cpu_buf->base == MAP_FAILED) {
+ cpu_buf->base = NULL;
+ err = -errno;
+ pr_warning("failed to mmap perf buffer on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ if (ioctl(cpu_buf->fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ pr_warning("failed to enable perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ return cpu_buf;
+
+error:
+ perf_buffer__free_cpu_buf(pb, cpu_buf);
+ return (struct perf_cpu_buf *)ERR_PTR(err);
+}
+
+static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
+ struct perf_buffer_params *p);
+
+struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
+ const struct perf_buffer_opts *opts)
+{
+ struct perf_buffer_params p = {};
+ struct perf_event_attr attr = {
+ .config = PERF_COUNT_SW_BPF_OUTPUT,
+ .type = PERF_TYPE_SOFTWARE,
+ .sample_type = PERF_SAMPLE_RAW,
+ .sample_period = 1,
+ .wakeup_events = 1,
+ };
+
+ p.attr = &attr;
+ p.sample_cb = opts ? opts->sample_cb : NULL;
+ p.lost_cb = opts ? opts->lost_cb : NULL;
+ p.ctx = opts ? opts->ctx : NULL;
+
+ return __perf_buffer__new(map_fd, page_cnt, &p);
+}
+
+struct perf_buffer *
+perf_buffer__new_raw(int map_fd, size_t page_cnt,
+ const struct perf_buffer_raw_opts *opts)
+{
+ struct perf_buffer_params p = {};
+
+ p.attr = opts->attr;
+ p.event_cb = opts->event_cb;
+ p.ctx = opts->ctx;
+ p.cpu_cnt = opts->cpu_cnt;
+ p.cpus = opts->cpus;
+ p.map_keys = opts->map_keys;
+
+ return __perf_buffer__new(map_fd, page_cnt, &p);
+}
+
+static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
+ struct perf_buffer_params *p)
+{
+ struct bpf_map_info map = {};
+ char msg[STRERR_BUFSIZE];
+ struct perf_buffer *pb;
+ __u32 map_info_len;
+ int err, i;
+
+ if (page_cnt & (page_cnt - 1)) {
+ pr_warning("page count should be power of two, but is %zu\n",
+ page_cnt);
+ return ERR_PTR(-EINVAL);
+ }
+
+ map_info_len = sizeof(map);
+ err = bpf_obj_get_info_by_fd(map_fd, &map, &map_info_len);
+ if (err) {
+ err = -errno;
+ pr_warning("failed to get map info for map FD %d: %s\n",
+ map_fd, libbpf_strerror_r(err, msg, sizeof(msg)));
+ return ERR_PTR(err);
+ }
+
+ if (map.type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+ pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+ map.name);
+ return ERR_PTR(-EINVAL);
+ }
+
+ pb = calloc(1, sizeof(*pb));
+ if (!pb)
+ return ERR_PTR(-ENOMEM);
+
+ pb->event_cb = p->event_cb;
+ pb->sample_cb = p->sample_cb;
+ pb->lost_cb = p->lost_cb;
+ pb->ctx = p->ctx;
+
+ pb->page_size = getpagesize();
+ pb->mmap_size = pb->page_size * page_cnt;
+ pb->map_fd = map_fd;
+
+ pb->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
+ if (pb->epoll_fd < 0) {
+ err = -errno;
+ pr_warning("failed to create epoll instance: %s\n",
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ if (p->cpu_cnt > 0) {
+ pb->cpu_cnt = p->cpu_cnt;
+ } else {
+ pb->cpu_cnt = libbpf_num_possible_cpus();
+ if (pb->cpu_cnt < 0) {
+ err = pb->cpu_cnt;
+ goto error;
+ }
+ if (map.max_entries < pb->cpu_cnt)
+ pb->cpu_cnt = map.max_entries;
+ }
+
+ pb->events = calloc(pb->cpu_cnt, sizeof(*pb->events));
+ if (!pb->events) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate events: out of memory\n");
+ goto error;
+ }
+ pb->cpu_bufs = calloc(pb->cpu_cnt, sizeof(*pb->cpu_bufs));
+ if (!pb->cpu_bufs) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate buffers: out of memory\n");
+ goto error;
+ }
+
+ for (i = 0; i < pb->cpu_cnt; i++) {
+ struct perf_cpu_buf *cpu_buf;
+ int cpu, map_key;
+
+ cpu = p->cpu_cnt > 0 ? p->cpus[i] : i;
+ map_key = p->cpu_cnt > 0 ? p->map_keys[i] : i;
+
+ cpu_buf = perf_buffer__open_cpu_buf(pb, p->attr, cpu, map_key);
+ if (IS_ERR(cpu_buf)) {
+ err = PTR_ERR(cpu_buf);
+ goto error;
+ }
+
+ pb->cpu_bufs[i] = cpu_buf;
+
+ err = bpf_map_update_elem(pb->map_fd, &map_key,
+ &cpu_buf->fd, 0);
+ if (err) {
+ err = -errno;
+ pr_warning("failed to set cpu #%d, key %d -> perf FD %d: %s\n",
+ cpu, map_key, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ pb->events[i].events = EPOLLIN;
+ pb->events[i].data.ptr = cpu_buf;
+ if (epoll_ctl(pb->epoll_fd, EPOLL_CTL_ADD, cpu_buf->fd,
+ &pb->events[i]) < 0) {
+ err = -errno;
+ pr_warning("failed to epoll_ctl cpu #%d perf FD %d: %s\n",
+ cpu, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+ }
+
+ return pb;
+
+error:
+ if (pb)
+ perf_buffer__free(pb);
+ return ERR_PTR(err);
+}
+
+struct perf_sample_raw {
+ struct perf_event_header header;
+ uint32_t size;
+ char data[0];
+};
+
+struct perf_sample_lost {
+ struct perf_event_header header;
+ uint64_t id;
+ uint64_t lost;
+ uint64_t sample_id;
+};
+
+static enum bpf_perf_event_ret
+perf_buffer__process_record(struct perf_event_header *e, void *ctx)
+{
+ struct perf_cpu_buf *cpu_buf = ctx;
+ struct perf_buffer *pb = cpu_buf->pb;
+ void *data = e;
+
+ /* user wants full control over parsing perf event */
+ if (pb->event_cb)
+ return pb->event_cb(pb->ctx, cpu_buf->cpu, e);
+
+ switch (e->type) {
+ case PERF_RECORD_SAMPLE: {
+ struct perf_sample_raw *s = data;
+
+ if (pb->sample_cb)
+ pb->sample_cb(pb->ctx, cpu_buf->cpu, s->data, s->size);
+ break;
+ }
+ case PERF_RECORD_LOST: {
+ struct perf_sample_lost *s = data;
+
+ if (pb->lost_cb)
+ pb->lost_cb(pb->ctx, cpu_buf->cpu, s->lost);
+ break;
+ }
+ default:
+ pr_warning("unknown perf sample type %d\n", e->type);
+ return LIBBPF_PERF_EVENT_ERROR;
+ }
+ return LIBBPF_PERF_EVENT_CONT;
+}
+
+static int perf_buffer__process_records(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf)
+{
+ enum bpf_perf_event_ret ret;
+
+ ret = bpf_perf_event_read_simple(cpu_buf->base, pb->mmap_size,
+ pb->page_size, &cpu_buf->buf,
+ &cpu_buf->buf_size,
+ perf_buffer__process_record, cpu_buf);
+ if (ret != LIBBPF_PERF_EVENT_CONT)
+ return ret;
+ return 0;
+}
+
+int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
+{
+ int i, cnt, err;
+
+ cnt = epoll_wait(pb->epoll_fd, pb->events, pb->cpu_cnt, timeout_ms);
+ for (i = 0; i < cnt; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->events[i].data.ptr;
+
+ err = perf_buffer__process_records(pb, cpu_buf);
+ if (err) {
+ pr_warning("error while processing records: %d\n", err);
+ return err;
+ }
+ }
+ return cnt < 0 ? -errno : cnt;
+}
+
struct bpf_prog_info_array_desc {
int array_offset; /* e.g. offset of jited_prog_insns */
int count_offset; /* e.g. offset of jited_prog_len */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index f55933784f95..5cbf459ece0b 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -358,6 +358,26 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
+struct perf_buffer;
+
+typedef void (*perf_buffer_sample_fn)(void *ctx, int cpu,
+ void *data, __u32 size);
+typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
+
+/* common use perf buffer options */
+struct perf_buffer_opts {
+ /* if specified, sample_cb is called for each sample */
+ perf_buffer_sample_fn sample_cb;
+ /* if specified, lost_cb is called for each batch of lost samples */
+ perf_buffer_lost_fn lost_cb;
+ /* ctx is provided to sample_cb and lost_cb */
+ void *ctx;
+};
+
+LIBBPF_API struct perf_buffer *
+perf_buffer__new(int map_fd, size_t page_cnt,
+ const struct perf_buffer_opts *opts);
+
enum bpf_perf_event_ret {
LIBBPF_PERF_EVENT_DONE = 0,
LIBBPF_PERF_EVENT_ERROR = -1,
@@ -365,6 +385,35 @@ enum bpf_perf_event_ret {
};
struct perf_event_header;
+
+typedef enum bpf_perf_event_ret
+(*perf_buffer_event_fn)(void *ctx, int cpu, struct perf_event_header *event);
+
+/* raw perf buffer options, giving most power and control */
+struct perf_buffer_raw_opts {
+ /* perf event attrs passed directly into perf_event_open() */
+ struct perf_event_attr *attr;
+ /* raw event callback */
+ perf_buffer_event_fn event_cb;
+ /* ctx is provided to event_cb */
+ void *ctx;
+ /* if cpu_cnt == 0, open all on all possible CPUs (up to the number of
+ * max_entries of given PERF_EVENT_ARRAY map)
+ */
+ int cpu_cnt;
+ /* if cpu_cnt > 0, cpus is an array of CPUs to open ring buffers on */
+ int *cpus;
+ /* if cpu_cnt > 0, map_keys specify map keys to set per-CPU FDs for */
+ int *map_keys;
+};
+
+LIBBPF_API struct perf_buffer *
+perf_buffer__new_raw(int map_fd, size_t page_cnt,
+ const struct perf_buffer_raw_opts *opts);
+
+LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
+LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
+
typedef enum bpf_perf_event_ret
(*bpf_perf_event_print_t)(struct perf_event_header *hdr,
void *private_data);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e6b7d4edbc93..f9d316e873d8 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -179,4 +179,8 @@ LIBBPF_0.0.4 {
btf_dump__new;
btf__parse_elf;
libbpf_num_possible_cpus;
+ perf_buffer__free;
+ perf_buffer__new;
+ perf_buffer__new_raw;
+ perf_buffer__poll;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH v7 bpf-next 5/5] libbpf: add perf_buffer_ prefix to README
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
perf_buffer "object" is part of libbpf API now, add it to the list of
libbpf function prefixes.
Suggested-by: Daniel Borkman <daniel@iogearbox.net>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/lib/bpf/README.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/README.rst b/tools/lib/bpf/README.rst
index cef7b77eab69..8928f7787f2d 100644
--- a/tools/lib/bpf/README.rst
+++ b/tools/lib/bpf/README.rst
@@ -9,7 +9,8 @@ described here. It's recommended to follow these conventions whenever a
new function or type is added to keep libbpf API clean and consistent.
All types and functions provided by libbpf API should have one of the
-following prefixes: ``bpf_``, ``btf_``, ``libbpf_``, ``xsk_``.
+following prefixes: ``bpf_``, ``btf_``, ``libbpf_``, ``xsk_``,
+``perf_buffer_``.
System call wrappers
--------------------
--
2.17.1
^ permalink raw reply related
* [PATCH v7 bpf-next 3/5] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
Add test verifying perf buffer API functionality.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
.../selftests/bpf/prog_tests/perf_buffer.c | 100 ++++++++++++++++++
.../selftests/bpf/progs/test_perf_buffer.c | 25 +++++
2 files changed, 125 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
new file mode 100644
index 000000000000..3f1ef95865ff
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sched.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+#ifdef __x86_64__
+#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
+#else
+#define SYS_KPROBE_NAME "sys_nanosleep"
+#endif
+
+static void on_sample(void *ctx, int cpu, void *data, __u32 size)
+{
+ int cpu_data = *(int *)data, duration = 0;
+ cpu_set_t *cpu_seen = ctx;
+
+ if (cpu_data != cpu)
+ CHECK(cpu_data != cpu, "check_cpu_data",
+ "cpu_data %d != cpu %d\n", cpu_data, cpu);
+
+ CPU_SET(cpu, cpu_seen);
+}
+
+void test_perf_buffer(void)
+{
+ int err, prog_fd, nr_cpus, i, duration = 0;
+ const char *prog_name = "kprobe/sys_nanosleep";
+ const char *file = "./test_perf_buffer.o";
+ struct perf_buffer_opts pb_opts = {};
+ struct bpf_map *perf_buf_map;
+ cpu_set_t cpu_set, cpu_seen;
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ struct perf_buffer *pb;
+ struct bpf_link *link;
+
+ nr_cpus = libbpf_num_possible_cpus();
+ if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
+ return;
+
+ /* load program */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ prog = bpf_object__find_program_by_title(obj, prog_name);
+ if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
+ goto out_close;
+
+ /* load map */
+ perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
+ if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
+ goto out_close;
+
+ /* attach kprobe */
+ link = bpf_program__attach_kprobe(prog, false /* retprobe */,
+ SYS_KPROBE_NAME);
+ if (CHECK(IS_ERR(link), "attach_kprobe", "err %ld\n", PTR_ERR(link)))
+ goto out_close;
+
+ /* set up perf buffer */
+ pb_opts.sample_cb = on_sample;
+ pb_opts.ctx = &cpu_seen;
+ pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts);
+ if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
+ goto out_detach;
+
+ /* trigger kprobe on every CPU */
+ CPU_ZERO(&cpu_seen);
+ for (i = 0; i < nr_cpus; i++) {
+ CPU_ZERO(&cpu_set);
+ CPU_SET(i, &cpu_set);
+
+ err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
+ &cpu_set);
+ if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
+ i, err))
+ goto out_detach;
+
+ usleep(1);
+ }
+
+ /* read perf buffer */
+ err = perf_buffer__poll(pb, 100);
+ if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
+ goto out_free_pb;
+
+ if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
+ "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
+ goto out_free_pb;
+
+out_free_pb:
+ perf_buffer__free(pb);
+out_detach:
+ bpf_link__destroy(link);
+out_close:
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
new file mode 100644
index 000000000000..876c27deb65a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
+ __uint(key_size, sizeof(int));
+ __uint(value_size, sizeof(int));
+} perf_buf_map SEC(".maps");
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+ int cpu = bpf_get_smp_processor_id();
+
+ bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
+ &cpu, sizeof(cpu));
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
--
2.17.1
^ permalink raw reply related
* [PATCH v7 bpf-next 4/5] tools/bpftool: switch map event_pipe to libbpf's perf_buffer
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
Switch event_pipe implementation to rely on new libbpf perf buffer API
(it's raw low-level variant).
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
tools/bpf/bpftool/map_perf_ring.c | 201 ++++++++++--------------------
1 file changed, 64 insertions(+), 137 deletions(-)
diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c
index 0507dfaf7a8f..3f108ab17797 100644
--- a/tools/bpf/bpftool/map_perf_ring.c
+++ b/tools/bpf/bpftool/map_perf_ring.c
@@ -28,7 +28,7 @@
#define MMAP_PAGE_CNT 16
-static bool stop;
+static volatile bool stop;
struct event_ring_info {
int fd;
@@ -44,32 +44,44 @@ struct perf_event_sample {
unsigned char data[];
};
+struct perf_event_lost {
+ struct perf_event_header header;
+ __u64 id;
+ __u64 lost;
+};
+
static void int_exit(int signo)
{
fprintf(stderr, "Stopping...\n");
stop = true;
}
+struct event_pipe_ctx {
+ bool all_cpus;
+ int cpu;
+ int idx;
+};
+
static enum bpf_perf_event_ret
-print_bpf_output(struct perf_event_header *event, void *private_data)
+print_bpf_output(void *private_data, int cpu, struct perf_event_header *event)
{
- struct perf_event_sample *e = container_of(event, struct perf_event_sample,
+ struct perf_event_sample *e = container_of(event,
+ struct perf_event_sample,
header);
- struct event_ring_info *ring = private_data;
- struct {
- struct perf_event_header header;
- __u64 id;
- __u64 lost;
- } *lost = (typeof(lost))event;
+ struct perf_event_lost *lost = container_of(event,
+ struct perf_event_lost,
+ header);
+ struct event_pipe_ctx *ctx = private_data;
+ int idx = ctx->all_cpus ? cpu : ctx->idx;
if (json_output) {
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "type");
jsonw_uint(json_wtr, e->header.type);
jsonw_name(json_wtr, "cpu");
- jsonw_uint(json_wtr, ring->cpu);
+ jsonw_uint(json_wtr, cpu);
jsonw_name(json_wtr, "index");
- jsonw_uint(json_wtr, ring->key);
+ jsonw_uint(json_wtr, idx);
if (e->header.type == PERF_RECORD_SAMPLE) {
jsonw_name(json_wtr, "timestamp");
jsonw_uint(json_wtr, e->time);
@@ -89,7 +101,7 @@ print_bpf_output(struct perf_event_header *event, void *private_data)
if (e->header.type == PERF_RECORD_SAMPLE) {
printf("== @%lld.%09lld CPU: %d index: %d =====\n",
e->time / 1000000000ULL, e->time % 1000000000ULL,
- ring->cpu, ring->key);
+ cpu, idx);
fprint_hex(stdout, e->data, e->size, " ");
printf("\n");
} else if (e->header.type == PERF_RECORD_LOST) {
@@ -103,87 +115,25 @@ print_bpf_output(struct perf_event_header *event, void *private_data)
return LIBBPF_PERF_EVENT_CONT;
}
-static void
-perf_event_read(struct event_ring_info *ring, void **buf, size_t *buf_len)
-{
- enum bpf_perf_event_ret ret;
-
- ret = bpf_perf_event_read_simple(ring->mem,
- MMAP_PAGE_CNT * get_page_size(),
- get_page_size(), buf, buf_len,
- print_bpf_output, ring);
- if (ret != LIBBPF_PERF_EVENT_CONT) {
- fprintf(stderr, "perf read loop failed with %d\n", ret);
- stop = true;
- }
-}
-
-static int perf_mmap_size(void)
-{
- return get_page_size() * (MMAP_PAGE_CNT + 1);
-}
-
-static void *perf_event_mmap(int fd)
-{
- int mmap_size = perf_mmap_size();
- void *base;
-
- base = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- if (base == MAP_FAILED) {
- p_err("event mmap failed: %s\n", strerror(errno));
- return NULL;
- }
-
- return base;
-}
-
-static void perf_event_unmap(void *mem)
-{
- if (munmap(mem, perf_mmap_size()))
- fprintf(stderr, "Can't unmap ring memory!\n");
-}
-
-static int bpf_perf_event_open(int map_fd, int key, int cpu)
+int do_event_pipe(int argc, char **argv)
{
- struct perf_event_attr attr = {
+ struct perf_event_attr perf_attr = {
.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_TIME,
.type = PERF_TYPE_SOFTWARE,
.config = PERF_COUNT_SW_BPF_OUTPUT,
+ .sample_period = 1,
+ .wakeup_events = 1,
};
- int pmu_fd;
-
- pmu_fd = sys_perf_event_open(&attr, -1, cpu, -1, 0);
- if (pmu_fd < 0) {
- p_err("failed to open perf event %d for CPU %d", key, cpu);
- return -1;
- }
-
- if (bpf_map_update_elem(map_fd, &key, &pmu_fd, BPF_ANY)) {
- p_err("failed to update map for event %d for CPU %d", key, cpu);
- goto err_close;
- }
- if (ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0)) {
- p_err("failed to enable event %d for CPU %d", key, cpu);
- goto err_close;
- }
-
- return pmu_fd;
-
-err_close:
- close(pmu_fd);
- return -1;
-}
-
-int do_event_pipe(int argc, char **argv)
-{
- int i, nfds, map_fd, index = -1, cpu = -1;
struct bpf_map_info map_info = {};
- struct event_ring_info *rings;
- size_t tmp_buf_sz = 0;
- void *tmp_buf = NULL;
- struct pollfd *pfds;
+ struct perf_buffer_raw_opts opts = {};
+ struct event_pipe_ctx ctx = {
+ .all_cpus = true,
+ .cpu = -1,
+ .idx = -1,
+ };
+ struct perf_buffer *pb;
__u32 map_info_len;
- bool do_all = true;
+ int err, map_fd;
map_info_len = sizeof(map_info);
map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len);
@@ -205,7 +155,7 @@ int do_event_pipe(int argc, char **argv)
char *endptr;
NEXT_ARG();
- cpu = strtoul(*argv, &endptr, 0);
+ ctx.cpu = strtoul(*argv, &endptr, 0);
if (*endptr) {
p_err("can't parse %s as CPU ID", **argv);
goto err_close_map;
@@ -216,7 +166,7 @@ int do_event_pipe(int argc, char **argv)
char *endptr;
NEXT_ARG();
- index = strtoul(*argv, &endptr, 0);
+ ctx.idx = strtoul(*argv, &endptr, 0);
if (*endptr) {
p_err("can't parse %s as index", **argv);
goto err_close_map;
@@ -228,45 +178,32 @@ int do_event_pipe(int argc, char **argv)
goto err_close_map;
}
- do_all = false;
+ ctx.all_cpus = false;
}
- if (!do_all) {
- if (index == -1 || cpu == -1) {
+ if (!ctx.all_cpus) {
+ if (ctx.idx == -1 || ctx.cpu == -1) {
p_err("cpu and index must be specified together");
goto err_close_map;
}
-
- nfds = 1;
} else {
- nfds = min(get_possible_cpus(), map_info.max_entries);
- cpu = 0;
- index = 0;
+ ctx.cpu = 0;
+ ctx.idx = 0;
}
- rings = calloc(nfds, sizeof(rings[0]));
- if (!rings)
+ opts.attr = &perf_attr;
+ opts.event_cb = print_bpf_output;
+ opts.ctx = &ctx;
+ opts.cpu_cnt = ctx.all_cpus ? 0 : 1;
+ opts.cpus = &ctx.cpu;
+ opts.map_keys = &ctx.idx;
+
+ pb = perf_buffer__new_raw(map_fd, MMAP_PAGE_CNT, &opts);
+ err = libbpf_get_error(pb);
+ if (err) {
+ p_err("failed to create perf buffer: %s (%d)",
+ strerror(err), err);
goto err_close_map;
-
- pfds = calloc(nfds, sizeof(pfds[0]));
- if (!pfds)
- goto err_free_rings;
-
- for (i = 0; i < nfds; i++) {
- rings[i].cpu = cpu + i;
- rings[i].key = index + i;
-
- rings[i].fd = bpf_perf_event_open(map_fd, rings[i].key,
- rings[i].cpu);
- if (rings[i].fd < 0)
- goto err_close_fds_prev;
-
- rings[i].mem = perf_event_mmap(rings[i].fd);
- if (!rings[i].mem)
- goto err_close_fds_current;
-
- pfds[i].fd = rings[i].fd;
- pfds[i].events = POLLIN;
}
signal(SIGINT, int_exit);
@@ -277,34 +214,24 @@ int do_event_pipe(int argc, char **argv)
jsonw_start_array(json_wtr);
while (!stop) {
- poll(pfds, nfds, 200);
- for (i = 0; i < nfds; i++)
- perf_event_read(&rings[i], &tmp_buf, &tmp_buf_sz);
+ err = perf_buffer__poll(pb, 200);
+ if (err < 0 && err != -EINTR) {
+ p_err("perf buffer polling failed: %s (%d)",
+ strerror(err), err);
+ goto err_close_pb;
+ }
}
- free(tmp_buf);
if (json_output)
jsonw_end_array(json_wtr);
- for (i = 0; i < nfds; i++) {
- perf_event_unmap(rings[i].mem);
- close(rings[i].fd);
- }
- free(pfds);
- free(rings);
+ perf_buffer__free(pb);
close(map_fd);
return 0;
-err_close_fds_prev:
- while (i--) {
- perf_event_unmap(rings[i].mem);
-err_close_fds_current:
- close(rings[i].fd);
- }
- free(pfds);
-err_free_rings:
- free(rings);
+err_close_pb:
+ perf_buffer__free(pb);
err_close_map:
close(map_fd);
return -1;
--
2.17.1
^ permalink raw reply related
* Re: Kernel BUG: epoll_wait() (and epoll_pwait) stall for 206 ms per call on sockets with a small-ish snd/rcv buffer.
From: Carlo Wood @ 2019-07-06 18:19 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <20190706181657.7ff57395@hikaru>
While investigating this further, I read on
http://www.masterraghu.com/subjects/np/introduction/unix_network_programming_v1.3/ch07lev1sec5.html
under "SO_RCVBUF and SO_SNDBUF Socket Options":
When setting the size of the TCP socket receive buffer, the
ordering of the function calls is important. This is because of
TCP's window scale option (Section 2.6), which is exchanged with
the peer on the SYN segments when the connection is established.
For a client, this means the SO_RCVBUF socket option must be set
before calling connect. For a server, this means the socket option
must be set for the listening socket before calling listen. Setting
this option for the connected socket will have no effect whatsoever
on the possible window scale option because accept does not return
with the connected socket until TCP's three-way handshake is
complete. That is why this option must be set for the listening
socket. (The sizes of the socket buffers are always inherited from
the listening socket by the newly created connected socket: pp.
462–463 of TCPv2.)
As mentioned in a previous post, I had already discovered about
needing to set the socket buffers before connect, but I didn't know
about setting them before the call to listen() in order to get the
buffer sizes inherited by the accepted sockets.
After fixing this in my test program, all problems disappeared when
keeping the send and receive buffers the same on both sides.
However, when only setting the send and receive buffers on the client
socket (not on the (accepted or) listen socket), epoll_wait() still
stalls 43ms. When the SO_SNDBUF is smaller than 33182 bytes.
Here is the latest version of my test program:
https://github.com/CarloWood/ai-evio-testsuite/blob/master/src/epoll_bug.c
I have to retract most of my "bug" report, it might even not really be
a bug then... but nevertheless, what remains strange is the fact
that setting the socket buffer sizes on the accepted sockets can lead
to so much crippling effect, while the quoted website states:
Setting this option for the connected socket will have no effect
whatsoever on the possible window scale option because accept does
not return with the connected socket until TCP's three-way
handshake is complete.
And when only setting the socket buffer sizes for the client socket
(that I use to send back received data; so this is the sending
side now) then why does epoll_wait() stall 43 ms per call when the
receiving side is using the default (much larger) socket buffer sizes?
That 43 ms is STILL crippling-- slowing down the transmission of the
data to a trickling speed compared to what it should be.
--
Carlo Wood <carlo@alinoe.com>
^ permalink raw reply
* [PATCH net-next v4 0/4] devlink: Introduce PCI PF, VF ports and attributes
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>
This patchset carry forwards the work initiated in [1] and discussion
futher concluded at [2].
To improve visibility of representor netdevice, its association with
PF or VF, physical port, two new devlink port flavours are added as
PCI PF and PCI VF ports.
A sample eswitch view can be seen below, which will be futher extended to
mdev subdevices of a PCI function in future.
Patch-1,2 extends devlink port attributes and port flavour.
Patch-3 extends mlx5 driver to register devlink ports for PF, VF and
physical link.
+---+ +---+
vf| | | | pf
+-+-+ +-+-+
physical link <---------+ | |
| | |
| | |
+-+-+ +-+-+ +-+-+
| 1 | | 2 | | 3 |
+--+---+-----+---+------+---+--+
| physical vf pf |
| port port port |
| |
| eswitch |
| |
+------------------------------+
[1] https://www.spinics.net/lists/netdev/msg555797.html
[2] https://marc.info/?l=linux-netdev&m=155354609408485&w=2
---
Changelog:
v3->v4:
- Addressed comments from Jiri.
- Split first patch to two patches.
- Renamed phys_port to physical to be consistent with pci_pf.
- Removed port_number from __devlink_port_attrs_set and moved
assignment to caller function.
- Used capital letter while moving old comment to new structure.
- Removed helper function is_devlink_phy_port_num_supported().
v2->v3:
- Made port_number and split_port_number applicable only to
physical port flavours.
v1->v2:
- Updated new APIs and mlx5 driver to drop port_number for PF, VF
attributes
- Updated port_number comment for its usage
- Limited putting port_number to physical ports
Parav Pandit (4):
devlink: Refactor physical port attributes
devlink: Introduce PCI PF port flavour and port attribute
devlink: Introduce PCI VF port flavour and port attribute
net/mlx5e: Register devlink ports for physical link, PCI PF, VFs
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 108 ++++++++++----
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 1 +
include/net/devlink.h | 31 +++-
include/uapi/linux/devlink.h | 11 ++
net/core/devlink.c | 135 +++++++++++++++---
5 files changed, 233 insertions(+), 53 deletions(-)
--
2.19.2
^ permalink raw reply
* [PATCH net-next v4 1/4] devlink: Refactor physical port attributes
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>
To support additional devlink port flavours and to support few common
and few different port attributes, make following changes.
1. Move physical port attributes to a different structure
2. Return such attritubes in netlink response only for physical ports
(PHYSICAL, CPU and DSA)
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
include/net/devlink.h | 13 +++++++--
net/core/devlink.c | 63 +++++++++++++++++++++++++++++--------------
2 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6625ea068d5e..c79a1370867a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -38,14 +38,23 @@ struct devlink {
char priv[0] __aligned(NETDEV_ALIGN);
};
+struct devlink_port_phys_attrs {
+ u32 port_number; /* Same value as "split group".
+ * A physical port which is visible to the user
+ * for a given port flavour.
+ */
+ u32 split_subport_number;
+};
+
struct devlink_port_attrs {
u8 set:1,
split:1,
switch_port:1;
enum devlink_port_flavour flavour;
- u32 port_number; /* same value as "split group" */
- u32 split_subport_number;
struct netdev_phys_item_id switch_id;
+ union {
+ struct devlink_port_phys_attrs physical;
+ };
};
struct devlink_port {
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 89c533778135..db6fa6bb9b33 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -515,14 +515,20 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
return 0;
if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
return -EMSGSIZE;
- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
+ if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
+ devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
+ devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA)
+ return 0;
+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
+ attrs->physical.port_number))
return -EMSGSIZE;
if (!attrs->split)
return 0;
- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
+ attrs->physical.port_number))
return -EMSGSIZE;
if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
- attrs->split_subport_number))
+ attrs->physical.split_subport_number))
return -EMSGSIZE;
return 0;
}
@@ -5738,6 +5744,29 @@ void devlink_port_type_clear(struct devlink_port *devlink_port)
}
EXPORT_SYMBOL_GPL(devlink_port_type_clear);
+static int __devlink_port_attrs_set(struct devlink_port *devlink_port,
+ enum devlink_port_flavour flavour,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len)
+{
+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
+
+ if (WARN_ON(devlink_port->registered))
+ return -EEXIST;
+ attrs->set = true;
+ attrs->flavour = flavour;
+ if (switch_id) {
+ attrs->switch_port = true;
+ if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
+ switch_id_len = MAX_PHYS_ITEM_ID_LEN;
+ memcpy(attrs->switch_id.id, switch_id, switch_id_len);
+ attrs->switch_id.id_len = switch_id_len;
+ } else {
+ attrs->switch_port = false;
+ }
+ return 0;
+}
+
/**
* devlink_port_attrs_set - Set port attributes
*
@@ -5760,23 +5789,15 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
unsigned char switch_id_len)
{
struct devlink_port_attrs *attrs = &devlink_port->attrs;
+ int ret;
- if (WARN_ON(devlink_port->registered))
+ ret = __devlink_port_attrs_set(devlink_port, flavour,
+ switch_id, switch_id_len);
+ if (ret)
return;
- attrs->set = true;
- attrs->flavour = flavour;
- attrs->port_number = port_number;
attrs->split = split;
- attrs->split_subport_number = split_subport_number;
- if (switch_id) {
- attrs->switch_port = true;
- if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
- switch_id_len = MAX_PHYS_ITEM_ID_LEN;
- memcpy(attrs->switch_id.id, switch_id, switch_id_len);
- attrs->switch_id.id_len = switch_id_len;
- } else {
- attrs->switch_port = false;
- }
+ attrs->physical.port_number = port_number;
+ attrs->physical.split_subport_number = split_subport_number;
}
EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
@@ -5792,10 +5813,12 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
switch (attrs->flavour) {
case DEVLINK_PORT_FLAVOUR_PHYSICAL:
if (!attrs->split)
- n = snprintf(name, len, "p%u", attrs->port_number);
+ n = snprintf(name, len, "p%u",
+ attrs->physical.port_number);
else
- n = snprintf(name, len, "p%us%u", attrs->port_number,
- attrs->split_subport_number);
+ n = snprintf(name, len, "p%us%u",
+ attrs->physical.port_number,
+ attrs->physical.split_subport_number);
break;
case DEVLINK_PORT_FLAVOUR_CPU:
case DEVLINK_PORT_FLAVOUR_DSA:
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 2/4] devlink: Introduce PCI PF port flavour and port attribute
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>
In an eswitch, PCI PF may have port which is normally represented
using a representor netdevice.
To have better visibility of eswitch port, its association with
PF and a representor netdevice, introduce a PCI PF port
flavour and port attriute.
When devlink port flavour is PCI PF, fill up PCI PF attributes of the
port.
Extend port name creation using PCI PF number on best effort basis.
So that vendor drivers can skip defining their own scheme.
$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
Changelog:
v3->v4:
- Addressed comments from Jiri.
- Renamed phys_port to physical to be consistent with pci_pf.
- Removed port_number from __devlink_port_attrs_set and moved
assigment to caller function.
- Used capital letter while moving old comment to new structure.
- Removed helper function is_devlink_phy_port_num_supported().
v2->v3:
- Address comments from Jakub.
- Made port_number and split_port_number applicable only to
physical port flavours by having in union.
v1->v2:
- Limited port_num attribute to physical ports
- Updated PCI PF attribute set API to not have port_number
---
include/net/devlink.h | 8 ++++++++
include/uapi/linux/devlink.h | 5 +++++
net/core/devlink.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index c79a1370867a..2a8eaaff3d4b 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -46,6 +46,10 @@ struct devlink_port_phys_attrs {
u32 split_subport_number;
};
+struct devlink_port_pci_pf_attrs {
+ u16 pf; /* Associated PCI PF for this port. */
+};
+
struct devlink_port_attrs {
u8 set:1,
split:1,
@@ -54,6 +58,7 @@ struct devlink_port_attrs {
struct netdev_phys_item_id switch_id;
union {
struct devlink_port_phys_attrs physical;
+ struct devlink_port_pci_pf_attrs pci_pf;
};
};
@@ -599,6 +604,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
u32 split_subport_number,
const unsigned char *switch_id,
unsigned char switch_id_len);
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len, u16 pf);
int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
u32 size, u16 ingress_pools_count,
u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5287b42c181f..f7323884c3fe 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,10 @@ enum devlink_port_flavour {
DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
* interconnect port.
*/
+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+ * the PCI PF. It is an internal
+ * port that faces the PCI PF.
+ */
};
enum devlink_param_cmode {
@@ -337,6 +341,7 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index db6fa6bb9b33..3717eae8a502 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -515,6 +515,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
return 0;
if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
return -EMSGSIZE;
+ if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+ if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
+ attrs->pci_pf.pf))
+ return -EMSGSIZE;
+ }
if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA)
@@ -5801,6 +5806,32 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
}
EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
+/**
+ * devlink_port_attrs_pci_pf_set - Set PCI PF port attributes
+ *
+ * @devlink_port: devlink port
+ * @pf: associated PF for the devlink port instance
+ * @switch_id: if the port is part of switch, this is buffer with ID,
+ * otwerwise this is NULL
+ * @switch_id_len: length of the switch_id buffer
+ */
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len, u16 pf)
+{
+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
+ int ret;
+
+ ret = __devlink_port_attrs_set(devlink_port,
+ DEVLINK_PORT_FLAVOUR_PCI_PF,
+ switch_id, switch_id_len);
+ if (ret)
+ return;
+
+ attrs->pci_pf.pf = pf;
+}
+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
+
static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
char *name, size_t len)
{
@@ -5827,6 +5858,9 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
*/
WARN_ON(1);
return -EINVAL;
+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
+ n = snprintf(name, len, "pf%u", attrs->pci_pf.pf);
+ break;
}
if (n >= len)
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 3/4] devlink: Introduce PCI VF port flavour and port attribute
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>
In an eswitch, PCI VF may have port which is normally represented using
a representor netdevice.
To have better visibility of eswitch port, its association with VF,
and its representor netdevice, introduce a PCI VF port flavour.
When devlink port flavour is PCI VF, fill up PCI VF attributes of
the port.
Extend port name creation using PCI PF and VF number scheme on best
effort basis, so that vendor drivers can skip defining their own scheme.
$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
include/net/devlink.h | 10 ++++++++++
include/uapi/linux/devlink.h | 6 ++++++
net/core/devlink.c | 38 ++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 2a8eaaff3d4b..a02f639ad519 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -50,6 +50,11 @@ struct devlink_port_pci_pf_attrs {
u16 pf; /* Associated PCI PF for this port. */
};
+struct devlink_port_pci_vf_attrs {
+ u16 pf; /* Associated PCI PF for this port. */
+ u16 vf; /* Associated PCI VF for of the PCI PF for this port. */
+};
+
struct devlink_port_attrs {
u8 set:1,
split:1,
@@ -59,6 +64,7 @@ struct devlink_port_attrs {
union {
struct devlink_port_phys_attrs physical;
struct devlink_port_pci_pf_attrs pci_pf;
+ struct devlink_port_pci_vf_attrs pci_vf;
};
};
@@ -607,6 +613,10 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
const unsigned char *switch_id,
unsigned char switch_id_len, u16 pf);
+void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len,
+ u16 pf, u16 vf);
int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
u32 size, u16 ingress_pools_count,
u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index f7323884c3fe..ffc993256527 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -173,6 +173,10 @@ enum devlink_port_flavour {
* the PCI PF. It is an internal
* port that faces the PCI PF.
*/
+ DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+ * for the PCI VF. It is an internal
+ * port that faces the PCI VF.
+ */
};
enum devlink_param_cmode {
@@ -342,6 +346,8 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
+ DEVLINK_ATTR_PORT_PCI_VF_NUMBER, /* u16 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3717eae8a502..5a1887c1b3c5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -519,6 +519,12 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
attrs->pci_pf.pf))
return -EMSGSIZE;
+ } else if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+ if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
+ attrs->pci_vf.pf) ||
+ nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_VF_NUMBER,
+ attrs->pci_vf.vf))
+ return -EMSGSIZE;
}
if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
@@ -5832,6 +5838,34 @@ void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
}
EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
+/**
+ * devlink_port_attrs_pci_vf_set - Set PCI VF port attributes
+ *
+ * @devlink_port: devlink port
+ * @pf: associated PF for the devlink port instance
+ * @vf: associated VF of a PF for the devlink port instance
+ * @switch_id: if the port is part of switch, this is buffer with ID,
+ * otwerwise this is NULL
+ * @switch_id_len: length of the switch_id buffer
+ */
+void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
+ const unsigned char *switch_id,
+ unsigned char switch_id_len,
+ u16 pf, u16 vf)
+{
+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
+ int ret;
+
+ ret = __devlink_port_attrs_set(devlink_port,
+ DEVLINK_PORT_FLAVOUR_PCI_VF,
+ switch_id, switch_id_len);
+ if (ret)
+ return;
+ attrs->pci_vf.pf = pf;
+ attrs->pci_vf.vf = vf;
+}
+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_vf_set);
+
static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
char *name, size_t len)
{
@@ -5861,6 +5895,10 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
case DEVLINK_PORT_FLAVOUR_PCI_PF:
n = snprintf(name, len, "pf%u", attrs->pci_pf.pf);
break;
+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
+ n = snprintf(name, len, "pf%uvf%u",
+ attrs->pci_vf.pf, attrs->pci_vf.vf);
+ break;
}
if (n >= len)
--
2.19.2
^ permalink raw reply related
* [PATCH net-next v4 4/4] net/mlx5e: Register devlink ports for physical link, PCI PF, VFs
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>
Register devlink port of physical port, PCI PF and PCI VF flavour
for each PF, VF when a given devlink instance is in switchdev mode.
Implement ndo_get_devlink_port callback API to make use of registered
devlink ports.
This eliminates ndo_get_phys_port_name() and ndo_get_port_parent_id()
callbacks. Hence, remove them.
An example output with 2 VFs, without a PF and single uplink port is
below.
$devlink port show
pci/0000:06:00.0/65535: type eth netdev ens2f0 flavour physical
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1
Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 108 +++++++++++++-----
.../net/ethernet/mellanox/mlx5/core/en_rep.h | 1 +
2 files changed, 78 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 529f8e4b32c6..6810b9fa0705 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -37,6 +37,7 @@
#include <net/act_api.h>
#include <net/netevent.h>
#include <net/arp.h>
+#include <net/devlink.h>
#include "eswitch.h"
#include "en.h"
@@ -1119,32 +1120,6 @@ static int mlx5e_rep_close(struct net_device *dev)
return ret;
}
-static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
- char *buf, size_t len)
-{
- struct mlx5e_priv *priv = netdev_priv(dev);
- struct mlx5e_rep_priv *rpriv = priv->ppriv;
- struct mlx5_eswitch_rep *rep = rpriv->rep;
- unsigned int fn;
- int ret;
-
- fn = PCI_FUNC(priv->mdev->pdev->devfn);
- if (fn >= MLX5_MAX_PORTS)
- return -EOPNOTSUPP;
-
- if (rep->vport == MLX5_VPORT_UPLINK)
- ret = snprintf(buf, len, "p%d", fn);
- else if (rep->vport == MLX5_VPORT_PF)
- ret = snprintf(buf, len, "pf%d", fn);
- else
- ret = snprintf(buf, len, "pf%dvf%d", fn, rep->vport - 1);
-
- if (ret >= len)
- return -EOPNOTSUPP;
-
- return 0;
-}
-
static int
mlx5e_rep_setup_tc_cls_flower(struct mlx5e_priv *priv,
struct tc_cls_flower_offload *cls_flower, int flags)
@@ -1298,17 +1273,24 @@ static int mlx5e_uplink_rep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan
return 0;
}
+static struct devlink_port *mlx5e_get_devlink_port(struct net_device *dev)
+{
+ struct mlx5e_priv *priv = netdev_priv(dev);
+ struct mlx5e_rep_priv *rpriv = priv->ppriv;
+
+ return &rpriv->dl_port;
+}
+
static const struct net_device_ops mlx5e_netdev_ops_rep = {
.ndo_open = mlx5e_rep_open,
.ndo_stop = mlx5e_rep_close,
.ndo_start_xmit = mlx5e_xmit,
- .ndo_get_phys_port_name = mlx5e_rep_get_phys_port_name,
.ndo_setup_tc = mlx5e_rep_setup_tc,
+ .ndo_get_devlink_port = mlx5e_get_devlink_port,
.ndo_get_stats64 = mlx5e_rep_get_stats,
.ndo_has_offload_stats = mlx5e_rep_has_offload_stats,
.ndo_get_offload_stats = mlx5e_rep_get_offload_stats,
.ndo_change_mtu = mlx5e_rep_change_mtu,
- .ndo_get_port_parent_id = mlx5e_rep_get_port_parent_id,
};
static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
@@ -1316,8 +1298,8 @@ static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
.ndo_stop = mlx5e_close,
.ndo_start_xmit = mlx5e_xmit,
.ndo_set_mac_address = mlx5e_uplink_rep_set_mac,
- .ndo_get_phys_port_name = mlx5e_rep_get_phys_port_name,
.ndo_setup_tc = mlx5e_rep_setup_tc,
+ .ndo_get_devlink_port = mlx5e_get_devlink_port,
.ndo_get_stats64 = mlx5e_get_stats,
.ndo_has_offload_stats = mlx5e_rep_has_offload_stats,
.ndo_get_offload_stats = mlx5e_rep_get_offload_stats,
@@ -1330,7 +1312,6 @@ static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
.ndo_get_vf_config = mlx5e_get_vf_config,
.ndo_get_vf_stats = mlx5e_get_vf_stats,
.ndo_set_vf_vlan = mlx5e_uplink_rep_set_vf_vlan,
- .ndo_get_port_parent_id = mlx5e_rep_get_port_parent_id,
.ndo_set_features = mlx5e_set_features,
};
@@ -1731,6 +1712,55 @@ static const struct mlx5e_profile mlx5e_uplink_rep_profile = {
.max_tc = MLX5E_MAX_NUM_TC,
};
+static bool
+is_devlink_port_supported(const struct mlx5_core_dev *dev,
+ const struct mlx5e_rep_priv *rpriv)
+{
+ return rpriv->rep->vport == MLX5_VPORT_UPLINK ||
+ rpriv->rep->vport == MLX5_VPORT_PF ||
+ mlx5_eswitch_is_vf_vport(dev->priv.eswitch, rpriv->rep->vport);
+}
+
+static int register_devlink_port(struct mlx5_core_dev *dev,
+ struct mlx5e_rep_priv *rpriv)
+{
+ struct devlink *devlink = priv_to_devlink(dev);
+ struct mlx5_eswitch_rep *rep = rpriv->rep;
+ struct netdev_phys_item_id ppid = {};
+ int ret;
+
+ if (!is_devlink_port_supported(dev, rpriv))
+ return 0;
+
+ ret = mlx5e_rep_get_port_parent_id(rpriv->netdev, &ppid);
+ if (ret)
+ return ret;
+
+ if (rep->vport == MLX5_VPORT_UPLINK)
+ devlink_port_attrs_set(&rpriv->dl_port,
+ DEVLINK_PORT_FLAVOUR_PHYSICAL,
+ PCI_FUNC(dev->pdev->devfn), false, 0,
+ &ppid.id[0], ppid.id_len);
+ else if (rep->vport == MLX5_VPORT_PF)
+ devlink_port_attrs_pci_pf_set(&rpriv->dl_port,
+ &ppid.id[0], ppid.id_len,
+ dev->pdev->devfn);
+ else if (mlx5_eswitch_is_vf_vport(dev->priv.eswitch, rpriv->rep->vport))
+ devlink_port_attrs_pci_vf_set(&rpriv->dl_port,
+ &ppid.id[0], ppid.id_len,
+ dev->pdev->devfn,
+ rep->vport - 1);
+
+ return devlink_port_register(devlink, &rpriv->dl_port, rep->vport);
+}
+
+static void unregister_devlink_port(struct mlx5_core_dev *dev,
+ struct mlx5e_rep_priv *rpriv)
+{
+ if (is_devlink_port_supported(dev, rpriv))
+ devlink_port_unregister(&rpriv->dl_port);
+}
+
/* e-Switch vport representors */
static int
mlx5e_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
@@ -1782,15 +1812,27 @@ mlx5e_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
goto err_detach_netdev;
}
+ err = register_devlink_port(dev, rpriv);
+ if (err) {
+ esw_warn(dev, "Failed to register devlink port %d\n",
+ rep->vport);
+ goto err_neigh_cleanup;
+ }
+
err = register_netdev(netdev);
if (err) {
pr_warn("Failed to register representor netdev for vport %d\n",
rep->vport);
- goto err_neigh_cleanup;
+ goto err_devlink_cleanup;
}
+ if (is_devlink_port_supported(dev, rpriv))
+ devlink_port_type_eth_set(&rpriv->dl_port, netdev);
return 0;
+err_devlink_cleanup:
+ unregister_devlink_port(dev, rpriv);
+
err_neigh_cleanup:
mlx5e_rep_neigh_cleanup(rpriv);
@@ -1813,9 +1855,13 @@ mlx5e_vport_rep_unload(struct mlx5_eswitch_rep *rep)
struct mlx5e_rep_priv *rpriv = mlx5e_rep_to_rep_priv(rep);
struct net_device *netdev = rpriv->netdev;
struct mlx5e_priv *priv = netdev_priv(netdev);
+ struct mlx5_core_dev *dev = priv->mdev;
void *ppriv = priv->ppriv;
+ if (is_devlink_port_supported(dev, rpriv))
+ devlink_port_type_clear(&rpriv->dl_port);
unregister_netdev(netdev);
+ unregister_devlink_port(dev, rpriv);
mlx5e_rep_neigh_cleanup(rpriv);
mlx5e_detach_netdev(priv);
if (rep->vport == MLX5_VPORT_UPLINK)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index d4585f3b8cb2..c56e6ee4350c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -86,6 +86,7 @@ struct mlx5e_rep_priv {
struct mlx5_flow_handle *vport_rx_rule;
struct list_head vport_sqs_list;
struct mlx5_rep_uplink_priv uplink_priv; /* valid for uplink rep */
+ struct devlink_port dl_port;
};
static inline
--
2.19.2
^ permalink raw reply related
* RE: [PATCH net-next v3 1/3] devlink: Introduce PCI PF port flavour and port attribute
From: Parav Pandit @ 2019-07-06 18:38 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, Saeed Mahameed,
jakub.kicinski@netronome.com
In-Reply-To: <20190706062611.GA2264@nanopsycho>
> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Saturday, July 6, 2019 11:56 AM
> To: Parav Pandit <parav@mellanox.com>
> Cc: netdev@vger.kernel.org; Jiri Pirko <jiri@mellanox.com>; Saeed Mahameed
> <saeedm@mellanox.com>; jakub.kicinski@netronome.com
> Subject: Re: [PATCH net-next v3 1/3] devlink: Introduce PCI PF port flavour and
> port attribute
>
> Sat, Jul 06, 2019 at 08:16:24AM CEST, parav@mellanox.com wrote:
> >In an eswitch, PCI PF may have port which is normally represented using
> >a representor netdevice.
> >To have better visibility of eswitch port, its association with PF and
> >a representor netdevice, introduce a PCI PF port flavour and port
> >attriute.
> >
> >When devlink port flavour is PCI PF, fill up PCI PF attributes of the
> >port.
> >
> >Extend port name creation using PCI PF number on best effort basis.
> >So that vendor drivers can skip defining their own scheme.
> >
> >$ devlink port show
> >pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
> >
> >Signed-off-by: Parav Pandit <parav@mellanox.com>
> >
> >---
> >Changelog:
> >v2->v3:
> > - Address comments from Jakub.
> > - Made port_number and split_port_number applicable only to
> > physical port flavours by having in union.
> >v1->v2:
> > - Limited port_num attribute to physical ports
> > - Updated PCI PF attribute set API to not have port_number
> >---
> > include/net/devlink.h | 21 +++++++-
> > include/uapi/linux/devlink.h | 5 ++
> > net/core/devlink.c | 97 ++++++++++++++++++++++++++++--------
> > 3 files changed, 100 insertions(+), 23 deletions(-)
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h index
> >6625ea068d5e..1455f60e4069 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -38,13 +38,27 @@ struct devlink {
> > char priv[0] __aligned(NETDEV_ALIGN); };
> >
> >+struct devlink_port_phys_attrs {
> >+ u32 port_number; /* same value as "split group".
>
> "Same" with capital letter.
>
Done in v4.
> >+ * A physical port which is visible to the user
> >+ * for a given port flavour.
> >+ */
> >+ u32 split_subport_number;
> >+};
> >+
> >+struct devlink_port_pci_pf_attrs {
> >+ u16 pf; /* Associated PCI PF for this port. */
> >+};
> >+
> > struct devlink_port_attrs {
> > u8 set:1,
> > split:1,
> > switch_port:1;
> > enum devlink_port_flavour flavour;
> >- u32 port_number; /* same value as "split group" */
> >- u32 split_subport_number;
> >+ union {
> >+ struct devlink_port_phys_attrs phys_port;
> >+ struct devlink_port_pci_pf_attrs pci_pf;
>
> Be consistent in naming: "phys", "pci_pf".
>
Done in v4 as "physical".
>
> >+ };
> > struct netdev_phys_item_id switch_id; };
> >
> >@@ -590,6 +604,9 @@ void devlink_port_attrs_set(struct devlink_port
> *devlink_port,
> > u32 split_subport_number,
> > const unsigned char *switch_id,
> > unsigned char switch_id_len);
> >+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
> >+ const unsigned char *switch_id,
> >+ unsigned char switch_id_len, u16 pf);
> > int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
> > u32 size, u16 ingress_pools_count,
> > u16 egress_pools_count, u16 ingress_tc_count, diff --
> git
> >a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index
> >5287b42c181f..f7323884c3fe 100644
> >--- a/include/uapi/linux/devlink.h
> >+++ b/include/uapi/linux/devlink.h
> >@@ -169,6 +169,10 @@ enum devlink_port_flavour {
> > DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
> > * interconnect port.
> > */
> >+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
> >+ * the PCI PF. It is an internal
> >+ * port that faces the PCI PF.
> >+ */
> > };
> >
> > enum devlink_param_cmode {
> >@@ -337,6 +341,7 @@ enum devlink_attr {
> > DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
> > DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
> >
> >+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u16 */
> > /* add new attributes above here, update the policy in devlink.c */
> >
> > __DEVLINK_ATTR_MAX,
> >diff --git a/net/core/devlink.c b/net/core/devlink.c index
> >89c533778135..9aa36104b471 100644
> >--- a/net/core/devlink.c
> >+++ b/net/core/devlink.c
> >@@ -506,6 +506,14 @@ static void devlink_notify(struct devlink *devlink,
> enum devlink_command cmd)
> > msg, 0, DEVLINK_MCGRP_CONFIG,
> GFP_KERNEL); }
> >
> >+static bool
> >+is_devlink_phy_port_num_supported(const struct devlink_port *dl_port)
> >+{
> >+ return (dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL
> ||
> >+ dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
> >+ dl_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA); }
> >+
> > static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> > struct devlink_port *devlink_port) { @@ -
> 515,14 +523,23 @@
> >static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> > return 0;
> > if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
> > return -EMSGSIZE;
> >- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs-
> >port_number))
> >+ if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> >+ if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> >+ attrs->pci_pf.pf))
> >+ return -EMSGSIZE;
> >+ }
> >+ if (!is_devlink_phy_port_num_supported(devlink_port))
>
> Please do the check here. No need for helper (the name with "is" and
> "supported" is weird anyway.
>
Done in v4.
>
> >+ return 0;
> >+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
> >+ attrs->phys_port.port_number))
> > return -EMSGSIZE;
> > if (!attrs->split)
> > return 0;
> >- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs-
> >port_number))
> >+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
> >+ attrs->phys_port.port_number))
>
> Better to split this into 2 patches. One pushing phys things into separate struct,
> the second the rest.
>
Done in v4.
>
> > return -EMSGSIZE;
> > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
> >- attrs->split_subport_number))
> >+ attrs->phys_port.split_subport_number))
> > return -EMSGSIZE;
> > return 0;
> > }
> >@@ -5738,6 +5755,30 @@ void devlink_port_type_clear(struct devlink_port
> >*devlink_port) } EXPORT_SYMBOL_GPL(devlink_port_type_clear);
> >
> >+static void __devlink_port_attrs_set(struct devlink_port *devlink_port,
> >+ enum devlink_port_flavour flavour,
> >+ u32 port_number,
> >+ const unsigned char *switch_id,
> >+ unsigned char switch_id_len)
> >+{
> >+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
> >+
> >+ if (WARN_ON(devlink_port->registered))
> >+ return;
> >+ attrs->set = true;
> >+ attrs->flavour = flavour;
> >+ attrs->phys_port.port_number = port_number;
> >+ if (switch_id) {
> >+ attrs->switch_port = true;
> >+ if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
> >+ switch_id_len = MAX_PHYS_ITEM_ID_LEN;
> >+ memcpy(attrs->switch_id.id, switch_id, switch_id_len);
> >+ attrs->switch_id.id_len = switch_id_len;
> >+ } else {
> >+ attrs->switch_port = false;
> >+ }
> >+}
> >+
> > /**
> > * devlink_port_attrs_set - Set port attributes
> > *
> >@@ -5761,25 +5802,34 @@ void devlink_port_attrs_set(struct devlink_port
> >*devlink_port, {
> > struct devlink_port_attrs *attrs = &devlink_port->attrs;
> >
> >- if (WARN_ON(devlink_port->registered))
> >- return;
> >- attrs->set = true;
> >- attrs->flavour = flavour;
> >- attrs->port_number = port_number;
> >+ __devlink_port_attrs_set(devlink_port, flavour, port_number,
> >+ switch_id, switch_id_len);
> > attrs->split = split;
> >- attrs->split_subport_number = split_subport_number;
> >- if (switch_id) {
> >- attrs->switch_port = true;
> >- if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
> >- switch_id_len = MAX_PHYS_ITEM_ID_LEN;
> >- memcpy(attrs->switch_id.id, switch_id, switch_id_len);
> >- attrs->switch_id.id_len = switch_id_len;
> >- } else {
> >- attrs->switch_port = false;
> >- }
> >+ attrs->phys_port.split_subport_number = split_subport_number;
> > }
> > EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
> >
> >+/**
> >+ * devlink_port_attrs_pci_pf_set - Set PCI PF port attributes
> >+ *
> >+ * @devlink_port: devlink port
> >+ * @pf: associated PF for the devlink port instance
> >+ * @switch_id: if the port is part of switch, this is buffer with ID,
> >+ * otwerwise this is NULL
> >+ * @switch_id_len: length of the switch_id buffer
> >+ */
> >+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
> >+ const unsigned char *switch_id,
> >+ unsigned char switch_id_len, u16 pf) {
> >+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
> >+
> >+ __devlink_port_attrs_set(devlink_port,
> DEVLINK_PORT_FLAVOUR_PCI_PF,
> >+ 0, switch_id, switch_id_len);
>
> Please have this done differently. __devlink_port_attrs_set() sets
> attrs->phys_port.port_number which does not make sense there.
>
Changed in v4.
>
> >+ attrs->pci_pf.pf = pf;
> >+}
> >+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
> >+
> > static int __devlink_port_phys_port_name_get(struct devlink_port
> *devlink_port,
> > char *name, size_t len)
> > {
> >@@ -5792,10 +5842,12 @@ static int
> __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
> > switch (attrs->flavour) {
> > case DEVLINK_PORT_FLAVOUR_PHYSICAL:
> > if (!attrs->split)
> >- n = snprintf(name, len, "p%u", attrs->port_number);
> >+ n = snprintf(name, len, "p%u",
> >+ attrs->phys_port.port_number);
> > else
> >- n = snprintf(name, len, "p%us%u", attrs->port_number,
> >- attrs->split_subport_number);
> >+ n = snprintf(name, len, "p%us%u",
> >+ attrs->phys_port.port_number,
> >+ attrs->phys_port.split_subport_number);
> > break;
> > case DEVLINK_PORT_FLAVOUR_CPU:
> > case DEVLINK_PORT_FLAVOUR_DSA:
> >@@ -5804,6 +5856,9 @@ static int
> __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
> > */
> > WARN_ON(1);
> > return -EINVAL;
> >+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
> >+ n = snprintf(name, len, "pf%u", attrs->pci_pf.pf);
> >+ break;
> > }
> >
> > if (n >= len)
> >--
> >2.19.2
> >
^ permalink raw reply
* Re: [PATCH bpf-next v2 0/6] xdp: Add devmap_hash map type
From: Y Song @ 2019-07-06 18:58 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156240283550.10171.1727292671613975908.stgit@alrua-x1>
On Sat, Jul 6, 2019 at 1:47 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> This series adds a new map type, devmap_hash, that works like the existing
> devmap type, but using a hash-based indexing scheme. This is useful for the use
> case where a devmap is indexed by ifindex (for instance for use with the routing
> table lookup helper). For this use case, the regular devmap needs to be sized
> after the maximum ifindex number, not the number of devices in it. A hash-based
> indexing scheme makes it possible to size the map after the number of devices it
> should contain instead.
>
> This was previously part of my patch series that also turned the regular
> bpf_redirect() helper into a map-based one; for this series I just pulled out
> the patches that introduced the new map type.
>
> Changelog:
>
> v2:
>
> - Split commit adding the new map type so uapi and tools changes are separate.
>
> Changes to these patches since the previous series:
>
> - Rebase on top of the other devmap changes (makes this one simpler!)
> - Don't enforce key==val, but allow arbitrary indexes.
> - Rename the type to devmap_hash to reflect the fact that it's just a hashmap now.
>
> ---
>
> Toke Høiland-Jørgensen (6):
> include/bpf.h: Remove map_insert_ctx() stubs
> xdp: Refactor devmap allocation code for reuse
> uapi/bpf: Add new devmap_hash type
> xdp: Add devmap_hash map type for looking up devices by hashed index
> tools/libbpf_probes: Add new devmap_hash type
> tools: Add definitions for devmap_hash map type
Thanks for re-organize the patch. I guess this can be tweaked a little more
to better suit for syncing between kernel and libbpf repo.
Let me provide a little bit background here. The below is
a sync done by Andrii from kernel/tools to libbpf repo.
=============
commit 39de6711795f6d1583ae96ed8d13892bc4475ac1
Author: Andrii Nakryiko <andriin@fb.com>
Date: Tue Jun 11 09:56:11 2019 -0700
sync: latest libbpf changes from kernel
Syncing latest libbpf commits from kernel repository.
Baseline commit: e672db03ab0e43e41ab6f8b2156a10d6e40f243d
Checkpoint commit: 5e2ac390fbd08b2a462db66cef2663e4db0d5191
Andrii Nakryiko (9):
libbpf: fix detection of corrupted BPF instructions section
libbpf: preserve errno before calling into user callback
libbpf: simplify endianness check
libbpf: check map name retrieved from ELF
libbpf: fix error code returned on corrupted ELF
libbpf: use negative fd to specify missing BTF
libbpf: simplify two pieces of logic
libbpf: typo and formatting fixes
libbpf: reduce unnecessary line wrapping
Hechao Li (1):
bpf: add a new API libbpf_num_possible_cpus()
Jonathan Lemon (2):
bpf/tools: sync bpf.h
libbpf: remove qidconf and better support external bpf programs.
Quentin Monnet (1):
libbpf: prevent overwriting of log_level in bpf_object__load_progs()
include/uapi/linux/bpf.h | 4 +
src/libbpf.c | 207 ++++++++++++++++++++++-----------------
src/libbpf.h | 16 +++
src/libbpf.map | 1 +
src/xsk.c | 103 ++++++-------------
5 files changed, 167 insertions(+), 164 deletions(-)
==========
You can see the commits at tools/lib/bpf and
commits at tools/include/uapi/{linux/[bpf.h, btf.h], ...}
are sync'ed to libbpf repo.
So we would like kernel commits to be aligned that way for better
automatic syncing.
Therefore, your current patch set could be changed from
> include/bpf.h: Remove map_insert_ctx() stubs
> xdp: Refactor devmap allocation code for reuse
> uapi/bpf: Add new devmap_hash type
> xdp: Add devmap_hash map type for looking up devices by hashed index
> tools/libbpf_probes: Add new devmap_hash type
> tools: Add definitions for devmap_hash map type
to
1. include/bpf.h: Remove map_insert_ctx() stubs
2. xdp: Refactor devmap allocation code for reuse
3. kernel non-tools changes (the above patch #3 and #4)
4. tools/include/uapi change (part of the above patch #6)
5. tools/libbpf_probes change
6. other tools/ change (the above patch #6 - new patch #4).
Thanks!
Yonghong
>
>
> include/linux/bpf.h | 11 -
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3
> include/uapi/linux/bpf.h | 1
> kernel/bpf/devmap.c | 325 ++++++++++++++++++++++++++-----
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 +
> tools/bpf/bpftool/map.c | 1
> tools/include/uapi/linux/bpf.h | 1
> tools/lib/bpf/libbpf_probes.c | 1
> tools/testing/selftests/bpf/test_maps.c | 16 ++
> 11 files changed, 310 insertions(+), 61 deletions(-)
>
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/6] include/bpf.h: Remove map_insert_ctx() stubs
From: Y Song @ 2019-07-06 18:59 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156240283571.10171.11997723639222073086.stgit@alrua-x1>
On Sat, Jul 6, 2019 at 1:47 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> When we changed the device and CPU maps to use linked lists instead of
> bitmaps, we also removed the need for the map_insert_ctx() helpers to keep
> track of the bitmaps inside each map. However, it seems I forgot to remove
> the function definitions stubs, so remove those here.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH bpf-next v2 2/6] xdp: Refactor devmap allocation code for reuse
From: Y Song @ 2019-07-06 19:02 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156240283578.10171.1470306115442701328.stgit@alrua-x1>
On Sat, Jul 6, 2019 at 1:47 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The subsequent patch to add a new devmap sub-type can re-use much of the
> initialisation and allocation code, so refactor it into separate functions.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> kernel/bpf/devmap.c | 137 +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 84 insertions(+), 53 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index d83cf8ccc872..a2fe16362129 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -60,7 +60,7 @@ struct xdp_bulk_queue {
> struct bpf_dtab_netdev {
> struct net_device *dev; /* must be first member, due to tracepoint */
> struct bpf_dtab *dtab;
> - unsigned int bit;
> + unsigned int idx; /* keep track of map index for tracepoint */
> struct xdp_bulk_queue __percpu *bulkq;
> struct rcu_head rcu;
> };
> @@ -75,28 +75,22 @@ struct bpf_dtab {
> static DEFINE_SPINLOCK(dev_map_lock);
> static LIST_HEAD(dev_map_list);
>
> -static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> +static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> + bool check_memlock)
> {
> - struct bpf_dtab *dtab;
> int err, cpu;
> u64 cost;
>
> - if (!capable(CAP_NET_ADMIN))
> - return ERR_PTR(-EPERM);
> -
> /* check sanity of attributes */
> if (attr->max_entries == 0 || attr->key_size != 4 ||
> attr->value_size != 4 || attr->map_flags & ~DEV_CREATE_FLAG_MASK)
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> /* Lookup returns a pointer straight to dev->ifindex, so make sure the
> * verifier prevents writes from the BPF side
> */
> attr->map_flags |= BPF_F_RDONLY_PROG;
>
> - dtab = kzalloc(sizeof(*dtab), GFP_USER);
> - if (!dtab)
> - return ERR_PTR(-ENOMEM);
>
> bpf_map_init_from_attr(&dtab->map, attr);
>
> @@ -107,9 +101,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> /* if map size is larger than memlock limit, reject it */
> err = bpf_map_charge_init(&dtab->map.memory, cost);
> if (err)
> - goto free_dtab;
> -
> - err = -ENOMEM;
> + return -EINVAL;
>
> dtab->flush_list = alloc_percpu(struct list_head);
> if (!dtab->flush_list)
> @@ -124,19 +116,38 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> if (!dtab->netdev_map)
> goto free_percpu;
>
> - spin_lock(&dev_map_lock);
> - list_add_tail_rcu(&dtab->list, &dev_map_list);
> - spin_unlock(&dev_map_lock);
> -
> - return &dtab->map;
> + return 0;
>
> free_percpu:
> free_percpu(dtab->flush_list);
> free_charge:
> bpf_map_charge_finish(&dtab->map.memory);
> -free_dtab:
> - kfree(dtab);
> - return ERR_PTR(err);
> + return -ENOMEM;
> +}
> +
[...]
^ permalink raw reply
* Re: [PATCH bpf-next v2 4/6] xdp: Add devmap_hash map type for looking up devices by hashed index
From: Y Song @ 2019-07-06 19:14 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156240283595.10171.8867063497268976931.stgit@alrua-x1>
On Sat, Jul 6, 2019 at 1:48 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> A common pattern when using xdp_redirect_map() is to create a device map
> where the lookup key is simply ifindex. Because device maps are arrays,
> this leaves holes in the map, and the map has to be sized to fit the
> largest ifindex, regardless of how many devices actually are actually
> needed in the map.
>
> This patch adds a second type of device map where the key is looked up
> using a hashmap, instead of being used as an array index. This allows maps
> to be densely packed, so they can be smaller.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> include/linux/bpf.h | 7 ++
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3 -
> kernel/bpf/devmap.c | 192 ++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 ++
> 6 files changed, 211 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index bfdb54dd2ad1..f9a506147c8a 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -713,6 +713,7 @@ struct xdp_buff;
> struct sk_buff;
>
> struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key);
> void __dev_map_flush(struct bpf_map *map);
> int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
> struct net_device *dev_rx);
> @@ -799,6 +800,12 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
> return NULL;
> }
>
> +static inline struct net_device *__dev_map_hash_lookup_elem(struct bpf_map *map,
> + u32 key)
> +{
> + return NULL;
> +}
> +
> static inline void __dev_map_flush(struct bpf_map *map)
> {
> }
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index eec5aeeeaf92..36a9c2325176 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -62,6 +62,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
> #ifdef CONFIG_NET
> BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_HASH, dev_map_hash_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
> #if defined(CONFIG_BPF_STREAM_PARSER)
> BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> index 68899fdc985b..8c8420230a10 100644
> --- a/include/trace/events/xdp.h
> +++ b/include/trace/events/xdp.h
> @@ -175,7 +175,8 @@ struct _bpf_dtab_netdev {
> #endif /* __DEVMAP_OBJ_TYPE */
>
> #define devmap_ifindex(fwd, map) \
> - ((map->map_type == BPF_MAP_TYPE_DEVMAP) ? \
> + ((map->map_type == BPF_MAP_TYPE_DEVMAP || \
> + map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) ? \
> ((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0)
>
> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index a2fe16362129..341af02f049d 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -37,6 +37,12 @@
> * notifier hook walks the map we know that new dev references can not be
> * added by the user because core infrastructure ensures dev_get_by_index()
> * calls will fail at this point.
> + *
> + * The devmap_hash type is a map type which interprets keys as ifindexes and
> + * indexes these using a hashmap. This allows maps that use ifindex as key to be
> + * densely packed instead of having holes in the lookup array for unused
> + * ifindexes. The setup and packet enqueue/send code is shared between the two
> + * types of devmap; only the lookup and insertion is different.
> */
> #include <linux/bpf.h>
> #include <net/xdp.h>
> @@ -59,6 +65,7 @@ struct xdp_bulk_queue {
>
> struct bpf_dtab_netdev {
> struct net_device *dev; /* must be first member, due to tracepoint */
> + struct hlist_node index_hlist;
> struct bpf_dtab *dtab;
> unsigned int idx; /* keep track of map index for tracepoint */
> struct xdp_bulk_queue __percpu *bulkq;
> @@ -70,11 +77,29 @@ struct bpf_dtab {
> struct bpf_dtab_netdev **netdev_map;
> struct list_head __percpu *flush_list;
> struct list_head list;
> +
> + /* these are only used for DEVMAP_HASH type maps */
> + unsigned int items;
> + struct hlist_head *dev_index_head;
> + spinlock_t index_lock;
> };
>
> static DEFINE_SPINLOCK(dev_map_lock);
> static LIST_HEAD(dev_map_list);
>
> +static struct hlist_head *dev_map_create_hash(void)
> +{
> + int i;
> + struct hlist_head *hash;
> +
> + hash = kmalloc_array(NETDEV_HASHENTRIES, sizeof(*hash), GFP_KERNEL);
> + if (hash != NULL)
> + for (i = 0; i < NETDEV_HASHENTRIES; i++)
> + INIT_HLIST_HEAD(&hash[i]);
> +
> + return hash;
> +}
> +
> static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> bool check_memlock)
> {
> @@ -98,6 +123,9 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
> cost += sizeof(struct list_head) * num_possible_cpus();
>
> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH)
> + cost += sizeof(struct hlist_head) * NETDEV_HASHENTRIES;
> +
> /* if map size is larger than memlock limit, reject it */
> err = bpf_map_charge_init(&dtab->map.memory, cost);
> if (err)
> @@ -116,8 +144,18 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> if (!dtab->netdev_map)
> goto free_percpu;
>
> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
> + dtab->dev_index_head = dev_map_create_hash();
> + if (!dtab->dev_index_head)
> + goto free_map_area;
> +
> + spin_lock_init(&dtab->index_lock);
> + }
> +
> return 0;
>
> +free_map_area:
> + bpf_map_area_free(dtab->netdev_map);
> free_percpu:
> free_percpu(dtab->flush_list);
> free_charge:
> @@ -199,6 +237,7 @@ static void dev_map_free(struct bpf_map *map)
>
> free_percpu(dtab->flush_list);
> bpf_map_area_free(dtab->netdev_map);
> + kfree(dtab->dev_index_head);
> kfree(dtab);
> }
>
> @@ -219,6 +258,70 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> return 0;
> }
>
> +static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
> + int idx)
> +{
> + return &dtab->dev_index_head[idx & (NETDEV_HASHENTRIES - 1)];
> +}
> +
> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> + struct hlist_head *head = dev_map_index_hash(dtab, key);
> + struct bpf_dtab_netdev *dev;
> +
> + hlist_for_each_entry_rcu(dev, head, index_hlist)
> + if (dev->idx == key)
> + return dev;
> +
> + return NULL;
> +}
> +
> +static int dev_map_hash_get_next_key(struct bpf_map *map, void *key,
> + void *next_key)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> + u32 idx, *next = next_key;
> + struct bpf_dtab_netdev *dev, *next_dev;
> + struct hlist_head *head;
> + int i = 0;
> +
> + if (!key)
> + goto find_first;
> +
> + idx = *(u32 *)key;
> +
> + dev = __dev_map_hash_lookup_elem(map, idx);
> + if (!dev)
> + goto find_first;
> +
> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&dev->index_hlist)),
> + struct bpf_dtab_netdev, index_hlist);
Just want to get a better understanding. Why do you want
hlist_entry_safe instead of hlist_entry?
Also, maybe rcu_dereference instead of rcu_dereference_raw?
dev_map_hash_get_next_key() is called in syscall.c within rcu_read_lock region.
> +
> + if (next_dev) {
> + *next = next_dev->idx;
> + return 0;
> + }
> +
> + i = idx & (NETDEV_HASHENTRIES - 1);
> + i++;
> +
> + find_first:
> + for (; i < NETDEV_HASHENTRIES; i++) {
> + head = dev_map_index_hash(dtab, i);
> +
> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
> + struct bpf_dtab_netdev,
> + index_hlist);
ditto. The same question as the above.
> + if (next_dev) {
> + *next = next_dev->idx;
> + return 0;
> + }
> + }
> +
> + return -ENOENT;
> +}
> +
> static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags,
> bool in_napi_ctx)
> {
> @@ -374,6 +477,15 @@ static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
> return dev ? &dev->ifindex : NULL;
> }
>
> +static void *dev_map_hash_lookup_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_dtab_netdev *obj = __dev_map_hash_lookup_elem(map,
> + *(u32 *)key);
> + struct net_device *dev = obj ? obj->dev : NULL;
When obj->dev could be NULL?
> +
> + return dev ? &dev->ifindex : NULL;
> +}
> +
> static void dev_map_flush_old(struct bpf_dtab_netdev *dev)
> {
> if (dev->dev->netdev_ops->ndo_xdp_xmit) {
> @@ -423,6 +535,27 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
> return 0;
> }
>
> +static int dev_map_hash_delete_elem(struct bpf_map *map, void *key)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> + struct bpf_dtab_netdev *old_dev;
> + int k = *(u32 *)key;
> +
> + old_dev = __dev_map_hash_lookup_elem(map, k);
> + if (!old_dev)
> + return 0;
> +
> + spin_lock(&dtab->index_lock);
> + if (!hlist_unhashed(&old_dev->index_hlist)) {
> + dtab->items--;
> + hlist_del_init_rcu(&old_dev->index_hlist);
> + }
> + spin_unlock(&dtab->index_lock);
> +
> + call_rcu(&old_dev->rcu, __dev_map_entry_free);
> + return 0;
> +}
> +
> static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
> struct bpf_dtab *dtab,
> u32 ifindex,
> @@ -503,6 +636,55 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
> map, key, value, map_flags);
> }
>
> +static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
> + void *key, void *value, u64 map_flags)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> + struct bpf_dtab_netdev *dev, *old_dev;
> + u32 ifindex = *(u32 *)value;
> + u32 idx = *(u32 *)key;
> +
> + if (unlikely(map_flags > BPF_EXIST || !ifindex))
> + return -EINVAL;
> +
> + old_dev = __dev_map_hash_lookup_elem(map, idx);
> + if (old_dev && (map_flags & BPF_NOEXIST))
> + return -EEXIST;
> +
> + dev = __dev_map_alloc_node(net, dtab, ifindex, idx);
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> +
> + spin_lock(&dtab->index_lock);
> +
> + if (old_dev) {
> + hlist_del_rcu(&old_dev->index_hlist);
> + } else {
> + if (dtab->items >= dtab->map.max_entries) {
> + spin_unlock(&dtab->index_lock);
> + call_rcu(&dev->rcu, __dev_map_entry_free);
> + return -ENOSPC;
> + }
> + dtab->items++;
> + }
> +
> + hlist_add_head_rcu(&dev->index_hlist,
> + dev_map_index_hash(dtab, idx));
> + spin_unlock(&dtab->index_lock);
> +
> + if (old_dev)
> + call_rcu(&old_dev->rcu, __dev_map_entry_free);
> +
> + return 0;
> +}
> +
> +static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
> + u64 map_flags)
> +{
> + return __dev_map_hash_update_elem(current->nsproxy->net_ns,
> + map, key, value, map_flags);
> +}
> +
> const struct bpf_map_ops dev_map_ops = {
> .map_alloc = dev_map_alloc,
> .map_free = dev_map_free,
> @@ -513,6 +695,16 @@ const struct bpf_map_ops dev_map_ops = {
> .map_check_btf = map_check_no_btf,
> };
>
> +const struct bpf_map_ops dev_map_hash_ops = {
> + .map_alloc = dev_map_alloc,
> + .map_free = dev_map_free,
> + .map_get_next_key = dev_map_hash_get_next_key,
> + .map_lookup_elem = dev_map_hash_lookup_elem,
> + .map_update_elem = dev_map_hash_update_elem,
> + .map_delete_elem = dev_map_hash_delete_elem,
> + .map_check_btf = map_check_no_btf,
> +};
> +
> static int dev_map_notification(struct notifier_block *notifier,
> ulong event, void *ptr)
> {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a2e763703c30..231b9e22827c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -3460,6 +3460,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> break;
> case BPF_MAP_TYPE_DEVMAP:
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> if (func_id != BPF_FUNC_redirect_map &&
> func_id != BPF_FUNC_map_lookup_elem)
> goto error;
> @@ -3542,6 +3543,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> break;
> case BPF_FUNC_redirect_map:
> if (map->map_type != BPF_MAP_TYPE_DEVMAP &&
> + map->map_type != BPF_MAP_TYPE_DEVMAP_HASH &&
> map->map_type != BPF_MAP_TYPE_CPUMAP &&
> map->map_type != BPF_MAP_TYPE_XSKMAP)
> goto error;
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 089aaea0ccc6..276961c4e0d4 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3517,7 +3517,8 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
> int err;
>
> switch (map->map_type) {
> - case BPF_MAP_TYPE_DEVMAP: {
> + case BPF_MAP_TYPE_DEVMAP:
> + case BPF_MAP_TYPE_DEVMAP_HASH: {
> struct bpf_dtab_netdev *dst = fwd;
>
> err = dev_map_enqueue(dst, xdp, dev_rx);
> @@ -3554,6 +3555,7 @@ void xdp_do_flush_map(void)
> if (map) {
> switch (map->map_type) {
> case BPF_MAP_TYPE_DEVMAP:
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> __dev_map_flush(map);
> break;
> case BPF_MAP_TYPE_CPUMAP:
> @@ -3574,6 +3576,8 @@ static inline void *__xdp_map_lookup_elem(struct bpf_map *map, u32 index)
> switch (map->map_type) {
> case BPF_MAP_TYPE_DEVMAP:
> return __dev_map_lookup_elem(map, index);
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> + return __dev_map_hash_lookup_elem(map, index);
> case BPF_MAP_TYPE_CPUMAP:
> return __cpu_map_lookup_elem(map, index);
> case BPF_MAP_TYPE_XSKMAP:
> @@ -3655,7 +3659,8 @@ static int xdp_do_generic_redirect_map(struct net_device *dev,
> ri->tgt_value = NULL;
> WRITE_ONCE(ri->map, NULL);
>
> - if (map->map_type == BPF_MAP_TYPE_DEVMAP) {
> + if (map->map_type == BPF_MAP_TYPE_DEVMAP ||
> + map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
> struct bpf_dtab_netdev *dst = fwd;
>
> err = dev_map_generic_redirect(dst, skb, xdp_prog);
>
^ permalink raw reply
* [PATCH] net, skbuff: Handle devmap managed page when skb->head_frag is true
From: Ujjal Roy @ 2019-07-06 19:57 UTC (permalink / raw)
To: Kernel, David S. Miller
When head_frag is true we have page in the SKB head. So, for devm
managed page we need to inform the device driver through callback.
Signed-off-by: Ujjal Roy <royujjal@gmail.com>
---
net/core/skbuff.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c8cd99c3603f..0d303e694efa 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -582,10 +582,16 @@ static void skb_free_head(struct sk_buff *skb)
{
unsigned char *head = skb->head;
- if (skb->head_frag)
+ if (skb->head_frag) {
+ struct page *page = virt_to_head_page(head);
+
+ if (put_devmap_managed_page(page))
+ return;
+
skb_free_frag(head);
- else
+ } else {
kfree(head);
+ }
}
static void skb_release_data(struct sk_buff *skb)
--
2.11.0
^ permalink raw reply related
* [net-next] bonding: fix value exported by Netlink for peer_notif_delay
From: Vincent Bernat @ 2019-07-06 21:01 UTC (permalink / raw)
To: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
netdev
Cc: Vincent Bernat
IFLA_BOND_PEER_NOTIF_DELAY was set to the value of downdelay instead
of peer_notif_delay. After this change, the correct value is exported.
Fixes: 07a4ddec3ce9 ("bonding: add an option to specify a delay between peer notifications")
Signed-off-by: Vincent Bernat <vincent@bernat.ch>
---
drivers/net/bonding/bond_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index a259860a7208..b43b51646b11 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -547,7 +547,7 @@ static int bond_fill_info(struct sk_buff *skb,
goto nla_put_failure;
if (nla_put_u32(skb, IFLA_BOND_PEER_NOTIF_DELAY,
- bond->params.downdelay * bond->params.miimon))
+ bond->params.peer_notif_delay * bond->params.miimon))
goto nla_put_failure;
if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier))
--
2.20.1
^ permalink raw reply related
* [PATCH iproute2-next] ip: bond: add peer notification delay support
From: Vincent Bernat @ 2019-07-06 21:11 UTC (permalink / raw)
To: netdev, Stephen Hemminger; +Cc: Vincent Bernat
Ability to tweak the delay between gratuitous ND/ARP packets has been
added in kernel commit 07a4ddec3ce9 ("bonding: add an option to
specify a delay between peer notifications"), through
IFLA_BOND_PEER_NOTIF_DELAY attribute. Add support to set and show this
value.
Example:
$ ip -d link set bond0 type bond peer_notif_delay 1000
$ ip -d link l dev bond0
2: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue
state UP mode DEFAULT group default qlen 1000
link/ether 50:54:33:00:00:01 brd ff:ff:ff:ff:ff:ff
bond mode active-backup active_slave eth0 miimon 100 updelay 0
downdelay 0 peer_notif_delay 1000 use_carrier 1 arp_interval 0
arp_validate none arp_all_targets any primary eth0
primary_reselect always fail_over_mac active xmit_hash_policy
layer2 resend_igmp 1 num_grat_arp 5 all_slaves_active 0 min_links
0 lp_interval 1 packets_per_slave 1 lacp_rate slow ad_select
stable tlb_dynamic_lb 1 addrgenmode eu
Signed-off-by: Vincent Bernat <vincent@bernat.ch>
---
include/uapi/linux/if_link.h | 1 +
ip/iplink_bond.c | 14 +++++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index b59554dd55cb..d36919fb4024 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -634,6 +634,7 @@ enum {
IFLA_BOND_AD_USER_PORT_KEY,
IFLA_BOND_AD_ACTOR_SYSTEM,
IFLA_BOND_TLB_DYNAMIC_LB,
+ IFLA_BOND_PEER_NOTIF_DELAY,
__IFLA_BOND_MAX,
};
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index c60f0e8ad0a0..fb62c955631e 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -120,6 +120,7 @@ static void print_explain(FILE *f)
"Usage: ... bond [ mode BONDMODE ] [ active_slave SLAVE_DEV ]\n"
" [ clear_active_slave ] [ miimon MIIMON ]\n"
" [ updelay UPDELAY ] [ downdelay DOWNDELAY ]\n"
+ " [ peer_notif_delay DELAY ]\n"
" [ use_carrier USE_CARRIER ]\n"
" [ arp_interval ARP_INTERVAL ]\n"
" [ arp_validate ARP_VALIDATE ]\n"
@@ -165,7 +166,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
__u8 xmit_hash_policy, num_peer_notif, all_slaves_active;
__u8 lacp_rate, ad_select, tlb_dynamic_lb;
__u16 ad_user_port_key, ad_actor_sys_prio;
- __u32 miimon, updelay, downdelay, arp_interval, arp_validate;
+ __u32 miimon, updelay, downdelay, peer_notif_delay, arp_interval, arp_validate;
__u32 arp_all_targets, resend_igmp, min_links, lp_interval;
__u32 packets_per_slave;
unsigned int ifindex;
@@ -200,6 +201,11 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
if (get_u32(&downdelay, *argv, 0))
invarg("invalid downdelay", *argv);
addattr32(n, 1024, IFLA_BOND_DOWNDELAY, downdelay);
+ } else if (matches(*argv, "peer_notif_delay") == 0) {
+ NEXT_ARG();
+ if (get_u32(&peer_notif_delay, *argv, 0))
+ invarg("invalid peer_notif_delay", *argv);
+ addattr32(n, 1024, IFLA_BOND_PEER_NOTIF_DELAY, peer_notif_delay);
} else if (matches(*argv, "use_carrier") == 0) {
NEXT_ARG();
if (get_u8(&use_carrier, *argv, 0))
@@ -410,6 +416,12 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
"downdelay %u ",
rta_getattr_u32(tb[IFLA_BOND_DOWNDELAY]));
+ if (tb[IFLA_BOND_PEER_NOTIF_DELAY])
+ print_uint(PRINT_ANY,
+ "peer_notif_delay",
+ "peer_notif_delay %u ",
+ rta_getattr_u32(tb[IFLA_BOND_PEER_NOTIF_DELAY]));
+
if (tb[IFLA_BOND_USE_CARRIER])
print_uint(PRINT_ANY,
"use_carrier",
--
2.20.1
^ permalink raw reply related
* Re: [PATCH net-next] tipc: use rcu dereference functions properly
From: David Miller @ 2019-07-06 22:15 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion
In-Reply-To: <CADvbK_eDnUMSaoT65hco2PF5-f1PO=CKBeMPz3sTRZvg5qKGVA@mail.gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 6 Jul 2019 14:48:48 +0800
> Hi, David, I saw this patch in "Changes Requested".
I just put it back to Under Review, thanks.
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2019-07-06
From: David Miller @ 2019-07-06 22:20 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <878stbabkb.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Sat, 06 Jul 2019 10:04:20 +0300
> here's a pull request to net-next tree for v5.3, more info below. I will
> be offline from Sunday to Thursday, but Johannes should be able to help
> during that time.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH net-next 0/4] bnxt_en: Add XDP_REDIRECT support.
From: David Miller @ 2019-07-06 22:26 UTC (permalink / raw)
To: michael.chan; +Cc: gospo, netdev, hawk, ast
In-Reply-To: <1562398578-26020-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Sat, 6 Jul 2019 03:36:14 -0400
> This patch series adds XDP_REDIRECT support by Andy Gospodarek.
I'll give Jesper et al. a chance to review this.
^ permalink raw reply
* More complex PBR rules
From: Markus Moeller @ 2019-07-06 23:06 UTC (permalink / raw)
To: netdev
Hi Network developers
I am new to this group and wonder if you can advise how I could implement
more complex PBR rules to achieve for example load balancing. The
requirement I have is to route based on e.g. a hash like:
hash(src-ip+dst-ip) mod N routes via gwX 0<X<=N ( load balance over
N gateways )
This would help in situations where I can not use a MAC for identifying a
gateway ( e.g. in cloud environments) .
Could someone point me to the kernel source code where PBR is performed ?
Thank you
Markus
^ permalink raw reply
* Re: [PATCH][next] 6lowpan: fix off-by-one comparison of index id with LOWPAN_IPHC_CTX_TABLE_SIZE
From: Colin Ian King @ 2019-07-06 23:23 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Alexander Aring, Jukka Rissanen, David S. Miller, linux-bluetooth,
linux-wpan, netdev, kernel-janitors, linux-kernel
In-Reply-To: <B6A1CB42-C239-42CA-B14E-483A02B930EB@holtmann.org>
On 06/07/2019 11:51, Marcel Holtmann wrote:
> Hi Colin,
>
>> The WARN_ON_ONCE check on id is off-by-one, it should be greater or equal
>> to LOWPAN_IPHC_CTX_TABLE_SIZE and not greater than. Fix this.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> net/6lowpan/debugfs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/6lowpan/debugfs.c b/net/6lowpan/debugfs.c
>> index 1c140af06d52..a510bed8165b 100644
>> --- a/net/6lowpan/debugfs.c
>> +++ b/net/6lowpan/debugfs.c
>> @@ -170,7 +170,7 @@ static void lowpan_dev_debugfs_ctx_init(struct net_device *dev,
>> struct dentry *root;
>> char buf[32];
>>
>> - WARN_ON_ONCE(id > LOWPAN_IPHC_CTX_TABLE_SIZE);
>> + WARN_ON_ONCE(id >= LOWPAN_IPHC_CTX_TABLE_SIZE);
>
> this patch no longer applied cleanly to bluetooth-next. Can you send me an updated version.
I'm confused by this, I just applied it OK on bluetooth-next [1] on the
head 9ce67c3235be71e8cf922a9b3d0b7359ed3f4ce5, am I applying this to the
wrong repo/branch?
[1]
git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
Colin
>
> Regards
>
> Marcel
>
^ permalink raw reply
* Re: [PATCH 1/2] proc: revalidate directories created with proc_net_mkdir()
From: Al Viro @ 2019-07-07 1:03 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: davem, netdev, Per.Hallsmark
In-Reply-To: <20190706165201.GA10550@avx2>
On Sat, Jul 06, 2019 at 07:52:02PM +0300, Alexey Dobriyan wrote:
> +struct proc_dir_entry *_proc_mkdir(const char *name, umode_t mode,
> + struct proc_dir_entry **parent, void *data)
Two underscores, please...
> + parent->nlink++;
> + pde = proc_register(parent, pde);
> + if (!pde)
> + parent->nlink++;
Really?
^ permalink raw reply
* Re: INFO: rcu detected stall in ext4_write_checks
From: Paul E. McKenney @ 2019-07-07 1:16 UTC (permalink / raw)
To: Theodore Ts'o, Dmitry Vyukov, syzbot, Andreas Dilger,
David Miller, eladr, Ido Schimmel, Jiri Pirko, John Stultz,
linux-ext4, LKML, netdev, syzkaller-bugs, Thomas Gleixner,
Peter Zijlstra, Ingo Molnar
In-Reply-To: <20190706180311.GW26519@linux.ibm.com>
On Sat, Jul 06, 2019 at 11:03:11AM -0700, Paul E. McKenney wrote:
> On Sat, Jul 06, 2019 at 11:02:26AM -0400, Theodore Ts'o wrote:
> > On Fri, Jul 05, 2019 at 11:16:31PM -0700, Paul E. McKenney wrote:
> > > I suppose RCU could take the dueling-banjos approach and use increasingly
> > > aggressive scheduler policies itself, up to and including SCHED_DEADLINE,
> > > until it started getting decent forward progress. However, that
> > > sounds like the something that just might have unintended consequences,
> > > particularly if other kernel subsystems were to also play similar
> > > games of dueling banjos.
> >
> > So long as the RCU threads are well-behaved, using SCHED_DEADLINE
> > shouldn't have much of an impact on the system --- and the scheduling
> > parameters that you can specify on SCHED_DEADLINE allows you to
> > specify the worst-case impact on the system while also guaranteeing
> > that the SCHED_DEADLINE tasks will urn in the first place. After all,
> > that's the whole point of SCHED_DEADLINE.
> >
> > So I wonder if the right approach is during the the first userspace
> > system call to shced_setattr to enable a (any) real-time priority
> > scheduler (SCHED_DEADLINE, SCHED_FIFO or SCHED_RR) on a userspace
> > thread, before that's allowed to proceed, the RCU kernel threads are
> > promoted to be SCHED_DEADLINE with appropriately set deadline
> > parameters. That way, a root user won't be able to shoot the system
> > in the foot, and since the vast majority of the time, there shouldn't
> > be any processes running with real-time priorities, we won't be
> > changing the behavior of a normal server system.
>
> It might well be. However, running the RCU kthreads at real-time
> priority does not come for free. For example, it tends to crank up the
> context-switch rate.
>
> Plus I have taken several runs at computing SCHED_DEADLINE parameters,
> but things like the rcuo callback-offload threads have computational
> requirements that are controlled not by RCU, and not just by the rest of
> the kernel, but also by userspace (keeping in mind the example of opening
> and closing a file in a tight loop, each pass of which queues a callback).
> I suspect that RCU is not the only kernel subsystem whose computational
> requirements are set not by the subsystem, but rather by external code.
>
> OK, OK, I suppose I could just set insanely large SCHED_DEADLINE
> parameters, following syzkaller's example, and then trust my ability to
> keep the RCU code from abusing the resulting awesome power. But wouldn't
> a much nicer approach be to put SCHED_DEADLINE between SCHED_RR/SCHED_FIFO
> priorities 98 and 99 or some such? Then the same (admittedly somewhat
> scary) result could be obtained much more simply via SCHED_FIFO or
> SCHED_RR priority 99.
>
> Some might argue that this is one of those situations where simplicity
> is not necessarily an advantage, but then again, you can find someone
> who will complain about almost anything. ;-)
>
> > (I suspect there might be some audio applications that might try to
> > set real-time priorities, but for desktop systems, it's probably more
> > important that the system not tie its self into knots since the
> > average desktop user isn't going to be well equipped to debug the
> > problem.)
>
> Not only that, but if core counts continue to increase, and if reliance
> on cloud computing continues to grow, there are going to be an increasing
> variety of mixed workloads in increasingly less-controlled environments.
>
> So, yes, it would be good to solve this problem in some reasonable way.
>
> I don't see this as urgent just yet, but I am sure you all will let
> me know if I am mistaken on that point.
>
> > > Alternatively, is it possible to provide stricter admission control?
> >
> > I think that's an orthogonal issue; better admission control would be
> > nice, but it looks to me that it's going to be fundamentally an issue
> > of tweaking hueristics, and a fool-proof solution that will protect
> > against all malicious userspace applications (including syzkaller) is
> > going to require solving the halting problem. So while it would be
> > nice to improve the admission control, I don't think that's a going to
> > be a general solution.
>
> Agreed, and my earlier point about the need to trust the coding abilities
> of those writing ultimate-priority code is all too consistent with your
> point about needing to solve the halting problem. Nevertheless, I believe
> that we could make something that worked reasonably well in practice.
>
> Here are a few components of a possible solution, in practice, but
> of course not in theory:
>
> 1. We set limits to SCHED_DEADLINE parameters, perhaps novel ones.
> For one example, insist on (say) 10 milliseconds of idle time
> every second on each CPU. Yes, you can configure beyond that
> given sufficient permissions, but if you do so, you just voided
> your warranty.
>
> 2. Only allow SCHED_DEADLINE on nohz_full CPUs. (Partial solution,
> given that such a CPU might be running in the kernel or have
> more than one runnable task. Just for fun, I will suggest the
> option of disabling SCHED_DEADLINE during such times.)
>
> 3. RCU detects slowdowns, and does something TBD to increase its
> priority, but only while the slowdown persists. This likely
> relies on scheduling-clock interrupts to detect the slowdowns,
> so there might be additional challenges on a fully nohz_full
> system.
4. SCHED_DEADLINE treats the other three scheduling classes as each
having a period, deadline, and a modest CPU consumption budget
for the members of the class in aggregate. But this has to have
been discussed before. How did that go?
> 5. Your idea here.
Thanx, Paul
^ 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