Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next 6/7] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
expected.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 151 ++++++++++++++++++
 .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
 2 files changed, 206 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c

diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
new file mode 100644
index 000000000000..5cc7e674a513
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+ssize_t get_base_addr() {
+	size_t start;
+	char buf[256];
+	FILE *f;
+
+	f = fopen("/proc/self/maps", "r");
+	if (!f)
+		return -errno;
+
+	while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
+		if (strcmp(buf, "r-xp") == 0) {
+			fclose(f);
+			return start;
+		}
+	}
+
+	fclose(f);
+	return -EINVAL;
+}
+
+void test_attach_probe(void)
+{
+	const char *kprobe_name = "kprobe/sys_nanosleep";
+	const char *kretprobe_name = "kretprobe/sys_nanosleep";
+	const char *uprobe_name = "uprobe/trigger_func";
+	const char *uretprobe_name = "uretprobe/trigger_func";
+	const int kprobe_idx = 0, kretprobe_idx = 1;
+	const int uprobe_idx = 2, uretprobe_idx = 3;
+	const char *file = "./test_attach_probe.o";
+	struct bpf_program *kprobe_prog, *kretprobe_prog;
+	struct bpf_program *uprobe_prog, *uretprobe_prog;
+	struct bpf_object *obj;
+	int err, prog_fd, duration = 0, res;
+	int kprobe_pfd = -1, kretprobe_pfd = -1;
+	int uprobe_pfd = -1, uretprobe_pfd = -1;
+	int results_map_fd;
+	size_t uprobe_offset;
+	ssize_t base_addr;
+
+	base_addr = get_base_addr();
+	if (CHECK(base_addr < 0, "get_base_addr",
+		  "failed to find base addr: %zd", base_addr))
+		return;
+	uprobe_offset = (size_t)&get_base_addr - base_addr;
+
+	/* load programs */
+	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;
+
+	kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
+	if (CHECK(!kprobe_prog, "find_probe",
+		  "prog '%s' not found\n", kprobe_name))
+		goto cleanup;
+	kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
+	if (CHECK(!kretprobe_prog, "find_probe",
+		  "prog '%s' not found\n", kretprobe_name))
+		goto cleanup;
+	uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
+	if (CHECK(!uprobe_prog, "find_probe",
+		  "prog '%s' not found\n", uprobe_name))
+		goto cleanup;
+	uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
+	if (CHECK(!uretprobe_prog, "find_probe",
+		  "prog '%s' not found\n", uretprobe_name))
+		goto cleanup;
+
+	/* load maps */
+	results_map_fd = bpf_find_map(__func__, obj, "results_map");
+	if (CHECK(results_map_fd < 0, "find_results_map",
+		  "err %d\n", results_map_fd))
+		goto cleanup;
+
+	kprobe_pfd = bpf_program__attach_kprobe(kprobe_prog,
+						false /* retprobe */,
+						"sys_nanosleep");
+	if (CHECK(kprobe_pfd < 0, "attach_kprobe", "err %d\n", kprobe_pfd))
+		goto cleanup;
+
+	kretprobe_pfd = bpf_program__attach_kprobe(kretprobe_prog,
+						   true /* retprobe */,
+						   "sys_nanosleep");
+	if (CHECK(kretprobe_pfd < 0, "attach_kretprobe",
+		  "err %d\n", kretprobe_pfd))
+		goto cleanup;
+
+	uprobe_pfd = bpf_program__attach_uprobe(uprobe_prog,
+						false /* retprobe */,
+						0 /* self pid */,
+						"/proc/self/exe",
+						uprobe_offset);
+	if (CHECK(uprobe_pfd < 0, "attach_uprobe", "err %d\n", uprobe_pfd))
+		goto cleanup;
+
+	uretprobe_pfd = bpf_program__attach_uprobe(uretprobe_prog,
+						   true /* retprobe */,
+						   -1 /* any pid */,
+						   "/proc/self/exe",
+						   uprobe_offset);
+	if (CHECK(uretprobe_pfd < 0, "attach_uretprobe",
+		  "err %d\n", uretprobe_pfd))
+		goto cleanup;
+
+	/* trigger & validate kprobe && kretprobe */
+	usleep(1);
+
+	err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
+	if (CHECK(err, "get_kprobe_res",
+		  "failed to get kprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
+		  "wrong kprobe res: %d\n", res))
+		goto cleanup;
+
+	err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
+	if (CHECK(err, "get_kretprobe_res",
+		  "failed to get kretprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
+		  "wrong kretprobe res: %d\n", res))
+		goto cleanup;
+
+	/* trigger & validate uprobe & uretprobe */
+	get_base_addr();
+
+	err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
+	if (CHECK(err, "get_uprobe_res",
+		  "failed to get uprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
+		  "wrong uprobe res: %d\n", res))
+		goto cleanup;
+
+	err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
+	if (CHECK(err, "get_uretprobe_res",
+		  "failed to get uretprobe res: %d\n", err))
+		goto cleanup;
+	if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
+		  "wrong uretprobe res: %d\n", res))
+		goto cleanup;
+
+cleanup:
+	libbpf_perf_event_disable_and_close(kprobe_pfd);
+	libbpf_perf_event_disable_and_close(kretprobe_pfd);
+	libbpf_perf_event_disable_and_close(uprobe_pfd);
+	libbpf_perf_event_disable_and_close(uretprobe_pfd);
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
new file mode 100644
index 000000000000..7a7c5cd728c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+	int type;
+	int max_entries;
+	int *key;
+	int *value;
+} results_map SEC(".maps") = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.max_entries = 4,
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+	const int key = 0, value = 1;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("kretprobe/sys_nanosleep")
+int handle_sys_getpid_return(struct pt_regs *ctx)
+{
+	const int key = 1, value = 2;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("uprobe/trigger_func")
+int handle_uprobe_entry(struct pt_regs *ctx)
+{
+	const int key = 2, value = 3;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+SEC("uretprobe/trigger_func")
+int handle_uprobe_return(struct pt_regs *ctx)
+{
+	const int key = 3, value = 4;
+
+	bpf_map_update_elem(&results_map, &key, &value, 0);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 5/7] selftests/bpf: switch test to new attach_perf_event API
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Use new bpf_program__attach_perf_event() in test previously relying on
direct ioctl manipulations.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../bpf/prog_tests/stacktrace_build_id_nmi.c     | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index 1c1a2f75f3d8..1bbdb0b82ac5 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
 void test_stacktrace_build_id_nmi(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/random/urandom_read";
 	const char *file = "./test_stacktrace_build_id.o";
 	int err, pmu_fd, prog_fd;
 	struct perf_event_attr attr = {
@@ -25,6 +26,7 @@ void test_stacktrace_build_id_nmi(void)
 		.config = PERF_COUNT_HW_CPU_CYCLES,
 	};
 	__u32 key, previous_key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
 	char buf[256];
 	int i, j;
@@ -39,6 +41,10 @@ void test_stacktrace_build_id_nmi(void)
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		return;
 
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
 	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
 			 0 /* cpu 0 */, -1 /* group id */,
 			 0 /* flags */);
@@ -47,16 +53,10 @@ void test_stacktrace_build_id_nmi(void)
 		  pmu_fd, errno))
 		goto close_prog;
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-		  err, errno))
+	err = bpf_program__attach_perf_event(prog, pmu_fd);
+	if (CHECK(err, "attach_perf_event", "err %d\n", err))
 		goto close_pmu;
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-		  err, errno))
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 4/7] libbpf: add tracepoint/raw tracepoint attach API
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Add APIs allowing to attach BPF program to kernel tracepoints. Raw
tracepoint attach API is also added for uniform per-BPF-program API,
but is mostly a wrapper around existing bpf_raw_tracepoint_open call.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 99 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  5 ++
 tools/lib/bpf/libbpf.map |  2 +
 3 files changed, 106 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 11329e05530e..cefe67ba160b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4176,6 +4176,105 @@ int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
 	return pfd;
 }
 
