Netdev List
 help / color / mirror / Atom feed
* [PATCH v2 net-next 5/8] samples/bpf: add multi-prog cgroup test case
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team
In-Reply-To: <20171003055028.1294791-1-ast@fb.com>

create 5 cgroups, attach 6 progs and check that progs are executed as:
cgrp1 (MULTI progs A, B) ->
   cgrp2 (OVERRIDE prog C) ->
     cgrp3 (MULTI prog D) ->
       cgrp4 (OVERRIDE prog E) ->
         cgrp5 (NONE prog F)
the event in cgrp5 triggers execution of F,D,A,B in that order.
if prog F is detached, the execution is E,D,A,B
if prog F and D are detached, the execution is E,A,B
if prog F, E and D are detached, the execution is C,A,B

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 samples/bpf/cgroup_helpers.c     |   4 +-
 samples/bpf/test_cgrp2_attach2.c | 188 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 185 insertions(+), 7 deletions(-)

diff --git a/samples/bpf/cgroup_helpers.c b/samples/bpf/cgroup_helpers.c
index 9d1be9426401..88bdcf4b1670 100644
--- a/samples/bpf/cgroup_helpers.c
+++ b/samples/bpf/cgroup_helpers.c
@@ -56,7 +56,7 @@ int setup_cgroup_environment(void)
 		return 1;
 	}
 
-	if (mount("none", CGROUP_MOUNT_PATH, "cgroup2", 0, NULL)) {
+	if (mount("none", CGROUP_MOUNT_PATH, "cgroup2", 0, NULL) && errno != EBUSY) {
 		log_err("mount cgroup2");
 		return 1;
 	}
@@ -163,7 +163,7 @@ int create_and_get_cgroup(char *path)
 
 	format_cgroup_path(cgroup_path, path);
 	if (mkdir(cgroup_path, 0777) && errno != EEXIST) {
-		log_err("mkdiring cgroup");
+		log_err("mkdiring cgroup %s .. %s", path, cgroup_path);
 		return 0;
 	}
 
diff --git a/samples/bpf/test_cgrp2_attach2.c b/samples/bpf/test_cgrp2_attach2.c
index 3049b1f26267..9a9f6836e5e9 100644
--- a/samples/bpf/test_cgrp2_attach2.c
+++ b/samples/bpf/test_cgrp2_attach2.c
@@ -30,7 +30,7 @@
 
 #define FOO		"/foo"
 #define BAR		"/foo/bar/"
-#define PING_CMD	"ping -c1 -w1 127.0.0.1"
+#define PING_CMD	"ping -c1 -w1 127.0.0.1 > /dev/null"
 
 char bpf_log_buf[BPF_LOG_BUF_SIZE];
 
@@ -55,8 +55,7 @@ static int prog_load(int verdict)
 	return ret;
 }
 
-
-int main(int argc, char **argv)
+static int test_foo_bar(void)
 {
 	int drop_prog, allow_prog, foo = 0, bar = 0, rc = 0;
 
@@ -189,8 +188,187 @@ int main(int argc, char **argv)
 	close(bar);
 	cleanup_cgroup_environment();
 	if (!rc)
-		printf("PASS\n");
+		printf("### override:PASS\n");
+	else
+		printf("### override:FAIL\n");
+	return rc;
+}
+
+static int map_fd = -1;
+
+static int prog_load_cnt(int verdict, int val)
+{
+	if (map_fd < 0)
+		map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 8, 1, 0);
+	if (map_fd < 0) {
+		printf("failed to create map '%s'\n", strerror(errno));
+		return -1;
+	}
+
+	struct bpf_insn prog[] = {
+		BPF_MOV32_IMM(BPF_REG_0, 0),
+		BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
+		BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+		BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
+		BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+		BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+		BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+		BPF_MOV64_IMM(BPF_REG_1, val), /* r1 = 1 */
+		BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+		BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
+		BPF_EXIT_INSN(),
+	};
+	size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
+	int ret;
+
+	ret = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
+			       prog, insns_cnt, "GPL", 0,
+			       bpf_log_buf, BPF_LOG_BUF_SIZE);
+
+	if (ret < 0) {
+		log_err("Loading program");
+		printf("Output from verifier:\n%s\n-------\n", bpf_log_buf);
+		return 0;
+	}
+	return ret;
+}
+
+
+static int test_multiprog(void)
+{
+	int cg1 = 0, cg2 = 0, cg3 = 0, cg4 = 0, cg5 = 0, key = 0;
+	int drop_prog, allow_prog[6] = {}, rc = 0;
+	unsigned long long value;
+	int i = 0;
+
+	for (i = 0; i < 6; i++) {
+		allow_prog[i] = prog_load_cnt(1, 1 << i);
+		if (!allow_prog[i])
+			goto err;
+	}
+	drop_prog = prog_load_cnt(0, 1);
+	if (!drop_prog)
+		goto err;
+
+	if (setup_cgroup_environment())
+		goto err;
+
+	cg1 = create_and_get_cgroup("/cg1");
+	if (!cg1)
+		goto err;
+	cg2 = create_and_get_cgroup("/cg1/cg2");
+	if (!cg2)
+		goto err;
+	cg3 = create_and_get_cgroup("/cg1/cg2/cg3");
+	if (!cg3)
+		goto err;
+	cg4 = create_and_get_cgroup("/cg1/cg2/cg3/cg4");
+	if (!cg4)
+		goto err;
+	cg5 = create_and_get_cgroup("/cg1/cg2/cg3/cg4/cg5");
+	if (!cg5)
+		goto err;
+
+	if (join_cgroup("/cg1/cg2/cg3/cg4/cg5"))
+		goto err;
+
+	if (bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS, 2)) {
+		log_err("Attaching prog to cg1");
+		goto err;
+	}
+	if (!bpf_prog_attach(allow_prog[0], cg1, BPF_CGROUP_INET_EGRESS, 2)) {
+		log_err("Unexpected success attaching the same prog to cg1");
+		goto err;
+	}
+	if (bpf_prog_attach(allow_prog[1], cg1, BPF_CGROUP_INET_EGRESS, 2)) {
+		log_err("Attaching prog2 to cg1");
+		goto err;
+	}
+	if (bpf_prog_attach(allow_prog[2], cg2, BPF_CGROUP_INET_EGRESS, 1)) {
+		log_err("Attaching prog to cg2");
+		goto err;
+	}
+	if (bpf_prog_attach(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS, 2)) {
+		log_err("Attaching prog to cg3");
+		goto err;
+	}
+	if (bpf_prog_attach(allow_prog[4], cg4, BPF_CGROUP_INET_EGRESS, 1)) {
+		log_err("Attaching prog to cg4");
+		goto err;
+	}
+	if (bpf_prog_attach(allow_prog[5], cg5, BPF_CGROUP_INET_EGRESS, 0)) {
+		log_err("Attaching prog to cg5");
+		goto err;
+	}
+	assert(system(PING_CMD) == 0);
+	assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
+	assert(value == 1 + 2 + 8 + 32);
+
+	/* detach bottom program and ping again */
+	if (bpf_prog_detach2(-1, cg5, BPF_CGROUP_INET_EGRESS)) {
+		log_err("Detaching prog from cg5");
+		goto err;
+	}
+	value = 0;
+	assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
+	assert(system(PING_CMD) == 0);
+	assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
+	assert(value == 1 + 2 + 8 + 16);
+
+	/* detach 3rd from bottom program and ping again */
+	errno = 0;
+	if (!bpf_prog_detach2(0, cg3, BPF_CGROUP_INET_EGRESS)) {
+		log_err("Unexpected success on detach from cg3");
+		goto err;
+	}
+	if (bpf_prog_detach2(allow_prog[3], cg3, BPF_CGROUP_INET_EGRESS)) {
+		log_err("Detaching from cg3");
+		goto err;
+	}
+	value = 0;
+	assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
+	assert(system(PING_CMD) == 0);
+	assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
+	assert(value == 1 + 2 + 16);
+
+	/* detach 2nd from bottom program and ping again */
+	if (bpf_prog_detach2(-1, cg4, BPF_CGROUP_INET_EGRESS)) {
+		log_err("Detaching prog from cg4");
+		goto err;
+	}
+	value = 0;
+	assert(bpf_map_update_elem(map_fd, &key, &value, 0) == 0);
+	assert(system(PING_CMD) == 0);
+	assert(bpf_map_lookup_elem(map_fd, &key, &value) == 0);
+	assert(value == 1 + 2 + 4);
+	goto out;
+err:
+	rc = 1;
+
+out:
+	for (i = 0; i < 6; i++)
+		if (allow_prog[i] > 0)
+			close(allow_prog[i]);
+	close(cg1);
+	close(cg2);
+	close(cg3);
+	close(cg4);
+	close(cg5);
+	cleanup_cgroup_environment();
+	if (!rc)
+		printf("### multi:PASS\n");
 	else
