Netdev List
 help / color / mirror / Atom feed
* [PATCH v4 perf,bpf 08/15] perf, bpf: save btf in a rbtree in perf_env
From: Song Liu @ 2019-02-26  0:20 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>

btf contains information necessary to annotate bpf programs. This patch
saves btf for bpf programs loaded in the system.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/util/bpf-event.c | 24 ++++++++++++++
 tools/perf/util/bpf-event.h |  7 ++++
 tools/perf/util/env.c       | 65 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/env.h       |  4 +++
 4 files changed, 100 insertions(+)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index ce81b2c43a51..370b830f2433 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -34,6 +34,29 @@ int machine__process_bpf_event(struct machine *machine __maybe_unused,
 	return 0;
 }
 
+static int perf_env__fetch_btf(struct perf_env *env,
+			       u32 btf_id,
+			       struct btf *btf)
+{
+	struct btf_node *node;
+	u32 data_size;
+	const void *data;
+
+	data = btf__get_raw_data(btf, &data_size);
+
+	node = malloc(data_size + sizeof(struct btf_node));
+
+	if (!node)
+		return -1;
+
+	node->id = btf_id;
+	node->data_size = data_size;
+	memcpy(node->data, data, data_size);
+
+	perf_env__insert_btf(env, node);
+	return 0;
+}
+
 /*
  * Synthesize PERF_RECORD_KSYMBOL and PERF_RECORD_BPF_EVENT for one bpf
  * program. One PERF_RECORD_BPF_EVENT is generated for the program. And
@@ -113,6 +136,7 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 			goto out;
 		}
 		has_btf = true;
+		perf_env__fetch_btf(env, info->btf_id, btf);
 	}
 
 	/* Synthesize PERF_RECORD_KSYMBOL */
diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
index fad932f7404f..b9ec394dc7c7 100644
--- a/tools/perf/util/bpf-event.h
+++ b/tools/perf/util/bpf-event.h
@@ -16,6 +16,13 @@ struct bpf_prog_info_node {
 	struct rb_node			rb_node;
 };
 
+struct btf_node {
+	struct rb_node	rb_node;
+	u32		id;
+	u32		data_size;
+	char		data[];
+};
+
 #ifdef HAVE_LIBBPF_SUPPORT
 int machine__process_bpf_event(struct machine *machine, union perf_event *event,
 			       struct perf_sample *sample);
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 650c14ad7e9b..291b21d8f858 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -63,6 +63,57 @@ struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 	return node;
 }
 
+void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node)
+{
+	struct rb_node *parent = NULL;
+	__u32 btf_id = btf_node->id;
+	struct btf_node *node;
+	struct rb_node **p;
+
+	down_write(&env->bpf_progs.lock);
+	p = &env->bpf_progs.btfs.rb_node;
+
+	while (*p != NULL) {
+		parent = *p;
+		node = rb_entry(parent, struct btf_node, rb_node);
+		if (btf_id < node->id) {
+			p = &(*p)->rb_left;
+		} else if (btf_id > node->id) {
+			p = &(*p)->rb_right;
+		} else {
+			pr_debug("duplicated btf %u\n", btf_id);
+			goto out;
+		}
+	}
+
+	rb_link_node(&btf_node->rb_node, parent, p);
+	rb_insert_color(&btf_node->rb_node, &env->bpf_progs.btfs);
+out:
+	up_write(&env->bpf_progs.lock);
+}
+
+struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id)
+{
+	struct btf_node *node = NULL;
+	struct rb_node *n;
+
+	down_read(&env->bpf_progs.lock);
+	n = env->bpf_progs.btfs.rb_node;
+
+	while (n) {
+		node = rb_entry(n, struct btf_node, rb_node);
+		if (btf_id < node->id)
+			n = n->rb_left;
+		else if (btf_id > node->id)
+			n = n->rb_right;
+		else
+			break;
+	}
+
+	up_read(&env->bpf_progs.lock);
+	return node;
+}
+
 /* purge data in bpf_prog_infos tree */
 static void perf_env__purge_bpf(struct perf_env *env)
 {
@@ -82,6 +133,19 @@ static void perf_env__purge_bpf(struct perf_env *env)
 		rb_erase_init(&node->rb_node, root);
 		free(node);
 	}
+
+	root = &env->bpf_progs.btfs;
+	next = rb_first(root);
+
+	while (next) {
+		struct btf_node *node;
+
+		node = rb_entry(next, struct btf_node, rb_node);
+		next = rb_next(&node->rb_node);
+		rb_erase_init(&node->rb_node, root);
+		free(node);
+	}
+
 	up_write(&env->bpf_progs.lock);
 }
 
@@ -119,6 +183,7 @@ void perf_env__exit(struct perf_env *env)
 static void init_bpf_rb_trees(struct perf_env *env)
 {
 	env->bpf_progs.prog_infos = RB_ROOT;
+	env->bpf_progs.btfs = RB_ROOT;
 	init_rwsem(&env->bpf_progs.lock);
 }
 
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 33ef4b2d2a29..b7fc4c71ba6a 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -74,10 +74,12 @@ struct perf_env {
 	struct {
 		struct rw_semaphore	lock;
 		struct rb_root		prog_infos;
+		struct rb_root		btfs;
 	} bpf_progs;
 };
 
 struct bpf_prog_info_node;
+struct btf_node;
 
 extern struct perf_env perf_env;
 
@@ -97,4 +99,6 @@ void perf_env__insert_bpf_prog_info(struct perf_env *env,
 				    struct bpf_prog_info_node *info_node);
 struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
 							__u32 prog_id);
+void perf_env__insert_btf(struct perf_env *env, struct btf_node *btf_node);
+struct btf_node *perf_env__find_btf(struct perf_env *env, __u32 btf_id);
 #endif /* __PERF_ENV_H */
-- 
2.17.1


^ permalink raw reply related

* [PATCH v4 perf,bpf 06/15] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Song Liu @ 2019-02-26  0:20 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: ast, daniel, kernel-team, peterz, acme, jolsa, namhyung, Song Liu
In-Reply-To: <20190226002019.3748539-1-songliubraving@fb.com>

bpf_prog_info contains information necessary to annotate bpf programs.
This patch saves bpf_prog_info for bpf programs loaded in the system.

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 tools/perf/util/bpf-event.c | 32 +++++++++++++-
 tools/perf/util/bpf-event.h |  7 ++-
 tools/perf/util/env.c       | 85 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/env.h       | 17 ++++++++
 4 files changed, 139 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index ff7ee149ec46..ce81b2c43a51 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -10,6 +10,7 @@
 #include "debug.h"
 #include "symbol.h"
 #include "machine.h"
+#include "env.h"
 #include "session.h"
 
 #define ptr_to_u64(ptr)    ((__u64)(unsigned long)(ptr))
@@ -54,17 +55,28 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 	struct bpf_event *bpf_event = &event->bpf_event;
 	struct bpf_prog_info_linear *info_linear;
 	struct perf_tool *tool = session->tool;
+	struct bpf_prog_info_node *info_node;
 	struct bpf_prog_info *info;
 	struct btf *btf = NULL;
 	bool has_btf = false;
+	struct perf_env *env;
 	u32 sub_prog_cnt, i;
 	int err = 0;
 	u64 arrays;
 
+	/*
+	 * for perf-record and perf-report use header.env;
+	 * otherwise, use global perf_env.
+	 */
+	env = session->data ? &session->header.env : &perf_env;
+
 	arrays = 1UL << BPF_PROG_INFO_JITED_KSYMS;
 	arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
 	arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
 	arrays |= 1UL << BPF_PROG_INFO_PROG_TAGS;
+	arrays |= 1UL << BPF_PROG_INFO_JITED_INSNS;
+	arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
+	arrays |= 1UL << BPF_PROG_INFO_JITED_LINE_INFO;
 
 	info_linear = bpf_program__get_prog_info_linear(fd, arrays);
 	if (IS_ERR_OR_NULL(info_linear)) {
@@ -153,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 						     machine, process);
 	}
 
-	/* Synthesize PERF_RECORD_BPF_EVENT */
 	if (opts->bpf_event) {
+		/* Synthesize PERF_RECORD_BPF_EVENT */
 		*bpf_event = (struct bpf_event){
 			.header = {
 				.type = PERF_RECORD_BPF_EVENT,
@@ -167,6 +179,24 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_session *session,
 		memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
 		memset((void *)event + event->header.size, 0, machine->id_hdr_size);
 		event->header.size += machine->id_hdr_size;
+
+		/* save bpf_prog_info to env */
+		info_node = malloc(sizeof(struct bpf_prog_info_node));
+
+		/*
+		 * Do not bail out for !info_node, as we still want to
+		 * call  perf_tool__process_synth_event()
+		 */
+		if (info_node) {
+			info_node->info_linear = info_linear;
+			perf_env__insert_bpf_prog_info(env, info_node);
+			info_linear = NULL;
+		}
+
+		/*
+		 * process after saving bpf_prog_info to env, so that
+		 * required information is ready for look up
+		 */
 		err = perf_tool__process_synth_event(tool, event,
 						     machine, process);
 	}
diff --git a/tools/perf/util/bpf-event.h b/tools/perf/util/bpf-event.h
index 6698683612a7..fad932f7404f 100644
--- a/tools/perf/util/bpf-event.h
+++ b/tools/perf/util/bpf-event.h
@@ -3,14 +3,19 @@
 #define __PERF_BPF_EVENT_H
 
 #include <linux/compiler.h>
+#include <linux/rbtree.h>
 #include "event.h"
 
 struct machine;
 union perf_event;
 struct perf_sample;
-struct perf_tool;
 struct record_opts;
 
+struct bpf_prog_info_node {
+	struct bpf_prog_info_linear	*info_linear;
+	struct rb_node			rb_node;
+};
+
 #ifdef HAVE_LIBBPF_SUPPORT
 int machine__process_bpf_event(struct machine *machine, union perf_event *event,
 			       struct perf_sample *sample);
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 4c23779e271a..650c14ad7e9b 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -3,15 +3,93 @@
 #include "env.h"
 #include "sane_ctype.h"
 #include "util.h"
+#include "bpf-event.h"
 #include <errno.h>
 #include <sys/utsname.h>
+#include <bpf/libbpf.h>
 
 struct perf_env perf_env;
 
+void perf_env__insert_bpf_prog_info(struct perf_env *env,
+				    struct bpf_prog_info_node *info_node)
+{
+	__u32 prog_id = info_node->info_linear->info.id;
+	struct bpf_prog_info_node *node;
+	struct rb_node *parent = NULL;
+	struct rb_node **p;
+
+	down_write(&env->bpf_progs.lock);
+	p = &env->bpf_progs.prog_infos.rb_node;
+
+	while (*p != NULL) {
+		parent = *p;
+		node = rb_entry(parent, struct bpf_prog_info_node, rb_node);
+		if (prog_id < node->info_linear->info.id) {
+			p = &(*p)->rb_left;
+		} else if (prog_id > node->info_linear->info.id) {
+			p = &(*p)->rb_right;
+		} else {
+			pr_debug("duplicated bpf prog info %u\n", prog_id);
+			goto out;
+		}
+	}
+
+	rb_link_node(&info_node->rb_node, parent, p);
+	rb_insert_color(&info_node->rb_node, &env->bpf_progs.prog_infos);
+out:
+	up_write(&env->bpf_progs.lock);
+}
+
+struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
+							__u32 prog_id)
+{
+	struct bpf_prog_info_node *node = NULL;
+	struct rb_node *n;
+
+	down_read(&env->bpf_progs.lock);
+	n = env->bpf_progs.prog_infos.rb_node;
+
+	while (n) {
+		node = rb_entry(n, struct bpf_prog_info_node, rb_node);
+		if (prog_id < node->info_linear->info.id)
+			n = n->rb_left;
+		else if (prog_id > node->info_linear->info.id)
+			n = n->rb_right;
+		else
+			break;
+	}
+
+	up_read(&env->bpf_progs.lock);
+	return node;
+}
+
+/* purge data in bpf_prog_infos tree */
+static void perf_env__purge_bpf(struct perf_env *env)
+{
+	struct rb_root *root;
+	struct rb_node *next;
+
+	down_write(&env->bpf_progs.lock);
+
+	root = &env->bpf_progs.prog_infos;
+	next = rb_first(root);
+
+	while (next) {
+		struct bpf_prog_info_node *node;
+
+		node = rb_entry(next, struct bpf_prog_info_node, rb_node);
+		next = rb_next(&node->rb_node);
+		rb_erase_init(&node->rb_node, root);
+		free(node);
+	}
+	up_write(&env->bpf_progs.lock);
+}
+
 void perf_env__exit(struct perf_env *env)
 {
 	int i;
 
+	perf_env__purge_bpf(env);
 	zfree(&env->hostname);
 	zfree(&env->os_release);
 	zfree(&env->version);
@@ -38,6 +116,12 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->memory_nodes);
 }
 