+static int determine_tracepoint_id(const char* tp_category, const char* tp_name)
+{
+	char file[PATH_MAX];
+	int ret;
+
+	ret = snprintf(file, sizeof(file),
+		       "/sys/kernel/debug/tracing/events/%s/%s/id",
+		       tp_category, tp_name);
+	if (ret < 0)
+		return -errno;
+	if (ret >= sizeof(file)) {
+		pr_debug("tracepoint %s/%s path is too long\n",
+			 tp_category, tp_name);
+		return -E2BIG;
+	}
+	return parse_uint_from_file(file);
+}
+
+static int perf_event_open_tracepoint(const char* tp_category,
+				      const char* tp_name)
+{
+	struct perf_event_attr attr = {};
+	char errmsg[STRERR_BUFSIZE];
+	int tp_id, pfd, err;
+
+	tp_id = determine_tracepoint_id(tp_category, tp_name);
+	if (tp_id < 0){
+		pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
+			   tp_category, tp_name,
+			   libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
+		return tp_id;
+	}
+
+	memset(&attr, 0, sizeof(attr));
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.size = sizeof(attr);
+	attr.config = tp_id;
+
+	pfd = syscall( __NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
+			-1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+	if (pfd < 0) {
+		err = -errno;
+		pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
+			   tp_category, tp_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+int bpf_program__attach_tracepoint(struct bpf_program *prog,
+				   const char *tp_category,
+				   const char *tp_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	int pfd, err;
+
+	pfd = perf_event_open_tracepoint(tp_category, tp_name);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   tp_category, tp_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return pfd;
+	}
+	err = bpf_program__attach_perf_event(prog, pfd);
+	if (err) {
+		libbpf_perf_event_disable_and_close(pfd);
+		pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
+			   bpf_program__title(prog, false),
+			   tp_category, tp_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+				       const char *tp_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	int bpf_fd, pfd;
+
+	bpf_fd = bpf_program__fd(prog);
+	if (bpf_fd < 0) {
+		pr_warning("program '%s': can't attach before loaded\n",
+			   bpf_program__title(prog, false));
+		return -EINVAL;
+	}
+	pfd = bpf_raw_tracepoint_open(tp_name, bpf_fd);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
+			   bpf_program__title(prog, false), tp_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return pfd;
+	}
+	return pfd;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index a7264f06aa5f..bf7020a565c6 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -176,6 +176,11 @@ LIBBPF_API int bpf_program__attach_uprobe(struct bpf_program *prog,
 					  pid_t pid,
 					  const char *binary_path,
 					  size_t func_offset);
+LIBBPF_API int bpf_program__attach_tracepoint(struct bpf_program *prog,
+					      const char *tp_category,
+					      const char *tp_name);
+LIBBPF_API int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+						  const char *tp_name);
 
 struct bpf_insn;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 1a982c2e1751..2382fbda4cbb 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -174,6 +174,8 @@ LIBBPF_0.0.4 {
 		bpf_object__load_xattr;
 		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
+		bpf_program__attach_raw_tracepoint;
+		bpf_program__attach_tracepoint;
 		bpf_program__attach_uprobe;
 		libbpf_num_possible_cpus;
 		libbpf_perf_event_disable_and_close;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 2/7] libbpf: add ability to attach/detach BPF to perf event
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

bpf_program__attach_perf_event allows to attach BPF program to existing
perf event, providing most generic and most low-level way to attach BPF
programs.

libbpf_perf_event_disable_and_close API is added to disable and close
existing perf event by its FD.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 41 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  4 ++++
 tools/lib/bpf/libbpf.map |  2 ++
 3 files changed, 47 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8ce3beba8551..2bb1fa008be3 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,6 +32,7 @@
 #include <linux/limits.h>
 #include <linux/perf_event.h>
 #include <linux/ring_buffer.h>
+#include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
@@ -3928,6 +3929,46 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
 	return 0;
 }
 
+int libbpf_perf_event_disable_and_close(int pfd)
+{
+	int err;
+
+	if (pfd < 0)
+		return 0;
+
+	err = ioctl(pfd, PERF_EVENT_IOC_DISABLE, 0);
+	close(pfd);
+	return err;
+}
+
+int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
+{
+	char errmsg[STRERR_BUFSIZE];
+	int bpf_fd, err;
+
+	bpf_fd = bpf_program__fd(prog);
+	if (bpf_fd < 0) {
+		pr_warning("program '%s': can't attach before loaded\n",
+			   bpf_program__title(prog, false));
+		return -EINVAL;
+	}
+	if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0) {
+		err = -errno;
+		pr_warning("program '%s': failed to attach to pfd %d: %s\n",
+			   bpf_program__title(prog, false), pfd,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+		err = -errno;
+		pr_warning("program '%s': failed to enable pfd %d: %s\n",
+			   bpf_program__title(prog, false), pfd,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return 0;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..76db1bbc0dac 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
 LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
 LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
 
+LIBBPF_API int libbpf_perf_event_disable_and_close(int pfd);
+LIBBPF_API int bpf_program__attach_perf_event(struct bpf_program *prog,
+					      int pfd);
+
 struct bpf_insn;
 
 /*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..d27406982b5a 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -172,5 +172,7 @@ LIBBPF_0.0.4 {
 		btf_dump__new;
 		btf__parse_elf;
 		bpf_object__load_xattr;
+		bpf_program__attach_perf_event;
 		libbpf_num_possible_cpus;
+		libbpf_perf_event_disable_and_close;
 } LIBBPF_0.0.3;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 1/7] libbpf: make libbpf_strerror_r agnostic to sign of error
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

It's often inconvenient to switch sign of error when passing it into
libbpf_strerror_r. It's better for it to handle that automatically.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/str_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 00e48ac5b806..b8064eedc177 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -11,7 +11,7 @@
  */
 char *libbpf_strerror_r(int err, char *dst, int len)
 {
-	int ret = strerror_r(err, dst, len);
+	int ret = strerror_r(err < 0 ? -err : err, dst, len);
 	if (ret)
 		snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
 	return dst;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 0/7] libbpf: add tracing attach APIs
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko

This patchset adds the following APIs to allow attaching BPF programs to
tracing entities:
- bpf_program__attach_perf_event for attaching to any opened perf event FD,
  allowing users full control;
- bpf_program__attach_kprobe for attaching to kernel probes (both entry and
  return probes);
- bpf_program__attach_uprobe for attaching to user probes (both entry/return);
- bpf_program__attach_tracepoint for attaching to kernel tracepoints;
- bpf_program__attach_raw_tracepoint for attaching to raw kernel tracepoint
  (wrapper around bpf_raw_tracepoint_open);

This set of APIs makes libbpf more useful for tracing applications.

Pre-patch #1 makes internal libbpf_strerror_r helper function work w/ negative
error codes, lifting the burder off callers to keep track of error sign.
Patch #2 adds attach_perf_event, which is the base for all other APIs.
Patch #3 adds kprobe/uprobe APIs.
Patch #4 adds tracepoint/raw_tracepoint APIs.
Patch #5 converts one existing test to use attach_perf_event.
Patch #6 adds new kprobe/uprobe tests.
Patch #7 converts all the selftests currently using tracepoint to new APIs.

Andrii Nakryiko (7):
  libbpf: make libbpf_strerror_r agnostic to sign of error
  libbpf: add ability to attach/detach BPF to perf event
  libbpf: add kprobe/uprobe attach API
  libbpf: add tracepoint/raw tracepoint attach API
  selftests/bpf: switch test to new attach_perf_event API
  selftests/bpf: add kprobe/uprobe selftests
  selftests/bpf: convert existing tracepoint tests to new APIs

 tools/lib/bpf/libbpf.c                        | 347 ++++++++++++++++++
 tools/lib/bpf/libbpf.h                        |  17 +
 tools/lib/bpf/libbpf.map                      |   6 +
 tools/lib/bpf/str_error.c                     |   2 +-
 .../selftests/bpf/prog_tests/attach_probe.c   | 151 ++++++++
 .../bpf/prog_tests/stacktrace_build_id.c      |  49 +--
 .../bpf/prog_tests/stacktrace_build_id_nmi.c  |  16 +-
 .../selftests/bpf/prog_tests/stacktrace_map.c |  42 +--
 .../bpf/prog_tests/stacktrace_map_raw_tp.c    |  14 +-
 .../bpf/prog_tests/task_fd_query_rawtp.c      |  10 +-
 .../bpf/prog_tests/task_fd_query_tp.c         |  51 +--
 .../bpf/prog_tests/tp_attach_query.c          |  56 +--
 .../selftests/bpf/progs/test_attach_probe.c   |  55 +++
 13 files changed, 650 insertions(+), 166 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c

-- 
2.17.1


^ permalink raw reply

* Re: [PATCH net-next v2 1/1] net: fastopen: robustness and endianness fixes for SipHash
From: kbuild test robot @ 2019-06-20 23:03 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: kbuild-all, netdev, Ard Biesheuvel, Eric Biggers, linux-crypto,
	herbert, edumazet, davem, kuznet, yoshfuji, jbaron, cpaasch,
	David.Laight, ycheng
In-Reply-To: <20190619065510.23514-2-ard.biesheuvel@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 8526 bytes --]

Hi Ard,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/net-fastopen-follow-up-tweaks-for-SipHash-switch/20190621-060434
config: i386-randconfig-x009-201924 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net//ipv4/tcp_fastopen.c: In function 'tcp_fastopen_reset_cipher':
>> net//ipv4/tcp_fastopen.c:82:16: warning: missing braces around initializer [-Wmissing-braces]
     ctx->key[0] = (siphash_key_t){
                   ^
      get_unaligned_le64(primary_key),
      {
   net//ipv4/tcp_fastopen.c:85:2:
     };
     }
   net//ipv4/tcp_fastopen.c:87:17: warning: missing braces around initializer [-Wmissing-braces]
      ctx->key[1] = (siphash_key_t){
                    ^
       get_unaligned_le64(backup_key),
       {
   net//ipv4/tcp_fastopen.c:90:3:
      };
      }
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_read
   Cyclomatic Complexity 1 include/linux/kasan-checks.h:kasan_check_write
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_read
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_set
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_add
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:arch_atomic_inc
   Cyclomatic Complexity 2 arch/x86/include/asm/atomic.h:arch_atomic_try_cmpxchg
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_read
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_set
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_add
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_inc
   Cyclomatic Complexity 1 include/asm-generic/atomic-instrumented.h:atomic_try_cmpxchg
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
   Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le64_to_cpup
   Cyclomatic Complexity 1 include/uapi/linux/byteorder/little_endian.h:__le32_to_cpup
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
   Cyclomatic Complexity 1 include/linux/percpu-defs.h:__this_cpu_preempt_check
   Cyclomatic Complexity 5 include/linux/string.h:memcmp
   Cyclomatic Complexity 1 include/asm-generic/getorder.h:__get_order
   Cyclomatic Complexity 6 arch/x86/include/asm/preempt.h:__preempt_count_add
   Cyclomatic Complexity 6 arch/x86/include/asm/preempt.h:__preempt_count_sub
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_lock
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock
   Cyclomatic Complexity 1 include/linux/rcupdate.h:__rcu_read_lock
   Cyclomatic Complexity 1 include/linux/rcupdate.h:__rcu_read_unlock
   Cyclomatic Complexity 1 include/linux/rcutiny.h:rcu_is_watching
   Cyclomatic Complexity 67 include/linux/slab.h:kmalloc_large
   Cyclomatic Complexity 3 include/linux/slab.h:kmalloc
   Cyclomatic Complexity 1 include/linux/slab.h:kzalloc
   Cyclomatic Complexity 1 include/linux/refcount.h:refcount_set
   Cyclomatic Complexity 1 include/linux/refcount.h:refcount_read
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_end_pointer
   Cyclomatic Complexity 1 include/linux/skbuff.h:__skb_insert
   Cyclomatic Complexity 1 include/linux/skbuff.h:__skb_queue_before
   Cyclomatic Complexity 1 include/linux/skbuff.h:__skb_queue_tail
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_transport_header
   Cyclomatic Complexity 1 include/linux/skbuff.h:skb_network_header
   Cyclomatic Complexity 1 include/net/net_namespace.h:read_pnet
   Cyclomatic Complexity 1 include/linux/unaligned/access_ok.h:get_unaligned_le64
   Cyclomatic Complexity 1 include/net/dst.h:dst_metric_raw
   Cyclomatic Complexity 1 include/net/sock.h:sk_has_account
   Cyclomatic Complexity 2 include/net/sock.h:sk_mem_charge
   Cyclomatic Complexity 1 include/net/sock.h:sock_net
   Cyclomatic Complexity 1 include/net/inet_sock.h:inet_sk
   Cyclomatic Complexity 1 include/net/inet_connection_sock.h:inet_csk
   Cyclomatic Complexity 1 include/linux/tcp.h:tcp_hdr
   Cyclomatic Complexity 1 include/linux/tcp.h:__tcp_hdrlen
   Cyclomatic Complexity 1 include/linux/tcp.h:tcp_hdrlen
   Cyclomatic Complexity 1 include/linux/tcp.h:tcp_rsk
   Cyclomatic Complexity 1 include/linux/tcp.h:tcp_sk
   Cyclomatic Complexity 1 include/linux/ipv6.h:ipv6_hdr
   Cyclomatic Complexity 1 include/linux/ip.h:ip_hdr
   Cyclomatic Complexity 4 include/net/tcp.h:tcp_fastopen_cookie_match
   Cyclomatic Complexity 1 include/net/tcp.h:tcp_fastopen_context_len
   Cyclomatic Complexity 3 include/net/tcp.h:tcp_segs_in
   Cyclomatic Complexity 8 include/net/tcp.h:tcp_fastopen_get_ctx
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_lock_acquire
   Cyclomatic Complexity 4 include/linux/rcupdate.h:rcu_read_lock
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_lock_release
   Cyclomatic Complexity 4 include/linux/rcupdate.h:rcu_read_unlock
   Cyclomatic Complexity 1 net//ipv4/tcp_fastopen.c:tcp_fastopen_ctx_free
   Cyclomatic Complexity 1 include/linux/lockdep.h:lock_is_held
   Cyclomatic Complexity 3 include/net/sock.h:lockdep_sock_is_held
   Cyclomatic Complexity 5 include/net/sock.h:__sk_dst_get
   Cyclomatic Complexity 2 include/net/dst.h:refdst_drop
   Cyclomatic Complexity 2 include/net/dst.h:skb_dst_drop
   Cyclomatic Complexity 5 include/net/dst.h:dst_metric
   Cyclomatic Complexity 5 net//ipv4/tcp_fastopen.c:tcp_fastopen_no_cookie
   Cyclomatic Complexity 3 include/linux/atomic-fallback.h:atomic_fetch_add_unless
   Cyclomatic Complexity 1 include/linux/atomic-fallback.h:atomic_add_unless
   Cyclomatic Complexity 1 include/linux/atomic-fallback.h:atomic_inc_not_zero
   Cyclomatic Complexity 6 include/net/sock.h:sk_dst_get
   Cyclomatic Complexity 1 include/linux/skbuff.h:__skb_pull
   Cyclomatic Complexity 2 include/linux/skbuff.h:skb_orphan
   Cyclomatic Complexity 1 include/net/sock.h:skb_set_owner_r
   Cyclomatic Complexity 2 include/net/sock.h:sock_put
   Cyclomatic Complexity 2 include/net/request_sock.h:__reqsk_free
   Cyclomatic Complexity 2 include/net/request_sock.h:reqsk_free
   Cyclomatic Complexity 2 include/net/request_sock.h:reqsk_put
   Cyclomatic Complexity 7 net//ipv4/tcp_fastopen.c:tcp_fastopen_queue_check
   Cyclomatic Complexity 11 include/linux/siphash.h:___siphash_aligned
   Cyclomatic Complexity 1 include/linux/siphash.h:siphash
   Cyclomatic Complexity 3 net//ipv4/tcp_fastopen.c:__tcp_fastopen_cookie_gen_cipher

vim +82 net//ipv4/tcp_fastopen.c

    68	
    69	int tcp_fastopen_reset_cipher(struct net *net, struct sock *sk,
    70				      void *primary_key, void *backup_key)
    71	{
    72		struct tcp_fastopen_context *ctx, *octx;
    73		struct fastopen_queue *q;
    74		int err = 0;
    75	
    76		ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
    77		if (!ctx) {
    78			err = -ENOMEM;
    79			goto out;
    80		}
    81	
  > 82		ctx->key[0] = (siphash_key_t){
    83			get_unaligned_le64(primary_key),
    84			get_unaligned_le64(primary_key + 8)
    85		};
    86		if (backup_key) {
    87			ctx->key[1] = (siphash_key_t){
    88				get_unaligned_le64(backup_key),
    89				get_unaligned_le64(backup_key + 8)
    90			};
    91			ctx->num = 2;
    92		} else {
    93			ctx->num = 1;
    94		}
    95	
    96		spin_lock(&net->ipv4.tcp_fastopen_ctx_lock);
    97		if (sk) {
    98			q = &inet_csk(sk)->icsk_accept_queue.fastopenq;
    99			octx = rcu_dereference_protected(q->ctx,
   100				lockdep_is_held(&net->ipv4.tcp_fastopen_ctx_lock));
   101			rcu_assign_pointer(q->ctx, ctx);
   102		} else {
   103			octx = rcu_dereference_protected(net->ipv4.tcp_fastopen_ctx,
   104				lockdep_is_held(&net->ipv4.tcp_fastopen_ctx_lock));
   105			rcu_assign_pointer(net->ipv4.tcp_fastopen_ctx, ctx);
   106		}
   107		spin_unlock(&net->ipv4.tcp_fastopen_ctx_lock);
   108	
   109		if (octx)
   110			call_rcu(&octx->rcu, tcp_fastopen_ctx_free);
   111	out:
   112		return err;
   113	}
   114	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29016 bytes --]

^ permalink raw reply

* Re: [PATCH net-next 5/5] ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF
From: kbuild test robot @ 2019-06-20 22:47 UTC (permalink / raw)
  To: Wei Wang
  Cc: kbuild-all, David Miller, netdev, Martin KaFai Lau, Eric Dumazet,
	Mahesh Bandewar, David Ahern, Wei Wang
In-Reply-To: <20190618182543.65477-6-tracywwnj@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]

Hi Wei,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Wei-Wang/ipv6-avoid-taking-refcnt-on-dst-during-route-lookup/20190621-052244
config: i386-randconfig-x077-201924 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ipvlan/ipvlan.h:23:0,
                    from drivers/net/ipvlan/ipvlan_main.c:5:
   include/net/ip6_route.h: In function 'ip6_route_output_flags':
>> include/net/ip6_route.h:103:15: error: 'struct net' has no member named 'ipv6'; did you mean 'ipv4'?
      dst = &net->ipv6.ip6_null_entry->dst;
                  ^~~~
                  ipv4

vim +103 include/net/ip6_route.h

    80	
    81	void ip6_route_input(struct sk_buff *skb);
    82	struct dst_entry *ip6_route_input_lookup(struct net *net,
    83						 struct net_device *dev,
    84						 struct flowi6 *fl6,
    85						 const struct sk_buff *skb, int flags);
    86	
    87	struct dst_entry *ip6_route_output_flags_noref(struct net *net,
    88						       const struct sock *sk,
    89						       struct flowi6 *fl6, int flags);
    90	
    91	static inline struct dst_entry *ip6_route_output_flags(struct net *net,
    92							       const struct sock *sk,
    93							       struct flowi6 *fl6,
    94							       int flags) {
    95		struct dst_entry *dst;
    96		struct rt6_info *rt6;
    97	
    98		rcu_read_lock();
    99		dst = ip6_route_output_flags_noref(net, sk, fl6, flags);
   100		rt6 = (struct rt6_info *)dst;
   101		/* For dst cached in uncached_list, refcnt is already taken. */
   102		if (list_empty(&rt6->rt6i_uncached) && !dst_hold_safe(dst)) {
 > 103			dst = &net->ipv6.ip6_null_entry->dst;
   104			dst_hold(dst);
   105		}
   106		rcu_read_unlock();
   107	
   108		return dst;
   109	}
   110	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36225 bytes --]

^ permalink raw reply

* Re: [RFC bpf-next 0/7] Programming socket lookup with BPF
From: Joe Stringer @ 2019-06-20 22:20 UTC (permalink / raw)
  To: Jakub Sitnicki; +Cc: Florian Westphal, netdev, bpf, kernel-team
In-Reply-To: <87sgs6ey43.fsf@cloudflare.com>