-		printf("FAIL\n");
+		printf("### multi:FAIL\n");
 	return rc;
 }
+
+int main(int argc, char **argv)
+{
+	int rc = 0;
+
+	rc = test_foo_bar();
+	if (rc)
+		return rc;
+
+	return test_multiprog();
+}
-- 
2.9.5

^ permalink raw reply related

* [PATCH v2 net-next 4/8] libbpf: introduce bpf_prog_detach2()
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team
In-Reply-To: <20171003055028.1294791-1-ast@fb.com>

introduce bpf_prog_detach2() that takes one more argument prog_fd
vs bpf_prog_detach() that takes only attach_fd and type.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/lib/bpf/bpf.c | 12 ++++++++++++
 tools/lib/bpf/bpf.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index daf624e4c720..d4b6ba8292ee 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -291,6 +291,18 @@ int bpf_prog_detach(int target_fd, enum bpf_attach_type type)
 	return sys_bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
 }
 
+int bpf_prog_detach2(int prog_fd, int target_fd, enum bpf_attach_type type)
+{
+	union bpf_attr attr;
+
+	bzero(&attr, sizeof(attr));
+	attr.target_fd	 = target_fd;
+	attr.attach_bpf_fd = prog_fd;
+	attr.attach_type = type;
+
+	return sys_bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
+}
+
 int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
 		      void *data_out, __u32 *size_out, __u32 *retval,
 		      __u32 *duration)
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 118d00535a0d..afd64727c9cf 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -66,6 +66,7 @@ int bpf_obj_get(const char *pathname);
 int bpf_prog_attach(int prog_fd, int attachable_fd, enum bpf_attach_type type,
 		    unsigned int flags);
 int bpf_prog_detach(int attachable_fd, enum bpf_attach_type type);
+int bpf_prog_detach2(int prog_fd, int attachable_fd, enum bpf_attach_type type);
 int bpf_prog_test_run(int prog_fd, int repeat, void *data, __u32 size,
 		      void *data_out, __u32 *size_out, __u32 *retval,
 		      __u32 *duration);
-- 
2.9.5

^ permalink raw reply related

* [PATCH v2 net-next 3/8] bpf: enforce return code for cgroup-bpf programs
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team
In-Reply-To: <20171003055028.1294791-1-ast@fb.com>

with addition of tnum logic the verifier got smart enough and
we can enforce return codes at program load time.
For now do so for cgroup-bpf program types.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/verifier.c                       | 40 ++++++++++++++++
 tools/testing/selftests/bpf/test_verifier.c | 72 +++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4cf9b72c59a0..52b022310f6a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3073,6 +3073,43 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	return 0;
 }
 