+static void init_bpf_rb_trees(struct perf_env *env)
+{
+	env->bpf_progs.prog_infos = RB_ROOT;
+	init_rwsem(&env->bpf_progs.lock);
+}
+
 int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[])
 {
 	int i;
@@ -59,6 +143,7 @@ int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[])
 
 	env->nr_cmdline = argc;
 
+	init_bpf_rb_trees(env);
 	return 0;
 out_free:
 	zfree(&env->cmdline_argv);
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index d01b8355f4ca..33ef4b2d2a29 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -3,7 +3,9 @@
 #define __PERF_ENV_H
 
 #include <linux/types.h>
+#include <linux/rbtree.h>
 #include "cpumap.h"
+#include "rwsem.h"
 
 struct cpu_topology_map {
 	int	socket_id;
@@ -64,8 +66,19 @@ struct perf_env {
 	struct memory_node	*memory_nodes;
 	unsigned long long	 memory_bsize;
 	u64                     clockid_res_ns;
+
+	/*
+	 * bpf_info_lock protects bpf rbtrees. This is needed because the
+	 * trees are accessed by different threads in perf-top
+	 */
+	struct {
+		struct rw_semaphore	lock;
+		struct rb_root		prog_infos;
+	} bpf_progs;
 };
 
+struct bpf_prog_info_node;
+
 extern struct perf_env perf_env;
 
 void perf_env__exit(struct perf_env *env);
@@ -80,4 +93,8 @@ const char *perf_env__arch(struct perf_env *env);
 const char *perf_env__raw_arch(struct perf_env *env);
 int perf_env__nr_cpus_avail(struct perf_env *env);
 
+void perf_env__insert_bpf_prog_info(struct perf_env *env,
+				    struct bpf_prog_info_node *info_node);
+struct bpf_prog_info_node *perf_env__find_bpf_prog_info(struct perf_env *env,
+							__u32 prog_id);
 #endif /* __PERF_ENV_H */
-- 
2.17.1


^ permalink raw reply related

* Re: [net-next 01/16] ice: Mark extack argument as __always_unused
From: Stephen Hemminger @ 2019-02-26  0:36 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Bruce Allan, netdev, nhorman, sassmann,
	Anirudh Venkataramanan, Andrew Bowers
In-Reply-To: <20190225184306.13505-2-jeffrey.t.kirsher@intel.com>

On Mon, 25 Feb 2019 10:42:51 -0800
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:

> From: Bruce Allan <bruce.w.allan@intel.com>
> 
> Commit 87b0984ebfab ("net: Add extack argument to ndo_fdb_add()") in
> net-next added an extended parameter to the .ndo_fdb_add op and changed
> ice_fdb_add() accordingly. Update the function header and add the
> __always_unused attribute to the new parameter to avoid -Wunused-parameter
> warnings.
> 
> Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 48f033928aa2..9d266d754445 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -2435,11 +2435,12 @@ static void ice_set_rx_mode(struct net_device *netdev)
>   * @addr: the MAC address entry being added
>   * @vid: VLAN id
>   * @flags: instructions from stack about fdb operation
> + * @extack: netlink extended ack
>   */
> -static int ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
> -		       struct net_device *dev, const unsigned char *addr,
> -		       u16 vid, u16 flags,
> -		       struct netlink_ext_ack *extack)
> +static int
> +ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],
> +	    struct net_device *dev, const unsigned char *addr, u16 vid,
> +	    u16 flags, struct netlink_ext_ack __always_unused *extack)
>  {
>  	int err;
>  

There are several log messages in ice_fdb_add, why don't you convert those
to be user friendly and use the extended ack mechanism?

^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Stephen Hemminger @ 2019-02-26  0:38 UTC (permalink / raw)
  To: Leslie Monis
  Cc: davem, netdev, Mohit P . Tahiliani, Dave Taht, Jamal Hadi Salim
In-Reply-To: <20190225191001.26797-1-lesliemonis@gmail.com>

On Tue, 26 Feb 2019 00:39:54 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> The current implementation of the PIE queuing discipline is according to the
> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> However, a lot of necessary modifications and enhancements have been proposed
> in RFC 8033, which have not yet been incorporated in the source code of Linux.
> This patch series helps in achieving the same.
> 
> Performance tests carried out using Flent [https://flent.org/]
> 
> Changes from v2 to v3:
>   - Used div_u64() instead of direct division after explicit type casting as
>     recommended by David
> 
> Changes from v1 to v2:
>   - Excluded the patch setting PIE dynamically active/inactive as the test
>     results were unsatisfactory
>   - Fixed a scaling issue when adding more auto-tuning cases which caused
>     local variables to underflow
>   - Changed the long if/else chain to a loop as suggested by Stephen
>   - Changed the position of the accu_prob variable in the pie_vars
>     structure as recommended by Stephen
> 
> Mohit P. Tahiliani (7):
>   net: sched: pie: change value of QUEUE_THRESHOLD
>   net: sched: pie: change default value of pie_params->target
>   net: sched: pie: change default value of pie_params->tupdate
>   net: sched: pie: change initial value of pie_vars->burst_time
>   net: sched: pie: add more cases to auto-tune alpha and beta
>   net: sched: pie: add derandomization mechanism
>   net: sched: pie: update references
> 
>  include/uapi/linux/pkt_sched.h |   2 +-
>  net/sched/sch_pie.c            | 107 ++++++++++++++++++++-------------
>  2 files changed, 66 insertions(+), 43 deletions(-)

Are you concerned at all that changes to default values might change
expected behavior of existing users?

^ permalink raw reply

* Re: [PATCH v3 bpf-next 0/4] bpf: per program stats
From: Andrii Nakryiko @ 2019-02-26  0:41 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, Daniel Borkmann, edumazet, netdev, bpf, Kernel Team
In-Reply-To: <20190225222842.2031962-1-ast@kernel.org>

On Mon, Feb 25, 2019 at 2:29 PM Alexei Starovoitov <ast@kernel.org> wrote:
>
> Introduce per program stats to monitor the usage BPF
>
> v2->v3:
> - rename to run_time_ns/run_cnt everywhere
>
> v1->v2:
> - fixed u64 stats on 32-bit archs. Thanks Eric
> - use more verbose run_time_ns in json output as suggested by Andrii
> - refactored prog_alloc and clarified behavior of stats in subprogs
>
> Alexei Starovoitov (4):
>   bpf: enable program stats
>   bpf: expose program stats via bpf_prog_info
>   tools/bpf: sync bpf.h into tools
>   tools/bpftool: recognize bpf_prog_info run_time_ns and run_cnt

Lgtm, thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>

>
>  include/linux/bpf.h                           |  9 +++++
>  include/linux/filter.h                        | 20 +++++++++-
>  include/uapi/linux/bpf.h                      |  2 +
>  kernel/bpf/core.c                             | 31 ++++++++++++++-
>  kernel/bpf/syscall.c                          | 39 ++++++++++++++++++-
>  kernel/bpf/verifier.c                         |  7 +++-
>  kernel/sysctl.c                               | 34 ++++++++++++++++
>  .../bpftool/Documentation/bpftool-prog.rst    |  4 +-
>  tools/bpf/bpftool/prog.c                      |  7 ++++
>  tools/include/uapi/linux/bpf.h                |  2 +
>  10 files changed, 148 insertions(+), 7 deletions(-)
>
> --
> 2.20.0
>

^ permalink raw reply

* [PATCH net-next] net: hns: use struct_size() in devm_kzalloc()
From: Gustavo A. R. Silva @ 2019-02-26  0:27 UTC (permalink / raw)
  To: Yisen Zhuang, Salil Mehta, David S. Miller
  Cc: netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

instance = devm_kzalloc(dev, sizeof(struct foo) + sizeof(struct boo) * count, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = devm_kzalloc(dev, struct_size(instance, entry, count), GFP_KERNEL);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
index 0942e4916d9d..3d07c8a7639d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -83,8 +83,9 @@ static int hns_ppe_common_get_cfg(struct dsaf_device *dsaf_dev, int comm_index)
 	else
 		ppe_num = HNS_PPE_DEBUG_NW_ENGINE_NUM;
 
-	ppe_common = devm_kzalloc(dsaf_dev->dev, sizeof(*ppe_common) +
-		ppe_num * sizeof(struct hns_ppe_cb), GFP_KERNEL);
+	ppe_common = devm_kzalloc(dsaf_dev->dev,
+				  struct_size(ppe_common, ppe_cb, ppe_num),
+				  GFP_KERNEL);
 	if (!ppe_common)
 		return -ENOMEM;
 
-- 
2.20.1


^ permalink raw reply related

* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: si-wei liu @ 2019-02-26  0:58 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Samudrala, Sridhar, Siwei Liu, Jiri Pirko, Stephen Hemminger,
	David Miller, Netdev, virtualization, virtio-dev,
	Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
	liran.alon
In-Reply-To: <20190222100753-mutt-send-email-mst@kernel.org>

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



On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
>>
>> On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
>>>
>>> On 2/21/2019 7:33 PM, si-wei liu wrote:
>>>>
>>>> On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
>>>>> On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
>>>>>> Sorry for replying to this ancient thread. There was some remaining
>>>>>> issue that I don't think the initial net_failover patch got addressed
>>>>>> cleanly, see:
>>>>>>
>>>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
>>>>>>
>>>>>> The renaming of 'eth0' to 'ens4' fails because the udev userspace was
>>>>>> not specifically writtten for such kernel automatic enslavement.
>>>>>> Specifically, if it is a bond or team, the slave would typically get
>>>>>> renamed *before* virtual device gets created, that's what udev can
>>>>>> control (without getting netdev opened early by the other part of
>>>>>> kernel) and other userspace components for e.g. initramfs,
>>>>>> init-scripts can coordinate well in between. The in-kernel
>>>>>> auto-enslavement of net_failover breaks this userspace convention,
>>>>>> which don't provides a solution if user care about consistent naming
>>>>>> on the slave netdevs specifically.
>>>>>>
>>>>>> Previously this issue had been specifically called out when IFF_HIDDEN
>>>>>> and the 1-netdev was proposed, but no one gives out a solution to this
>>>>>> problem ever since. Please share your mind how to proceed and solve
>>>>>> this userspace issue if netdev does not welcome a 1-netdev model.
>>>>> Above says:
>>>>>
>>>>>      there's no motivation in the systemd/udevd community at
>>>>>      this point to refactor the rename logic and make it work well with
>>>>>      3-netdev.
>>>>>
>>>>> What would the fix be? Skip slave devices?
>>>>>
>>>> There's nothing user can get if just skipping slave devices - the
>>>> name is still unchanged and unpredictable e.g. eth0, or eth1 the
>>>> next reboot, while the rest may conform to the naming scheme (ens3
>>>> and such). There's no way one can fix this in userspace alone - when
>>>> the failover is created the enslaved netdev was opened by the kernel
>>>> earlier than the userspace is made aware of, and there's no
>>>> negotiation protocol for kernel to know when userspace has done
>>>> initial renaming of the interface. I would expect netdev list should
>>>> at least provide the direction in general for how this can be
>>>> solved...
>
> I was just wondering what did you mean when you said
> "refactor the rename logic and make it work well with 3-netdev" -
> was there a proposal udev rejected?
No. I never believed this particular issue can be fixed in userspace 
alone. Previously someone had said it could be, but I never see any work 
or relevant discussion ever happened in various userspace communities 
(for e.g. dracut, initramfs-tools, systemd, udev, and NetworkManager). 
IMHO the root of the issue derives from the kernel, it makes more sense 
to start from netdev, work out and decide on a solution: see what can be 
done in the kernel in order to fix it, then after that engage userspace 
community for the feasibility...

> Anyway, can we write a time diagram for what happens in which order that
> leads to failure?  That would help look for triggers that we can tie
> into, or add new ones.
>

See attached diagram.

>
>
>
>
>>> Is there an issue if slave device names are not predictable? The user/admin scripts are expected
>>> to only work with the master failover device.
>> Where does this expectation come from?
>>
>> Admin users may have ethtool or tc configurations that need to deal with
>> predictable interface name. Third-party app which was built upon specifying
>> certain interface name can't be modified to chase dynamic names.
>>
>> Specifically, we have pre-canned image that uses ethtool to fine tune VF
>> offload settings post boot for specific workload. Those images won't work
>> well if the name is constantly changing just after couple rounds of live
>> migration.
> It should be possible to specify the ethtool configuration on the
> master and have it automatically propagated to the slave.
>
> BTW this is something we should look at IMHO.
I was elaborating a few examples that the expectation and assumption 
that user/admin scripts only deal with master failover device is 
incorrect. It had never been taken good care of, although I did try to 
emphasize it from the very beginning.

Basically what you said about propagating the ethtool configuration down 
to the slave is the key pursuance of 1-netdev model. However, what I am 
seeking now is any alternative that can also fix the specific udev 
rename problem, before concluding that 1-netdev is the only solution. 
Generally a 1-netdev scheme would take time to implement, while I'm 
trying to find a way out to fix this particular naming problem under 
3-netdev.

>
>>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
>>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
>>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
>>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.
>> Yes, that's one possible implementation (IMHO the key is to make 1-netdev
>> model as much transparent to a real NIC as possible, while a hidden netns is
>> just the vehicle). However, I recall there was resistance around this
>> discussion that even the concept of hiding itself is a taboo for Linux
>> netdev. I would like to summon potential alternatives before concluding
>> 1-netdev is the only solution too soon.
>>
>> Thanks,
>> -Siwei
> Your scripts would not work at all then, right?
At this point we don't claim images with such usage as SR-IOV live 
migrate-able. We would flag it as live migrate-able until this ethtool 
config issue is fully addressed and a transparent live migration 
solution emerges in upstream eventually.


Thanks,
-Siwei
>
>
>>>> -Siwei
>>>>
>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>


[-- Attachment #2: net_failover_rename_race.txt --]
[-- Type: text/plain, Size: 3587 bytes --]


  net_failover(kernel)                            |    network.service (user)    |          systemd-udevd (user)
--------------------------------------------------+------------------------------+--------------------------------------------
(standby virtio-net and net_failover              |                              |
devices created and initialized,                  |                              |
i.e. virtnet_probe()->                            |                              |
       net_failover_create()                      |                              |
was done.)                                        |                              |
                                                  |                              |
                                                  |  runs `ifup ens3' ->         |
                                                  |    ip link set dev ens3 up   |
net_failover_open()                               |                              |
  dev_open(virtnet_dev)                           |                              |
    virtnet_open(virtnet_dev)                     |                              |
  netif_carrier_on(failover_dev)                  |                              |
  ...                                             |                              |
                                                  |                              |
(VF hot plugged in)                               |                              |
ixgbevf_probe()                                   |                              |
 register_netdev(ixgbevf_netdev)                  |                              |
  netdev_register_kobject(ixgbevf_netdev)         |                              |
   kobject_add(ixgbevf_dev)                       |                              |
    device_add(ixgbevf_dev)                       |                              |
     kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) |                              |
      netlink_broadcast()                         |                              |
  ...                                             |                              |
  call_netdevice_notifiers(NETDEV_REGISTER)       |                              |
   failover_event(..., NETDEV_REGISTER, ...)      |                              |
    failover_slave_register(ixgbevf_netdev)       |                              |
     net_failover_slave_register(ixgbevf_netdev)  |                              |
      dev_open(ixgbevf_netdev)                    |                              |
                                                  |                              |
                                                  |                              |
                                                  |                              |   received ADD uevent from netlink fd
                                                  |                              |   ...
                                                  |                              |   udev-builtin-net_id.c:dev_pci_slot()
                                                  |                              |   (decided to renamed 'eth0' )
                                                  |                              |     ip link set dev eth0 name ens4
(dev_change_name() returns -EBUSY as              |                              |
ixgbevf_netdev->flags has IFF_UP)                 |                              |
                                                  |                              |


^ permalink raw reply

* Re: [PATCH net-next v3 0/7] net: sched: pie: align PIE implementation with RFC 8033
From: Dave Taht @ 2019-02-26  1:02 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Leslie Monis, David S. Miller, Linux Kernel Network Developers,
	Mohit P . Tahiliani, Jamal Hadi Salim
In-Reply-To: <20190225163811.4a6b477b@shemminger-XPS-13-9360>

On Mon, Feb 25, 2019 at 4:38 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Tue, 26 Feb 2019 00:39:54 +0530
> Leslie Monis <lesliemonis@gmail.com> wrote:
>
> > The current implementation of the PIE queuing discipline is according to the
> > IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> > [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> > However, a lot of necessary modifications and enhancements have been proposed
> > in RFC 8033, which have not yet been incorporated in the source code of Linux.
> > This patch series helps in achieving the same.
> >
> > Performance tests carried out using Flent [https://flent.org/]
> >
> > Changes from v2 to v3:
> >   - Used div_u64() instead of direct division after explicit type casting as
> >     recommended by David
> >
> > Changes from v1 to v2:
> >   - Excluded the patch setting PIE dynamically active/inactive as the test
> >     results were unsatisfactory
> >   - Fixed a scaling issue when adding more auto-tuning cases which caused
> >     local variables to underflow
> >   - Changed the long if/else chain to a loop as suggested by Stephen
> >   - Changed the position of the accu_prob variable in the pie_vars
> >     structure as recommended by Stephen
> >
> > Mohit P. Tahiliani (7):
> >   net: sched: pie: change value of QUEUE_THRESHOLD
> >   net: sched: pie: change default value of pie_params->target
> >   net: sched: pie: change default value of pie_params->tupdate
> >   net: sched: pie: change initial value of pie_vars->burst_time
> >   net: sched: pie: add more cases to auto-tune alpha and beta
> >   net: sched: pie: add derandomization mechanism
> >   net: sched: pie: update references
> >
> >  include/uapi/linux/pkt_sched.h |   2 +-
> >  net/sched/sch_pie.c            | 107 ++++++++++++++++++++-------------
> >  2 files changed, 66 insertions(+), 43 deletions(-)
>
> Are you concerned at all that changes to default values might change
> expected behavior of existing users?

There's existing users?

Most of these changes are really subtle and came out as we went along.
I'd have to *really search* to find my notes from that period,
and I have not evaluated the sum of these new changes, but overall
they were a slight improvement from the defaults as we shipped it way
back when.

I'm not being snarky - are there existing users? Every last person I
know that ever evaluated pie switched to fq_codel. Cisco ended up
going with another bufferbloat-fighting solution in one new device and
that team dissolved.

docsis-pie, well, that's a standard. and that's a bit different from
this pie, but this pie is closer to that pie. And it works ok.

I certainly hope leslie's group has done similar testing to what we
did then ? If there's results I can dig up my old results and see what
compares. I imagine they plan a paper. our old flent test code and
data is all published, but am not willing to spend another minute of
my life on a single queued aqm design unless someone writes me a very
big check for it, which I'd then spend on fixing P4 to run fq-codel.

fq-pie (which is waiting in the wings behind this set of patches) is
ok, as at least, the BSD version was competetive with fq_codel in most
respects. I told 'em they needed to look hard at the rate estimator.

The ECN handling problem mentioned is there on all known versions of
pie, but it's that it reverts to drop too soon for ecn to be
as much benefit as it could be.

and then l4s, which is the subject of 3 upcoming talks at netdevconf,
well... not gonna talk about it. I'm willing to give them their day in
court.

sorry to come across as grumpy. should linux be rfc compliant even if
the rfc has issues? damned if I know.



-- 

Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-205-9740

^ permalink raw reply

* Re: [PATCH net-next] can: kvaser_usb: Use struct_size() in alloc_candev()
From: Gustavo A. R. Silva @ 2019-02-26  0:48 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller
  Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190208031035.GA2665@embeddedor>

Hi all,

Friendly ping:

Who can take this?

Thanks
--
Gustavo

On 2/7/19 9:10 PM, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     void *entry[];
> };
> 
> instance = alloc(sizeof(struct foo) + count * sizeof(void *));
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> instance = alloc(struct_size(instance, entry, count));
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
> index c89c7d4900d7..0f1d3e807d63 100644
> --- a/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
> +++ b/drivers/net/can/usb/kvaser_usb/kvaser_usb_core.c
> @@ -643,8 +643,7 @@ static int kvaser_usb_init_one(struct kvaser_usb *dev,
>  			return err;
>  	}
>  
> -	netdev = alloc_candev(sizeof(*priv) +
> -			      dev->max_tx_urbs * sizeof(*priv->tx_contexts),
> +	netdev = alloc_candev(struct_size(priv, tx_contexts, dev->max_tx_urbs),
>  			      dev->max_tx_urbs);
>  	if (!netdev) {
>  		dev_err(&dev->intf->dev, "Cannot alloc candev\n");
> 

^ permalink raw reply

* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Martin Lau @ 2019-02-26  1:30 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Daniel Borkmann,
	Eric Dumazet, Kernel Team
In-Reply-To: <20190225231438.GC32115@mini-arch>

On Mon, Feb 25, 2019 at 03:14:38PM -0800, Stanislav Fomichev wrote:
[ ... ]

> > 
> > To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
> > attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
> > during load time if the prog uses this new helper.
> > The newly added prog->enforce_expected_attach_type bit will also be set
> > if this new helper is used.  This bit is for backward compatibility reason
> > because currently prog->expected_attach_type has been ignored in
> > BPF_PROG_TYPE_CGROUP_SKB.  During attach time,
> > prog->expected_attach_type is only enforced if the
> > prog->enforce_expected_attach_type bit is set.
> > i.e. prog->expected_attach_type is only enforced if this new helper
> > is used by the prog.
> > 
[ ... ]

> > @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> >  	case BPF_PROG_TYPE_CGROUP_SOCK:
> >  	case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> >  		return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> > +	case BPF_PROG_TYPE_CGROUP_SKB:
> > +		return prog->enforce_expected_attach_type &&
> > +			prog->expected_attach_type != attach_type ?
> > +			-EINVAL : 0;
> >  	default:
> >  		return 0;
> >  	}
[ ... ]

> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 97916eedfe69..ca57ef25279c 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> >  	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
> >  };
> >  
> > +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
> > +{
> > +	struct sock *sk = (struct sock *)tp;
> > +
> > +	if (sk->sk_state == TCP_ESTABLISHED) {
> > +		tcp_enter_cwr(sk);
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> > +	.func        = bpf_tcp_enter_cwr,
> > +	.gpl_only    = false,
> > +	.ret_type    = RET_INTEGER,
> > +	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
> > +};
> >  #endif /* CONFIG_INET */
> >  
> >  bool bpf_helper_changes_pkt_data(void *func)
> > @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> >  #ifdef CONFIG_INET
> >  	case BPF_FUNC_tcp_sock:
> >  		return &bpf_tcp_sock_proto;
> 
> [...]
> > +	case BPF_FUNC_tcp_enter_cwr:
> > +		if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> > +			prog->enforce_expected_attach_type = 1;
> > +			return &bpf_tcp_enter_cwr_proto;
> Instead of this back and forth with enforce_expected_attach_type, can we
> just do here:
> 
> if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
> 	return &bpf_tcp_enter_cwr_proto;
> else
> 	return null;
> 
> Wouldn't it have the same effect?
The attr->expected_attach_type is currently ignored (i.e. not checked)
during the bpf load time.

How to avoid breaking backward compatibility without selectively
enforcing prog->expected_attach_type during attach time?

^ permalink raw reply

* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Stephen Hemminger @ 2019-02-26  1:39 UTC (permalink / raw)
  To: si-wei liu
  Cc: Michael S. Tsirkin, Samudrala, Sridhar, Siwei Liu, Jiri Pirko,
	David Miller, Netdev, virtualization, virtio-dev,
	Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
	liran.alon
In-Reply-To: <e6a53bd1-83ab-f170-406a-03276e8c87e2@oracle.com>

On Mon, 25 Feb 2019 16:58:07 -0800
si-wei liu <si-wei.liu@oracle.com> wrote:

> On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> > On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:  
> >>
> >> On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:  
> >>>
> >>> On 2/21/2019 7:33 PM, si-wei liu wrote:  
> >>>>
> >>>> On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:  
> >>>>> On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:  
> >>>>>> Sorry for replying to this ancient thread. There was some remaining
> >>>>>> issue that I don't think the initial net_failover patch got addressed
> >>>>>> cleanly, see:
> >>>>>>
> >>>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
> >>>>>>
> >>>>>> The renaming of 'eth0' to 'ens4' fails because the udev userspace was
> >>>>>> not specifically writtten for such kernel automatic enslavement.
> >>>>>> Specifically, if it is a bond or team, the slave would typically get
> >>>>>> renamed *before* virtual device gets created, that's what udev can
> >>>>>> control (without getting netdev opened early by the other part of
> >>>>>> kernel) and other userspace components for e.g. initramfs,
> >>>>>> init-scripts can coordinate well in between. The in-kernel
> >>>>>> auto-enslavement of net_failover breaks this userspace convention,
> >>>>>> which don't provides a solution if user care about consistent naming
> >>>>>> on the slave netdevs specifically.
> >>>>>>
> >>>>>> Previously this issue had been specifically called out when IFF_HIDDEN
> >>>>>> and the 1-netdev was proposed, but no one gives out a solution to this
> >>>>>> problem ever since. Please share your mind how to proceed and solve
> >>>>>> this userspace issue if netdev does not welcome a 1-netdev model.  
> >>>>> Above says:
> >>>>>
> >>>>>      there's no motivation in the systemd/udevd community at
> >>>>>      this point to refactor the rename logic and make it work well with
> >>>>>      3-netdev.
> >>>>>
> >>>>> What would the fix be? Skip slave devices?
> >>>>>  
> >>>> There's nothing user can get if just skipping slave devices - the
> >>>> name is still unchanged and unpredictable e.g. eth0, or eth1 the
> >>>> next reboot, while the rest may conform to the naming scheme (ens3
> >>>> and such). There's no way one can fix this in userspace alone - when
> >>>> the failover is created the enslaved netdev was opened by the kernel
> >>>> earlier than the userspace is made aware of, and there's no
> >>>> negotiation protocol for kernel to know when userspace has done
> >>>> initial renaming of the interface. I would expect netdev list should
> >>>> at least provide the direction in general for how this can be
> >>>> solved...  
> >
> > I was just wondering what did you mean when you said
> > "refactor the rename logic and make it work well with 3-netdev" -
> > was there a proposal udev rejected?  
> No. I never believed this particular issue can be fixed in userspace 
> alone. Previously someone had said it could be, but I never see any work 
> or relevant discussion ever happened in various userspace communities 
> (for e.g. dracut, initramfs-tools, systemd, udev, and NetworkManager). 
> IMHO the root of the issue derives from the kernel, it makes more sense 
> to start from netdev, work out and decide on a solution: see what can be 
> done in the kernel in order to fix it, then after that engage userspace 
> community for the feasibility...
> 
> > Anyway, can we write a time diagram for what happens in which order that
> > leads to failure?  That would help look for triggers that we can tie
> > into, or add new ones.
> >  
> 
> See attached diagram.
> 
> >
> >
> >
> >  
> >>> Is there an issue if slave device names are not predictable? The user/admin scripts are expected
> >>> to only work with the master failover device.  
> >> Where does this expectation come from?
> >>
> >> Admin users may have ethtool or tc configurations that need to deal with
> >> predictable interface name. Third-party app which was built upon specifying
> >> certain interface name can't be modified to chase dynamic names.
> >>
> >> Specifically, we have pre-canned image that uses ethtool to fine tune VF
> >> offload settings post boot for specific workload. Those images won't work
> >> well if the name is constantly changing just after couple rounds of live
> >> migration.  
> > It should be possible to specify the ethtool configuration on the
> > master and have it automatically propagated to the slave.
> >
> > BTW this is something we should look at IMHO.  
> I was elaborating a few examples that the expectation and assumption 
> that user/admin scripts only deal with master failover device is 
> incorrect. It had never been taken good care of, although I did try to 
> emphasize it from the very beginning.
> 
> Basically what you said about propagating the ethtool configuration down 
> to the slave is the key pursuance of 1-netdev model. However, what I am 
> seeking now is any alternative that can also fix the specific udev 
> rename problem, before concluding that 1-netdev is the only solution. 
> Generally a 1-netdev scheme would take time to implement, while I'm 
> trying to find a way out to fix this particular naming problem under 
> 3-netdev.
> 
> >  
> >>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> >>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
> >>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> >>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.  
> >> Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> >> model as much transparent to a real NIC as possible, while a hidden netns is
> >> just the vehicle). However, I recall there was resistance around this
> >> discussion that even the concept of hiding itself is a taboo for Linux
> >> netdev. I would like to summon potential alternatives before concluding
> >> 1-netdev is the only solution too soon.
> >>
> >> Thanks,
> >> -Siwei  
> > Your scripts would not work at all then, right?  
> At this point we don't claim images with such usage as SR-IOV live 
> migrate-able. We would flag it as live migrate-able until this ethtool 
> config issue is fully addressed and a transparent live migration 
> solution emerges in upstream eventually.

The hyper-v netvsc with 1-dev model uses a timeout to allow  udev to do its rename.
I proposed a patch to key state change off of the udev rename, but that patch was
rejected.


^ permalink raw reply

* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Hangbin Liu @ 2019-02-26  1:43 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Stefano Brivio, David S . Miller, David Ahern
In-Reply-To: <1dd57d9d-fed2-67b8-ac28-7ef3681eeed2@gmail.com>

On Mon, Feb 25, 2019 at 08:39:27AM -0800, Eric Dumazet wrote:
> >> On 02/24/2019 08:12 PM, Hangbin Liu wrote:
> >>> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
> >>> at running time.
> >>>
> >>
> >>
> >> Why is it better exactly ?
> >>
> >> IPv6 can be enabled on the host, but disabled per device
> >>
> >> /proc/sys/net/ipv6/conf/{name}/disable_ipv6
> > 
> > Sorry, it looks I didn't make it clear in the commit description.
> > This issue only occurs when IPv6 is disabled at boot time as there is
> > no IPv6 route entry. Disable ipv6 on specific interface is not affected.
> > So check ipv6_mod_enabled() is enough and we don't need to worry about
> > the rcu_read_lock or the dev status.
> > 
> > Should I update the commit description?
> 
> Certainly. Are you telling us skb->dev could be NULL here ?
> 
> Because rcu_read_lock() should already be asserted.
> 

No. I know skb->dev is not NULL and we have rcu_read_lock() here. But can we
guarantee the skb->dev won't be NULL forever? Maybe I'm a little sensitive.

I mean for only checking if ipv6 is disable at boot time, use
ipv6_mod_enabled() is more suitable.

Thanks
Hangbin

^ permalink raw reply

* [PATCH net-next] net: remove unused struct inet_frag_queue.fragments field
From: Peter Oskolkov @ 2019-02-26  1:43 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: Peter Oskolkov, linux-wpan, Peter Oskolkov

Now that all users of struct inet_frag_queue have been converted
to use 'rb_fragments', remove the unused 'fragments' field.

Build with `make allyesconfig` succeeded. ip_defrag selftest passed.

Signed-off-by: Peter Oskolkov <posk@google.com>
---
 include/net/inet_frag.h                 |  4 +--
 net/ieee802154/6lowpan/reassembly.c     |  1 -
 net/ipv4/inet_fragment.c                | 44 ++++++++-----------------
 net/ipv4/ip_fragment.c                  |  2 --
 net/ipv6/netfilter/nf_conntrack_reasm.c |  1 -
 net/ipv6/reassembly.c                   |  1 -
 6 files changed, 14 insertions(+), 39 deletions(-)

diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index b02bf737d019..378904ee9129 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -56,7 +56,6 @@ struct frag_v6_compare_key {
  * @timer: queue expiration timer
  * @lock: spinlock protecting this frag
  * @refcnt: reference count of the queue
- * @fragments: received fragments head
  * @rb_fragments: received fragments rb-tree root
  * @fragments_tail: received fragments tail
  * @last_run_head: the head of the last "run". see ip_fragment.c
@@ -77,8 +76,7 @@ struct inet_frag_queue {
 	struct timer_list	timer;
 	spinlock_t		lock;
 	refcount_t		refcnt;
-	struct sk_buff		*fragments;  /* used in 6lopwpan IPv6. */
-	struct rb_root		rb_fragments; /* Used in IPv4/IPv6. */
+	struct rb_root		rb_fragments;
 	struct sk_buff		*fragments_tail;
 	struct sk_buff		*last_run_head;
 	ktime_t			stamp;
diff --git a/net/ieee802154/6lowpan/reassembly.c b/net/ieee802154/6lowpan/reassembly.c
index bd61633d2c32..4196bcd4105a 100644
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -179,7 +179,6 @@ static int lowpan_frag_reasm(struct lowpan_frag_queue *fq, struct sk_buff *skb,
 
 	skb->dev = ldev;
 	skb->tstamp = fq->q.stamp;
-	fq->q.fragments = NULL;
 	fq->q.rb_fragments = RB_ROOT;
 	fq->q.fragments_tail = NULL;
 	fq->q.last_run_head = NULL;
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index 9f69411251d0..737808e27f8b 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -203,7 +203,6 @@ EXPORT_SYMBOL(inet_frag_rbtree_purge);
 
 void inet_frag_destroy(struct inet_frag_queue *q)
 {
-	struct sk_buff *fp;
 	struct netns_frags *nf;
 	unsigned int sum, sum_truesize = 0;
 	struct inet_frags *f;
@@ -212,20 +211,9 @@ void inet_frag_destroy(struct inet_frag_queue *q)
 	WARN_ON(del_timer(&q->timer) != 0);
 
 	/* Release all fragment data. */
-	fp = q->fragments;
 	nf = q->net;
 	f = nf->f;
-	if (fp) {
-		do {
-			struct sk_buff *xp = fp->next;
-
-			sum_truesize += fp->truesize;
-			kfree_skb(fp);
-			fp = xp;
-		} while (fp);
-	} else {
-		sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
-	}
+	sum_truesize = inet_frag_rbtree_purge(&q->rb_fragments);
 	sum = sum_truesize + f->qsize;
 
 	call_rcu(&q->rcu, inet_frag_destroy_rcu);
@@ -489,26 +477,20 @@ EXPORT_SYMBOL(inet_frag_reasm_finish);
 
 struct sk_buff *inet_frag_pull_head(struct inet_frag_queue *q)
 {
-	struct sk_buff *head;
+	struct sk_buff *head, *skb;
 
-	if (q->fragments) {
-		head = q->fragments;
-		q->fragments = head->next;
-	} else {
-		struct sk_buff *skb;
+	head = skb_rb_first(&q->rb_fragments);
+	if (!head)
+		return NULL;
+	skb = FRAG_CB(head)->next_frag;
+	if (skb)
+		rb_replace_node(&head->rbnode, &skb->rbnode,
+				&q->rb_fragments);
+	else
+		rb_erase(&head->rbnode, &q->rb_fragments);
+	memset(&head->rbnode, 0, sizeof(head->rbnode));
+	barrier();
 
-		head = skb_rb_first(&q->rb_fragments);
-		if (!head)
-			return NULL;
-		skb = FRAG_CB(head)->next_frag;
-		if (skb)
-			rb_replace_node(&head->rbnode, &skb->rbnode,
-					&q->rb_fragments);
-		else
-			rb_erase(&head->rbnode, &q->rb_fragments);
-		memset(&head->rbnode, 0, sizeof(head->rbnode));
-		barrier();
-	}
 	if (head == q->fragments_tail)
 		q->fragments_tail = NULL;
 
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 486ecb0aeb87..cf2b0a6a3337 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -261,7 +261,6 @@ static int ip_frag_reinit(struct ipq *qp)
 	qp->q.flags = 0;
 	qp->q.len = 0;
 	qp->q.meat = 0;
-	qp->q.fragments = NULL;
 	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
 	qp->q.last_run_head = NULL;
@@ -451,7 +450,6 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *skb,
 	ip_send_check(iph);
 
 	__IP_INC_STATS(net, IPSTATS_MIB_REASMOKS);
-	qp->q.fragments = NULL;
 	qp->q.rb_fragments = RB_ROOT;
 	qp->q.fragments_tail = NULL;
 	qp->q.last_run_head = NULL;
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index cb1b4772dac0..3de0e9b0a482 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -365,7 +365,6 @@ static int nf_ct_frag6_reasm(struct frag_queue *fq, struct sk_buff *skb,
 					 skb_network_header_len(skb),
 					 skb->csum);
 
-	fq->q.fragments = NULL;
 	fq->q.rb_fragments = RB_ROOT;
 	fq->q.fragments_tail = NULL;
 	fq->q.last_run_head = NULL;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 24264d0a4b85..1a832f5e190b 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -304,7 +304,6 @@ static int ip6_frag_reasm(struct frag_queue *fq, struct sk_buff *skb,
 	rcu_read_lock();
 	__IP6_INC_STATS(net, __in6_dev_get(dev), IPSTATS_MIB_REASMOKS);
 	rcu_read_unlock();
-	fq->q.fragments = NULL;
 	fq->q.rb_fragments = RB_ROOT;
 	fq->q.fragments_tail = NULL;
 	fq->q.last_run_head = NULL;
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* Re: [PATCH] tools: testing: selftests: Remove duplicate headers
From: Michael Ellerman @ 2019-02-26  1:48 UTC (permalink / raw)
  To: Souptick Joarder, bamv2005, shuah, davem, benh, paulus, adobriyan,
	mathieu.desnoyers, peterz, paulmck, boqun.feng, john.stultz, tglx,
	sboyd, akpm
  Cc: linux-gpio, linux-kselftest, linux-kernel, netdev, linuxppc-dev,
	linux-fsdevel, sabyasachi.linux
In-Reply-To: <20190223070901.GA10274@jordon-HP-15-Notebook-PC>

Souptick Joarder <jrdr.linux@gmail.com> writes:
> Remove duplicate headers which are included twice.
>
> Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
> Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> ---
...
>  tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c | 1 -

I took this hunk via the powerpc tree.

> diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> index 167135b..af1b802 100644
> --- a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> +++ b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> @@ -11,7 +11,6 @@
>  #include <sys/wait.h>
>  #include <unistd.h>
>  #include <setjmp.h>
> -#include <signal.h>
>  
>  #include "ebb.h"


cheers

^ permalink raw reply

* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Hangbin Liu @ 2019-02-26  1:55 UTC (permalink / raw)
  To: David Ahern; +Cc: Eric Dumazet, netdev, Stefano Brivio, David S . Miller
In-Reply-To: <df9f765f-2f73-d136-e020-de1b63963535@cumulusnetworks.com>

On Mon, Feb 25, 2019 at 09:57:42AM -0700, David Ahern wrote:
> On 2/25/19 9:39 AM, Eric Dumazet wrote:
> >>> On 02/24/2019 08:12 PM, Hangbin Liu wrote:
> >>>> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
> >>>> at running time.
> >>>
> >>> Why is it better exactly ?
> >>>
> >>> IPv6 can be enabled on the host, but disabled per device
> >>>
> >>> /proc/sys/net/ipv6/conf/{name}/disable_ipv6
> >>
> >> Sorry, it looks I didn't make it clear in the commit description.
> >> This issue only occurs when IPv6 is disabled at boot time as there is
> >> no IPv6 route entry. Disable ipv6 on specific interface is not affected.
> >> So check ipv6_mod_enabled() is enough and we don't need to worry about
> >> the rcu_read_lock or the dev status.
> >>
> >> Should I update the commit description?
> > 
> > Certainly. Are you telling us skb->dev could be NULL here ?
> > 
> > Because rcu_read_lock() should already be asserted.
> > 
> 
> Same response as geneve. The existing check is more appropriate and
> relevant for the code path: is ipv6 enabled on this device versus is
> ipv6 enabled at all.

Hi David,

Just as I said, this issue only occurs when IPv6 is disabled at boot time
as there is no IPv6 route entry. Disable ipv6 on specific interface should
not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
would be more suitable in this scenario. Did I miss something?

Thanks
Hangbin

^ permalink raw reply

* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-26  2:05 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: si-wei liu, Samudrala, Sridhar, Siwei Liu, Jiri Pirko,
	David Miller, Netdev, virtualization, virtio-dev,
	Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
	liran.alon
In-Reply-To: <20190225173912.26b93422@shemminger-XPS-13-9360>

On Mon, Feb 25, 2019 at 05:39:12PM -0800, Stephen Hemminger wrote:
> > >>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > >>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > >>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > >>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.  
> > >> Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > >> model as much transparent to a real NIC as possible, while a hidden netns is
> > >> just the vehicle). However, I recall there was resistance around this
> > >> discussion that even the concept of hiding itself is a taboo for Linux
> > >> netdev. I would like to summon potential alternatives before concluding
> > >> 1-netdev is the only solution too soon.
> > >>
> > >> Thanks,
> > >> -Siwei  
> > > Your scripts would not work at all then, right?  
> > At this point we don't claim images with such usage as SR-IOV live 
> > migrate-able. We would flag it as live migrate-able until this ethtool 
> > config issue is fully addressed and a transparent live migration 
> > solution emerges in upstream eventually.
> 
> The hyper-v netvsc with 1-dev model uses a timeout to allow  udev to do its rename.
> I proposed a patch to key state change off of the udev rename, but that patch was
> rejected.

Of course that would mean nothing works without udev - was
that the objection? Could you help me find that discussion pls?

-- 
MST

^ permalink raw reply

* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Michael S. Tsirkin @ 2019-02-26  2:08 UTC (permalink / raw)
  To: si-wei liu
  Cc: Samudrala, Sridhar, Siwei Liu, Jiri Pirko, Stephen Hemminger,
	David Miller, Netdev, virtualization, virtio-dev,
	Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski, Jason Wang,
	liran.alon
In-Reply-To: <e6a53bd1-83ab-f170-406a-03276e8c87e2@oracle.com>

On Mon, Feb 25, 2019 at 04:58:07PM -0800, si-wei liu wrote:
> 
> 
> On 2/22/2019 7:14 AM, Michael S. Tsirkin wrote:
> > On Thu, Feb 21, 2019 at 11:55:11PM -0800, si-wei liu wrote:
> > > 
> > > On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
> > > > 
> > > > On 2/21/2019 7:33 PM, si-wei liu wrote:
> > > > > 
> > > > > On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
> > > > > > On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
> > > > > > > Sorry for replying to this ancient thread. There was some remaining
> > > > > > > issue that I don't think the initial net_failover patch got addressed
> > > > > > > cleanly, see:
> > > > > > > 
> > > > > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
> > > > > > > 
> > > > > > > The renaming of 'eth0' to 'ens4' fails because the udev userspace was
> > > > > > > not specifically writtten for such kernel automatic enslavement.
> > > > > > > Specifically, if it is a bond or team, the slave would typically get
> > > > > > > renamed *before* virtual device gets created, that's what udev can
> > > > > > > control (without getting netdev opened early by the other part of
> > > > > > > kernel) and other userspace components for e.g. initramfs,
> > > > > > > init-scripts can coordinate well in between. The in-kernel
> > > > > > > auto-enslavement of net_failover breaks this userspace convention,
> > > > > > > which don't provides a solution if user care about consistent naming
> > > > > > > on the slave netdevs specifically.
> > > > > > > 
> > > > > > > Previously this issue had been specifically called out when IFF_HIDDEN
> > > > > > > and the 1-netdev was proposed, but no one gives out a solution to this
> > > > > > > problem ever since. Please share your mind how to proceed and solve
> > > > > > > this userspace issue if netdev does not welcome a 1-netdev model.
> > > > > > Above says:
> > > > > > 
> > > > > >      there's no motivation in the systemd/udevd community at
> > > > > >      this point to refactor the rename logic and make it work well with
> > > > > >      3-netdev.
> > > > > > 
> > > > > > What would the fix be? Skip slave devices?
> > > > > > 
> > > > > There's nothing user can get if just skipping slave devices - the
> > > > > name is still unchanged and unpredictable e.g. eth0, or eth1 the
> > > > > next reboot, while the rest may conform to the naming scheme (ens3
> > > > > and such). There's no way one can fix this in userspace alone - when
> > > > > the failover is created the enslaved netdev was opened by the kernel
> > > > > earlier than the userspace is made aware of, and there's no
> > > > > negotiation protocol for kernel to know when userspace has done
> > > > > initial renaming of the interface. I would expect netdev list should
> > > > > at least provide the direction in general for how this can be
> > > > > solved...
> > 
> > I was just wondering what did you mean when you said
> > "refactor the rename logic and make it work well with 3-netdev" -
> > was there a proposal udev rejected?
> No. I never believed this particular issue can be fixed in userspace alone.
> Previously someone had said it could be, but I never see any work or
> relevant discussion ever happened in various userspace communities (for e.g.
> dracut, initramfs-tools, systemd, udev, and NetworkManager). IMHO the root
> of the issue derives from the kernel, it makes more sense to start from
> netdev, work out and decide on a solution: see what can be done in the
> kernel in order to fix it, then after that engage userspace community for
> the feasibility...
> 
> > Anyway, can we write a time diagram for what happens in which order that
> > leads to failure?  That would help look for triggers that we can tie
> > into, or add new ones.
> > 
> 
> See attached diagram.
> 
> > 
> > 
> > 
> > 
> > > > Is there an issue if slave device names are not predictable? The user/admin scripts are expected
> > > > to only work with the master failover device.
> > > Where does this expectation come from?
> > > 
> > > Admin users may have ethtool or tc configurations that need to deal with
> > > predictable interface name. Third-party app which was built upon specifying
> > > certain interface name can't be modified to chase dynamic names.
> > > 
> > > Specifically, we have pre-canned image that uses ethtool to fine tune VF
> > > offload settings post boot for specific workload. Those images won't work
> > > well if the name is constantly changing just after couple rounds of live
> > > migration.
> > It should be possible to specify the ethtool configuration on the
> > master and have it automatically propagated to the slave.
> > 
> > BTW this is something we should look at IMHO.
> I was elaborating a few examples that the expectation and assumption that
> user/admin scripts only deal with master failover device is incorrect. It
> had never been taken good care of, although I did try to emphasize it from
> the very beginning.
> 
> Basically what you said about propagating the ethtool configuration down to
> the slave is the key pursuance of 1-netdev model. However, what I am seeking
> now is any alternative that can also fix the specific udev rename problem,
> before concluding that 1-netdev is the only solution. Generally a 1-netdev
> scheme would take time to implement, while I'm trying to find a way out to
> fix this particular naming problem under 3-netdev.
> 
> > 
> > > > Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
> > > > about moving them to a hidden network namespace so that they are not visible from the default namespace.
> > > > I looked into this sometime back, but did not find the right kernel api to create a network namespace within
> > > > kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> > > Yes, that's one possible implementation (IMHO the key is to make 1-netdev
> > > model as much transparent to a real NIC as possible, while a hidden netns is
> > > just the vehicle). However, I recall there was resistance around this
> > > discussion that even the concept of hiding itself is a taboo for Linux
> > > netdev. I would like to summon potential alternatives before concluding
> > > 1-netdev is the only solution too soon.
> > > 
> > > Thanks,
> > > -Siwei
> > Your scripts would not work at all then, right?
> At this point we don't claim images with such usage as SR-IOV live
> migrate-able. We would flag it as live migrate-able until this ethtool
> config issue is fully addressed and a transparent live migration solution
> emerges in upstream eventually.
> 
> 
> Thanks,
> -Siwei
> > 
> > 
> > > > > -Siwei
> > > > > 
> > > > > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > 
> 

> 
>   net_failover(kernel)                            |    network.service (user)    |          systemd-udevd (user)
> --------------------------------------------------+------------------------------+--------------------------------------------
> (standby virtio-net and net_failover              |                              |
> devices created and initialized,                  |                              |
> i.e. virtnet_probe()->                            |                              |
>        net_failover_create()                      |                              |
> was done.)                                        |                              |
>                                                   |                              |
>                                                   |  runs `ifup ens3' ->         |
>                                                   |    ip link set dev ens3 up   |
> net_failover_open()                               |                              |
>   dev_open(virtnet_dev)                           |                              |
>     virtnet_open(virtnet_dev)                     |                              |
>   netif_carrier_on(failover_dev)                  |                              |
>   ...                                             |                              |
>                                                   |                              |
> (VF hot plugged in)                               |                              |
> ixgbevf_probe()                                   |                              |
>  register_netdev(ixgbevf_netdev)                  |                              |
>   netdev_register_kobject(ixgbevf_netdev)         |                              |
>    kobject_add(ixgbevf_dev)                       |                              |
>     device_add(ixgbevf_dev)                       |                              |
>      kobject_uevent(&ixgbevf_dev->kobj, KOBJ_ADD) |                              |
>       netlink_broadcast()                         |                              |
>   ...                                             |                              |
>   call_netdevice_notifiers(NETDEV_REGISTER)       |                              |
>    failover_event(..., NETDEV_REGISTER, ...)      |                              |
>     failover_slave_register(ixgbevf_netdev)       |                              |
>      net_failover_slave_register(ixgbevf_netdev)  |                              |
>       dev_open(ixgbevf_netdev)                    |                              |
>                                                   |                              |
>                                                   |                              |
>                                                   |                              |   received ADD uevent from netlink fd
>                                                   |                              |   ...
>                                                   |                              |   udev-builtin-net_id.c:dev_pci_slot()
>                                                   |                              |   (decided to renamed 'eth0' )
>                                                   |                              |     ip link set dev eth0 name ens4
> (dev_change_name() returns -EBUSY as              |                              |
> ixgbevf_netdev->flags has IFF_UP)                 |                              |
>                                                   |                              |
> 

Given renaming slaves does not work anyway: would it work if we just
hard-coded slave names instead?

E.g.
1. fail slave renames
2. rename of failover to XX automatically renames standby to XXnsby
   and primary to XXnpry


-- 
MST

^ permalink raw reply

* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: David Ahern @ 2019-02-26  2:15 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: Eric Dumazet, netdev, Stefano Brivio, David S . Miller
In-Reply-To: <20190226015533.GR10051@dhcp-12-139.nay.redhat.com>

On 2/25/19 6:55 PM, Hangbin Liu wrote:
> Just as I said, this issue only occurs when IPv6 is disabled at boot time
> as there is no IPv6 route entry. Disable ipv6 on specific interface should
> not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
> would be more suitable in this scenario. Did I miss something?

From a readability perspective the code path depends on whether ipv6 is
enabled on the device. I think it is better to leave that as it is.

^ permalink raw reply

* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: Hangbin Liu @ 2019-02-26  2:17 UTC (permalink / raw)
  To: David Ahern; +Cc: Sabrina Dubroca, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <212750de-2d6a-ebb9-5079-bacf234f6cf6@gmail.com>

On Mon, Feb 25, 2019 at 11:46:51AM -0700, David Ahern wrote:
> On 2/25/19 4:14 AM, Sabrina Dubroca wrote:
> > 2019-02-25, 15:47:00 +0800, Hangbin Liu wrote:
> >> @@ -14,6 +15,7 @@ int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
> >>  	case IPPROTO_TCP:
> >>  	case IPPROTO_UDP:
> >>  	case IPPROTO_ICMP:
> >> +	case IPPROTO_ICMPV6:
> > 
> > Is IPPROTO_ICMPV6 supposed to be valid in the IPv4 code path calling
> > this function? 
> 
> no. I do not see how that makes sense.

I also thought about this issue. Currently we didn't check the ipproto in both
IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
This looks don't make any serious problem. It's just a user mis-configuration,
the kernel check the proto number and won't match normal IP/IPv6 headers.

But yes, we should make it more strict, do you think if I should add a new
rtm_getroute_parse_ip6_proto() function, or just add a family parameter
in previous function?

The #if IS_ENABLED(CONFIG_IPV6) guardian makes sense. I will fix it.

Thanks
Hangbin

^ permalink raw reply

* Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto
From: David Ahern @ 2019-02-26  2:23 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: Sabrina Dubroca, netdev, Roopa Prabhu, David S . Miller
In-Reply-To: <20190226021739.GS10051@dhcp-12-139.nay.redhat.com>

On 2/25/19 7:17 PM, Hangbin Liu wrote:
> I also thought about this issue. Currently we didn't check the ipproto in both
> IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
> This looks don't make any serious problem. It's just a user mis-configuration,
> the kernel check the proto number and won't match normal IP/IPv6 headers.
> 
> But yes, we should make it more strict, do you think if I should add a new
> rtm_getroute_parse_ip6_proto() function, or just add a family parameter
> in previous function?

I see now. rtm_getroute_parse_ip_proto is used for ipv4 and ipv6. For v4
IPPROTO_ICMPV6 should not be allowed and for v6 IPPROTO_ICMP should
fail. You could a version argument to rtm_getroute_parse_ip_proto and
fail as needed.

^ permalink raw reply

* Re: [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jakub Kicinski @ 2019-02-26  3:07 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190225103127.4f17f900@cakuba.netronome.com>

On Mon, 25 Feb 2019 10:31:27 -0800, Jakub Kicinski wrote:
> +static bool devlink_is_registered(struct devlink *devlink)
> +{
> +       return list_empty(&devlink->list);
> +}

Nevermind, this won't really help the drivers that much, those
registering devlink after netdevs will have to take care of the
potential race by marking the devlink instance as dead in their priv
data structure from the thread doing the devlink_unregister, using
their own locking.

^ permalink raw reply

* Re: [PATCH v3 bpf-next 1/4] bpf: enable program stats
From: Stanislav Fomichev @ 2019-02-26  3:10 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, davem@davemloft.net, daniel@iogearbox.net,
	edumazet@google.com, netdev@vger.kernel.org, bpf@vger.kernel.org,
	Kernel Team
In-Reply-To: <2eeb7f8d-d184-07d1-2b7b-76c93b4b1bfe@fb.com>

On 02/25, Alexei Starovoitov wrote:
> On 2/25/19 3:07 PM, Stanislav Fomichev wrote:
> >> +#define BPF_PROG_RUN(prog, ctx)	({				\
> >> +	u32 ret;						\
> >> +	cant_sleep();						\
> >> +	if (static_branch_unlikely(&bpf_stats_enabled_key)) {	\
> >> +		struct bpf_prog_stats *stats;			\
> >> +		u64 start = sched_clock();			\
> > QQ: why sched_clock() and not, for example, ktime_get_ns() which we do
> > in the bpf_test_run()? Or even why not local_clock?
> > I'm just wondering what king of trade off we are doing here
> > regarding precision vs run time cost.
> 
> 
> I'm making this decision based on documentation:
> Documentation/timers/timekeeping.txt
> "Compared to clock sources, sched_clock() has to be very fast: it is 
> called much more often, especially by the scheduler. If you have to do 
> trade-offs between accuracy compared to the clock source, you may 
> sacrifice accuracy for speed in sched_clock()."
So sched_clock is fast, but imprecise; and ktime_get_ns (and
lock_clock?) are slow(er), but more precise?

If that's the case, would it make sense to use a more precise
measurement? I suppose the BPF program execution time is on the order of
nanoseconds and if sched_close has msec or usec resolution, all we get is
essentially noise?

I understand that you want this feature to have almost no overhead, but
since it's gated by the static key, should we aim for a higher precision?

^ permalink raw reply

* Re: [PATCH v2 bpf-next 2/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: Stanislav Fomichev @ 2019-02-26  3:32 UTC (permalink / raw)
  To: Martin Lau
  Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Daniel Borkmann,
	Eric Dumazet, Kernel Team
In-Reply-To: <20190226012959.4gttysrj3khqumc5@kafai-mbp.dhcp.thefacebook.com>

On 02/26, Martin Lau wrote:
> On Mon, Feb 25, 2019 at 03:14:38PM -0800, Stanislav Fomichev wrote:
> [ ... ]
> 
> > > 
> > > To ensure it is only called from BPF_CGROUP_INET_EGRESS, the
> > > attr->expected_attach_type must be specified as BPF_CGROUP_INET_EGRESS
> > > during load time if the prog uses this new helper.
> > > The newly added prog->enforce_expected_attach_type bit will also be set
> > > if this new helper is used.  This bit is for backward compatibility reason
> > > because currently prog->expected_attach_type has been ignored in
> > > BPF_PROG_TYPE_CGROUP_SKB.  During attach time,
> > > prog->expected_attach_type is only enforced if the
> > > prog->enforce_expected_attach_type bit is set.
> > > i.e. prog->expected_attach_type is only enforced if this new helper
> > > is used by the prog.
> > > 
> [ ... ]
> 
> > > @@ -1725,6 +1733,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> > >  	case BPF_PROG_TYPE_CGROUP_SOCK:
> > >  	case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> > >  		return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> > > +	case BPF_PROG_TYPE_CGROUP_SKB:
> > > +		return prog->enforce_expected_attach_type &&
> > > +			prog->expected_attach_type != attach_type ?
> > > +			-EINVAL : 0;
> > >  	default:
> > >  		return 0;
> > >  	}
> [ ... ]
> 
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index 97916eedfe69..ca57ef25279c 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -5426,6 +5426,24 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> > >  	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
> > >  };
> > >  
> > > +BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
> > > +{
> > > +	struct sock *sk = (struct sock *)tp;
> > > +
> > > +	if (sk->sk_state == TCP_ESTABLISHED) {
> > > +		tcp_enter_cwr(sk);
> > > +		return 0;
> > > +	}
> > > +
> > > +	return -EINVAL;
> > > +}
> > > +
> > > +static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> > > +	.func        = bpf_tcp_enter_cwr,
> > > +	.gpl_only    = false,
> > > +	.ret_type    = RET_INTEGER,
> > > +	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
> > > +};
> > >  #endif /* CONFIG_INET */
> > >  
> > >  bool bpf_helper_changes_pkt_data(void *func)
> > > @@ -5585,6 +5603,13 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> > >  #ifdef CONFIG_INET
> > >  	case BPF_FUNC_tcp_sock:
> > >  		return &bpf_tcp_sock_proto;
> > 
> > [...]
> > > +	case BPF_FUNC_tcp_enter_cwr:
> > > +		if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> > > +			prog->enforce_expected_attach_type = 1;
> > > +			return &bpf_tcp_enter_cwr_proto;
> > Instead of this back and forth with enforce_expected_attach_type, can we
> > just do here:
> > 
> > if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
> > 	return &bpf_tcp_enter_cwr_proto;
> > else
> > 	return null;
> > 
> > Wouldn't it have the same effect?
> The attr->expected_attach_type is currently ignored (i.e. not checked)
> during the bpf load time.
But nothing stops you form checking prog->expected_attach_type in
the cg_skb_func_proto, right? That is done at the time of loading.
So depending on prog->expected_attach_type just return null or non-null
and the verifier will take care of the rest. Then, at attach time just
make sure we are attaching it to the expected_attach_type.

We also should not have any existing use cases for
BPF_FUNC_tcp_enter_cwr I suppose.

In other words, why something like below won't work? Am I missing something?

---

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b155cd17c1bd..86dc7cd00f34 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1678,6 +1678,10 @@ static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
 	case BPF_PROG_TYPE_CGROUP_SOCK:
 	case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
 		return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
+	case BPF_PROG_TYPE_CGROUP_SKB:
+		if (prog->expected_attach_type)
+			return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
+		return 0;
 	default:
 		return 0;
 	}
diff --git a/net/core/filter.c b/net/core/filter.c
index 7559d6835ecb..56f70468fc7a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5396,6 +5396,11 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	switch (func_id) {
 	case BPF_FUNC_get_local_storage:
 		return &bpf_get_local_storage_proto;
+	case BPF_FUNC_tcp_enter_cwr:
+		if (prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
+			return &bpf_tcp_enter_cwr_proto;
+		else
+			return NULL;
 	default:
 		return sk_filter_func_proto(func_id, prog);
 	}

> 
> How to avoid breaking backward compatibility without selectively
> enforcing prog->expected_attach_type during attach time?

^ permalink raw reply related

* [PATCH net-next v4 0/6] devlink: make ethtool compat reliable
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski

Hi!

This is a follow up to the series which added device flash
updates via devlink. I went with the approach of adding a
new NDO in the end. It seems to end up looking cleaner.

First patch removes the option to build devlink as a module.
Users can still decide to not build it, but the module option
ends up not being worth the maintenance cost.

Next two patches add a NDO which can be used to ask the driver
to return a devlink instance associated with a given netdev,
instead of iterating over devlink ports. Drivers which implement
this NDO must take into account the potential impact on the
visibility of the devlink instance.

With the new NDO in place we can remove NFP ethtool flash update
code.

Fifth patch makes sure we hold a reference to dev while
callbacks are active.

Last but not least the NULL-check of devlink->ops is moved
to instance allocation time.

Last but not least missing checks for devlink->ops are added.
There is currently no driver registering devlink without ops,
so can just fix this in -next.

v2 (Michal): add netdev_to_devlink() in patch 3.
v3 (Florian):
 - add missing checks for devlink->ops;
 - move locking/holding into devlink_compat_ functions.
v4 (Jiri):
 - hold devlink_mutex around callbacks (patch 2);
 - require non-NULL ops (patch 6).
 
Jakub Kicinski (6):
  net: devlink: turn devlink into a built-in
  devlink: create a special NDO for getting the devlink instance
  nfp: add .ndo_get_devlink
  nfp: remove ethtool flashing fallback
  devlink: hold a reference to the netdevice around ethtool compat
  devlink: require non-NULL ops for devlink instances

 drivers/infiniband/hw/bnxt_re/Kconfig         |   1 -
 drivers/infiniband/hw/mlx4/Kconfig            |   1 -
 drivers/net/Kconfig                           |   1 -
 drivers/net/ethernet/broadcom/Kconfig         |   1 -
 drivers/net/ethernet/cavium/Kconfig           |   1 -
 drivers/net/ethernet/mellanox/mlx4/Kconfig    |   1 -
 .../net/ethernet/mellanox/mlx5/core/Kconfig   |   1 -
 drivers/net/ethernet/mellanox/mlxsw/Kconfig   |   1 -
 drivers/net/ethernet/netronome/Kconfig        |   1 -
 drivers/net/ethernet/netronome/nfp/nfp_app.h  |   2 +
 .../net/ethernet/netronome/nfp/nfp_devlink.c  |  11 ++
 .../ethernet/netronome/nfp/nfp_net_common.c   |   1 +
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  |  24 ----
 .../net/ethernet/netronome/nfp/nfp_net_repr.c |   1 +
 include/linux/netdevice.h                     |   7 +
 include/net/devlink.h                         |  19 ++-
 net/Kconfig                                   |  11 +-
 net/core/devlink.c                            | 126 +++++++-----------
 net/core/ethtool.c                            |  13 +-
 net/dsa/Kconfig                               |   2 +-
 20 files changed, 90 insertions(+), 136 deletions(-)

-- 
2.19.2


^ permalink raw reply

* [PATCH net-next v4 1/6] net: devlink: turn devlink into a built-in
From: Jakub Kicinski @ 2019-02-26  3:34 UTC (permalink / raw)
  To: davem, jiri
  Cc: mkubecek, andrew, f.fainelli, netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190226033407.32625-1-jakub.kicinski@netronome.com>

Being able to build devlink as a module causes growing pains.
First all drivers had to add a meta dependency to make sure
they are not built in when devlink is built as a module.  Now
we are struggling to invoke ethtool compat code reliably.

Make devlink code built-in, users can still not build it at
all but the dynamically loadable module option is removed.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/infiniband/hw/bnxt_re/Kconfig           |  1 -
 drivers/infiniband/hw/mlx4/Kconfig              |  1 -
 drivers/net/Kconfig                             |  1 -
 drivers/net/ethernet/broadcom/Kconfig           |  1 -
 drivers/net/ethernet/cavium/Kconfig             |  1 -
 drivers/net/ethernet/mellanox/mlx4/Kconfig      |  1 -
 drivers/net/ethernet/mellanox/mlx5/core/Kconfig |  1 -
 drivers/net/ethernet/mellanox/mlxsw/Kconfig     |  1 -
 drivers/net/ethernet/netronome/Kconfig          |  1 -
 include/net/devlink.h                           | 10 ++++------
 net/Kconfig                                     | 11 +----------
 net/core/devlink.c                              | 15 ++-------------
 net/dsa/Kconfig                                 |  2 +-
 13 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/Kconfig b/drivers/infiniband/hw/bnxt_re/Kconfig
index 18f5ed082f41..19982a4a9bba 100644
--- a/drivers/infiniband/hw/bnxt_re/Kconfig
+++ b/drivers/infiniband/hw/bnxt_re/Kconfig
@@ -1,7 +1,6 @@
 config INFINIBAND_BNXT_RE
     tristate "Broadcom Netxtreme HCA support"
     depends on ETHERNET && NETDEVICES && PCI && INET && DCB
-    depends on MAY_USE_DEVLINK
     select NET_VENDOR_BROADCOM
     select BNXT
     ---help---
diff --git a/drivers/infiniband/hw/mlx4/Kconfig b/drivers/infiniband/hw/mlx4/Kconfig
index d1de3285fd88..4e9936731867 100644
--- a/drivers/infiniband/hw/mlx4/Kconfig
+++ b/drivers/infiniband/hw/mlx4/Kconfig
@@ -2,7 +2,6 @@ config MLX4_INFINIBAND
 	tristate "Mellanox ConnectX HCA support"
 	depends on NETDEVICES && ETHERNET && PCI && INET
 	depends on INFINIBAND_USER_ACCESS || !INFINIBAND_USER_ACCESS
-	depends on MAY_USE_DEVLINK
 	select NET_VENDOR_MELLANOX
 	select MLX4_CORE
 	---help---
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 6210757772ed..5e4ca082cfcd 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -505,7 +505,6 @@ source "drivers/net/hyperv/Kconfig"
 config NETDEVSIM
 	tristate "Simulated networking device"
 	depends on DEBUG_FS
-	depends on MAY_USE_DEVLINK
 	help
 	  This driver is a developer testing tool and software model that can
 	  be used to test various control path networking APIs, especially
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index c1d3ee9baf7e..716bfbba59cf 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -194,7 +194,6 @@ config SYSTEMPORT
 config BNXT
 	tristate "Broadcom NetXtreme-C/E support"
 	depends on PCI
-	depends on MAY_USE_DEVLINK
 	select FW_LOADER
 	select LIBCRC32C
 	---help---
diff --git a/drivers/net/ethernet/cavium/Kconfig b/drivers/net/ethernet/cavium/Kconfig
index 05f4a3b21e29..6650e2a5f171 100644
--- a/drivers/net/ethernet/cavium/Kconfig
+++ b/drivers/net/ethernet/cavium/Kconfig
@@ -64,7 +64,6 @@ config CAVIUM_PTP
 config LIQUIDIO
 	tristate "Cavium LiquidIO support"
 	depends on 64BIT && PCI
-	depends on MAY_USE_DEVLINK
 	depends on PCI
 	imply PTP_1588_CLOCK
 	select FW_LOADER
diff --git a/drivers/net/ethernet/mellanox/mlx4/Kconfig b/drivers/net/ethernet/mellanox/mlx4/Kconfig
index f200b8c420d5..ff8057ed97ee 100644
--- a/drivers/net/ethernet/mellanox/mlx4/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx4/Kconfig
@@ -4,7 +4,6 @@
 
 config MLX4_EN
 	tristate "Mellanox Technologies 1/10/40Gbit Ethernet support"
-	depends on MAY_USE_DEVLINK
 	depends on PCI && NETDEVICES && ETHERNET && INET
 	select MLX4_CORE
 	imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
index 37a551436e4a..6debffb8336b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Kconfig
@@ -4,7 +4,6 @@
 
 config MLX5_CORE
 	tristate "Mellanox 5th generation network adapters (ConnectX series) core driver"
-	depends on MAY_USE_DEVLINK
 	depends on PCI
 	imply PTP_1588_CLOCK
 	imply VXLAN
diff --git a/drivers/net/ethernet/mellanox/mlxsw/Kconfig b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
index b9a25aed5d11..9c195dfed031 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/Kconfig
+++ b/drivers/net/ethernet/mellanox/mlxsw/Kconfig
@@ -4,7 +4,6 @@
 
 config MLXSW_CORE
 	tristate "Mellanox Technologies Switch ASICs support"
-	depends on MAY_USE_DEVLINK
 	---help---
 	  This driver supports Mellanox Technologies Switch ASICs family.
 
diff --git a/drivers/net/ethernet/netronome/Kconfig b/drivers/net/ethernet/netronome/Kconfig
index 66f15b05b65e..549898d5d450 100644
--- a/drivers/net/ethernet/netronome/Kconfig
+++ b/drivers/net/ethernet/netronome/Kconfig
@@ -19,7 +19,6 @@ config NFP
 	tristate "Netronome(R) NFP4000/NFP6000 NIC driver"
 	depends on PCI && PCI_MSI
 	depends on VXLAN || VXLAN=n
-	depends on MAY_USE_DEVLINK
 	---help---
 	  This driver supports the Netronome(R) NFP4000/NFP6000 based
 	  cards working as a advanced Ethernet NIC.  It works with both
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a2da49dd9147..f9f7fe974652 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -707,6 +707,10 @@ devlink_health_reporter_priv(struct devlink_health_reporter *reporter);
 int devlink_health_report(struct devlink_health_reporter *reporter,
 			  const char *msg, void *priv_ctx);
 
+void devlink_compat_running_version(struct net_device *dev,
+				    char *buf, size_t len);
+int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
+
 #else
 
 static inline struct devlink *devlink_alloc(const struct devlink_ops *ops,
@@ -1190,13 +1194,7 @@ devlink_health_report(struct devlink_health_reporter *reporter,
 {
 	return 0;
 }
-#endif
 
-#if IS_REACHABLE(CONFIG_NET_DEVLINK)
-void devlink_compat_running_version(struct net_device *dev,
-				    char *buf, size_t len);
-int devlink_compat_flash_update(struct net_device *dev, const char *file_name);
-#else
 static inline void
 devlink_compat_running_version(struct net_device *dev, char *buf, size_t len)
 {
diff --git a/net/Kconfig b/net/Kconfig
index 62da6148e9f8..1efe1f9ee492 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -429,21 +429,12 @@ config NET_SOCK_MSG
 	  with the help of BPF programs.
 
 config NET_DEVLINK
-	tristate "Network physical/parent device Netlink interface"
+	bool "Network physical/parent device Netlink interface"
 	help
 	  Network physical/parent device Netlink interface provides
 	  infrastructure to support access to physical chip-wide config and
 	  monitoring.
 
-config MAY_USE_DEVLINK
-	tristate
-	default m if NET_DEVLINK=m
-	default y if NET_DEVLINK=y || NET_DEVLINK=n
-	help
-	  Drivers using the devlink infrastructure should have a dependency
-	  on MAY_USE_DEVLINK to ensure they do not cause link errors when
-	  devlink is a loadable module and the driver using it is built-in.
-
 config PAGE_POOL
        bool
 
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4f31ddc883e7..05e04ea0a5c7 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6482,20 +6482,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
 	return -EOPNOTSUPP;
 }
 
-static int __init devlink_module_init(void)
+static int __init devlink_init(void)
 {
 	return genl_register_family(&devlink_nl_family);
 }
 
-static void __exit devlink_module_exit(void)
-{
-	genl_unregister_family(&devlink_nl_family);
-}
-
-module_init(devlink_module_init);
-module_exit(devlink_module_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Jiri Pirko <jiri@mellanox.com>");
-MODULE_DESCRIPTION("Network physical device Netlink interface");
-MODULE_ALIAS_GENL_FAMILY(DEVLINK_GENL_NAME);
+subsys_initcall(devlink_init);
diff --git a/net/dsa/Kconfig b/net/dsa/Kconfig
index 91e52973ee13..fab49132345f 100644
--- a/net/dsa/Kconfig
+++ b/net/dsa/Kconfig
@@ -6,7 +6,7 @@ config HAVE_NET_DSA
 
 config NET_DSA
 	tristate "Distributed Switch Architecture"
-	depends on HAVE_NET_DSA && MAY_USE_DEVLINK
+	depends on HAVE_NET_DSA
 	depends on BRIDGE || BRIDGE=n
 	select NET_SWITCHDEV
 	select PHYLINK
-- 
2.19.2


^ 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