On Wed, Jun 19, 2019 at 2:14 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> Hey Florian,
>
> Thanks for taking a look at it.
>
> On Tue, Jun 18, 2019 at 03:52 PM CEST, Florian Westphal wrote:
> > Jakub Sitnicki <jakub@cloudflare.com> wrote:
> >>  - XDP programs using bpf_sk_lookup helpers, like load balancers, can't
> >>    find the listening socket to check for SYN cookies with TPROXY redirect.
> >
> > Sorry for the question, but where is the problem?
> > (i.e., is it with TPROXY or bpf side)?
>
> The way I see it is that the problem is that we have mappings for
> steering traffic into sockets split between two places: (1) the socket
> lookup tables, and (2) the TPROXY rules.
>
> BPF programs that need to check if there is a socket the packet is
> destined for have access to the socket lookup tables, via the mentioned
> bpf_sk_lookup helper, but are unaware of TPROXY redirects.
>
> For TCP we're able to look up from BPF if there are any established,
> request, and "normal" listening sockets. The listening sockets that
> receive connections via TPROXY are invisible to BPF progs.
>
> Why are we interested in finding all listening sockets? To check if any
> of them had SYN queue overflow recently and if we should honor SYN
> cookies.

Why are they invisible? Can't you look them up with bpf_skc_lookup_tcp()?

^ permalink raw reply

* [PATCH] FDDI: defza: Include linux/io-64-nonatomic-lo-hi.h
From: Paul Burton @ 2019-06-20 22:13 UTC (permalink / raw)
  To: Maciej W. Rozycki, David S. Miller, netdev@vger.kernel.org
  Cc: Paul Burton, Serge Semin, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org

Currently arch/mips/include/asm/io.h provides 64b memory accessor
functions such as readq & writeq even on MIPS32 platforms where those
accessors cannot actually perform a 64b memory access. They instead
BUG(). This is unfortunate for drivers which either #ifdef on the
presence of these accessors, or can function with non-atomic
implementations of them found in either linux/io-64-nonatomic-lo-hi.h or
linux/io-64-nonatomic-hi-lo.h. As such we're preparing to remove the
definitions of these 64b accessor functions for MIPS32 kernels.

In preparation for this, include linux/io-64-nonatomic-lo-hi.h in
defza.c in order to provide a non-atomic implementation of the
readq_relaxed & writeq_relaxed functions that are used by this code. In
practice this will have no runtime effect, since use of the 64b accessor
functions is conditional upon sizeof(unsigned long) == 8, ie. upon
CONFIG_64BIT=y. This means the calls to these non-atomic readq & writeq
implementations will be optimized out anyway, but we need their
definitions to keep the compiler happy.

For 64bit kernels using this code this change should also have no effect
because asm/io.h will continue to provide the definitions of
readq_relaxed & writeq_relaxed, which linux/io-64-nonatomic-lo-hi.h
checks for before defining itself.

Signed-off-by: Paul Burton <paul.burton@mips.com>
Cc: Serge Semin <Sergey.Semin@t-platforms.ru>
Cc: "Maciej W. Rozycki" <macro@linux-mips.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-mips@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Maciej, David, if you'd be happy to provide an Ack so that I can take
this through the mips-next branch that would be great; that'll let me
apply it prior to the asm/io.h change.
---
 drivers/net/fddi/defza.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/fddi/defza.c b/drivers/net/fddi/defza.c
index c5cae8e74dc4..060712c666bf 100644
--- a/drivers/net/fddi/defza.c
+++ b/drivers/net/fddi/defza.c
@@ -33,6 +33,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/ioport.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH net-next v4 1/7] igb: clear out tstamp after sending the packet
From: Eric Dumazet @ 2019-06-20 21:56 UTC (permalink / raw)
  To: Patel, Vedang, Eric Dumazet
  Cc: netdev@vger.kernel.org, Kirsher, Jeffrey T, David Miller,
	Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	intel-wired-lan@lists.osuosl.org, Gomes, Vinicius, l@dorileo.org,
	Jakub Kicinski, Murali Karicheri, Sergei Shtylyov
In-Reply-To: <D1A9515C-D317-40F3-81A2-451F7228A853@intel.com>



On 6/20/19 1:32 PM, Patel, Vedang wrote:
> 

> Ahh.. that’s clearly a false statement. Skb->tstamp is cleared so
> that it is not interpreted as a software timestamp when trying to
> send the Hardware TX timestamp to the userspace. I will rephrase the
> commit message in the next version.
> 
> Some more details: The problem occurs when using the txtime-assist
> mode of taprio with packets which also request the hardware transmit
> timestamp (e.g. PTP packets). Whenever txtime-assist mode is set,
> taprio will assign a hardware transmit timestamp to all the packets
> (in skb->tstamp). PTP packets will also request the hardware transmit
> timestamp be sent to the userspace after packet is transmitted.
> 
> Whenever a new timestamp is detected by the driver (this work is done
> in igb_ptp_tx_work() which calls igb_ptp_tx_hwtstamps() in
> igb_ptp.c[1]), it will queue the timestamp in the ERR_QUEUE for the
> userspace to read. When the userspace is ready, it will issue a
> recvmsg() call to collect this timestamp. The problem is in this
> recvmsg() call. If the skb->tstamp is not cleared out, it will be
> interpreted as a software timestamp and the hardware tx timestamp
> will not be successfully sent to the userspace. Look at
> skb_is_swtx_tstamp() and the callee function __sock_recv_timestamp()
> in net/socket.c for more details.


That amount of details in the changelog would be really nice ;)



^ permalink raw reply

* Re: [PATCH net-next 02/18] ionic: Add hardware init and device commands
From: Andrew Lunn @ 2019-06-20 21:54 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-3-snelson@pensando.io>

On Thu, Jun 20, 2019 at 01:24:08PM -0700, Shannon Nelson wrote:
> +	err = ionic_debugfs_add_dev(ionic);
> +	if (err) {
> +		dev_err(dev, "Cannot add device debugfs: %d , aborting\n", err);
> +		goto err_out_clear_drvdata;
> +	}

Hi Shannon

debugfs should not fail, since it is optional. debugfs failing should
also not be considered fatal. And lastly, debugfs is not very liked by
network people, so you should avoid it as much as possible. Use
ethtool, lspci, devlink, etc.

> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int blob_open(struct inode *inode, struct file *filp)
> +{
> +	filp->private_data = inode->i_private;
> +	return 0;
> +}
> +
> +static ssize_t blob_read(struct file *filp, char __user *buffer,
> +			 size_t count, loff_t *ppos)
> +{
> +	struct debugfs_blob_wrapper *blob = filp->private_data;
> +
> +	if (*ppos >= blob->size)
> +		return 0;
> +	if (*ppos + count > blob->size)
> +		count = blob->size - *ppos;
> +
> +	if (copy_to_user(buffer, blob->data + *ppos, count))
> +		return -EFAULT;
> +
> +	*ppos += count;
> +
> +	return count;
> +}
> +
> +static ssize_t blob_write(struct file *filp, const char __user *buffer,
> +			  size_t count, loff_t *ppos)
> +{
> +	struct debugfs_blob_wrapper *blob = filp->private_data;
> +
> +	if (*ppos >= blob->size)
> +		return 0;
> +	if (*ppos + count > blob->size)
> +		count = blob->size - *ppos;
> +
> +	if (copy_from_user(blob->data + *ppos, buffer, count))
> +		return -EFAULT;
> +
> +	*ppos += count;
> +
> +	return count;
> +}

Write is pretty much a no-no. We don't want proprietary user space
tools manipulating the hardware/driver state.

> +
> +static const struct file_operations blob_fops = {
> +	.owner = THIS_MODULE,
> +	.open = blob_open,
> +	.read = blob_read,
> +	.write = blob_write,
> +};
> +
> +struct dentry *debugfs_create_blob(const char *name, umode_t mode,
> +				   struct dentry *parent,
> +				   struct debugfs_blob_wrapper *blob)
> +{
> +	return debugfs_create_file(name, mode | 0200, parent, blob,
> +				   &blob_fops);
> +}
> +