+static int check_return_code(struct bpf_verifier_env *env)
+{
+	struct bpf_reg_state *reg;
+	struct tnum range = tnum_range(0, 1);
+
+	switch (env->prog->type) {
+	case BPF_PROG_TYPE_CGROUP_SKB:
+	case BPF_PROG_TYPE_CGROUP_SOCK:
+	case BPF_PROG_TYPE_SOCK_OPS:
+		break;
+	default:
+		return 0;
+	}
+
+	reg = &env->cur_state.regs[BPF_REG_0];
+	if (reg->type != SCALAR_VALUE) {
+		verbose("At program exit the register R0 is not a known value (%s)\n",
+			reg_type_str[reg->type]);
+		return -EINVAL;
+	}
+
+	if (!tnum_in(range, reg->var_off)) {
+		verbose("At program exit the register R0 ");
+		if (!tnum_is_unknown(reg->var_off)) {
+			char tn_buf[48];
+
+			tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
+			verbose("has value %s", tn_buf);
+		} else {
+			verbose("has unknown scalar value");
+		}
+		verbose(" should have been 0 or 1\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /* non-recursive DFS pseudo code
  * 1  procedure DFS-iterative(G,v):
  * 2      label v as discovered
@@ -3863,6 +3900,9 @@ static int do_check(struct bpf_verifier_env *env)
 					return -EACCES;
 				}
 
+				err = check_return_code(env);
+				if (err)
+					return err;
 process_bpf_exit:
 				insn_idx = pop_stack(env, &prev_insn_idx);
 				if (insn_idx < 0) {
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 290d5056c165..cc91d0159f43 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -6892,6 +6892,78 @@ static struct bpf_test tests[] = {
 		.result = ACCEPT,
 		.prog_type = BPF_PROG_TYPE_XDP,
 	},
+	{
+		"bpf_exit with invalid return code. test1",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "R0 has value (0x0; 0xffffffff)",
+		.result = REJECT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test2",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
+			BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 1),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test3",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
+			BPF_ALU64_IMM(BPF_AND, BPF_REG_0, 3),
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "R0 has value (0x0; 0x3)",
+		.result = REJECT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test4",
+		.insns = {
+			BPF_MOV64_IMM(BPF_REG_0, 1),
+			BPF_EXIT_INSN(),
+		},
+		.result = ACCEPT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test5",
+		.insns = {
+			BPF_MOV64_IMM(BPF_REG_0, 2),
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "R0 has value (0x2; 0x0)",
+		.result = REJECT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test6",
+		.insns = {
+			BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "R0 is not a known value (ctx)",
+		.result = REJECT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
+	{
+		"bpf_exit with invalid return code. test7",
+		.insns = {
+			BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1, 0),
+			BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 4),
+			BPF_ALU64_REG(BPF_MUL, BPF_REG_0, BPF_REG_2),
+			BPF_EXIT_INSN(),
+		},
+		.errstr = "R0 has unknown scalar value",
+		.result = REJECT,
+		.prog_type = BPF_PROG_TYPE_CGROUP_SOCK,
+	},
 };
 
 static int probe_filter_length(const struct bpf_insn *fp)
-- 
2.9.5

^ permalink raw reply related

* [PATCH v2 net-next 6/8] libbpf: sync bpf.h
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team
In-Reply-To: <20171003055028.1294791-1-ast@fb.com>

tools/include/uapi/linux/bpf.h got out of sync with actual kernel header.
Update it.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
---
 tools/include/uapi/linux/bpf.h | 55 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 52 insertions(+), 3 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 6d2137b4cf38..cb2b9f95160a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -92,6 +92,7 @@ enum bpf_cmd {
 	BPF_PROG_GET_FD_BY_ID,
 	BPF_MAP_GET_FD_BY_ID,
 	BPF_OBJ_GET_INFO_BY_FD,
+	BPF_PROG_QUERY,
 };
 
 enum bpf_map_type {
@@ -143,11 +144,47 @@ enum bpf_attach_type {
 
 #define MAX_BPF_ATTACH_TYPE __MAX_BPF_ATTACH_TYPE
 
-/* If BPF_F_ALLOW_OVERRIDE flag is used in BPF_PROG_ATTACH command
- * to the given target_fd cgroup the descendent cgroup will be able to
- * override effective bpf program that was inherited from this cgroup
+/* cgroup-bpf attach flags used in BPF_PROG_ATTACH command
+ *
+ * NONE(default): No further bpf programs allowed in the subtree.
+ *
+ * BPF_F_ALLOW_OVERRIDE: If a sub-cgroup installs some bpf program,
+ * the program in this cgroup yields to sub-cgroup program.
+ *
+ * BPF_F_ALLOW_MULTI: If a sub-cgroup installs some bpf program,
+ * that cgroup program gets run in addition to the program in this cgroup.
+ *
+ * Only one program is allowed to be attached to a cgroup with
+ * NONE or BPF_F_ALLOW_OVERRIDE flag.
+ * Attaching another program on top of NONE or BPF_F_ALLOW_OVERRIDE will
+ * release old program and attach the new one. Attach flags has to match.
+ *
+ * Multiple programs are allowed to be attached to a cgroup with
+ * BPF_F_ALLOW_MULTI flag. They are executed in FIFO order
+ * (those that were attached first, run first)
+ * The programs of sub-cgroup are executed first, then programs of
+ * this cgroup and then programs of parent cgroup.
+ * When children program makes decision (like picking TCP CA or sock bind)
+ * parent program has a chance to override it.
+ *
+ * A cgroup with MULTI or OVERRIDE flag allows any attach flags in sub-cgroups.
+ * A cgroup with NONE doesn't allow any programs in sub-cgroups.
+ * Ex1:
+ * cgrp1 (MULTI progs A, B) ->
+ *    cgrp2 (OVERRIDE prog C) ->
+ *      cgrp3 (MULTI prog D) ->
+ *        cgrp4 (OVERRIDE prog E) ->
+ *          cgrp5 (NONE prog F)
+ * the event in cgrp5 triggers execution of F,D,A,B in that order.
+ * if prog F is detached, the execution is E,D,A,B
+ * if prog F and D are detached, the execution is E,A,B
+ * if prog F, E and D are detached, the execution is C,A,B
+ *
+ * All eligible programs are executed regardless of return code from
+ * earlier programs.
  */
 #define BPF_F_ALLOW_OVERRIDE	(1U << 0)
+#define BPF_F_ALLOW_MULTI	(1U << 1)
 
 /* If BPF_F_STRICT_ALIGNMENT is used in BPF_PROG_LOAD command, the
  * verifier will perform strict alignment checking as if the kernel
@@ -175,6 +212,9 @@ enum bpf_attach_type {
 /* Specify numa node during map creation */
 #define BPF_F_NUMA_NODE		(1U << 2)
 
+/* flags for BPF_PROG_QUERY */
+#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
+
 #define BPF_OBJ_NAME_LEN 16U
 
 union bpf_attr {
@@ -253,6 +293,15 @@ union bpf_attr {
 		__u32		info_len;
 		__aligned_u64	info;
 	} info;
+
+	struct { /* anonymous struct used by BPF_PROG_QUERY command */
+		__u32		target_fd;	/* container object to query */
+		__u32		attach_type;
+		__u32		query_flags;
+		__u32		attach_flags;
+		__aligned_u64	prog_ids;
+		__u32		prog_cnt;
+	} query;
 } __attribute__((aligned(8)));
 
 /* BPF helper function descriptions:
-- 
2.9.5

^ permalink raw reply related

* [PATCH v2 net-next 2/8] bpf: introduce BPF_PROG_QUERY command
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team
In-Reply-To: <20171003055028.1294791-1-ast@fb.com>

introduce BPF_PROG_QUERY command to retrieve a set of either
attached programs to given cgroup or a set of effective programs
that will execute for events within a cgroup

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Martin KaFai Lau <kafai@fb.com>
for cgroup bits
Acked-by: Tejun Heo <tj@kernel.org>
---
 include/linux/bpf-cgroup.h |  4 ++++
 include/linux/bpf.h        |  3 +++
 include/uapi/linux/bpf.h   | 13 +++++++++++++
 kernel/bpf/cgroup.c        | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/core.c          | 38 ++++++++++++++++++++++++++++++++++++++
 kernel/bpf/syscall.c       | 34 ++++++++++++++++++++++++++++++++++
 kernel/cgroup/cgroup.c     | 10 ++++++++++
 7 files changed, 148 insertions(+)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 102e56fbb6de..359b6f5d3d90 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -44,12 +44,16 @@ int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
 			enum bpf_attach_type type, u32 flags);
 int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 			enum bpf_attach_type type, u32 flags);
+int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
+		       union bpf_attr __user *uattr);
 
 /* Wrapper for __cgroup_bpf_*() protected by cgroup_mutex */
 int cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *prog,
 		      enum bpf_attach_type type, u32 flags);
 int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 		      enum bpf_attach_type type, u32 flags);
+int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
+		     union bpf_attr __user *uattr);
 
 int __cgroup_bpf_run_filter_skb(struct sock *sk,
 				struct sk_buff *skb,
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a6964b75f070..a67daea731ab 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -260,6 +260,9 @@ struct bpf_prog_array {
 
 struct bpf_prog_array __rcu *bpf_prog_array_alloc(u32 prog_cnt, gfp_t flags);
 void bpf_prog_array_free(struct bpf_prog_array __rcu *progs);
+int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
+int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
+				__u32 __user *prog_ids, u32 cnt);
 
 #define BPF_PROG_RUN_ARRAY(array, ctx, func)		\
 	({						\
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 762f74bc6c47..cb2b9f95160a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -92,6 +92,7 @@ enum bpf_cmd {
 	BPF_PROG_GET_FD_BY_ID,
 	BPF_MAP_GET_FD_BY_ID,
 	BPF_OBJ_GET_INFO_BY_FD,
+	BPF_PROG_QUERY,
 };
 
 enum bpf_map_type {
@@ -211,6 +212,9 @@ enum bpf_attach_type {
 /* Specify numa node during map creation */
 #define BPF_F_NUMA_NODE		(1U << 2)
 
+/* flags for BPF_PROG_QUERY */
+#define BPF_F_QUERY_EFFECTIVE	(1U << 0)
+
 #define BPF_OBJ_NAME_LEN 16U
 
 union bpf_attr {
@@ -289,6 +293,15 @@ union bpf_attr {
 		__u32		info_len;
 		__aligned_u64	info;
 	} info;
+
+	struct { /* anonymous struct used by BPF_PROG_QUERY command */
+		__u32		target_fd;	/* container object to query */
+		__u32		attach_type;
+		__u32		query_flags;
+		__u32		attach_flags;
+		__aligned_u64	prog_ids;
+		__u32		prog_cnt;
+	} query;
 } __attribute__((aligned(8)));
 
 /* BPF helper function descriptions:
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 6b7500bbdb53..e88abc0865d5 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -384,6 +384,52 @@ int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 	return err;
 }
 
+/* Must be called with cgroup_mutex held to avoid races. */
+int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
+		       union bpf_attr __user *uattr)
+{
+	__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
+	enum bpf_attach_type type = attr->query.attach_type;
+	struct list_head *progs = &cgrp->bpf.progs[type];
+	u32 flags = cgrp->bpf.flags[type];
+	int cnt, ret = 0, i;
+
+	if (attr->query.query_flags & BPF_F_QUERY_EFFECTIVE)
+		cnt = bpf_prog_array_length(cgrp->bpf.effective[type]);
+	else
+		cnt = prog_list_length(progs);
+
+	if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)))
+		return -EFAULT;
+	if (copy_to_user(&uattr->query.prog_cnt, &cnt, sizeof(cnt)))
+		return -EFAULT;
+	if (attr->query.prog_cnt == 0 || !prog_ids || !cnt)
+		/* return early if user requested only program count + flags */
+		return 0;
+	if (attr->query.prog_cnt < cnt) {
+		cnt = attr->query.prog_cnt;
+		ret = -ENOSPC;
+	}
+
+	if (attr->query.query_flags & BPF_F_QUERY_EFFECTIVE) {
+		return bpf_prog_array_copy_to_user(cgrp->bpf.effective[type],
+						   prog_ids, cnt);
+	} else {
+		struct bpf_prog_list *pl;
+		u32 id;
+
+		i = 0;
+		list_for_each_entry(pl, progs, node) {
+			id = pl->prog->aux->id;
+			if (copy_to_user(prog_ids + i, &id, sizeof(id)))
+				return -EFAULT;
+			if (++i == cnt)
+				break;
+		}
+	}
+	return ret;
+}
+
 /**
  * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
  * @sk: The socket sending or receiving traffic
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6b49e1991ae7..eba966c09053 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1412,6 +1412,44 @@ void bpf_prog_array_free(struct bpf_prog_array __rcu *progs)
 	kfree_rcu(progs, rcu);
 }
 
+int bpf_prog_array_length(struct bpf_prog_array __rcu *progs)
+{
+	struct bpf_prog **prog;
+	u32 cnt = 0;
+
+	rcu_read_lock();
+	prog = rcu_dereference(progs)->progs;
+	for (; *prog; prog++)
+		cnt++;
+	rcu_read_unlock();
+	return cnt;
+}
+
+int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
+				__u32 __user *prog_ids, u32 cnt)
+{
+	struct bpf_prog **prog;
+	u32 i = 0, id;
+
+	rcu_read_lock();
+	prog = rcu_dereference(progs)->progs;
+	for (; *prog; prog++) {
+		id = (*prog)->aux->id;
+		if (copy_to_user(prog_ids + i, &id, sizeof(id))) {
+			rcu_read_unlock();
+			return -EFAULT;
+		}
+		if (++i == cnt) {
+			prog++;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	if (*prog)
+		return -ENOSPC;
+	return 0;
+}
+
 static void bpf_prog_free_deferred(struct work_struct *work)
 {
 	struct bpf_prog_aux *aux;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 51bee695d32c..0048cb24ba7b 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1272,6 +1272,37 @@ static int bpf_prog_detach(const union bpf_attr *attr)
 	return ret;
 }
 
+#define BPF_PROG_QUERY_LAST_FIELD query.prog_cnt
+
+static int bpf_prog_query(const union bpf_attr *attr,
+			  union bpf_attr __user *uattr)
+{
+	struct cgroup *cgrp;
+	int ret;
+
+	if (!capable(CAP_NET_ADMIN))
+		return -EPERM;
+	if (CHECK_ATTR(BPF_PROG_QUERY))
+		return -EINVAL;
+	if (attr->query.query_flags & ~BPF_F_QUERY_EFFECTIVE)
+		return -EINVAL;
+
+	switch (attr->query.attach_type) {
+	case BPF_CGROUP_INET_INGRESS:
+	case BPF_CGROUP_INET_EGRESS:
+	case BPF_CGROUP_INET_SOCK_CREATE:
+	case BPF_CGROUP_SOCK_OPS:
+		break;
+	default:
+		return -EINVAL;
+	}
+	cgrp = cgroup_get_from_fd(attr->query.target_fd);
+	if (IS_ERR(cgrp))
+		return PTR_ERR(cgrp);
+	ret = cgroup_bpf_query(cgrp, attr, uattr);
+	cgroup_put(cgrp);
+	return ret;
+}
 #endif /* CONFIG_CGROUP_BPF */
 
 #define BPF_PROG_TEST_RUN_LAST_FIELD test.duration
@@ -1568,6 +1599,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	case BPF_PROG_DETACH:
 		err = bpf_prog_detach(&attr);
 		break;
+	case BPF_PROG_QUERY:
+		err = bpf_prog_query(&attr, uattr);
+		break;
 #endif
 	case BPF_PROG_TEST_RUN:
 		err = bpf_prog_test_run(&attr, uattr);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 57eb866ae78d..269512b94a94 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5761,4 +5761,14 @@ int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 	mutex_unlock(&cgroup_mutex);
 	return ret;
 }
+int cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
+		     union bpf_attr __user *uattr)
+{
+	int ret;
+
+	mutex_lock(&cgroup_mutex);
+	ret = __cgroup_bpf_query(cgrp, attr, uattr);
+	mutex_unlock(&cgroup_mutex);
+	return ret;
+}
 #endif /* CONFIG_CGROUP_BPF */