> +static const struct debugfs_reg32 dev_cmd_regs[] = {
> +	{ .name = "db", .offset = 0, },
> +	{ .name = "done", .offset = 4, },
> +	{ .name = "cmd.word[0]", .offset = 8, },
> +	{ .name = "cmd.word[1]", .offset = 12, },
> +	{ .name = "cmd.word[2]", .offset = 16, },
> +	{ .name = "cmd.word[3]", .offset = 20, },
> +	{ .name = "cmd.word[4]", .offset = 24, },
> +	{ .name = "cmd.word[5]", .offset = 28, },
> +	{ .name = "cmd.word[6]", .offset = 32, },
> +	{ .name = "cmd.word[7]", .offset = 36, },
> +	{ .name = "cmd.word[8]", .offset = 40, },
> +	{ .name = "cmd.word[9]", .offset = 44, },
> +	{ .name = "cmd.word[10]", .offset = 48, },
> +	{ .name = "cmd.word[11]", .offset = 52, },
> +	{ .name = "cmd.word[12]", .offset = 56, },
> +	{ .name = "cmd.word[13]", .offset = 60, },
> +	{ .name = "cmd.word[14]", .offset = 64, },
> +	{ .name = "cmd.word[15]", .offset = 68, },
> +	{ .name = "comp.word[0]", .offset = 72, },
> +	{ .name = "comp.word[1]", .offset = 76, },
> +	{ .name = "comp.word[2]", .offset = 80, },
> +	{ .name = "comp.word[3]", .offset = 84, },
> +};
> +
> +int ionic_debugfs_add_dev_cmd(struct ionic *ionic)
> +{
> +	struct debugfs_regset32 *dev_cmd_regset;
> +	struct device *dev = ionic->dev;
> +	struct dentry *dentry;
> +
> +	dev_cmd_regset = devm_kzalloc(dev, sizeof(*dev_cmd_regset),
> +				      GFP_KERNEL);
> +	if (!dev_cmd_regset)
> +		return -ENOMEM;
> +	dev_cmd_regset->regs = dev_cmd_regs;
> +	dev_cmd_regset->nregs = ARRAY_SIZE(dev_cmd_regs);
> +	dev_cmd_regset->base = ionic->idev.dev_cmd_regs;
> +
> +	dentry = debugfs_create_regset32("dev_cmd", 0400,
> +					 ionic->dentry, dev_cmd_regset);
> +	if (IS_ERR_OR_NULL(dentry))
> +		return PTR_ERR(dentry);

ethtool -d seems like a more acceptable method for exporting
registers.

> +static int identity_show(struct seq_file *seq, void *v)
> +{
> +	struct ionic *ionic = seq->private;
> +	struct identity *ident = &ionic->ident;
> +	struct ionic_dev *idev = &ionic->idev;
> +
> +	seq_printf(seq, "asic_type:        0x%x\n", idev->dev_info.asic_type);
> +	seq_printf(seq, "asic_rev:         0x%x\n", idev->dev_info.asic_rev);
> +	seq_printf(seq, "serial_num:       %s\n", idev->dev_info.serial_num);
> +	seq_printf(seq, "fw_version:       %s\n", idev->dev_info.fw_version);
> +	seq_printf(seq, "fw_status:        0x%x\n",
> +		   ioread8(&idev->dev_info_regs->fw_status));
> +	seq_printf(seq, "fw_heartbeat:     0x%x\n",
> +		   ioread32(&idev->dev_info_regs->fw_heartbeat));

devlink just gained a much more flexible version of ethtool -i. Please
remove all this and use that.

> +int ionic_dev_setup(struct ionic *ionic)
> +{
> +	struct ionic_dev_bar *bar = ionic->bars;
> +	unsigned int num_bars = ionic->num_bars;
> +	struct ionic_dev *idev = &ionic->idev;
> +	struct device *dev = ionic->dev;
> +	u32 sig;
> +
> +	/* BAR0: dev_cmd and interrupts */
> +	if (num_bars < 1) {
> +		dev_info(dev, "No bars found, aborting\n");

dev_err(), since this is fatal. The same for most of these dev_info()
calls.

> +enum os_type {
> +	IONIC_OS_TYPE_LINUX   = 1,
> +	IONIC_OS_TYPE_WIN     = 2,
> +	IONIC_OS_TYPE_DPDK    = 3,
> +	IONIC_OS_TYPE_FREEBSD = 4,
> +	IONIC_OS_TYPE_IPXE    = 5,
> +	IONIC_OS_TYPE_ESXI    = 6,
> +};
> +
> +/**
> + * union drv_identity - driver identity information
> + * @os_type:          OS type (see enum os_type)
> + * @os_dist:          OS distribution, numeric format
> + * @os_dist_str:      OS distribution, string format
> + * @kernel_ver:       Kernel version, numeric format
> + * @kernel_ver_str:   Kernel version, string format
> + * @driver_ver_str:   Driver version, string format
> + */
> +union drv_identity {
> +	struct {
> +		__le32 os_type;
> +		__le32 os_dist;
> +		char   os_dist_str[128];
> +		__le32 kernel_ver;
> +		char   kernel_ver_str[32];
> +		char   driver_ver_str[32];
> +	};
> +	__le32 words[512];
> +};

> +int ionic_identify(struct ionic *ionic)
> +{
> +	struct identity *ident = &ionic->ident;
> +	struct ionic_dev *idev = &ionic->idev;
> +	size_t sz;
> +	int err;
> +
> +	memset(ident, 0, sizeof(*ident));
> +
> +	ident->drv.os_type = cpu_to_le32(IONIC_OS_TYPE_LINUX);
> +	ident->drv.os_dist = 0;
> +	strncpy(ident->drv.os_dist_str, utsname()->release,
> +		sizeof(ident->drv.os_dist_str) - 1);
> +	ident->drv.kernel_ver = cpu_to_le32(LINUX_VERSION_CODE);
> +	strncpy(ident->drv.kernel_ver_str, utsname()->version,
> +		sizeof(ident->drv.kernel_ver_str) - 1);
> +	strncpy(ident->drv.driver_ver_str, DRV_VERSION,
> +		sizeof(ident->drv.driver_ver_str) - 1);

Creepy.

	Andrew

^ permalink raw reply

* RE: kernel panic: corrupted stack end in corrupted
From: John Fastabend @ 2019-06-20 21:53 UTC (permalink / raw)
  To: syzbot, ast, daniel, john.fastabend, linux-kernel, netdev,
	syzkaller-bugs
In-Reply-To: <00000000000097ca41058bc129cc@google.com>

syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    29f785ff Merge branch 'fixes' of git://git.kernel.org/pub/..
> git tree:       net
> console output: https://syzkaller.appspot.com/x/log.txt?x=1158d411a00000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=e5c77f8090a3b96b
> dashboard link: https://syzkaller.appspot.com/bug?extid=b764c7ca388222ddfb17
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=100ca932a00000
> 
> The bug was bisected to:
> 
> commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
> Author: John Fastabend <john.fastabend@gmail.com>
> Date:   Sat Jun 30 13:17:47 2018 +0000
> 
>      bpf: sockhash fix omitted bucket lock in sock_close
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=135e8a3aa00000
> final crash:    https://syzkaller.appspot.com/x/report.txt?x=10de8a3aa00000
> console output: https://syzkaller.appspot.com/x/log.txt?x=175e8a3aa00000
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+b764c7ca388222ddfb17@syzkaller.appspotmail.com
> Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
> 
> Kernel panic - not syncing: corrupted stack end detected inside scheduler
> CPU: 0 PID: 8770 Comm: syz-executor.4 Not tainted 5.2.0-rc5+ #57
> Hardware name: Google GooglSeaBIOS (version 1.8.2-20190503_170316-google)
> 

I believe this is the same or similar bug to the others found with
sockmap + ulp running. Will have fixes shortly.

Thanks,
John

^ permalink raw reply

* Re: [PATCH v2 2/3] net: fddi: skfp: Include generic PCI definitions
From: Bjorn Helgaas @ 2019-06-20 21:37 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci
In-Reply-To: <20190620180754.15413-3-puranjay12@gmail.com>

On Thu, Jun 20, 2019 at 11:37:53PM +0530, Puranjay Mohan wrote:
> Include the uapi/linux/pci_regs.h header file which contains the generic
> PCI defines.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> ---
>  drivers/net/fddi/skfp/drvfbi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/fddi/skfp/drvfbi.c b/drivers/net/fddi/skfp/drvfbi.c
> index b324c1acf195..e8245cb281f8 100644
> --- a/drivers/net/fddi/skfp/drvfbi.c
> +++ b/drivers/net/fddi/skfp/drvfbi.c
> @@ -20,7 +20,7 @@
>  #include "h/supern_2.h"
>  #include "h/skfbiinc.h"
>  #include <linux/bitrev.h>
> -
> +#include <linux/pci_regs.h>

You removed the blank line between the list of include files and the
SCCS ID (now there's an anachronism) below.  That blank line is part
of typical Linux style and you should keep it.

>  #ifndef	lint
>  static const char ID_sccs[] = "@(#)drvfbi.c	1.63 99/02/11 (C) SK " ;
>  #endif
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [PATCH v2 3/3] net: fddi: skfp: Remove unused private PCI definitions
From: Bjorn Helgaas @ 2019-06-20 21:37 UTC (permalink / raw)
  To: Puranjay Mohan
  Cc: Shuah Khan, Bjorn Helgaas, netdev, Linux Kernel Mailing List,
	linux-kernel-mentees, linux-pci
In-Reply-To: <20190620180754.15413-4-puranjay12@gmail.com>

On Thu, Jun 20, 2019 at 11:37:54PM +0530, Puranjay Mohan wrote:
> Remove unused and private PCI definitions from skfbi.h because generic PCI
> symbols are already included from pci_regs.h.
> 
> Signed-off-by: Puranjay Mohan <puranjay12@gmail.com>
> ---
>  drivers/net/fddi/skfp/h/skfbi.h | 207 +-------------------------------
>  1 file changed, 1 insertion(+), 206 deletions(-)
> 
> diff --git a/drivers/net/fddi/skfp/h/skfbi.h b/drivers/net/fddi/skfp/h/skfbi.h
> index a05ce16171be..a8af80148022 100644
> --- a/drivers/net/fddi/skfp/h/skfbi.h
> +++ b/drivers/net/fddi/skfp/h/skfbi.h
> @@ -27,46 +27,6 @@
>  /*
>   * Configuration Space header
>   */

This comment should be removed because it goes along with the
definitions below (the ones from PCI_VENDOR_ID to PCI_MAX_LAT).

> -#define	PCI_VENDOR_ID	0x00	/* 16 bit	Vendor ID */
> -#define	PCI_DEVICE_ID	0x02	/* 16 bit	Device ID */

> @@ -74,179 +34,14 @@
>   * Note: The temperature and voltage sensors are relocated on a different
>   *	 I2C bus.
>   */
> -#define I2C_ADDR_VPD	0xA0	/* I2C address for the VPD EEPROM */ 
> +#define I2C_ADDR_VPD	0xA0	/* I2C address for the VPD EEPROM */

You removed the space at the end of the I2C_ADDR_VPD line.  That space
*is* a whitespace error, but generally you should avoid fixing random
unrelated issues in the middle of your patch.  Those random fixes make
it harder because the reviewer is expecting to see unused PCI-related
things removed and seeing an I2C diff is surprising and forces him/her
to do some investigation.

> -/*	PCI_STATUS	16 bit	Status */
>  #define	PCI_PERR	0x8000	/* Bit 15:	Parity Error */
>  #define	PCI_SERR	0x4000	/* Bit 14:	Signaled SERR */
>  #define	PCI_RMABORT	0x2000	/* Bit 13:	Received Master Abort */

These should be replaced by PCI_STATUS_DETECTED_PARITY and similar
generic definitions.  You could do this in your 1/1 patch along with
PCI_REVISION_ID.

> -#define	PCI_RTABORT	0x1000	/* Bit 12:	Received Target Abort */
>  #define	PCI_STABORT	0x0800	/* Bit 11:	Sent Target Abort */
> -#define	PCI_DEVSEL	0x0600	/* Bit 10..9:	DEVSEL Timing */
> -#define	PCI_DEV_FAST	(0<<9)	/*		fast */
> -#define	PCI_DEV_MEDIUM	(1<<9)	/*		medium */
> -#define	PCI_DEV_SLOW	(2<<9)	/*		slow */
>  #define	PCI_DATAPERR	0x0100	/* Bit 8:	DATA Parity error detected */
> -#define	PCI_FB2BCAP	0x0080	/* Bit 7:	Fast Back-to-Back Capability */
> -#define	PCI_UDF		0x0040	/* Bit 6:	User Defined Features */
> -#define PCI_66MHZCAP	0x0020	/* Bit 5:	66 MHz PCI bus clock capable */
> -#define PCI_NEWCAP	0x0010	/* Bit 4:	New cap. list implemented */
> -
>  #define PCI_ERRBITS	(PCI_PERR|PCI_SERR|PCI_RMABORT|PCI_STABORT|PCI_DATAPERR)

^ permalink raw reply

* Re: [PATCH iproute2 v2 0/3] refactor the cmd_exec()
From: Stephen Hemminger @ 2019-06-20 21:31 UTC (permalink / raw)
  To: Matteo Croce; +Cc: netdev, David Ahern, Andrea Claudi
In-Reply-To: <20190618144935.31405-1-mcroce@redhat.com>

On Tue, 18 Jun 2019 16:49:32 +0200
Matteo Croce <mcroce@redhat.com> wrote:

> Refactor the netns and ipvrf code so less steps are needed to exec commands
> in a netns or a VRF context.
> Also remove some code which became dead. bloat-o-meter shows a tiny saving.
> 
> Matteo Croce (3):
>   netns: switch netns in the child when executing commands
>   ip vrf: use hook to change VRF in the child
>   netns: make netns_{save,restore} static
> 
>  include/namespace.h |  2 --
>  include/utils.h     |  6 ++---
>  ip/ip.c             |  1 -
>  ip/ipnetns.c        | 61 ++++++++++++++++++++++++++++++++++-----------
>  ip/ipvrf.c          | 12 ++++++---
>  lib/exec.c          |  7 +++++-
>  lib/namespace.c     | 31 -----------------------
>  lib/utils.c         | 27 --------------------
>  8 files changed, 63 insertions(+), 84 deletions(-)
> 

Ok, applied.

^ permalink raw reply

* Re: [PATCH net-next 01/18] ionic: Add basic framework for IONIC Network device driver
From: Andrew Lunn @ 2019-06-20 21:24 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-2-snelson@pensando.io>

> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -0,0 +1,27 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#ifndef _IONIC_H_
> +#define _IONIC_H_
> +
> +#define DRV_NAME		"ionic"
> +#define DRV_DESCRIPTION		"Pensando Ethernet NIC Driver"
> +#define DRV_VERSION		"0.11.0-k"

DRV_VERSION is pretty useless. What you really want to know is the
kernel git tree and commit. The big distributions might backport this
version of the driver back to the old kernel with a million
patches. At which point 0.11.0-k tells you nothing much.
> +
> +// TODO: register these with the official include/linux/pci_ids.h
> +#define PCI_VENDOR_ID_PENSANDO			0x1dd8

That file has a comment:

 *      Do not add new entries to this file unless the definitions
 *      are shared between multiple drivers.

Is it going to be shared?

 +
> +#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF	0x1002
> +#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF	0x1003
> +#define PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT	0x1004
> +
> +#define IONIC_SUBDEV_ID_NAPLES_25	0x4000
> +#define IONIC_SUBDEV_ID_NAPLES_100_4	0x4001
> +#define IONIC_SUBDEV_ID_NAPLES_100_8	0x4002
> +
> +struct ionic {
> +	struct pci_dev *pdev;
> +	struct device *dev;
> +};
> +
> +#endif /* _IONIC_H_ */
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus.h b/drivers/net/ethernet/pensando/ionic/ionic_bus.h
> new file mode 100644
> index 000000000000..94ba0afc6f38
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#ifndef _IONIC_BUS_H_
> +#define _IONIC_BUS_H_
> +
> +int ionic_bus_register_driver(void);
> +void ionic_bus_unregister_driver(void);
> +
> +#endif /* _IONIC_BUS_H_ */
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> new file mode 100644
> index 000000000000..ab6206c162d4
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/pci.h>
> +
> +#include "ionic.h"
> +#include "ionic_bus.h"
> +
> +/* Supported devices */
> +static const struct pci_device_id ionic_id_table[] = {
> +	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF) },
> +	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_VF) },
> +	{ PCI_VDEVICE(PENSANDO, PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT) },
> +	{ 0, }	/* end of table */
> +};
> +MODULE_DEVICE_TABLE(pci, ionic_id_table);
> +
> +static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ionic *ionic;
> +
> +	ionic = devm_kzalloc(dev, sizeof(*ionic), GFP_KERNEL);
> +	if (!ionic)
> +		return -ENOMEM;
> +
> +	ionic->pdev = pdev;
> +	pci_set_drvdata(pdev, ionic);
> +	ionic->dev = dev;
> +	dev_info(ionic->dev, "attached\n");

probed would be more accurate. But in general, please avoid all but
the minimum of such info messages.

> +
> +	return 0;
> +}
> +
> +static void ionic_remove(struct pci_dev *pdev)
> +{
> +	struct ionic *ionic = pci_get_drvdata(pdev);
> +
> +	pci_set_drvdata(pdev, NULL);
> +	dev_info(ionic->dev, "removed\n");

Not very useful dev_info().

Also, i think the core will NULL out the drive data for you. But you
should check.
> +}
> +
> +static struct pci_driver ionic_driver = {
> +	.name = DRV_NAME,
> +	.id_table = ionic_id_table,
> +	.probe = ionic_probe,
> +	.remove = ionic_remove,
> +};
> +
> +int ionic_bus_register_driver(void)
> +{
> +	return pci_register_driver(&ionic_driver);
> +}
> +
> +void ionic_bus_unregister_driver(void)
> +{
> +	pci_unregister_driver(&ionic_driver);
> +}

It looks like you can use module_pci_driver() and remove a lot of
boilerplate.

	Andrew

^ permalink raw reply

* Re: [PATCH net-next v4 2/7] etf: Add skip_sock_check
From: Patel, Vedang @ 2019-06-20 21:13 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev@vger.kernel.org, Kirsher, Jeffrey T, David Miller,
	Jamal Hadi Salim, xiyou.wangcong@gmail.com, jiri@resnulli.us,
	intel-wired-lan@lists.osuosl.org, Gomes, Vinicius, l@dorileo.org,
	jakub.kicinski@netronome.com, m-karicheri2@ti.com
In-Reply-To: <c304970a-1973-cdce-17b5-682f28856306@cogentembedded.com>



> On Jun 20, 2019, at 1:16 AM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:
> 
> On 19.06.2019 20:40, Vedang Patel wrote:
> 
>> Currently, etf expects a socket with SO_TXTIME option set for each packet
>> it encounters. So, it will drop all other packets. But, in the future
>> commits we are planning to add functionality which where tstamp value will
> 
>   One of "which" and "where", not both. :-)
> 
Yeah. It’s a typo. Will fix it in next version.

Thanks,
Vedang
>> be set by another qdisc. Also, some packets which are generated from within
>> the kernel (e.g. ICMP packets) do not have any socket associated with them.
>> So, this commit adds support for skip_sock_check. When this option is set,
>> etf will skip checking for a socket and other associated options for all
>> skbs.
>> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
> [...]
> 
> MBR, Sergei


^ permalink raw reply

* Re: network unstable on odroid-c1/meson8b.
From: Aymeric @ 2019-06-20 20:54 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: netdev, linux-amlogic, Martin Blumenstingl
In-Reply-To: <1f34f3b6-2c70-9ff3-3f5a-597e4bd9c66f@gmail.com>


Le 20/06/2019 à 17:53, Heiner Kallweit a écrit :
> On 20.06.2019 09:55, Aymeric wrote:
>> Hi,
>> On 2019-06-20 00:14, Heiner Kallweit wrote:
>>> On 19.06.2019 22:18, Aymeric wrote:
>>>> Hello all,
>>>>
>>> Kernel 3.10 didn't have a dedicated RTL8211F PHY driver yet, therefore
>>> I assume the genphy driver was used. Do you have a line with
>>> "attached PHY driver" in dmesg output of the vendor kernel?
>> No.
>> Here is the full output of the dmesg from vendor kernel [¹].
>>
>> I've also noticed something strange, it might be linked, but mac address of the board is set to a random value when using mainline kernel and I've to set it manually but not when using vendor kernel.
>>
>>> The dedicated PHY driver takes care of the tx delay, if the genphy
>>> driver is used we have to rely on what uboot configured.
>>> But if we indeed had an issue with a misconfigured delay, I think
>>> the connection shouldn't be fine with just another link partner.
>>> Just to have it tested you could make rtl8211f_config_init() in
>>> drivers/net/phy/realtek.c a no-op (in current kernels).
>>>
>> I'm not an expert here, just adding a "return 0;" here[²] would be enough?
>>
>>> And you could compare at least the basic PHY registers 0x00 - 0x30
>>> with both kernel versions, e.g. with phytool.
>>>
>> They are not the same but I don't know what I'm looking for, so for kernel 3.10 [³] and for kernel 5.1.12 [⁴].
>>
>> Aymeric
>>
>> [¹]: https://paste.aplu.fr/?38ef95b44ebdbfc3#G666/YbhgU+O+tdC/2HaimUCigm8ZTB44qvQip/HJ5A=
>> [²]: https://github.com/torvalds/linux/blob/241e39004581475b2802cd63c111fec43bb0123e/drivers/net/phy/realtek.c#L164
>> [³]: https://paste.aplu.fr/?2dde1c32d5c68f4c#6xIa8MjTm6jpI6citEJAqFTLMMHDjFZRet/M00/EwjU=
>> [⁴]: https://paste.aplu.fr/?32130e9bcb05dde7#N/xdnvb5GklcJtiOxMpTCm+9gsUliRwH8X3dcwSV+ng=
>>
> The vendor kernel has some, but not really much magic:
> https://github.com/hardkernel/linux/blob/odroidc-3.10.y/drivers/amlogic/ethernet/phy/am_rtl8211f.c
> The write to RTL8211F_PHYCR2 is overwritten later, therefore we don't have to consider it.
>
> The following should make the current Realtek PHY driver behave like in the vendor driver.
> Could you test it?

(sending again for mailing list, sorry, I forgot to force it in plaintext…)

I've applied your patch and tried but it doesn't change anything.

Here is dmesg output and phytool results.

https://paste.aplu.fr/?9735c99907528929#SeCgwR45cgnbDA1tXIVBHCBT8RNct2r41jU6vsguLVc=

-- 
Aymeric

^ permalink raw reply

* Re: Stats for XDP actions
From: Toke Høiland-Jørgensen @ 2019-06-20 20:42 UTC (permalink / raw)
  To: David Ahern, Jesper Dangaard Brouer
  Cc: David Miller, mst, makita.toshiaki, jasowang, netdev,
	virtualization, hawk, Jakub Kicinski, John Fastabend,
	Daniel Borkmann, Saeed Mahameed, Tariq Toukan
In-Reply-To: <44ae964a-d3dd-6b7f-4bcc-21e07525bf41@gmail.com>

David Ahern <dsahern@gmail.com> writes:

> On 4/18/19 8:24 AM, Toke Høiland-Jørgensen wrote:
>>>>
>>>
>>> Understood. Hopefully in March I will get some time to come back to this
>>> and propose an idea on what I would like to see - namely, the admin has
>>> a config option at load time to enable driver counters versus custom map
>>> counters. (meaning the operator of the node chooses standard stats over
>>> strict performance.) But of course that means the drivers have the code
>>> to collect those stats.
>> 
>> Hi David
>> 
>> I don't recall seeing any follow-up on this. Did you have a chance to
>> formulate your ideas? :)
>> 
>
> Not yet. Almost done with the nexthop changes. Once that is out of the
> way I can come back to this.

Ping? :)

-Toke

^ permalink raw reply

* Re: [PATCH net-next v4 1/7] igb: clear out tstamp after sending the packet
From: Patel, Vedang @ 2019-06-20 20:32 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev@vger.kernel.org, Kirsher, Jeffrey T, David Miller,
	Jamal Hadi Salim, Cong Wang, Jiri Pirko,
	intel-wired-lan@lists.osuosl.org, Gomes, Vinicius, l@dorileo.org,
	Jakub Kicinski, Murali Karicheri, Sergei Shtylyov
In-Reply-To: <99e834ed-1c78-d35c-84dc-511d377284a1@gmail.com>