-- 
2.9.5

^ permalink raw reply related

* [PATCH v2 net-next 0/8] bpf: muli prog support for cgroup-bpf
From: Alexei Starovoitov @ 2017-10-03  5:50 UTC (permalink / raw)
  To: David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, David Ahern, netdev, kernel-team

v1->v2:
- fixed accidentally swapped two lines which caused static_key not going to zero
- addressed Martin's feedback and changed prog_query to be consistent
  with verifier output: return -enospc and fill supplied buffer instead
  of just returning -enospc when buffer is too small to fit all prog_ids

v1:
cgroup-bpf use cases are getting more advanced and running only
one program per cgroup is no longer enough. Therefore introduce
support for attaching multiple programs per cgroup and running
a set of effective programs.

These patches introduces BPF_F_ALLOW_MULTI flag for BPF_PROG_ATTACH cmd.
The default is still NONE and behavior of BPF_F_ALLOW_OVERRIDE flag
is unchanged.
The difference between three possible flags for BPF_PROG_ATTACH command:
- NONE(default): No further bpf programs allowed in the subtree.
- BPF_F_ALLOW_OVERRIDE: If a sub-cgroup installs some bpf program,
  the program in this cgroup yields to sub-cgroup program.
- BPF_F_ALLOW_MULTI: If a sub-cgroup installs some bpf program,
  that cgroup program gets run in addition to the program in this cgroup.

Most of the logic is in patch 1. Even when cgroup doesn't have
any programs attached its set of effective program can be non-empty.
To quickly execute them and avoid penalizing cgroups without
any effective programs introduce 'struct bpf_prog_array'
which has an optimization for cgroups with zero effective programs.

Patch 2 introduces BPF_PROG_QUERY command for introspection
Patch 3 makes verifier more strict for cgroup-bpf program types.
Patch 4+ are tests.

More details in individual patches

Alexei Starovoitov (8):
  bpf: multi program support for cgroup+bpf
  bpf: introduce BPF_PROG_QUERY command
  bpf: enforce return code for cgroup-bpf programs
  libbpf: introduce bpf_prog_detach2()
  samples/bpf: add multi-prog cgroup test case
  libbpf: sync bpf.h
  libbpf: add support for BPF_PROG_QUERY
  samples/bpf: use bpf_prog_query() interface

 include/linux/bpf-cgroup.h                  |  54 ++-
 include/linux/bpf.h                         |  35 ++
 include/linux/filter.h                      |   2 +-
 include/uapi/linux/bpf.h                    |  55 ++-
 kernel/bpf/cgroup.c                         | 513 +++++++++++++++++++++-------
 kernel/bpf/core.c                           |  69 ++++
 kernel/bpf/syscall.c                        |  71 +++-
 kernel/bpf/verifier.c                       |  40 +++
 kernel/cgroup/cgroup.c                      |  38 ++-
 samples/bpf/cgroup_helpers.c                |   4 +-
 samples/bpf/test_cgrp2_attach2.c            | 224 +++++++++++-
 tools/include/uapi/linux/bpf.h              |  55 ++-
 tools/lib/bpf/bpf.c                         |  32 ++
 tools/lib/bpf/bpf.h                         |   4 +-
 tools/testing/selftests/bpf/test_verifier.c |  72 ++++
 15 files changed, 1086 insertions(+), 182 deletions(-)

-- 
2.9.5

^ permalink raw reply

* [PATCH net-next] cxgb4: add new T6 pci device id's
From: Ganesh Goudar @ 2017-10-03  5:40 UTC (permalink / raw)
  To: netdev, davem; +Cc: nirranjan, indranil, venkatesh, Ganesh Goudar

Add 0x6085 T6 device id.

Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
index 37d90d6..633e975 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_pci_id_tbl.h
@@ -202,6 +202,7 @@ CH_PCI_DEVICE_ID_TABLE_DEFINE_BEGIN
 	CH_PCI_ID_TABLE_FENTRY(0x6082), /* Custom T6225-CR SFP28 */
 	CH_PCI_ID_TABLE_FENTRY(0x6083), /* Custom T62100-CR QSFP28 */
 	CH_PCI_ID_TABLE_FENTRY(0x6084), /* Custom T64100-CR QSFP28 */
+	CH_PCI_ID_TABLE_FENTRY(0x6085), /* Custom T6240-SO */
 CH_PCI_DEVICE_ID_TABLE_DEFINE_END;
 
 #endif /* __T4_PCI_ID_TBL_H__ */
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Herbert Xu @ 2017-10-03  5:26 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Jia-Ju Bai, David S. Miller, Neil Horman,
	vyasevich-Re5JQEeQqe8AvxtiuMwx3w, Kalle Valo,
	Linux Crypto Mailing List, Network Development,
	linux-sctp-u79uwXL29TY76Z2rM5mHXA, Linux Wireless List
In-Reply-To: <CALCETrWdXjTTTywbb3duCEsLYNxkeGx7bf3SM4PYKeErCyiUNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Oct 02, 2017 at 09:18:24PM -0700, Andy Lutomirski wrote:
> > On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org> wrote:
> >
> > The SCTP program may sleep under a spinlock, and the function call path is:
> > sctp_generate_t3_rtx_event (acquire the spinlock)
> >  sctp_do_sm
> >    sctp_side_effects
> >      sctp_cmd_interpreter
> >        sctp_make_init_ack
> >          sctp_pack_cookie
> >            crypto_shash_setkey
> >              shash_setkey_unaligned
> >                kmalloc(GFP_KERNEL)
> 
> I'm going to go out on a limb here: why on Earth is out crypto API so
> full of indirection that we allocate memory at all here?

The crypto API operates on a one key per-tfm basis.  So normally
tfm allocation and key setting is done once only and not done on
the data path.

I have looked at the SCTP code and it appears to fit this paradigm.
That is, we should be able to allocate the tfm and set the key when
the key is actually generated via get_random_bytes, rather than every
time the key is used which is not only a waste but as you see runs
into API issues.

Usually if you're invoking setkey from a non-sleeping code-path
you're probably doing something wrong.

As someone else noted recently, there is no single forum for
reviewing code that uses the crypto API so buggy code like this
is not surprising.

> We're synchronously computing a hash of a small amount of data using
> either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
> right.  There's a sane way to do this that doesn't need kmalloc,
> alloca, or fancy indirection.  And then there's crypto_shash_xyz().

There are some legitimate cases where you want to use a different
key for every hashing operation.  But so far these are uses have
been very few so there has been no need to provide an API for them.

Cheers,
-- 
Email: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* RE: [PATCH v2 3/6] staging: fsl-dpaa2/ethsw: Add ethtool support
From: Razvan Stefanescu @ 2017-10-03  5:22 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: devel@driverdev.osuosl.org, arnd@arndb.de,
	gregkh@linuxfoundation.org, Alexandru Marginean, agraf@suse.de,
	linux-kernel@vger.kernel.org, stuyoder@gmail.com,
	netdev@vger.kernel.org, Bogdan Purcareata, Laurentiu Tudor
In-Reply-To: <20171002153718.GJ17713@lunn.ch>



> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, October 02, 2017 18:37
> To: Razvan Stefanescu <razvan.stefanescu@nxp.com>
> Cc: gregkh@linuxfoundation.org; devel@driverdev.osuosl.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org; agraf@suse.de;
> arnd@arndb.de; Alexandru Marginean <alexandru.marginean@nxp.com>;
> Bogdan Purcareata <bogdan.purcareata@nxp.com>; Ruxandra Ioana Radulescu
> <ruxandra.radulescu@nxp.com>; Laurentiu Tudor <laurentiu.tudor@nxp.com>;
> stuyoder@gmail.com
> Subject: Re: [PATCH v2 3/6] staging: fsl-dpaa2/ethsw: Add ethtool support
> 
> Hi Razvan
> 
> > +static void ethsw_get_drvinfo(struct net_device *netdev,
> > +			      struct ethtool_drvinfo *drvinfo)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	u16 version_major, version_minor;
> > +	int err;
> > +
> > +	strlcpy(drvinfo->driver, KBUILD_MODNAME, sizeof(drvinfo->driver));
> > +	strlcpy(drvinfo->version, ethsw_drv_version, sizeof(drvinfo->version));
> 
> Software driver versions are mostly useless. I would suggest you
> remove this.
> 
>        Andrew
Thank you. I'll remove it in v3.

Best regards,
Razvan S.

^ permalink raw reply

* Re: [PATCH net-next 1/8] bpf: multi program support for cgroup+bpf
From: Alexei Starovoitov @ 2017-10-03  5:00 UTC (permalink / raw)
  To: David Ahern, David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, netdev, kernel-team
In-Reply-To: <1211d4c0-b812-af08-cbd4-72eb9259ee78@cumulusnetworks.com>

On 10/2/17 9:26 PM, David Ahern wrote:
> On 10/2/17 9:21 PM, Alexei Starovoitov wrote:
>>
>> i'm not sure what you're trying to say.
>> The first loop quoted above is inside cgroup_bpf_put()
>> which is called when cgroup is destroyed. At this point
>> we're detaching and prog_put all attached programs.
>> While there is only one static_branch_inc() in __cgroup_bpf_attach()
>> that is called every time the prog is attached to a cgroup.
>> So what's the concern?
>
> just asking if cgroup_bpf_enabled_key is 0 when all programs are removed
> -- ie., that the inc's and dec's are equal. Reviewing this patch it was
> not clear that they are.

after some debugging turned out there is a typo in attach code
that leaks prog in the case of override.
Strangely kmemleak didn't catch it.
Will respin.

^ permalink raw reply

* Re: [PATCH net] net: br: Fix igmp snooping offload with CONFIG_BRIDGE_VLAN_FILTERING
From: Toshiaki Makita @ 2017-10-03  3:29 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: David Miller, Vivien Didelot, netdev
In-Reply-To: <1506992111-25004-1-git-send-email-andrew@lunn.ch>

On 2017/10/03 9:55, Andrew Lunn wrote:
> With CONFIG_BRIDGE_VLAN_FILTERING enabled, but the feature not enabled
> via /sys/class/net/brX/bridge/vlan_filtering, mdb offloaded to the
> kernel have the wrong VID.
> 
> When an interface is added to the bridge, switchdev is first used to
> notify the hardware that a port has joined a bridge. This is
> immediately followed by the default_pvid, 1, being added to the
> interface via another switchdev call.
> 
> The bridge will then perform IGMP snooping, and offload an mdb entries
> to the switch as needed. With vlan filtering disabled, the vid is left
> as 0. This causes the switch to put the static mdb into the wrong
> vlan, and so frames are not forwarded by the mdb entry.
> 
> If vlan filtering is disable, use the default_pvid, not 0.
> 
> Fixes: f1fecb1d10ec ("bridge: Reflect MDB entries to hardware")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  net/bridge/br_vlan.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 233a30040c91..aa3589891797 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -492,6 +492,7 @@ bool br_allowed_ingress(const struct net_bridge *br,
>  	 */
>  	if (!br->vlan_enabled) {
>  		BR_INPUT_SKB_CB(skb)->vlan_filtered = false;
> +		*vid = br_get_pvid(vg);
>  		return true;
>  	}
>  

This does not look correct.
This will update fdb with vid which is not 0.
Pvid can be different between each port even when vlan_filtering is
disabled so unicast forwarding (fdb learning) will break.
Also, fdb is visible to userspace so this can break userspace which
expects fdb entries with 0 as well.

Why does the switch driver use pvid while vlan_filtering is disabled?
The (software) bridge does not use pvid for forwarding and use fdb/mdb
entires with vid 0 when vlan_filtering is disabled even if pvid has been
configured.

-- 
Toshiaki Makita

^ permalink raw reply

* Re: [PATCH net-next v2 3/3] tools: bpftool: add documentation
From: Alexei Starovoitov @ 2017-10-03  4:29 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, daniel, oss-drivers, David Beckett
In-Reply-To: <20171002183509.76b2cc65@cakuba>

On Mon, Oct 02, 2017 at 06:35:09PM -0700, Jakub Kicinski wrote:
> > will pretty print them as verifier output as well?
> 
> We tried to use LLVM as a library for this but the interface is
> painfully unstable and it's a heavy dependency.  The current thinking
> is to try to put the instruction printing code in some higher level
> library, but I would rather leave that as a follow up.

follow up, of course.
Not depending on llvm is must have for this tool.
I think we need tiny and simple tools first.
Since you're using gpl+bsd license for this tool I think
it would be fine to copy-paste verifier's pretty print code into it.

^ permalink raw reply

* Re: [PATCH net-next 1/8] bpf: multi program support for cgroup+bpf
From: David Ahern @ 2017-10-03  4:26 UTC (permalink / raw)
  To: Alexei Starovoitov, David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, netdev, kernel-team
In-Reply-To: <f5d34464-42a9-b51b-edc4-06c44b3c4845@fb.com>

On 10/2/17 9:21 PM, Alexei Starovoitov wrote:
> 
> i'm not sure what you're trying to say.
> The first loop quoted above is inside cgroup_bpf_put()
> which is called when cgroup is destroyed. At this point
> we're detaching and prog_put all attached programs.
> While there is only one static_branch_inc() in __cgroup_bpf_attach()
> that is called every time the prog is attached to a cgroup.
> So what's the concern?

just asking if cgroup_bpf_enabled_key is 0 when all programs are removed
-- ie., that the inc's and dec's are equal. Reviewing this patch it was
not clear that they are.

> Note we're doing branch_dec only for progs in prog_list.
> Just like we do branch_inc only for progs in prog_list.
> Computing prog_array doesn't involve manipulations with prog's refcnt
> and no branch_inc/dec either.
> 

^ permalink raw reply

* Re: [PATCH net-next 1/8] bpf: multi program support for cgroup+bpf
From: Alexei Starovoitov @ 2017-10-03  4:21 UTC (permalink / raw)
  To: David Ahern, David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, netdev, kernel-team
In-Reply-To: <71bda584-d828-7472-7655-85a454dbe297@cumulusnetworks.com>

On 10/2/17 8:54 PM, David Ahern wrote:
> On 10/2/17 4:48 PM, Alexei Starovoitov wrote:
>> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
>> index 546113430049..70f679a94804 100644
>> --- a/kernel/bpf/cgroup.c
>> +++ b/kernel/bpf/cgroup.c
>> @@ -27,129 +27,361 @@ void cgroup_bpf_put(struct cgroup *cgrp)
>>  {
>>  	unsigned int type;
>>
>> -	for (type = 0; type < ARRAY_SIZE(cgrp->bpf.prog); type++) {
>> -		struct bpf_prog *prog = cgrp->bpf.prog[type];
>> -
>> -		if (prog) {
>> -			bpf_prog_put(prog);
>> +	for (type = 0; type < ARRAY_SIZE(cgrp->bpf.progs); type++) {
>> +		struct list_head *progs = &cgrp->bpf.progs[type];
>> +		struct bpf_prog_list *pl, *tmp;
>> +
>> +		list_for_each_entry_safe(pl, tmp, progs, node) {
>> +			list_del(&pl->node);
>> +			bpf_prog_put(pl->prog);
>> +			kfree(pl);
>>  			static_branch_dec(&cgroup_bpf_enabled_key);
>>  		}
>> +		bpf_prog_array_free(cgrp->bpf.effective[type]);
>> +	}
>> +}
>> +
>
> ...
>
>>
>> -	if (prog)
>> -		static_branch_inc(&cgroup_bpf_enabled_key);
>> +	/* all allocations were successful. Activate all prog arrays */
>> +	css_for_each_descendant_pre(css, &cgrp->self) {
>> +		struct cgroup *desc = container_of(css, struct cgroup, self);
>>
>> +		activate_effective_progs(desc, type, desc->bpf.inactive);
>> +		desc->bpf.inactive = NULL;
>> +	}
>> +
>> +	static_branch_inc(&cgroup_bpf_enabled_key);
>>  	if (old_prog) {
>>  		bpf_prog_put(old_prog);
>>  		static_branch_dec(&cgroup_bpf_enabled_key);
>>  	}
>>  	return 0;
>
> It's not clear to me that the static_branch_inc and static_branch_dec's
> are equal since the dec is in the loop over each program in the list,
> but the inc is not in a loop.

i'm not sure what you're trying to say.
The first loop quoted above is inside cgroup_bpf_put()
which is called when cgroup is destroyed. At this point
we're detaching and prog_put all attached programs.
While there is only one static_branch_inc() in __cgroup_bpf_attach()
that is called every time the prog is attached to a cgroup.
So what's the concern?
Note we're doing branch_dec only for progs in prog_list.
Just like we do branch_inc only for progs in prog_list.
Computing prog_array doesn't involve manipulations with prog's refcnt
and no branch_inc/dec either.

^ permalink raw reply

* Re: [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Andy Lutomirski @ 2017-10-03  4:18 UTC (permalink / raw)
  To: Jia-Ju Bai
  Cc: David S. Miller, Herbert Xu, Neil Horman, vyasevich,
	Andrew Lutomirski, Kalle Valo, Linux Crypto Mailing List,
	Network Development, linux-sctp, Linux Wireless List
In-Reply-To: <1506997522-26684-1-git-send-email-baijiaju1990@163.com>

> On Oct 2, 2017, at 7:25 PM, Jia-Ju Bai <baijiaju1990@163.com> wrote:
>
> The SCTP program may sleep under a spinlock, and the function call path is:
> sctp_generate_t3_rtx_event (acquire the spinlock)
>  sctp_do_sm
>    sctp_side_effects
>      sctp_cmd_interpreter
>        sctp_make_init_ack
>          sctp_pack_cookie
>            crypto_shash_setkey
>              shash_setkey_unaligned
>                kmalloc(GFP_KERNEL)
>

I'm going to go out on a limb here: why on Earth is out crypto API so
full of indirection that we allocate memory at all here?

We're synchronously computing a hash of a small amount of data using
either HMAC-SHA1 or HMAC-SHA256 (determined at runtime) if I read it
right.  There's a sane way to do this that doesn't need kmalloc,
alloca, or fancy indirection.  And then there's crypto_shash_xyz().

--Andy, who is sick of seeing stupid bugs caused by the fact that it's
basically impossible to use the crypto API in a sane way.

P.S. gnulib has:

int hmac_sha256 (const void *key, size_t keylen, const void *in,
size_t inlen, void *resbuf);

An init/update/final-style API would be nice, but something like this
would be a phenomenal improvement over what we have.

^ permalink raw reply

* Re: [PATCH net-next 1/8] bpf: multi program support for cgroup+bpf
From: David Ahern @ 2017-10-03  3:54 UTC (permalink / raw)
  To: Alexei Starovoitov, David S . Miller
  Cc: Daniel Borkmann, Tejun Heo, netdev, kernel-team
In-Reply-To: <20171002234857.3707580-2-ast@fb.com>

On 10/2/17 4:48 PM, Alexei Starovoitov wrote:
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 546113430049..70f679a94804 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -27,129 +27,361 @@ void cgroup_bpf_put(struct cgroup *cgrp)
>  {
>  	unsigned int type;
>  
> -	for (type = 0; type < ARRAY_SIZE(cgrp->bpf.prog); type++) {
> -		struct bpf_prog *prog = cgrp->bpf.prog[type];
> -
> -		if (prog) {
> -			bpf_prog_put(prog);
> +	for (type = 0; type < ARRAY_SIZE(cgrp->bpf.progs); type++) {
> +		struct list_head *progs = &cgrp->bpf.progs[type];
> +		struct bpf_prog_list *pl, *tmp;
> +
> +		list_for_each_entry_safe(pl, tmp, progs, node) {
> +			list_del(&pl->node);
> +			bpf_prog_put(pl->prog);
> +			kfree(pl);
>  			static_branch_dec(&cgroup_bpf_enabled_key);
>  		}
> +		bpf_prog_array_free(cgrp->bpf.effective[type]);
> +	}
> +}
> +

...

>  
> -	if (prog)
> -		static_branch_inc(&cgroup_bpf_enabled_key);
> +	/* all allocations were successful. Activate all prog arrays */
> +	css_for_each_descendant_pre(css, &cgrp->self) {
> +		struct cgroup *desc = container_of(css, struct cgroup, self);
>  
> +		activate_effective_progs(desc, type, desc->bpf.inactive);
> +		desc->bpf.inactive = NULL;
> +	}
> +
> +	static_branch_inc(&cgroup_bpf_enabled_key);
>  	if (old_prog) {
>  		bpf_prog_put(old_prog);
>  		static_branch_dec(&cgroup_bpf_enabled_key);
>  	}
>  	return 0;

It's not clear to me that the static_branch_inc and static_branch_dec's
are equal since the dec is in the loop over each program in the list,
but the inc is not in a loop.

^ permalink raw reply

* [PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Jia-Ju Bai @ 2017-10-03  2:25 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q,
	nhorman-2XuSBdqkA4R54TAoqtyWWQ, vyasevich-Re5JQEeQqe8AvxtiuMwx3w,
	luto-DgEjT+Ai2ygdnm+yROfE0A, kvalo-sgV2jX0FEOL9JmXXK+q4OQ
  Cc: linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-sctp-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Jia-Ju Bai

The SCTP program may sleep under a spinlock, and the function call path is:
sctp_generate_t3_rtx_event (acquire the spinlock)
  sctp_do_sm
    sctp_side_effects
      sctp_cmd_interpreter
        sctp_make_init_ack
          sctp_pack_cookie
            crypto_shash_setkey
              shash_setkey_unaligned
                kmalloc(GFP_KERNEL)

For the same reason, the orinoco driver may sleep in interrupt handler, 
and the function call path is:
orinoco_rx_isr_tasklet
  orinoco_rx
    orinoco_mic
      crypto_shash_setkey
        shash_setkey_unaligned
          kmalloc(GFP_KERNEL)

To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai <baijiaju1990-9Onoh4P/yGk@public.gmane.org>
---
 crypto/shash.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..8fcecc6 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -41,7 +41,7 @@ static int shash_setkey_unaligned(struct crypto_shash *tfm, const u8 *key,
 	int err;
 
 	absize = keylen + (alignmask & ~(crypto_tfm_ctx_alignment() - 1));
-	buffer = kmalloc(absize, GFP_KERNEL);
+	buffer = kmalloc(absize, GFP_ATOMIC);
 	if (!buffer)
 		return -ENOMEM;
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] Fix a sleep-in-atomic bug in shash_setkey_unaligned
From: Jia-Ju Bai @ 2017-10-03  2:22 UTC (permalink / raw)
  To: davem, herbert, nhorman, vyasevich, luto, kvalo
  Cc: linux-crypto, netdev, linux-sctp, linux-wireless, Jia-Ju Bai

For the same reason, the orinoco driver may sleep in interrupt handler, 
and the function call path is:
orinoco_rx_isr_tasklet
  orinoco_rx
    orinoco_mic
      crypto_shash_setkey
        shash_setkey_unaligned
          kmalloc(GFP_KERNEL)

To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
 crypto/shash.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..8fcecc6 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -41,7 +41,7 @@ static int shash_setkey_unaligned(struct crypto_shash *tfm, const u8 *key,
 	int err;
 
 	absize = keylen + (alignmask & ~(crypto_tfm_ctx_alignment() - 1));
-	buffer = kmalloc(absize, GFP_KERNEL);
+	buffer = kmalloc(absize, GFP_ATOMIC);
 	if (!buffer)
 		return -ENOMEM;
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 05/18] net: use ARRAY_SIZE
From: Jérémy Lefaure @ 2017-10-03  1:23 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
	Jeff Kirsher, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, Larry Finger, Chaoming Li,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel, intel-wired-lan, linux-usb
In-Reply-To: <87h8vh64sq.fsf@codeaurora.org>

On Mon, 02 Oct 2017 16:46:29 +0300
Kalle Valo <kvalo@codeaurora.org> wrote:

> We have a tree for wireless so usually it's better to submit wireless
> changes on their own but here I assume Dave will apply this to his tree.
> If not, please resubmit the wireless part in a separate patch.
Ok, I note that.

I'll wait Dave's answer and I'll split this patch if needed.

Thank you,
Jérémy

^ permalink raw reply

* Re: [PATCH net-next v2 3/3] tools: bpftool: add documentation
From: Jakub Kicinski @ 2017-10-03  1:35 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: netdev, daniel, oss-drivers, David Beckett
In-Reply-To: <20171003005500.yh2gbnofm5ckn54x@ast-mbp>

On Mon, 2 Oct 2017 17:55:02 -0700, Alexei Starovoitov wrote:
> > +EXAMPLES
> > +========
> > +**# bpftool prog show**
> > +::
> > +
> > +  10: xdp  name:some_prog  tag 00:5a:3d:21:23:62:0c:8b  
> 
> could you please remove ':' in the output to match what
> show_fdinfo and kallsyms do ?

Ack.

> > +	loaded_at:2024.771  uid:0  
> 
> may be translate that to something human readable?

Oh yes, the code will print a proper date/time, I forgot to
regenerate the doc :S

> > +	xlated:528B  jited:370B  memlock:4096B  map_ids:10
> > +
> > +| 
> > +| **# bpftool prog dump xlated id 10 file /tmp/t**
> > +| **# ls -l /tmp/t**
> > +|   -rw------- 1 root root 560 Jul 22 01:42 /tmp/t
> > +
> > +| 
> > +| **# mount -t bpf none /sys/fs/bpf/**
> > +| **# bpftool prog pin id 10 /sys/fs/bpf/prog**
> > +| **# bpftool prog dum jited pinned /sys/fs/bpf/prog**
> > +
> > +::
> > +
> > +    push   %rbp
> > +    mov    %rsp,%rbp
> > +    sub    $0x228,%rsp
> > +    sub    $0x28,%rbp
> > +    mov    %rbx,0x0(%rbp)  
> 
> imo too many steps to dump disasm output.
> Can it print it if we just say:
> bpftool prog dump jited id 10
> and
> dump xlated

Yes those will work.  This example kind of shows pinning and dumping at
the some time.  Perhaps that's ill advised.

> will pretty print them as verifier output as well?

We tried to use LLVM as a library for this but the interface is
painfully unstable and it's a heavy dependency.  The current thinking
is to try to put the instruction printing code in some higher level
library, but I would rather leave that as a follow up.

> All that can be changed later. Thanks for the doc.
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Thanks!

^ permalink raw reply

* Re: [PATCH 00/18] use ARRAY_SIZE macro
From: Jérémy Lefaure @ 2017-10-03  1:33 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: alsa-devel, nouveau, dri-devel, dm-devel, brcm80211-dev-list,
	devel, linux-scsi, linux-rdma, amd-gfx, Jason Gunthorpe,
	linux-acpi, linux-video, intel-wired-lan, Greg KH, linux-media,
	intel-gfx, ecryptfs, brcm80211-dev-list.pdl, linux-raid,
	openipmi-developer, intel-gvt-dev, devel, linux-nfs, jlayton,
	netdev, linux-usb, linux-wireless, linux-kernel, linux-integrity,
	"
In-Reply-To: <20171002192224.GD1903@fieldses.org>

On Mon, 2 Oct 2017 15:22:24 -0400
bfields@fieldses.org (J. Bruce Fields) wrote:

> Mainly I'd just like to know which you're asking for.  Do you want me to
> apply this, or to ACK it so someone else can?  If it's sent as a series
> I tend to assume the latter.
> 
> But in this case I'm assuming it's the former, so I'll pick up the nfsd
> one....
Could you to apply the NFSD patch ("nfsd: use ARRAY_SIZE") with the
Reviewed-by: Jeff Layton <jlayton@redhat.com>) tag please ?

This patch is an individual patch and it should not have been sent in a
series (sorry about that).

Thank you,
Jérémy

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer

^ permalink raw reply

* Re: [PATCH 05/18] net: use ARRAY_SIZE
From: Jérémy Lefaure @ 2017-10-03  1:22 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
	Jeff Kirsher, Arend van Spriel, Franky Lin, Hante Meuleman,
	Chi-Hsien Lin, Wright Feng, Kalle Valo, Larry Finger, Chaoming Li,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <
In-Reply-To: <CAHp75VfC2e+h8GCiB-1Rbe=_z3siBBo0WfqEf3qz8yWh8Cm3RQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, 2 Oct 2017 16:07:36 +0300
Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> > +       {&gainctrl_lut_core0_rev0, ARRAY_SIZE(gainctrl_lut_core0_rev0), 26, 192,
> > +        32},  
> 
> For all such cases I would rather put on one line disregard checkpatch
> warning for better readability.
I agree that it would be better. I didn't know that it was possible to
not follow this rule for anything else than a string.

I am waiting for more comments and I will send a v2.

Thank you,
Jérémy

^ permalink raw reply

* [PATCH RESEND net 2/9] net/mac89x0: Remove dead or unreachable code
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1506992619.git.fthain@telegraphics.com.au>

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 36 -----------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index f910f0f386d6..8fc43c865621 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -119,10 +119,6 @@ struct net_local {
 };
 
 /* Index to functions, as function prototypes. */
-
-#if 0
-extern void reset_chip(struct net_device *dev);
-#endif
 static int net_open(struct net_device *dev);
 static int net_send_packet(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t net_interrupt(int irq, void *dev_id);
@@ -132,10 +128,6 @@ static int net_close(struct net_device *dev);
 static struct net_device_stats *net_get_stats(struct net_device *dev);
 static int set_mac_address(struct net_device *dev, void *addr);
 
-
-/* Example routines you must write ;->. */
-#define tx_done(dev) 1
-
 /* For reading/writing registers ISA-style */
 static inline int
 readreg_io(struct net_device *dev, int portno)
@@ -179,7 +171,6 @@ static const struct net_device_ops mac89x0_netdev_ops = {
 struct net_device * __init mac89x0_probe(int unit)
 {
 	struct net_device *dev;
-	static int once_is_enough;
 	struct net_local *lp;
 	static unsigned version_printed;
 	int i, slot;
@@ -200,10 +191,6 @@ struct net_device * __init mac89x0_probe(int unit)
 		netdev_boot_setup_check(dev);
 	}
 
-	if (once_is_enough)
-		goto out;
-	once_is_enough = 1;
-
 	/* We might have to parameterize this later */
 	slot = 0xE;
 	/* Get out now if there's a real NuBus card in slot E */
@@ -295,24 +282,6 @@ struct net_device * __init mac89x0_probe(int unit)
 	return ERR_PTR(err);
 }
 
-#if 0
-/* This is useful for something, but I don't know what yet. */
-void __init reset_chip(struct net_device *dev)
-{
-	int reset_start_time;
-
-	writereg(dev, PP_SelfCTL, readreg(dev, PP_SelfCTL) | POWER_ON_RESET);
-
-	/* wait 30 ms */
-	msleep_interruptible(30);
-
-	/* Wait until the chip is reset */
-	reset_start_time = jiffies;
-	while( (readreg(dev, PP_SelfST) & INIT_DONE) == 0 && jiffies - reset_start_time < 2)
-		;
-}
-#endif
-
 /* Open/initialize the board.  This is called (in the current kernel)
    sometime after booting when the 'ifconfig' program is run.
 
@@ -414,11 +383,6 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 	struct net_local *lp;
 	int ioaddr, status;
 
-	if (dev == NULL) {
-		printk ("net_interrupt(): irq %d for unknown device.\n", irq);
-		return IRQ_NONE;
-	}
-
 	ioaddr = dev->base_addr;
 	lp = netdev_priv(dev);
 
-- 
2.13.5

^ permalink raw reply related

* [PATCH RESEND net 4/9] net/mac89x0: Replace custom debug logging with netif_* calls
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1506992619.git.fthain@telegraphics.com.au>

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 47 ++++++++++++-----------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 4393d9b89f33..278df230c5f6 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -61,18 +61,6 @@
 static const char version[] =
 "cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
 
-/* ======================= configure the driver here ======================= */
-
-/* use 0 for production, 1 for verification, >2 for debug */
-#ifndef NET_DEBUG
-#define NET_DEBUG 0
-#endif
-
-/* ======================= end of configuration ======================= */
-
-
-/* Always include 'config.h' first in case the user wants to turn on
-   or override something. */
 #include <linux/module.h>
 
 /*
@@ -107,10 +95,13 @@ static const char version[] =
 
 #include "cs89x0.h"
 
-static unsigned int net_debug = NET_DEBUG;
+static int debug = -1;
+module_param(debug, int, 0);
+MODULE_PARM_DESC(debug, "debug message level");
 
 /* Information that need to be kept for each board. */
 struct net_local {
+	int msg_enable;
 	int chip_type;		/* one of: CS8900, CS8920, CS8920M */
 	char chip_revision;	/* revision letter of the chip ('A'...) */
 	int send_cmd;		/* the propercommand used to send a packet. */
@@ -174,7 +165,6 @@ struct net_device * __init mac89x0_probe(int unit)
 {
 	struct net_device *dev;
 	struct net_local *lp;
-	static unsigned version_printed;
 	int i, slot;
 	unsigned rev_type = 0;
 	unsigned long ioaddr;
@@ -220,6 +210,8 @@ struct net_device * __init mac89x0_probe(int unit)
 	/* Initialize the net_device structure. */
 	lp = netdev_priv(dev);
 
+	lp->msg_enable = netif_msg_init(debug, 0);
+
 	/* Fill in the 'dev' fields. */
 	dev->base_addr = ioaddr;
 	dev->mem_start = (unsigned long)
@@ -242,8 +234,7 @@ struct net_device * __init mac89x0_probe(int unit)
 	if (lp->chip_type != CS8900 && lp->chip_revision >= 'C')
 		lp->send_cmd = TX_NOW;
 
-	if (net_debug && version_printed++ == 0)
-		printk(version);
+	netif_dbg(lp, drv, dev, "%s", version);
 
 	pr_info("CS89%c0%s rev %c found at %#8lx\n",
 	        lp->chip_type == CS8900 ? '0' : '2',
@@ -340,11 +331,9 @@ net_send_packet(struct sk_buff *skb, struct net_device *dev)
 	struct net_local *lp = netdev_priv(dev);
 	unsigned long flags;
 
-	if (net_debug > 3)
-		printk("%s: sent %d byte packet of type %x\n",
-		       dev->name, skb->len,
-		       (skb->data[ETH_ALEN+ETH_ALEN] << 8)
-		       | skb->data[ETH_ALEN+ETH_ALEN+1]);
+	netif_dbg(lp, tx_queued, dev, "sent %d byte packet of type %x\n",
+	          skb->len, skb->data[ETH_ALEN + ETH_ALEN] << 8 |
+	                    skb->data[ETH_ALEN + ETH_ALEN + 1]);
 
 	/* keep the upload from being interrupted, since we
 	   ask the chip to start transmitting before the
@@ -393,7 +382,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
            faster than you can read them off, you're screwed.  Hasta la
            vista, baby!  */
 	while ((status = swab16(nubus_readw(dev->base_addr + ISQ_PORT)))) {
-		if (net_debug > 4)printk("%s: event=%04x\n", dev->name, status);
+		netif_dbg(lp, intr, dev, "status=%04x\n", status);
 		switch(status & ISQ_EVENT_MASK) {
 		case ISQ_RECEIVER_EVENT:
 			/* Got a packet(s). */
@@ -423,7 +412,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 				netif_wake_queue(dev);
 			}
 			if (status & TX_UNDERRUN) {
-				if (net_debug > 0) printk("%s: transmit underrun\n", dev->name);
+				netif_dbg(lp, tx_err, dev, "transmit underrun\n");
                                 lp->send_underrun++;
                                 if (lp->send_underrun == 3) lp->send_cmd = TX_AFTER_381;
                                 else if (lp->send_underrun == 6) lp->send_cmd = TX_AFTER_ALL;
@@ -444,6 +433,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 static void
 net_rx(struct net_device *dev)
 {
+	struct net_local *lp = netdev_priv(dev);
 	struct sk_buff *skb;
 	int status, length;
 
@@ -475,10 +465,9 @@ net_rx(struct net_device *dev)
 	skb_copy_to_linear_data(skb, (void *)(dev->mem_start + PP_RxFrame),
 				length);
 
-	if (net_debug > 3)printk("%s: received %d byte packet of type %x\n",
-                                 dev->name, length,
-                                 (skb->data[ETH_ALEN+ETH_ALEN] << 8)
-				 | skb->data[ETH_ALEN+ETH_ALEN+1]);
+	netif_dbg(lp, rx_status, dev, "received %d byte packet of type %x\n",
+	          length, skb->data[ETH_ALEN + ETH_ALEN] << 8 |
+	                  skb->data[ETH_ALEN + ETH_ALEN + 1]);
 
         skb->protocol=eth_type_trans(skb,dev);
 	netif_rx(skb);
@@ -566,16 +555,12 @@ static int set_mac_address(struct net_device *dev, void *addr)
 #ifdef MODULE
 
 static struct net_device *dev_cs89x0;
-static int debug;
 
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "CS89[02]0 debug level (0-5)");
 MODULE_LICENSE("GPL");
 
 int __init
 init_module(void)
 {
-	net_debug = debug;
         dev_cs89x0 = mac89x0_probe(-1);
 	if (IS_ERR(dev_cs89x0)) {
 		pr_warn("No card found\n");
-- 
2.13.5

^ permalink raw reply related

* [PATCH RESEND net 3/9] net/mac89x0: Fix and modernize log messages
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1506992619.git.fthain@telegraphics.com.au>

Fix misplaced newlines in conditional log messages.
Add missing printk severity levels.
Log the MAC address after the interface gets a meaningful name.
Drop deprecated "out of memory" message as per checkpatch advice.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 8fc43c865621..4393d9b89f33 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -56,6 +56,8 @@
   local_irq_{dis,en}able()
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 static const char version[] =
 "cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
 
@@ -243,16 +245,14 @@ struct net_device * __init mac89x0_probe(int unit)
 	if (net_debug && version_printed++ == 0)
 		printk(version);
 
-	printk(KERN_INFO "%s: cs89%c0%s rev %c found at %#8lx",
-	       dev->name,
-	       lp->chip_type==CS8900?'0':'2',
-	       lp->chip_type==CS8920M?"M":"",
-	       lp->chip_revision,
-	       dev->base_addr);
+	pr_info("CS89%c0%s rev %c found at %#8lx\n",
+	        lp->chip_type == CS8900 ? '0' : '2',
+	        lp->chip_type == CS8920M ? "M" : "",
+	        lp->chip_revision, dev->base_addr);
 
 	/* Try to read the MAC address */
 	if ((readreg(dev, PP_SelfST) & (EEPROM_PRESENT | EEPROM_OK)) == 0) {
-		printk("\nmac89x0: No EEPROM, giving up now.\n");
+		pr_info("No EEPROM, giving up now\n");
 		goto out1;
         } else {
                 for (i = 0; i < ETH_ALEN; i += 2) {
@@ -265,15 +265,14 @@ struct net_device * __init mac89x0_probe(int unit)
 
 	dev->irq = SLOT2IRQ(slot);
 
-	/* print the IRQ and ethernet address. */
-
-	printk(" IRQ %d ADDR %pM\n", dev->irq, dev->dev_addr);
-
 	dev->netdev_ops		= &mac89x0_netdev_ops;
 
 	err = register_netdev(dev);
 	if (err)
 		goto out1;
+
+	netdev_info(dev, "MAC %pM, IRQ %d\n", dev->dev_addr, dev->irq);
+
 	return NULL;
 out1:
 	nubus_writew(0, dev->base_addr + ADD_PORT);
@@ -468,7 +467,6 @@ net_rx(struct net_device *dev)
 	/* Malloc up new buffer. */
 	skb = alloc_skb(length, GFP_ATOMIC);
 	if (skb == NULL) {
-		printk("%s: Memory squeeze, dropping packet.\n", dev->name);
 		dev->stats.rx_dropped++;
 		return;
 	}
@@ -556,7 +554,7 @@ static int set_mac_address(struct net_device *dev, void *addr)
 		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, saddr->sa_data, ETH_ALEN);
-	printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);
+	netdev_info(dev, "Setting MAC address to %pM\n", dev->dev_addr);
 
 	/* set the Ethernet address */
 	for (i=0; i < ETH_ALEN/2; i++)
@@ -580,7 +578,7 @@ init_module(void)
 	net_debug = debug;
         dev_cs89x0 = mac89x0_probe(-1);
 	if (IS_ERR(dev_cs89x0)) {
-                printk(KERN_WARNING "mac89x0.c: No card found\n");
+		pr_warn("No card found\n");
 		return PTR_ERR(dev_cs89x0);
 	}
 	return 0;
-- 
2.13.5

^ 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