> On Jun 20, 2019, at 10:07 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
> 
> 
> On 6/20/19 9:49 AM, Patel, Vedang wrote:
>> 
>> 
>>> On Jun 20, 2019, at 3:47 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On 6/19/19 10:40 AM, Vedang Patel wrote:
>>>> skb->tstamp is being used at multiple places. On the transmit side, it
>>>> is used to determine the launchtime of the packet. It is also used to
>>>> determine the software timestamp after the packet has been transmitted.
>>>> 
>>>> So, clear out the tstamp value after it has been read so that we do not
>>>> report false software timestamp on the receive side.
>>>> 
>>>> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
>>>> ---
>>>> drivers/net/ethernet/intel/igb/igb_main.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>> 
>>>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>>>> index fc925adbd9fa..f66dae72fe37 100644
>>>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>>>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>>>> @@ -5688,6 +5688,7 @@ static void igb_tx_ctxtdesc(struct igb_ring *tx_ring,
>>>> 	 */
>>>> 	if (tx_ring->launchtime_enable) {
>>>> 		ts = ns_to_timespec64(first->skb->tstamp);
>>>> +		first->skb->tstamp = 0;
>>> 
>>> Please provide more explanations.
>>> 
>>> Why only this driver would need this ?
>>> 
>> Currently, igb is the only driver which uses the skb->tstamp option on the transmit side (to set the hardware transmit timestamp). All the other drivers only use it on the receive side (to collect and send the hardware transmit timestamp to the userspace after packet has been sent).
>> 
>> So, any driver which supports the hardware txtime in the future will have to clear skb->tstamp to make sure that hardware tx transmit and tx timestamping can be done on the same packet.
> 
> The changelog is rather confusing :
> 
> "So, clear out the tstamp value after it has been read so that we do not
> report false software timestamp on the receive side."
> 
> I have hard time understanding why sending an skb through this driver
> could cause a problem on receive side ?
> 
Ahh.. that’s clearly a false statement. Skb->tstamp is cleared so that it is not interpreted as a software timestamp when trying to send the Hardware TX timestamp to the userspace. I will rephrase the commit message in the next version.

Some more details:
The problem occurs when using the txtime-assist mode of taprio with packets which also request the hardware transmit timestamp (e.g. PTP packets). Whenever txtime-assist mode is set, taprio will assign a hardware transmit timestamp to all the packets (in skb->tstamp). PTP packets will also request the hardware transmit timestamp be sent to the userspace after packet is transmitted.

Whenever a new timestamp is detected by the driver (this work is done in igb_ptp_tx_work() which calls igb_ptp_tx_hwtstamps() in igb_ptp.c[1]), it will queue the timestamp in the ERR_QUEUE for the userspace to read. When the userspace is ready, it will issue a recvmsg() call to collect this timestamp. The problem is in this recvmsg() call. If the skb->tstamp is not cleared out, it will be interpreted as a software timestamp and the hardware tx timestamp will not be successfully sent to the userspace. Look at skb_is_swtx_tstamp() and the callee function __sock_recv_timestamp() in net/socket.c for more details. 

Thanks, 
Vedang

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/intel/igb/igb_ptp.c?h=v5.2-rc5#n666

> I suggest to rephrase it to clear the confusion.
> 
>> 
>> Thanks,
>> Vedang
>>> 
>>>> 		context_desc->seqnum_seed = cpu_to_le32(ts.tv_nsec / 32);
>>>> 	} else {
>>>> 		context_desc->seqnum_seed = 0;
>>>> 
>> 


^ permalink raw reply

* [PATCH net-next 03/18] ionic: Add port management commands
From: Shannon Nelson @ 2019-06-20 20:24 UTC (permalink / raw)
  To: snelson, netdev
In-Reply-To: <20190620202424.23215-1-snelson@pensando.io>

The port management commands apply to the physical port
associated with the PCI device, which might be shared among
several logical interfaces.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic.h   |   4 +
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  16 +++
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 116 ++++++++++++++++++
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  15 +++
 .../net/ethernet/pensando/ionic/ionic_main.c  |  95 ++++++++++++++
 5 files changed, 246 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index f19503c5aca6..a1ed9bc486dd 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -45,4 +45,8 @@ int ionic_identify(struct ionic *ionic);
 int ionic_init(struct ionic *ionic);
 int ionic_reset(struct ionic *ionic);
 
+int ionic_port_identify(struct ionic *ionic);
+int ionic_port_init(struct ionic *ionic);
+int ionic_port_reset(struct ionic *ionic);
+
 #endif /* _IONIC_H_ */
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 018ed00ff566..407988f17796 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -145,10 +145,25 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto err_out_teardown;
 	}
 
+	/* Configure the ports */
+	err = ionic_port_identify(ionic);
+	if (err) {
+		dev_err(dev, "Cannot identify port: %d, aborting\n", err);
+		goto err_out_reset;
+	}
+
+	err = ionic_port_init(ionic);
+	if (err) {
+		dev_err(dev, "Cannot init port: %d, aborting\n", err);
+		goto err_out_reset;
+	}
+
 	dev_info(ionic->dev, "attached\n");
 
 	return 0;
 
+err_out_reset:
+	ionic_reset(ionic);
 err_out_teardown:
 	ionic_dev_teardown(ionic);
 err_out_unmap_bars:
@@ -172,6 +187,7 @@ static void ionic_remove(struct pci_dev *pdev)
 	struct ionic *ionic = pci_get_drvdata(pdev);
 
 	if (ionic) {
+		ionic_port_reset(ionic);
 		ionic_reset(ionic);
 		ionic_dev_teardown(ionic);
 		ionic_unmap_bars(ionic);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index f3e457853a5a..55fd2881aac3 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -130,3 +130,119 @@ void ionic_dev_cmd_reset(struct ionic_dev *idev)
 
 	ionic_dev_cmd_go(idev, &cmd);
 }
+
+/* Port commands */
+void ionic_dev_cmd_port_identify(struct ionic_dev *idev)
+{
+	union dev_cmd cmd = {
+		.port_init.opcode = CMD_OPCODE_PORT_IDENTIFY,
+		.port_init.index = 0,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_init(struct ionic_dev *idev)
+{
+	union dev_cmd cmd = {
+		.port_init.opcode = CMD_OPCODE_PORT_INIT,
+		.port_init.index = 0,
+		.port_init.info_pa = cpu_to_le64(idev->port_info_pa),
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_reset(struct ionic_dev *idev)
+{
+	union dev_cmd cmd = {
+		.port_reset.opcode = CMD_OPCODE_PORT_RESET,
+		.port_reset.index = 0,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_state(struct ionic_dev *idev, u8 state)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_STATE,
+		.port_setattr.state = state,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_speed(struct ionic_dev *idev, u32 speed)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_SPEED,
+		.port_setattr.speed = cpu_to_le32(speed),
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_mtu(struct ionic_dev *idev, u32 mtu)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_MTU,
+		.port_setattr.mtu = cpu_to_le32(mtu),
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_autoneg(struct ionic_dev *idev, u8 an_enable)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_AUTONEG,
+		.port_setattr.an_enable = an_enable,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_fec(struct ionic_dev *idev, u8 fec_type)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_FEC,
+		.port_setattr.fec_type = fec_type,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_PAUSE,
+		.port_setattr.pause_type = pause_type,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+void ionic_dev_cmd_port_loopback(struct ionic_dev *idev, u8 loopback_mode)
+{
+	union dev_cmd cmd = {
+		.port_setattr.opcode = CMD_OPCODE_PORT_SETATTR,
+		.port_setattr.index = 0,
+		.port_setattr.attr = IONIC_PORT_ATTR_LOOPBACK,
+		.port_setattr.loopback_mode = loopback_mode,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 66afab3ee396..fe5e1b0e8d55 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -123,6 +123,10 @@ struct ionic_dev {
 	struct ionic_intr __iomem *intr_ctrl;
 	u64 __iomem *intr_status;
 
+	struct port_info *port_info;
+	dma_addr_t port_info_pa;
+	u32 port_info_sz;
+
 	struct ionic_devinfo dev_info;
 };
 
@@ -141,4 +145,15 @@ void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver);
 void ionic_dev_cmd_init(struct ionic_dev *idev);
 void ionic_dev_cmd_reset(struct ionic_dev *idev);
 
+void ionic_dev_cmd_port_identify(struct ionic_dev *idev);
+void ionic_dev_cmd_port_init(struct ionic_dev *idev);
+void ionic_dev_cmd_port_reset(struct ionic_dev *idev);
+void ionic_dev_cmd_port_state(struct ionic_dev *idev, u8 state);
+void ionic_dev_cmd_port_speed(struct ionic_dev *idev, u32 speed);
+void ionic_dev_cmd_port_mtu(struct ionic_dev *idev, u32 mtu);
+void ionic_dev_cmd_port_autoneg(struct ionic_dev *idev, u8 an_enable);
+void ionic_dev_cmd_port_fec(struct ionic_dev *idev, u8 fec_type);
+void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type);
+void ionic_dev_cmd_port_loopback(struct ionic_dev *idev, u8 loopback_mode);
+
 #endif /* _IONIC_DEV_H_ */
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 2ac1ed12a81d..36314f865b94 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -309,6 +309,101 @@ int ionic_reset(struct ionic *ionic)
 	return err;
 }
 
+int ionic_port_identify(struct ionic *ionic)
+{
+	struct identity *ident = &ionic->ident;
+	struct ionic_dev *idev = &ionic->idev;
+	struct device *dev = ionic->dev;
+	size_t sz;
+	int err;
+
+	mutex_lock(&ionic->dev_cmd_lock);
+
+	ionic_dev_cmd_port_identify(idev);
+	err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+	if (!err) {
+		sz = min(sizeof(ident->port), sizeof(idev->dev_cmd_regs->data));
+		memcpy_fromio(&ident->port, &idev->dev_cmd_regs->data, sz);
+	}
+
+	mutex_unlock(&ionic->dev_cmd_lock);
+
+	dev_dbg(dev, "speed %d\n", ident->port.config.speed);
+	dev_dbg(dev, "mtu %d\n", ident->port.config.mtu);
+	dev_dbg(dev, "state %d\n", ident->port.config.state);
+	dev_dbg(dev, "an_enable %d\n", ident->port.config.an_enable);
+	dev_dbg(dev, "fec_type %d\n", ident->port.config.fec_type);
+	dev_dbg(dev, "pause_type %d\n", ident->port.config.pause_type);
+	dev_dbg(dev, "loopback_mode %d\n", ident->port.config.loopback_mode);
+
+	return err;
+}
+
+int ionic_port_init(struct ionic *ionic)
+{
+	struct identity *ident = &ionic->ident;
+	struct ionic_dev *idev = &ionic->idev;
+	size_t sz;
+	int err;
+
+	if (idev->port_info)
+		return 0;
+
+	idev->port_info_sz = ALIGN(sizeof(*idev->port_info), PAGE_SIZE);
+	idev->port_info = dma_alloc_coherent(ionic->dev, idev->port_info_sz,
+					     &idev->port_info_pa,
+					     GFP_KERNEL);
+	if (!idev->port_info) {
+		dev_err(ionic->dev, "Failed to allocate port info, aborting\n");
+		return -ENOMEM;
+	}
+
+	sz = min(sizeof(ident->port.config), sizeof(idev->dev_cmd_regs->data));
+
+	mutex_lock(&ionic->dev_cmd_lock);
+
+	memcpy_toio(&idev->dev_cmd_regs->data, &ident->port.config, sz);
+	ionic_dev_cmd_port_init(idev);
+	err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+
+	ionic_dev_cmd_port_state(&ionic->idev, PORT_ADMIN_STATE_UP);
+	(void)ionic_dev_cmd_wait(ionic, devcmd_timeout);
+
+	mutex_unlock(&ionic->dev_cmd_lock);
+	if (err) {
+		dev_err(ionic->dev, "Failed to init port\n");
+		return err;
+	}
+
+	return 0;
+}
+
+int ionic_port_reset(struct ionic *ionic)
+{
+	struct ionic_dev *idev = &ionic->idev;
+	int err;
+
+	if (!idev->port_info)
+		return 0;
+
+	mutex_lock(&ionic->dev_cmd_lock);
+	ionic_dev_cmd_port_reset(idev);
+	err = ionic_dev_cmd_wait(ionic, devcmd_timeout);
+	mutex_unlock(&ionic->dev_cmd_lock);
+	if (err) {
+		dev_err(ionic->dev, "Failed to reset port\n");
+		return err;
+	}
+
+	dma_free_coherent(ionic->dev, idev->port_info_sz,
+			  idev->port_info, idev->port_info_pa);
+
+	idev->port_info = NULL;
+	idev->port_info_pa = 0;
+
+	return err;
+}
+
 static int __init ionic_init_module(void)
 {
 	ionic_struct_size_checks();
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 08/18] ionic: Add notifyq support
From: Shannon Nelson @ 2019-06-20 20:24 UTC (permalink / raw)
  To: snelson, netdev
In-Reply-To: <20190620202424.23215-1-snelson@pensando.io>

The AdminQ is fine for sending messages and requests to the NIC,
but we also need to have events published from the NIC to the
driver.  The NotifyQ handles this for us, using the same interrupt
as AdminQ.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../ethernet/pensando/ionic/ionic_debugfs.c   |  16 ++
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 187 +++++++++++++++++-
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   4 +
 3 files changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
index e01126f3f6bd..5ebfaa320edf 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
@@ -298,6 +298,7 @@ int ionic_debugfs_add_qcq(struct lif *lif, struct qcq *qcq)
 	struct debugfs_blob_wrapper *desc_blob;
 	struct device *dev = lif->ionic->dev;
 	struct intr *intr = &qcq->intr;
+	struct dentry *stats_dentry;
 	struct queue *q = &qcq->q;
 	struct cq *cq = &qcq->cq;
 
@@ -391,6 +392,21 @@ int ionic_debugfs_add_qcq(struct lif *lif, struct qcq *qcq)
 					intr_ctrl_regset);
 	}
 
+	if (qcq->flags & QCQ_F_NOTIFYQ) {
+		stats_dentry = debugfs_create_dir("notifyblock", qcq_dentry);
+		if (IS_ERR_OR_NULL(stats_dentry))
+			return PTR_ERR(stats_dentry);
+
+		debugfs_create_u64("eid", 0400, stats_dentry,
+				   (u64 *)&lif->info->status.eid);
+		debugfs_create_u16("link_status", 0400, stats_dentry,
+				   (u16 *)&lif->info->status.link_status);
+		debugfs_create_u32("link_speed", 0400, stats_dentry,
+				   (u32 *)&lif->info->status.link_speed);
+		debugfs_create_u16("link_down_count", 0400, stats_dentry,
+				   (u16 *)&lif->info->status.link_down_count);
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 1cfc571dc13e..2a5c2383e2f3 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -12,6 +12,8 @@
 #include "ionic_lif.h"
 #include "ionic_debugfs.h"
 
+static int ionic_notifyq_clean(struct lif *lif, int budget);
+
 static bool ionic_adminq_service(struct cq *cq, struct cq_info *cq_info)
 {
 	struct admin_comp *comp = cq_info->cq_desc;
@@ -26,7 +28,94 @@ static bool ionic_adminq_service(struct cq *cq, struct cq_info *cq_info)
 
 static int ionic_adminq_napi(struct napi_struct *napi, int budget)
 {
-	return ionic_napi(napi, budget, ionic_adminq_service, NULL, NULL);
+	struct lif *lif = napi_to_cq(napi)->lif;
+	int n_work = 0;
+	int a_work = 0;
+
+	if (likely(lif->notifyqcq && lif->notifyqcq->flags & QCQ_F_INITED))
+		n_work = ionic_notifyq_clean(lif, budget);
+	a_work = ionic_napi(napi, budget, ionic_adminq_service, NULL, NULL);
+
+	return max(n_work, a_work);
+}
+
+static bool ionic_notifyq_service(struct cq *cq, struct cq_info *cq_info)
+{
+	union notifyq_comp *comp = cq_info->cq_desc;
+	struct net_device *netdev;
+	struct queue *q;
+	struct lif *lif;
+	u64 eid;
+
+	q = cq->bound_q;
+	lif = q->info[0].cb_arg;
+	netdev = lif->netdev;
+	eid = le64_to_cpu(comp->event.eid);
+
+	/* Have we run out of new completions to process? */
+	if (eid <= lif->last_eid)
+		return false;
+
+	lif->last_eid = eid;
+
+	dev_dbg(lif->ionic->dev, "notifyq event:\n");
+	dynamic_hex_dump("event ", DUMP_PREFIX_OFFSET, 16, 1,
+			 comp, sizeof(*comp), true);
+
+	switch (le16_to_cpu(comp->event.ecode)) {
+	case EVENT_OPCODE_LINK_CHANGE:
+		netdev_info(netdev, "Notifyq EVENT_OPCODE_LINK_CHANGE eid=%lld\n",
+			    eid);
+		netdev_info(netdev,
+			    "  link_status=%d link_speed=%d\n",
+			    le16_to_cpu(comp->link_change.link_status),
+			    le32_to_cpu(comp->link_change.link_speed));
+		break;
+	case EVENT_OPCODE_RESET:
+		netdev_info(netdev, "Notifyq EVENT_OPCODE_RESET eid=%lld\n",
+			    eid);
+		netdev_info(netdev, "  reset_code=%d state=%d\n",
+			    comp->reset.reset_code,
+			    comp->reset.state);
+		break;
+	case EVENT_OPCODE_HEARTBEAT:
+		netdev_info(netdev, "Notifyq EVENT_OPCODE_HEARTBEAT eid=%lld\n",
+			    eid);
+		break;
+	case EVENT_OPCODE_LOG:
+		netdev_info(netdev, "Notifyq EVENT_OPCODE_LOG eid=%lld\n", eid);
+		print_hex_dump(KERN_INFO, "notifyq ", DUMP_PREFIX_OFFSET, 16, 1,
+			       comp->log.data, sizeof(comp->log.data), true);
+		break;
+	default:
+		netdev_warn(netdev, "Notifyq bad event ecode=%d eid=%lld\n",
+			    comp->event.ecode, eid);
+		break;
+	}
+
+	return true;
+}
+
+static int ionic_notifyq_clean(struct lif *lif, int budget)
+{
+	struct ionic_dev *idev = &lif->ionic->idev;
+	struct cq *cq = &lif->notifyqcq->cq;
+	u32 work_done;
+
+	work_done = ionic_cq_service(cq, budget, ionic_notifyq_service,
+				     NULL, NULL);
+	if (work_done)
+		ionic_intr_credits(idev->intr_ctrl, cq->bound_intr->index,
+				   work_done, IONIC_INTR_CRED_RESET_COALESCE);
+
+	/* If we ran out of budget, there are more events
+	 * to process and napi will reschedule us soon
+	 */
+	if (work_done == budget)
+		goto return_to_napi;
+
+return_to_napi:
+	return work_done;
 }
 
 static irqreturn_t ionic_isr(int irq, void *data)
@@ -62,6 +151,17 @@ static void ionic_intr_free(struct lif *lif, int index)
 		clear_bit(index, lif->ionic->intrs);
 }
 
+static void ionic_link_qcq_interrupts(struct qcq *src_qcq, struct qcq *n_qcq)
+{
+	if (WARN_ON(n_qcq->flags & QCQ_F_INTR)) {
+		ionic_intr_free(n_qcq->cq.lif, n_qcq->intr.index);
+		n_qcq->flags &= ~QCQ_F_INTR;
+	}
+
+	n_qcq->intr.vector = src_qcq->intr.vector;
+	n_qcq->intr.index = src_qcq->intr.index;
+}
+
 static int ionic_qcq_alloc(struct lif *lif, unsigned int type,
 			   unsigned int index,
 			   const char *name, unsigned int flags,
@@ -236,11 +336,36 @@ static int ionic_qcqs_alloc(struct lif *lif)
 	if (err)
 		return err;
 
+	if (lif->ionic->nnqs_per_lif) {
+		flags = QCQ_F_NOTIFYQ;
+		err = ionic_qcq_alloc(lif, IONIC_QTYPE_NOTIFYQ, 0, "notifyq",
+				      flags, IONIC_NOTIFYQ_LENGTH,
+				      sizeof(struct notifyq_cmd),
+				      sizeof(union notifyq_comp),
+				      0, lif->kern_pid, &lif->notifyqcq);
+		if (err)
+			goto err_out_free_adminqcq;
+
+		/* Let the notifyq ride on the adminq interrupt */
+		ionic_link_qcq_interrupts(lif->adminqcq, lif->notifyqcq);
+	}
+
 	return 0;
+
+err_out_free_adminqcq:
+	ionic_qcq_free(lif, lif->adminqcq);
+	lif->adminqcq = NULL;
+
+	return err;
 }
 
 static void ionic_qcqs_free(struct lif *lif)
 {
+	if (lif->notifyqcq) {
+		ionic_qcq_free(lif, lif->notifyqcq);
+		lif->notifyqcq = NULL;
+	}
+
 	if (lif->adminqcq) {
 		ionic_qcq_free(lif, lif->adminqcq);
 		lif->adminqcq = NULL;
@@ -402,6 +527,7 @@ static void ionic_lif_deinit(struct lif *lif)
 	clear_bit(LIF_INITED, lif->state);
 
 	napi_disable(&lif->adminqcq->napi);
+	ionic_lif_qcq_deinit(lif, lif->notifyqcq);
 	ionic_lif_qcq_deinit(lif, lif->adminqcq);
 
 	ionic_lif_reset(lif);
@@ -489,6 +615,57 @@ static int ionic_lif_adminq_init(struct lif *lif)
 	return 0;
 }
 
+static int ionic_lif_notifyq_init(struct lif *lif)
+{
+	struct device *dev = lif->ionic->dev;
+	struct qcq *qcq = lif->notifyqcq;
+	struct queue *q = &qcq->q;
+	int err;
+
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.q_init = {
+			.opcode = CMD_OPCODE_Q_INIT,
+			.lif_index = cpu_to_le16(lif->index),
+			.type = q->type,
+			.index = cpu_to_le32(q->index),
+			.flags = cpu_to_le16(IONIC_QINIT_F_IRQ |
+					     IONIC_QINIT_F_ENA),
+			.intr_index = cpu_to_le16(lif->adminqcq->intr.index),
+			.pid = cpu_to_le16(q->pid),
+			.ring_size = ilog2(q->num_descs),
+			.ring_base = cpu_to_le64(q->base_pa),
+		}
+	};
+
+	dev_dbg(dev, "notifyq_init.pid %d\n", ctx.cmd.q_init.pid);
+	dev_dbg(dev, "notifyq_init.index %d\n", ctx.cmd.q_init.index);
+	dev_dbg(dev, "notifyq_init.ring_base 0x%llx\n", ctx.cmd.q_init.ring_base);
+	dev_dbg(dev, "notifyq_init.ring_size %d\n", ctx.cmd.q_init.ring_size);
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
+	q->hw_type = ctx.comp.q_init.hw_type;
+	q->hw_index = le32_to_cpu(ctx.comp.q_init.hw_index);
+	q->dbval = IONIC_DBELL_QID(q->hw_index);
+
+	dev_dbg(dev, "notifyq->hw_type %d\n", q->hw_type);
+	dev_dbg(dev, "notifyq->hw_index %d\n", q->hw_index);
+
+	/* preset the callback info */
+	q->info[0].cb_arg = lif;
+
+	qcq->flags |= QCQ_F_INITED;
+
+	err = ionic_debugfs_add_qcq(lif, qcq);
+	if (err)
+		dev_warn(dev, "debugfs add for notifyq failed %d\n", err);
+
+	return 0;
+}
+
 static int ionic_lif_init(struct lif *lif)
 {
 	struct ionic_dev *idev = &lif->ionic->idev;
@@ -543,10 +720,18 @@ static int ionic_lif_init(struct lif *lif)
 	if (err)
 		goto err_out_adminq_deinit;
 
+	if (lif->ionic->nnqs_per_lif) {
+		err = ionic_lif_notifyq_init(lif);
+		if (err)
+			goto err_out_notifyq_deinit;
+	}
+
 	set_bit(LIF_INITED, lif->state);
 
 	return 0;
 
+err_out_notifyq_deinit:
+	ionic_lif_qcq_deinit(lif, lif->notifyqcq);
 err_out_adminq_deinit:
 	ionic_lif_qcq_deinit(lif, lif->adminqcq);
 	ionic_lif_reset(lif);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index b9e2dd799a05..0a88a7df6c2b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -7,6 +7,7 @@
 #include <linux/pci.h>
 
 #define IONIC_ADMINQ_LENGTH	16	/* must be a power of two */
+#define IONIC_NOTIFYQ_LENGTH	64	/* must be a power of two */
 
 #define GET_NAPI_CNTR_IDX(work_done)	(work_done)
 #define MAX_NUM_NAPI_CNTR	(NAPI_POLL_WEIGHT + 1)
@@ -26,6 +27,7 @@ struct rx_stats {
 #define QCQ_F_INITED		BIT(0)
 #define QCQ_F_SG		BIT(1)
 #define QCQ_F_INTR		BIT(2)
+#define QCQ_F_NOTIFYQ		BIT(5)
 
 struct napi_stats {
 	u64 poll_count;
@@ -78,6 +80,8 @@ struct lif {
 	u64 __iomem *kern_dbpage;
 	spinlock_t adminq_lock;		/* lock for AdminQ operations */
 	struct qcq *adminqcq;
+	struct qcq *notifyqcq;
+	u64 last_eid;
 	unsigned int neqs;
 	unsigned int nxqs;
 
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 12/18] ionic: Add async link status check and basic stats
From: Shannon Nelson @ 2019-06-20 20:24 UTC (permalink / raw)
  To: snelson, netdev
In-Reply-To: <20190620202424.23215-1-snelson@pensando.io>

Add code to handle the link status event, and wire up the
basic netdev hardware stats.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 117 ++++++++++++++++++
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   1 +
 2 files changed, 118 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index fa48648b68b7..8bf865f50054 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -15,6 +15,7 @@
 static void ionic_lif_rx_mode(struct lif *lif, unsigned int rx_mode);
 static int ionic_lif_addr_add(struct lif *lif, const u8 *addr);
 static int ionic_lif_addr_del(struct lif *lif, const u8 *addr);
+static void ionic_link_status_check(struct lif *lif);
 
 static int ionic_set_nic_features(struct lif *lif, netdev_features_t features);
 static int ionic_notifyq_clean(struct lif *lif, int budget);
@@ -43,6 +44,9 @@ static void ionic_lif_deferred_work(struct work_struct *work)
 		case DW_TYPE_RX_ADDR_DEL:
 			ionic_lif_addr_del(lif, w->addr);
 			break;
+		case DW_TYPE_LINK_STATUS:
+			ionic_link_status_check(lif);
+			break;
 		default:
 			break;
 		};
@@ -68,6 +72,7 @@ int ionic_open(struct net_device *netdev)
 
 	set_bit(LIF_UP, lif->state);
 
+	ionic_link_status_check(lif);
 	if (netif_carrier_ok(netdev))
 		netif_tx_wake_all_queues(netdev);
 
@@ -150,6 +155,40 @@ static int ionic_adminq_napi(struct napi_struct *napi, int budget)
 	return max(n_work, a_work);
 }
 
+static void ionic_link_status_check(struct lif *lif)
+{
+	struct net_device *netdev = lif->netdev;
+	u16 link_status;
+	bool link_up;
+
+	clear_bit(LIF_LINK_CHECK_NEEDED, lif->state);
+
+	link_status = le16_to_cpu(lif->info->status.link_status);
+	link_up = link_status == PORT_OPER_STATUS_UP;
+
+	/* filter out the no-change cases */
+	if ((link_up && netif_carrier_ok(netdev)) ||
+	    (!link_up && !netif_carrier_ok(netdev)))
+		return;
+
+	if (link_up) {
+		netdev_info(netdev, "Link up - %d Gbps\n",
+			    le32_to_cpu(lif->info->status.link_speed) / 1000);
+
+		if (test_bit(LIF_UP, lif->state)) {
+			netif_tx_wake_all_queues(lif->netdev);
+			netif_carrier_on(netdev);
+		}
+	} else {
+		netdev_info(netdev, "Link down\n");
+
+		/* carrier off first to avoid watchdog timeout */
+		netif_carrier_off(netdev);
+		if (test_bit(LIF_UP, lif->state))
+			netif_tx_stop_all_queues(netdev);
+	}
+}
+
 static bool ionic_notifyq_service(struct cq *cq, struct cq_info *cq_info)
 {
 	union notifyq_comp *comp = cq_info->cq_desc;
@@ -181,6 +220,9 @@ static bool ionic_notifyq_service(struct cq *cq, struct cq_info *cq_info)
 			    "  link_status=%d link_speed=%d\n",
 			    le16_to_cpu(comp->link_change.link_status),
 			    le32_to_cpu(comp->link_change.link_speed));
+
+		set_bit(LIF_LINK_CHECK_NEEDED, lif->state);
+
 		break;
 	case EVENT_OPCODE_RESET:
 		netdev_info(netdev, "Notifyq EVENT_OPCODE_RESET eid=%lld\n",
@@ -225,10 +267,81 @@ static int ionic_notifyq_clean(struct lif *lif, int budget)
 	if (work_done == budget)
 		goto return_to_napi;
 
+	/* After outstanding events are processed we can check on
+	 * the link status and any outstanding interrupt credits.
+	 *
+	 * We wait until here to check on the link status in case
+	 * there was a long list of link events from a flap episode.
+	 */
+	if (test_bit(LIF_LINK_CHECK_NEEDED, lif->state)) {
+		struct deferred_work *work;
+
+		work = kzalloc(sizeof(*work), GFP_ATOMIC);
+		if (!work) {
+			netdev_err(lif->netdev, "%s OOM\n", __func__);
+		} else {
+			work->type = DW_TYPE_LINK_STATUS;
+			ionic_lif_deferred_enqueue(&lif->deferred, work);
+		}
+	}
+
 return_to_napi:
 	return work_done;
 }
 
+static void ionic_get_stats64(struct net_device *netdev,
+			      struct rtnl_link_stats64 *ns)
+{
+	struct lif *lif = netdev_priv(netdev);
+	struct lif_stats *ls;
+
+	memset(ns, 0, sizeof(*ns));
+	ls = &lif->info->stats;
+
+	ns->rx_packets = le64_to_cpu(ls->rx_ucast_packets) +
+			 le64_to_cpu(ls->rx_mcast_packets) +
+			 le64_to_cpu(ls->rx_bcast_packets);
+
+	ns->tx_packets = le64_to_cpu(ls->tx_ucast_packets) +
+			 le64_to_cpu(ls->tx_mcast_packets) +
+			 le64_to_cpu(ls->tx_bcast_packets);
+
+	ns->rx_bytes = le64_to_cpu(ls->rx_ucast_bytes) +
+		       le64_to_cpu(ls->rx_mcast_bytes) +
+		       le64_to_cpu(ls->rx_bcast_bytes);
+
+	ns->tx_bytes = le64_to_cpu(ls->tx_ucast_bytes) +
+		       le64_to_cpu(ls->tx_mcast_bytes) +
+		       le64_to_cpu(ls->tx_bcast_bytes);
+
+	ns->rx_dropped = le64_to_cpu(ls->rx_ucast_drop_packets) +
+			 le64_to_cpu(ls->rx_mcast_drop_packets) +
+			 le64_to_cpu(ls->rx_bcast_drop_packets);
+
+	ns->tx_dropped = le64_to_cpu(ls->tx_ucast_drop_packets) +
+			 le64_to_cpu(ls->tx_mcast_drop_packets) +
+			 le64_to_cpu(ls->tx_bcast_drop_packets);
+
+	ns->multicast = le64_to_cpu(ls->rx_mcast_packets);
+
+	ns->rx_over_errors = le64_to_cpu(ls->rx_queue_empty);
+
+	ns->rx_missed_errors = le64_to_cpu(ls->rx_dma_error) +
+			       le64_to_cpu(ls->rx_queue_disabled) +
+			       le64_to_cpu(ls->rx_desc_fetch_error) +
+			       le64_to_cpu(ls->rx_desc_data_error);
+
+	ns->tx_aborted_errors = le64_to_cpu(ls->tx_dma_error) +
+				le64_to_cpu(ls->tx_queue_disabled) +
+				le64_to_cpu(ls->tx_desc_fetch_error) +
+				le64_to_cpu(ls->tx_desc_data_error);
+
+	ns->rx_errors = ns->rx_over_errors +
+			ns->rx_missed_errors;
+
+	ns->tx_errors = ns->tx_aborted_errors;
+}
+
 static int ionic_lif_addr_add(struct lif *lif, const u8 *addr)
 {
 	struct ionic_admin_ctx ctx = {
@@ -594,6 +707,7 @@ static int ionic_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto,
 static const struct net_device_ops ionic_netdev_ops = {
 	.ndo_open               = ionic_open,
 	.ndo_stop               = ionic_stop,
+	.ndo_get_stats64	= ionic_get_stats64,
 	.ndo_set_rx_mode	= ionic_set_rx_mode,
 	.ndo_set_features	= ionic_set_features,
 	.ndo_set_mac_address	= ionic_set_mac_address,
@@ -1442,6 +1556,8 @@ static int ionic_lif_init(struct lif *lif)
 
 	set_bit(LIF_INITED, lif->state);
 
+	ionic_link_status_check(lif);
+
 	return 0;
 
 err_out_notifyq_deinit:
@@ -1485,6 +1601,7 @@ int ionic_lifs_register(struct ionic *ionic)
 		return err;
 	}
 
+	ionic_link_status_check(ionic->master_lif);
 	ionic->master_lif->registered = true;
 
 	return 0;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index c3ecf1df9c2c..efe680da9a5c 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -86,6 +86,7 @@ struct deferred {
 enum lif_state_flags {
 	LIF_INITED,
 	LIF_UP,
+	LIF_LINK_CHECK_NEEDED,
 	LIF_QUEUE_RESET,
 
 	/* leave this as last */
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next 11/18] ionic: Add Rx filter and rx_mode nod support
From: Shannon Nelson @ 2019-06-20 20:24 UTC (permalink / raw)
  To: snelson, netdev
In-Reply-To: <20190620202424.23215-1-snelson@pensando.io>

Add the Rx filtering and rx_mode NDO callbacks.  Also add
the deferred work thread handling needed to manage the filter
requests otuside of the netif_addr_lock spinlock.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 391 +++++++++++++++++-
 .../net/ethernet/pensando/ionic/ionic_lif.h   |  29 ++
 2 files changed, 415 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index ac4a8dbd33fb..fa48648b68b7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -12,9 +12,54 @@
 #include "ionic_lif.h"
 #include "ionic_debugfs.h"
 
+static void ionic_lif_rx_mode(struct lif *lif, unsigned int rx_mode);
+static int ionic_lif_addr_add(struct lif *lif, const u8 *addr);
+static int ionic_lif_addr_del(struct lif *lif, const u8 *addr);
+
 static int ionic_set_nic_features(struct lif *lif, netdev_features_t features);
 static int ionic_notifyq_clean(struct lif *lif, int budget);
 
+static void ionic_lif_deferred_work(struct work_struct *work)
+{
+	struct lif *lif = container_of(work, struct lif, deferred.work);
+	struct deferred *def = &lif->deferred;
+	struct deferred_work *w = NULL;
+
+	spin_lock_bh(&def->lock);
+	if (!list_empty(&def->list)) {
+		w = list_first_entry(&def->list, struct deferred_work, list);
+		list_del(&w->list);
+	}
+	spin_unlock_bh(&def->lock);
+
+	if (w) {
+		switch (w->type) {
+		case DW_TYPE_RX_MODE:
+			ionic_lif_rx_mode(lif, w->rx_mode);
+			break;
+		case DW_TYPE_RX_ADDR_ADD:
+			ionic_lif_addr_add(lif, w->addr);
+			break;
+		case DW_TYPE_RX_ADDR_DEL:
+			ionic_lif_addr_del(lif, w->addr);
+			break;
+		default:
+			break;
+		};
+		kfree(w);
+		schedule_work(&def->work);
+	}
+}
+
+static void ionic_lif_deferred_enqueue(struct deferred *def,
+				       struct deferred_work *work)
+{
+	spin_lock_bh(&def->lock);
+	list_add_tail(&work->list, &def->list);
+	spin_unlock_bh(&def->lock);
+	schedule_work(&def->work);
+}
+
 int ionic_open(struct net_device *netdev)
 {
 	struct lif *lif = netdev_priv(netdev);
@@ -184,6 +229,235 @@ static int ionic_notifyq_clean(struct lif *lif, int budget)
 	return work_done;
 }
 
+static int ionic_lif_addr_add(struct lif *lif, const u8 *addr)
+{
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.rx_filter_add = {
+			.opcode = CMD_OPCODE_RX_FILTER_ADD,
+			.lif_index = cpu_to_le16(lif->index),
+			.match = cpu_to_le16(RX_FILTER_MATCH_MAC),
+		},
+	};
+	struct rx_filter *f;
+	int err;
+
+	/* don't bother if we already have it */
+	spin_lock_bh(&lif->rx_filters.lock);
+	f = ionic_rx_filter_by_addr(lif, addr);
+	spin_unlock_bh(&lif->rx_filters.lock);
+	if (f)
+		return 0;
+
+	netdev_dbg(lif->netdev, "rx_filter add ADDR %pM (id %d)\n", addr,
+		   ctx.comp.rx_filter_add.filter_id);
+
+	memcpy(ctx.cmd.rx_filter_add.mac.addr, addr, ETH_ALEN);
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
+	return ionic_rx_filter_save(lif, 0, RXQ_INDEX_ANY, 0, &ctx);
+}
+
+static int ionic_lif_addr_del(struct lif *lif, const u8 *addr)
+{
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.rx_filter_del = {
+			.opcode = CMD_OPCODE_RX_FILTER_DEL,
+			.lif_index = cpu_to_le16(lif->index),
+		},
+	};
+	struct rx_filter *f;
+	int err;
+
+	spin_lock_bh(&lif->rx_filters.lock);
+	f = ionic_rx_filter_by_addr(lif, addr);
+	if (!f) {
+		spin_unlock_bh(&lif->rx_filters.lock);
+		return -ENOENT;
+	}
+
+	ctx.cmd.rx_filter_del.filter_id = cpu_to_le32(f->filter_id);
+	ionic_rx_filter_free(lif, f);
+	spin_unlock_bh(&lif->rx_filters.lock);
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
+	netdev_dbg(lif->netdev, "rx_filter del ADDR %pM (id %d)\n", addr,
+		   ctx.cmd.rx_filter_del.filter_id);
+
+	return 0;
+}
+
+static int ionic_lif_addr(struct lif *lif, const u8 *addr, bool add)
+{
+	struct ionic *ionic = lif->ionic;
+	struct deferred_work *work;
+	unsigned int nmfilters;
+	unsigned int nufilters;
+
+	if (add) {
+		/* Do we have space for this filter?  We test the counters
+		 * here before checking the need for deferral so that we
+		 * can return an overflow error to the stack.
+		 */
+		nmfilters = le32_to_cpu(ionic->ident.lif.eth.max_mcast_filters);
+		nufilters = le32_to_cpu(ionic->ident.lif.eth.max_ucast_filters);
+
+		if ((is_multicast_ether_addr(addr) && lif->nmcast < nmfilters))
+			lif->nmcast++;
+		else if (!is_multicast_ether_addr(addr) &&
+			 lif->nucast < nufilters)
+			lif->nucast++;
+		else
+			return -ENOSPC;
+	} else {
+		if (is_multicast_ether_addr(addr) && lif->nmcast)
+			lif->nmcast--;
+		else if (!is_multicast_ether_addr(addr) && lif->nucast)
+			lif->nucast--;
+	}
+
+	if (in_interrupt()) {
+		work = kzalloc(sizeof(*work), GFP_ATOMIC);
+		if (!work) {
+			netdev_err(lif->netdev, "%s OOM\n", __func__);
+			return -ENOMEM;
+		}
+		work->type = add ? DW_TYPE_RX_ADDR_ADD : DW_TYPE_RX_ADDR_DEL;
+		memcpy(work->addr, addr, ETH_ALEN);
+		netdev_dbg(lif->netdev, "deferred: rx_filter %s %pM\n",
+			   add ? "add" : "del", addr);
+		ionic_lif_deferred_enqueue(&lif->deferred, work);
+	} else {
+		netdev_dbg(lif->netdev, "rx_filter %s %pM\n",
+			   add ? "add" : "del", addr);
+		if (add)
+			return ionic_lif_addr_add(lif, addr);
+		else
+			return ionic_lif_addr_del(lif, addr);
+	}
+
+	return 0;
+}
+
+static int ionic_addr_add(struct net_device *netdev, const u8 *addr)
+{
+	return ionic_lif_addr(netdev_priv(netdev), addr, true);
+}
+
+static int ionic_addr_del(struct net_device *netdev, const u8 *addr)
+{
+	return ionic_lif_addr(netdev_priv(netdev), addr, false);
+}
+
+static void ionic_lif_rx_mode(struct lif *lif, unsigned int rx_mode)
+{
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.rx_mode_set = {
+			.opcode = CMD_OPCODE_RX_MODE_SET,
+			.lif_index = cpu_to_le16(lif->index),
+			.rx_mode = cpu_to_le16(rx_mode),
+		},
+	};
+	char buf[128];
+	int err;
+	int i;
+#define REMAIN(__x) (sizeof(buf) - (__x))
+
+	i = snprintf(buf, sizeof(buf), "rx_mode 0x%04x -> 0x%04x:",
+		     lif->rx_mode, rx_mode);
+	if (rx_mode & RX_MODE_F_UNICAST)
+		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_UNICAST");
+	if (rx_mode & RX_MODE_F_MULTICAST)
+		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_MULTICAST");
+	if (rx_mode & RX_MODE_F_BROADCAST)
+		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_BROADCAST");
+	if (rx_mode & RX_MODE_F_PROMISC)
+		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_PROMISC");
+	if (rx_mode & RX_MODE_F_ALLMULTI)
+		i += snprintf(&buf[i], REMAIN(i), " RX_MODE_F_ALLMULTI");
+	netdev_dbg(lif->netdev, "lif%d %s\n", lif->index, buf);
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		netdev_warn(lif->netdev, "set rx_mode 0x%04x failed: %d\n",
+			    rx_mode, err);
+	else
+		lif->rx_mode = rx_mode;
+}
+
+static void _ionic_lif_rx_mode(struct lif *lif, unsigned int rx_mode)
+{
+	struct deferred_work *work;
+
+	if (in_interrupt()) {
+		work = kzalloc(sizeof(*work), GFP_ATOMIC);
+		if (!work) {
+			netdev_err(lif->netdev, "%s OOM\n", __func__);
+			return;
+		}
+		work->type = DW_TYPE_RX_MODE;
+		work->rx_mode = rx_mode;
+		netdev_dbg(lif->netdev, "deferred: rx_mode\n");
+		ionic_lif_deferred_enqueue(&lif->deferred, work);
+	} else {
+		ionic_lif_rx_mode(lif, rx_mode);
+	}
+}
+
+static void ionic_set_rx_mode(struct net_device *netdev)
+{
+	struct lif *lif = netdev_priv(netdev);
+	struct identity *ident = &lif->ionic->ident;
+	unsigned int nfilters;
+	unsigned int rx_mode;
+
+	rx_mode = RX_MODE_F_UNICAST;
+	rx_mode |= (netdev->flags & IFF_MULTICAST) ? RX_MODE_F_MULTICAST : 0;
+	rx_mode |= (netdev->flags & IFF_BROADCAST) ? RX_MODE_F_BROADCAST : 0;
+	rx_mode |= (netdev->flags & IFF_PROMISC) ? RX_MODE_F_PROMISC : 0;
+	rx_mode |= (netdev->flags & IFF_ALLMULTI) ? RX_MODE_F_ALLMULTI : 0;
+
+	/* sync unicast addresses
+	 * next check to see if we're in an overflow state
+	 *    if so, we track that we overflowed and enable NIC PROMISC
+	 *    else if the overflow is set and not needed
+	 *       we remove our overflow flag and check the netdev flags
+	 *       to see if we can disable NIC PROMISC
+	 */
+	__dev_uc_sync(netdev, ionic_addr_add, ionic_addr_del);
+	nfilters = le32_to_cpu(ident->lif.eth.max_ucast_filters);
+	if (netdev_uc_count(netdev) + 1 > nfilters) {
+		rx_mode |= RX_MODE_F_PROMISC;
+		lif->uc_overflow = true;
+	} else if (lif->uc_overflow) {
+		lif->uc_overflow = false;
+		if (!(netdev->flags & IFF_PROMISC))
+			rx_mode &= ~RX_MODE_F_PROMISC;
+	}
+
+	/* same for multicast */
+	__dev_mc_sync(netdev, ionic_addr_add, ionic_addr_del);
+	nfilters = le32_to_cpu(ident->lif.eth.max_mcast_filters);
+	if (netdev_mc_count(netdev) > nfilters) {
+		rx_mode |= RX_MODE_F_ALLMULTI;
+		lif->mc_overflow = true;
+	} else if (lif->mc_overflow) {
+		lif->mc_overflow = false;
+		if (!(netdev->flags & IFF_ALLMULTI))
+			rx_mode &= ~RX_MODE_F_ALLMULTI;
+	}
+
+	if (lif->rx_mode != rx_mode)
+		_ionic_lif_rx_mode(lif, rx_mode);
+}
+
 static int ionic_set_features(struct net_device *netdev,
 			      netdev_features_t features)
 {
@@ -200,8 +474,26 @@ static int ionic_set_features(struct net_device *netdev,
 
 static int ionic_set_mac_address(struct net_device *netdev, void *sa)
 {
-	netdev_info(netdev, "%s: stubbed\n", __func__);
-	return 0;
+	struct sockaddr *addr = sa;
+	u8 *mac;
+
+	mac = (u8 *)addr->sa_data;
+	if (ether_addr_equal(netdev->dev_addr, mac))
+		return 0;
+
+	if (!is_valid_ether_addr(mac))
+		return -EADDRNOTAVAIL;
+
+	if (!is_zero_ether_addr(netdev->dev_addr)) {
+		netdev_info(netdev, "deleting mac addr %pM\n",
+			    netdev->dev_addr);
+		ionic_addr_del(netdev, netdev->dev_addr);
+	}
+
+	memcpy(netdev->dev_addr, mac, netdev->addr_len);
+	netdev_info(netdev, "updating mac addr %pM\n", mac);
+
+	return ionic_addr_add(netdev, mac);
 }
 
 static int ionic_change_mtu(struct net_device *netdev, int new_mtu)
@@ -241,20 +533,68 @@ static void ionic_tx_timeout(struct net_device *netdev)
 static int ionic_vlan_rx_add_vid(struct net_device *netdev, __be16 proto,
 				 u16 vid)
 {
-	netdev_info(netdev, "%s: stubbed\n", __func__);
-	return 0;
+	struct lif *lif = netdev_priv(netdev);
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.rx_filter_add = {
+			.opcode = CMD_OPCODE_RX_FILTER_ADD,
+			.lif_index = cpu_to_le16(lif->index),
+			.match = cpu_to_le16(RX_FILTER_MATCH_VLAN),
+			.vlan.vlan = cpu_to_le16(vid),
+		},
+	};
+	int err;
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
+	netdev_dbg(netdev, "rx_filter add VLAN %d (id %d)\n", vid,
+		   ctx.comp.rx_filter_add.filter_id);
+
+	return ionic_rx_filter_save(lif, 0, RXQ_INDEX_ANY, 0, &ctx);
 }
 
 static int ionic_vlan_rx_kill_vid(struct net_device *netdev, __be16 proto,
 				  u16 vid)
 {
-	netdev_info(netdev, "%s: stubbed\n", __func__);
+	struct lif *lif = netdev_priv(netdev);
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.rx_filter_del = {
+			.opcode = CMD_OPCODE_RX_FILTER_DEL,
+			.lif_index = cpu_to_le16(lif->index),
+		},
+	};
+	struct rx_filter *f;
+	int err;
+
+	spin_lock_bh(&lif->rx_filters.lock);
+
+	f = ionic_rx_filter_by_vlan(lif, vid);
+	if (!f) {
+		spin_unlock_bh(&lif->rx_filters.lock);
+		return -ENOENT;
+	}
+
+	netdev_dbg(netdev, "rx_filter del VLAN %d (id %d)\n", vid,
+		   le32_to_cpu(ctx.cmd.rx_filter_del.filter_id));
+
+	ctx.cmd.rx_filter_del.filter_id = cpu_to_le32(f->filter_id);
+	ionic_rx_filter_free(lif, f);
+	spin_unlock_bh(&lif->rx_filters.lock);
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
 	return 0;
 }
 
 static const struct net_device_ops ionic_netdev_ops = {
 	.ndo_open               = ionic_open,
 	.ndo_stop               = ionic_stop,
+	.ndo_set_rx_mode	= ionic_set_rx_mode,
 	.ndo_set_features	= ionic_set_features,
 	.ndo_set_mac_address	= ionic_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
@@ -555,6 +895,10 @@ static struct lif *ionic_lif_alloc(struct ionic *ionic, unsigned int index)
 
 	spin_lock_init(&lif->adminq_lock);
 
+	spin_lock_init(&lif->deferred.lock);
+	INIT_LIST_HEAD(&lif->deferred.list);
+	INIT_WORK(&lif->deferred.work, ionic_lif_deferred_work);
+
 	/* allocate lif info */
 	lif->info_sz = ALIGN(sizeof(*lif->info), PAGE_SIZE);
 	lif->info = dma_alloc_coherent(dev, lif->info_sz,
@@ -616,6 +960,8 @@ static void ionic_lif_free(struct lif *lif)
 	ionic_qcqs_free(lif);
 	ionic_lif_reset(lif);
 
+	cancel_work_sync(&lif->deferred.work);
+
 	/* free lif info */
 	if (lif->info) {
 		dma_free_coherent(dev, lif->info_sz, lif->info, lif->info_pa);
@@ -991,6 +1337,37 @@ static int ionic_init_nic_features(struct lif *lif)
 	return 0;
 }
 
+static int ionic_station_set(struct lif *lif)
+{
+	struct net_device *netdev = lif->netdev;
+	struct ionic_admin_ctx ctx = {
+		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
+		.cmd.lif_getattr = {
+			.opcode = CMD_OPCODE_LIF_GETATTR,
+			.index = cpu_to_le16(lif->index),
+			.attr = IONIC_LIF_ATTR_MAC,
+		},
+	};
+	int err;
+
+	err = ionic_adminq_post_wait(lif, &ctx);
+	if (err)
+		return err;
+
+	if (!is_zero_ether_addr(netdev->dev_addr)) {
+		netdev_dbg(lif->netdev, "deleting station MAC addr %pM\n",
+			   netdev->dev_addr);
+		ionic_lif_addr(lif, netdev->dev_addr, false);
+	}
+	memcpy(netdev->dev_addr, ctx.comp.lif_getattr.mac,
+	       netdev->addr_len);
+	netdev_dbg(lif->netdev, "adding station MAC addr %pM\n",
+		   netdev->dev_addr);
+	ionic_lif_addr(lif, netdev->dev_addr, true);
+
+	return 0;
+}
+
 static int ionic_lif_init(struct lif *lif)
 {
 	struct ionic_dev *idev = &lif->ionic->idev;
@@ -1059,6 +1436,10 @@ static int ionic_lif_init(struct lif *lif)
 	if (err)
 		goto err_out_notifyq_deinit;
 
+	err = ionic_station_set(lif);
+	if (err)
+		goto err_out_notifyq_deinit;
+
 	set_bit(LIF_INITED, lif->state);
 
 	return 0;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index 8129fa20695a..c3ecf1df9c2c 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -60,6 +60,29 @@ struct qcq {
 #define napi_to_qcq(napi)	container_of(napi, struct qcq, napi)
 #define napi_to_cq(napi)	(&napi_to_qcq(napi)->cq)
 
+enum deferred_work_type {
+	DW_TYPE_RX_MODE,
+	DW_TYPE_RX_ADDR_ADD,
+	DW_TYPE_RX_ADDR_DEL,
+	DW_TYPE_LINK_STATUS,
+	DW_TYPE_LIF_RESET,
+};
+
+struct deferred_work {
+	struct list_head list;
+	enum deferred_work_type type;
+	union {
+		unsigned int rx_mode;
+		u8 addr[ETH_ALEN];
+	};
+};
+
+struct deferred {
+	spinlock_t lock;		/* lock for deferred work list */
+	struct list_head list;
+	struct work_struct work;
+};
+
 enum lif_state_flags {
 	LIF_INITED,
 	LIF_UP,
@@ -87,13 +110,19 @@ struct lif {
 	u64 last_eid;
 	unsigned int neqs;
 	unsigned int nxqs;
+	unsigned int rx_mode;
 	u64 hw_features;
+	bool mc_overflow;
+	unsigned int nmcast;
+	bool uc_overflow;
+	unsigned int nucast;
 
 	struct lif_info *info;
 	dma_addr_t info_pa;
 	u32 info_sz;
 
 	struct rx_filters rx_filters;
+	struct deferred deferred;
 	unsigned long *dbid_inuse;
 	unsigned int dbid_count;
 	struct dentry *dentry;
-- 
2.17.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox