[patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Currently we have per-net notifier, which allows to get only
notifications relevant to particular network namespace. That is enough
for drivers that have netdevs local in a particular namespace (cannot
move elsewhere).

However if netdev can change namespace, per-net notifier cannot be used.
Introduce dev_net variant that is basically per-net notifier with an
extension that re-registers the per-net notifier upon netdev namespace
change. Basically the per-net notifier follows the netdev into
namespace.

Jiri Pirko (4):
  net: call call_netdevice_unregister_net_notifiers from unregister
  net: push code from net notifier reg/unreg into helpers
  net: introduce dev_net notifier register/unregister variants
  mlx5: Use dev_net netdevice notifier registrations

 .../net/ethernet/mellanox/mlx5/core/en/fs.h   |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c |   3 +-
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |  14 ++-
 .../net/ethernet/mellanox/mlx5/core/en_rep.h  |   1 +
 .../net/ethernet/mellanox/mlx5/core/en_tc.c   |   9 +-
 drivers/net/ethernet/mellanox/mlx5/core/lag.c |   8 +-
 drivers/net/ethernet/mellanox/mlx5/core/lag.h |   1 +
 .../ethernet/mellanox/mlx5/core/mlx5_core.h   |   2 +-
 include/linux/netdevice.h                     |  17 +++
 net/core/dev.c                                | 118 +++++++++++++-----
 10 files changed, 132 insertions(+), 42 deletions(-)

-- 
2.21.0

[patch net-next 1/4] net: call call_netdevice_unregister_net_notifiers from unregister

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

The function does the same thing as the existing code, so rather call
call_netdevice_unregister_net_notifiers() instead of code duplication.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/dev.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2c277b8aba38..2c90722195f8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1764,7 +1764,6 @@ EXPORT_SYMBOL(register_netdevice_notifier);
 
 int unregister_netdevice_notifier(struct notifier_block *nb)
 {
-	struct net_device *dev;
 	struct net *net;
 	int err;
 
@@ -1775,16 +1774,9 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
 	if (err)
 		goto unlock;
 
-	for_each_net(net) {
-		for_each_netdev(net, dev) {
-			if (dev->flags & IFF_UP) {
-				call_netdevice_notifier(nb, NETDEV_GOING_DOWN,
-							dev);
-				call_netdevice_notifier(nb, NETDEV_DOWN, dev);
-			}
-			call_netdevice_notifier(nb, NETDEV_UNREGISTER, dev);
-		}
-	}
+	for_each_net(net)
+		call_netdevice_unregister_net_notifiers(nb, net);
+
 unlock:
 	rtnl_unlock();
 	up_write(&pernet_ops_rwsem);
-- 
2.21.0

Re: [patch net-next 1/4] net: call call_netdevice_unregister_net_notifiers from unregister

[David Ahern]

On 12/20/19 5:35 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> The function does the same thing as the existing code, so rather call
> call_netdevice_unregister_net_notifiers() instead of code duplication.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  net/core/dev.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

[patch net-next 2/4] net: push code from net notifier reg/unreg into helpers

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Push the code which is done under rtnl lock in net notifier register and
unregister function into separate helpers.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/dev.c | 60 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2c90722195f8..932ee131c8c9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1784,6 +1784,42 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL(unregister_netdevice_notifier);
 
+static int __register_netdevice_notifier_net(struct net *net,
+					     struct notifier_block *nb,
+					     bool ignore_call_fail)
+{
+	int err;
+
+	err = raw_notifier_chain_register(&net->netdev_chain, nb);
+	if (err)
+		return err;
+	if (dev_boot_phase)
+		return 0;
+
+	err = call_netdevice_register_net_notifiers(nb, net);
+	if (err && !ignore_call_fail)
+		goto chain_unregister;
+
+	return 0;
+
+chain_unregister:
+	raw_notifier_chain_unregister(&netdev_chain, nb);
+	return err;
+}
+
+static int __unregister_netdevice_notifier_net(struct net *net,
+					       struct notifier_block *nb)
+{
+	int err;
+
+	err = raw_notifier_chain_unregister(&net->netdev_chain, nb);
+	if (err)
+		return err;
+
+	call_netdevice_unregister_net_notifiers(nb, net);
+	return 0;
+}
+
 /**
  * register_netdevice_notifier_net - register a per-netns network notifier block
  * @net: network namespace
@@ -1804,23 +1840,9 @@ int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb)
 	int err;
 
 	rtnl_lock();
-	err = raw_notifier_chain_register(&net->netdev_chain, nb);
-	if (err)
-		goto unlock;
-	if (dev_boot_phase)
-		goto unlock;
-
-	err = call_netdevice_register_net_notifiers(nb, net);
-	if (err)
-		goto chain_unregister;
-
-unlock:
+	err = __register_netdevice_notifier_net(net, nb, false);
 	rtnl_unlock();
 	return err;
-
-chain_unregister:
-	raw_notifier_chain_unregister(&netdev_chain, nb);
-	goto unlock;
 }
 EXPORT_SYMBOL(register_netdevice_notifier_net);
 
@@ -1846,13 +1868,7 @@ int unregister_netdevice_notifier_net(struct net *net,
 	int err;
 
 	rtnl_lock();
-	err = raw_notifier_chain_unregister(&net->netdev_chain, nb);
-	if (err)
-		goto unlock;
-
-	call_netdevice_unregister_net_notifiers(nb, net);
-
-unlock:
+	err = __unregister_netdevice_notifier_net(net, nb);
 	rtnl_unlock();
 	return err;
 }
-- 
2.21.0

Re: [patch net-next 2/4] net: push code from net notifier reg/unreg into helpers

[David Ahern]

On 12/20/19 5:35 AM, Jiri Pirko wrote:
> @@ -1784,6 +1784,42 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
>  }
>  EXPORT_SYMBOL(unregister_netdevice_notifier);
>  
> +static int __register_netdevice_notifier_net(struct net *net,
> +					     struct notifier_block *nb,
> +					     bool ignore_call_fail)
> +{
> +	int err;
> +
> +	err = raw_notifier_chain_register(&net->netdev_chain, nb);
> +	if (err)
> +		return err;
> +	if (dev_boot_phase)
> +		return 0;
> +
> +	err = call_netdevice_register_net_notifiers(nb, net);
> +	if (err && !ignore_call_fail)
> +		goto chain_unregister;
> +
> +	return 0;
> +
> +chain_unregister:
> +	raw_notifier_chain_unregister(&netdev_chain, nb);

why is the error path using the global netdev_chain when the register is
relative to a namespace? yes, I realize existing code does that and this
is maintaining that behavior.

Re: [patch net-next 2/4] net: push code from net notifier reg/unreg into helpers

[Jiri Pirko]

Fri, Dec 20, 2019 at 07:07:59PM CET, dsahern@gmail.com wrote:
>On 12/20/19 5:35 AM, Jiri Pirko wrote:
>> @@ -1784,6 +1784,42 @@ int unregister_netdevice_notifier(struct notifier_block *nb)
>>  }
>>  EXPORT_SYMBOL(unregister_netdevice_notifier);
>>  
>> +static int __register_netdevice_notifier_net(struct net *net,
>> +					     struct notifier_block *nb,
>> +					     bool ignore_call_fail)
>> +{
>> +	int err;
>> +
>> +	err = raw_notifier_chain_register(&net->netdev_chain, nb);
>> +	if (err)
>> +		return err;
>> +	if (dev_boot_phase)
>> +		return 0;
>> +
>> +	err = call_netdevice_register_net_notifiers(nb, net);
>> +	if (err && !ignore_call_fail)
>> +		goto chain_unregister;
>> +
>> +	return 0;
>> +
>> +chain_unregister:
>> +	raw_notifier_chain_unregister(&netdev_chain, nb);
>
>why is the error path using the global netdev_chain when the register is
>relative to a namespace? yes, I realize existing code does that and this

A bug. Will fix. Thanks!

>is maintaining that behavior.
>

[patch net-next 3/4] net: introduce dev_net notifier register/unregister variants

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Introduce dev_net variants of netdev notifier register/unregister functions
and allow per-net notifier to follow the netdevice into the namespace it is
moved to.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/linux/netdevice.h | 17 +++++++++++++++
 net/core/dev.c            | 46 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7a8ed11f5d45..89ccd4c8d9ea 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -937,6 +937,11 @@ struct netdev_name_node {
 int netdev_name_node_alt_create(struct net_device *dev, const char *name);
 int netdev_name_node_alt_destroy(struct net_device *dev, const char *name);
 
+struct netdev_net_notifier {
+	struct list_head list;
+	struct notifier_block *nb;
+};
+
 /*
  * This structure defines the management hooks for network devices.
  * The following hooks can be defined; unless noted otherwise, they are
@@ -1790,6 +1795,10 @@ enum netdev_priv_flags {
  *
  *	@wol_enabled:	Wake-on-LAN is enabled
  *
+ *	@net_notifier_list:	List of per-net netdev notifier block
+ *				that follow this device when it is moved
+ *				to another network namespace.
+ *
  *	FIXME: cleanup struct net_device such that network protocol info
  *	moves out.
  */
@@ -2080,6 +2089,8 @@ struct net_device {
 	struct lock_class_key	addr_list_lock_key;
 	bool			proto_down;
 	unsigned		wol_enabled:1;
+
+	struct list_head	net_notifier_list;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -2523,6 +2534,12 @@ int unregister_netdevice_notifier(struct notifier_block *nb);
 int register_netdevice_notifier_net(struct net *net, struct notifier_block *nb);
 int unregister_netdevice_notifier_net(struct net *net,
 				      struct notifier_block *nb);
+int register_netdevice_notifier_dev_net(struct net_device *dev,
+					struct notifier_block *nb,
+					struct netdev_net_notifier *nn);
+int unregister_netdevice_notifier_dev_net(struct net_device *dev,
+					  struct notifier_block *nb,
+					  struct netdev_net_notifier *nn);
 
 struct netdev_notifier_info {
 	struct net_device	*dev;
diff --git a/net/core/dev.c b/net/core/dev.c
index 932ee131c8c9..f59d2116db8d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1874,6 +1874,48 @@ int unregister_netdevice_notifier_net(struct net *net,
 }
 EXPORT_SYMBOL(unregister_netdevice_notifier_net);
 
+int register_netdevice_notifier_dev_net(struct net_device *dev,
+					struct notifier_block *nb,
+					struct netdev_net_notifier *nn)
+{
+	int err;
+
+	rtnl_lock();
+	err = __register_netdevice_notifier_net(dev_net(dev), nb, false);
+	if (!err) {
+		nn->nb = nb;
+		list_add(&nn->list, &dev->net_notifier_list);
+	}
+	rtnl_unlock();
+	return err;
+}
+EXPORT_SYMBOL(register_netdevice_notifier_dev_net);
+
+int unregister_netdevice_notifier_dev_net(struct net_device *dev,
+					  struct notifier_block *nb,
+					  struct netdev_net_notifier *nn)
+{
+	int err;
+
+	rtnl_lock();
+	list_del(&nn->list);
+	err = __unregister_netdevice_notifier_net(dev_net(dev), nb);
+	rtnl_unlock();
+	return err;
+}
+EXPORT_SYMBOL(unregister_netdevice_notifier_dev_net);
+
+static void move_netdevice_notifiers_dev_net(struct net_device *dev,
+					     struct net *net)
+{
+	struct netdev_net_notifier *nn;
+
+	list_for_each_entry(nn, &dev->net_notifier_list, list) {
+		__unregister_netdevice_notifier_net(dev_net(dev), nn->nb);
+		__register_netdevice_notifier_net(net, nn->nb, true);
+	}
+}
+
 /**
  *	call_netdevice_notifiers_info - call all network notifier blocks
  *	@val: value passed unmodified to notifier function
@@ -9770,6 +9812,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	INIT_LIST_HEAD(&dev->adj_list.lower);
 	INIT_LIST_HEAD(&dev->ptype_all);
 	INIT_LIST_HEAD(&dev->ptype_specific);
+	INIT_LIST_HEAD(&dev->net_notifier_list);
 #ifdef CONFIG_NET_SCHED
 	hash_init(dev->qdisc_hash);
 #endif
@@ -10031,6 +10074,9 @@ int dev_change_net_namespace(struct net_device *dev, struct net *net, const char
 	kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
 	netdev_adjacent_del_links(dev);
 
+	/* Move per-net netdevice notifiers that are following the netdevice */
+	move_netdevice_notifiers_dev_net(dev, net);
+
 	/* Actually switch the network namespace */
 	dev_net_set(dev, net);
 	dev->ifindex = new_ifindex;
-- 
2.21.0

Re: [patch net-next 3/4] net: introduce dev_net notifier register/unregister variants

[Saeed Mahameed]

On Fri, 2019-12-20 at 13:35 +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
> 
> Introduce dev_net variants of netdev notifier register/unregister
> functions
> and allow per-net notifier to follow the netdevice into the namespace
> it is
> moved to.
> 
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>  include/linux/netdevice.h | 17 +++++++++++++++
>  net/core/dev.c            | 46
> +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 7a8ed11f5d45..89ccd4c8d9ea 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -937,6 +937,11 @@ struct netdev_name_node {
>  int netdev_name_node_alt_create(struct net_device *dev, const char
> *name);
>  int netdev_name_node_alt_destroy(struct net_device *dev, const char
> *name);
>  
> +struct netdev_net_notifier {
> +	struct list_head list;
> +	struct notifier_block *nb;
> +};
> +
>  /*
>   * This structure defines the management hooks for network devices.
>   * The following hooks can be defined; unless noted otherwise, they
> are
> @@ -1790,6 +1795,10 @@ enum netdev_priv_flags {
>   *
>   *	@wol_enabled:	Wake-on-LAN is enabled
>   *
> + *	@net_notifier_list:	List of per-net netdev notifier block
> + *				that follow this device when it is
> moved
> + *				to another network namespace.
> + *
>   *	FIXME: cleanup struct net_device such that network protocol
> info
>   *	moves out.
>   */
> @@ -2080,6 +2089,8 @@ struct net_device {
>  	struct lock_class_key	addr_list_lock_key;
>  	bool			proto_down;
>  	unsigned		wol_enabled:1;
> +
> +	struct list_head	net_notifier_list;
>  };
>  #define to_net_dev(d) container_of(d, struct net_device, dev)
>  
> @@ -2523,6 +2534,12 @@ int unregister_netdevice_notifier(struct
> notifier_block *nb);
>  int register_netdevice_notifier_net(struct net *net, struct
> notifier_block *nb);
>  int unregister_netdevice_notifier_net(struct net *net,
>  				      struct notifier_block *nb);
> +int register_netdevice_notifier_dev_net(struct net_device *dev,
> +					struct notifier_block *nb,
> +					struct netdev_net_notifier
> *nn);
> +int unregister_netdevice_notifier_dev_net(struct net_device *dev,
> +					  struct notifier_block *nb,
> +					  struct netdev_net_notifier
> *nn);
>  
>  struct netdev_notifier_info {
>  	struct net_device	*dev;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 932ee131c8c9..f59d2116db8d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1874,6 +1874,48 @@ int unregister_netdevice_notifier_net(struct
> net *net,
>  }
>  EXPORT_SYMBOL(unregister_netdevice_notifier_net);
>  
> +int register_netdevice_notifier_dev_net(struct net_device *dev,
> +					struct notifier_block *nb,
> +					struct netdev_net_notifier *nn)
> +{
> +	int err;
> +
> +	rtnl_lock();
> +	err = __register_netdevice_notifier_net(dev_net(dev), nb,
> false);
> +	if (!err) {
> +		nn->nb = nb;

looks like there is 1 to 1 mapping between nn and nb, 
to save the driver developers the headache of dealing with two objects
just embed the nb object into the nn object and let the driver deal
with nn objects only.

> +		list_add(&nn->list, &dev->net_notifier_list);
> +	}
> +	rtnl_unlock();
> +	return err;
> +}
> +EXPORT_SYMBOL(register_netdevice_notifier_dev_net);
> +
> +int unregister_netdevice_notifier_dev_net(struct net_device *dev,
> +					  struct notifier_block *nb,
> +					  struct netdev_net_notifier
> *nn)
> +{
> +	int err;
> +
> +	rtnl_lock();
> +	list_del(&nn->list);
> +	err = __unregister_netdevice_notifier_net(dev_net(dev), nb);
> +	rtnl_unlock();
> +	return err;
> +}
> +EXPORT_SYMBOL(unregister_netdevice_notifier_dev_net);
> +
> +static void move_netdevice_notifiers_dev_net(struct net_device *dev,
> +					     struct net *net)
> +{
> +	struct netdev_net_notifier *nn;
> +
> +	list_for_each_entry(nn, &dev->net_notifier_list, list) {
> +		__unregister_netdevice_notifier_net(dev_net(dev), nn-
> >nb);
> +		__register_netdevice_notifier_net(net, nn->nb, true);
> +	}
> +}
> +
>  /**
>   *	call_netdevice_notifiers_info - call all network notifier
> blocks
>   *	@val: value passed unmodified to notifier function
> @@ -9770,6 +9812,7 @@ struct net_device *alloc_netdev_mqs(int
> sizeof_priv, const char *name,
>  	INIT_LIST_HEAD(&dev->adj_list.lower);
>  	INIT_LIST_HEAD(&dev->ptype_all);
>  	INIT_LIST_HEAD(&dev->ptype_specific);
> +	INIT_LIST_HEAD(&dev->net_notifier_list);
>  #ifdef CONFIG_NET_SCHED
>  	hash_init(dev->qdisc_hash);
>  #endif
> @@ -10031,6 +10074,9 @@ int dev_change_net_namespace(struct
> net_device *dev, struct net *net, const char
>  	kobject_uevent(&dev->dev.kobj, KOBJ_REMOVE);
>  	netdev_adjacent_del_links(dev);
>  
> +	/* Move per-net netdevice notifiers that are following the
> netdevice */
> +	move_netdevice_notifiers_dev_net(dev, net);
> +
>  	/* Actually switch the network namespace */
>  	dev_net_set(dev, net);
>  	dev->ifindex = new_ifindex;

Re: [patch net-next 3/4] net: introduce dev_net notifier register/unregister variants

[Jiri Pirko]

Fri, Dec 20, 2019 at 08:29:31PM CET, saeedm@mellanox.com wrote:
>On Fri, 2019-12-20 at 13:35 +0100, Jiri Pirko wrote:

[...]


>> +int register_netdevice_notifier_dev_net(struct net_device *dev,
>> +					struct notifier_block *nb,
>> +					struct netdev_net_notifier *nn)
>> +{
>> +	int err;
>> +
>> +	rtnl_lock();
>> +	err = __register_netdevice_notifier_net(dev_net(dev), nb,
>> false);
>> +	if (!err) {
>> +		nn->nb = nb;
>
>looks like there is 1 to 1 mapping between nn and nb, 
>to save the driver developers the headache of dealing with two objects
>just embed the nb object into the nn object and let the driver deal
>with nn objects only.

Sure. That was my thinking too. The problem is that in event handler,
the arg is struct notifier_block *nb.
So the user would have to know the struct netdev_net_notifier internals
in order to extract the container of struct notifier_block.

I think is is better to not to hide this.

[...]

[patch net-next 4/4] mlx5: Use dev_net netdevice notifier registrations

[Jiri Pirko]

From: Jiri Pirko <jiri@mellanox.com>

Register the dev_net notifier and allow the per-net notifier to follow
the device into different namespace.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/fs.h    |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  3 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c   | 14 +++++++++++---
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.h   |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_tc.c    |  9 +++++++--
 drivers/net/ethernet/mellanox/mlx5/core/lag.c      |  8 +++++---
 drivers/net/ethernet/mellanox/mlx5/core/lag.h      |  1 +
 .../net/ethernet/mellanox/mlx5/core/mlx5_core.h    |  2 +-
 8 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h b/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
index 68d593074f6c..a3b098a46ad1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/fs.h
@@ -21,6 +21,7 @@ struct mlx5e_tc_table {
 	DECLARE_HASHTABLE(hairpin_tbl, 8);
 
 	struct notifier_block     netdevice_nb;
+	struct netdev_net_notifier	netdevice_nn;
 };
 
 struct mlx5e_flow_table {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 68f1c8cb302b..d9a900e567f8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5166,6 +5166,7 @@ static void mlx5e_nic_enable(struct mlx5e_priv *priv)
 
 static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 {
+	struct net_device *netdev = priv->netdev;
 	struct mlx5_core_dev *mdev = priv->mdev;
 
 #ifdef CONFIG_MLX5_CORE_EN_DCB
@@ -5186,7 +5187,7 @@ static void mlx5e_nic_disable(struct mlx5e_priv *priv)
 		mlx5e_monitor_counter_cleanup(priv);
 
 	mlx5e_disable_async_events(priv);
-	mlx5_lag_remove(mdev);
+	mlx5_lag_remove(mdev, netdev);
 }
 
 int mlx5e_update_nic_rx(struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index f175cb24bb67..5a275690dc6d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -1688,7 +1688,9 @@ static int mlx5e_init_rep_tx(struct mlx5e_priv *priv)
 		/* init indirect block notifications */
 		INIT_LIST_HEAD(&uplink_priv->tc_indr_block_priv_list);
 		uplink_priv->netdevice_nb.notifier_call = mlx5e_nic_rep_netdevice_event;
-		err = register_netdevice_notifier(&uplink_priv->netdevice_nb);
+		err = register_netdevice_notifier_dev_net(rpriv->netdev,
+							  &uplink_priv->netdevice_nb,
+							  &uplink_priv->netdevice_nn);
 		if (err) {
 			mlx5_core_err(priv->mdev, "Failed to register netdev notifier\n");
 			goto tc_esw_cleanup;
@@ -1707,12 +1709,17 @@ static int mlx5e_init_rep_tx(struct mlx5e_priv *priv)
 static void mlx5e_cleanup_rep_tx(struct mlx5e_priv *priv)
 {
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
+	struct mlx5_rep_uplink_priv *uplink_priv;
 
 	mlx5e_destroy_tises(priv);
 
 	if (rpriv->rep->vport == MLX5_VPORT_UPLINK) {
+		uplink_priv = &rpriv->uplink_priv;
+
 		/* clean indirect TC block notifications */
-		unregister_netdevice_notifier(&rpriv->uplink_priv.netdevice_nb);
+		unregister_netdevice_notifier_dev_net(rpriv->netdev,
+						      &uplink_priv->netdevice_nb,
+						      &uplink_priv->netdevice_nn);
 		mlx5e_rep_indr_clean_block_privs(rpriv);
 
 		/* delete shared tc flow table */
@@ -1787,6 +1794,7 @@ static void mlx5e_uplink_rep_enable(struct mlx5e_priv *priv)
 
 static void mlx5e_uplink_rep_disable(struct mlx5e_priv *priv)
 {
+	struct net_device *netdev = priv->netdev;
 	struct mlx5_core_dev *mdev = priv->mdev;
 	struct mlx5e_rep_priv *rpriv = priv->ppriv;
 
@@ -1795,7 +1803,7 @@ static void mlx5e_uplink_rep_disable(struct mlx5e_priv *priv)
 #endif
 	mlx5_notifier_unregister(mdev, &priv->events_nb);
 	cancel_work_sync(&rpriv->uplink_priv.reoffload_flows_work);
-	mlx5_lag_remove(mdev);
+	mlx5_lag_remove(mdev, netdev);
 }
 
 static const struct mlx5e_profile mlx5e_rep_profile = {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index 31f83c8adcc9..3f756d51435f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -73,6 +73,7 @@ struct mlx5_rep_uplink_priv {
 	 */
 	struct list_head	    tc_indr_block_priv_list;
 	struct notifier_block	    netdevice_nb;
+	struct netdev_net_notifier  netdevice_nn;
 
 	struct mlx5_tun_entropy tun_entropy;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 9b32a9c0f497..59cc898b7a21 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -4142,7 +4142,10 @@ int mlx5e_tc_nic_init(struct mlx5e_priv *priv)
 		return err;
 
 	tc->netdevice_nb.notifier_call = mlx5e_tc_netdev_event;
-	if (register_netdevice_notifier(&tc->netdevice_nb)) {
+	err = register_netdevice_notifier_dev_net(priv->netdev,
+						  &tc->netdevice_nb,
+						  &tc->netdevice_nn);
+	if (err) {
 		tc->netdevice_nb.notifier_call = NULL;
 		mlx5_core_warn(priv->mdev, "Failed to register netdev notifier\n");
 	}
@@ -4164,7 +4167,9 @@ void mlx5e_tc_nic_cleanup(struct mlx5e_priv *priv)
 	struct mlx5e_tc_table *tc = &priv->fs.tc;
 
 	if (tc->netdevice_nb.notifier_call)
-		unregister_netdevice_notifier(&tc->netdevice_nb);
+		unregister_netdevice_notifier_dev_net(priv->netdev,
+						      &tc->netdevice_nb,
+						      &tc->netdevice_nn);
 
 	mutex_destroy(&tc->mod_hdr.lock);
 	mutex_destroy(&tc->hairpin_tbl_lock);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag.c
index fc0d9583475d..b91eabc09fbc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag.c
@@ -586,7 +586,8 @@ void mlx5_lag_add(struct mlx5_core_dev *dev, struct net_device *netdev)
 
 	if (!ldev->nb.notifier_call) {
 		ldev->nb.notifier_call = mlx5_lag_netdev_event;
-		if (register_netdevice_notifier(&ldev->nb)) {
+		if (register_netdevice_notifier_dev_net(netdev, &ldev->nb,
+							&ldev->nn)) {
 			ldev->nb.notifier_call = NULL;
 			mlx5_core_err(dev, "Failed to register LAG netdev notifier\n");
 		}
@@ -599,7 +600,7 @@ void mlx5_lag_add(struct mlx5_core_dev *dev, struct net_device *netdev)
 }
 
 /* Must be called with intf_mutex held */
-void mlx5_lag_remove(struct mlx5_core_dev *dev)
+void mlx5_lag_remove(struct mlx5_core_dev *dev, struct net_device *netdev)
 {
 	struct mlx5_lag *ldev;
 	int i;
@@ -619,7 +620,8 @@ void mlx5_lag_remove(struct mlx5_core_dev *dev)
 
 	if (i == MLX5_MAX_PORTS) {
 		if (ldev->nb.notifier_call)
-			unregister_netdevice_notifier(&ldev->nb);
+			unregister_netdevice_notifier_dev_net(netdev, &ldev->nb,
+							      &ldev->nn);
 		mlx5_lag_mp_cleanup(ldev);
 		cancel_delayed_work_sync(&ldev->bond_work);
 		mlx5_lag_dev_free(ldev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag.h b/drivers/net/ethernet/mellanox/mlx5/core/lag.h
index f1068aac6406..316ab09e2664 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lag.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lag.h
@@ -44,6 +44,7 @@ struct mlx5_lag {
 	struct workqueue_struct   *wq;
 	struct delayed_work       bond_work;
 	struct notifier_block     nb;
+	struct netdev_net_notifier	nn;
 	struct lag_mp             lag_mp;
 };
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index da67b28d6e23..fcce9e0fc82c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
@@ -157,7 +157,7 @@ int mlx5_query_qcam_reg(struct mlx5_core_dev *mdev, u32 *qcam,
 			u8 feature_group, u8 access_reg_group);
 
 void mlx5_lag_add(struct mlx5_core_dev *dev, struct net_device *netdev);
-void mlx5_lag_remove(struct mlx5_core_dev *dev);
+void mlx5_lag_remove(struct mlx5_core_dev *dev, struct net_device *netdev);
 
 int mlx5_irq_table_init(struct mlx5_core_dev *dev);
 void mlx5_irq_table_cleanup(struct mlx5_core_dev *dev);
-- 
2.21.0

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[David Ahern]

On 12/20/19 5:35 AM, Jiri Pirko wrote:
> However if netdev can change namespace, per-net notifier cannot be used.
> Introduce dev_net variant that is basically per-net notifier with an
> extension that re-registers the per-net notifier upon netdev namespace
> change. Basically the per-net notifier follows the netdev into
> namespace.

This is getting convoluted.

If the driver wants notifications in a new namespace, then it should
register for notifiers in the new namespace. The info for
NETDEV_UNREGISTER event could indicate the device is getting moved to a
new namespace and the driver register for notifications in the new
namespace. If the drivers are trying to be efficient wrt to
notifications then it will need to unregister when all netdevices leave
a namespace which it means it has work to do on namespace changes.

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[Jiri Pirko]

Fri, Dec 20, 2019 at 07:30:22PM CET, dsahern@gmail.com wrote:
>On 12/20/19 5:35 AM, Jiri Pirko wrote:
>> However if netdev can change namespace, per-net notifier cannot be used.
>> Introduce dev_net variant that is basically per-net notifier with an
>> extension that re-registers the per-net notifier upon netdev namespace
>> change. Basically the per-net notifier follows the netdev into
>> namespace.
>
>This is getting convoluted.
>
>If the driver wants notifications in a new namespace, then it should
>register for notifiers in the new namespace. The info for
>NETDEV_UNREGISTER event could indicate the device is getting moved to a
>new namespace and the driver register for notifications in the new

Yes, I considered this option. However, that would lead to having a pair
of notifier block struct for every registration and basically the same
tracking code would be implemented in every driver.

That is why i chose this implementation where there is still one
notifier block structure and the core takes care of the tracking for
all.


>namespace. If the drivers are trying to be efficient wrt to
>notifications then it will need to unregister when all netdevices leave
>a namespace which it means it has work to do on namespace changes.

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[David Ahern]

On 12/21/19 1:14 AM, Jiri Pirko wrote:
> Fri, Dec 20, 2019 at 07:30:22PM CET, dsahern@gmail.com wrote:
>> On 12/20/19 5:35 AM, Jiri Pirko wrote:
>>> However if netdev can change namespace, per-net notifier cannot be used.
>>> Introduce dev_net variant that is basically per-net notifier with an
>>> extension that re-registers the per-net notifier upon netdev namespace
>>> change. Basically the per-net notifier follows the netdev into
>>> namespace.
>>
>> This is getting convoluted.
>>
>> If the driver wants notifications in a new namespace, then it should
>> register for notifiers in the new namespace. The info for
>> NETDEV_UNREGISTER event could indicate the device is getting moved to a
>> new namespace and the driver register for notifications in the new
> 
> Yes, I considered this option. However, that would lead to having a pair
> of notifier block struct for every registration and basically the same
> tracking code would be implemented in every driver.
> 
> That is why i chose this implementation where there is still one
> notifier block structure and the core takes care of the tracking for
> all.
> 

This design has core code only handling half of the problem - automatic
registration in new namespaces for a netdev but not dealing with drivers
receiving notifications in namespaces they no longer care about. If a
driver cares for granularity, it can deal with namespace changes on its
own. If that's too much, use the global registration.

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[Jiri Pirko]

Sun, Dec 22, 2019 at 05:57:35AM CET, dsahern@gmail.com wrote:
>On 12/21/19 1:14 AM, Jiri Pirko wrote:
>> Fri, Dec 20, 2019 at 07:30:22PM CET, dsahern@gmail.com wrote:
>>> On 12/20/19 5:35 AM, Jiri Pirko wrote:
>>>> However if netdev can change namespace, per-net notifier cannot be used.
>>>> Introduce dev_net variant that is basically per-net notifier with an
>>>> extension that re-registers the per-net notifier upon netdev namespace
>>>> change. Basically the per-net notifier follows the netdev into
>>>> namespace.
>>>
>>> This is getting convoluted.
>>>
>>> If the driver wants notifications in a new namespace, then it should
>>> register for notifiers in the new namespace. The info for
>>> NETDEV_UNREGISTER event could indicate the device is getting moved to a
>>> new namespace and the driver register for notifications in the new
>> 
>> Yes, I considered this option. However, that would lead to having a pair
>> of notifier block struct for every registration and basically the same
>> tracking code would be implemented in every driver.
>> 
>> That is why i chose this implementation where there is still one
>> notifier block structure and the core takes care of the tracking for
>> all.
>> 
>
>This design has core code only handling half of the problem - automatic
>registration in new namespaces for a netdev but not dealing with drivers
>receiving notifications in namespaces they no longer care about. If a

I do not follow. This patchset assures that driver does not get
notification of namespace it does not care about. I'm not sure what do
you mean by "half of the problem".


>driver cares for granularity, it can deal with namespace changes on its
>own. If that's too much, use the global registration.

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[David Ahern]

On 1/6/20 2:15 AM, Jiri Pirko wrote:
> 
> I do not follow. This patchset assures that driver does not get
> notification of namespace it does not care about. I'm not sure what do
> you mean by "half of the problem".

originally, notifiers were only global. drivers registered for them, got
the replay of existing data, and notifications across namespaces.

You added code to allow drivers to register for events in a specific
namespace.

Now you are saying that is not enough as devices can be moved from one
namespace to another and you want core code to automagically register a
driver for events as its devices are moved.

My point is if a driver is trying to be efficient and not handle events
in namespaces it does not care about (the argument for per-namespace
notifiers) then something needs to track that a driver no longer cares
about events in a given namespace once all devices are moved out of Only
the driver knows that and IMHO the driver should be the one managing
what namespaces it wants events.

Example:
driver A has 2 devices eth0, eth1. It registers for events ONLY in
init_net. eth0 is moved to ns0. eth1 is moved to ns1. On the move, core
code registers driver A for events in ns0 and ns1.

Driver A now no longer cares about events in init_net, yet it still
receives them. If this is not a big concern for driver A, then why the
namespace only registration? ie., just use the global and move on. If it
is a concern (your point in this thread), you have not solved the
unregister problem.

ie., I don't like the automagic registration in the new namespace.
drivers should be explicit about what it wants.

> 
> 
>> driver cares for granularity, it can deal with namespace changes on its
>> own. If that's too much, use the global registration.

Re: [patch net-next 0/4] net: allow per-net notifier to follow netdev into namespace

[Jiri Pirko]

Mon, Jan 06, 2020 at 05:37:21PM CET, dsahern@gmail.com wrote:
>On 1/6/20 2:15 AM, Jiri Pirko wrote:
>> 
>> I do not follow. This patchset assures that driver does not get
>> notification of namespace it does not care about. I'm not sure what do
>> you mean by "half of the problem".
>
>originally, notifiers were only global. drivers registered for them, got
>the replay of existing data, and notifications across namespaces.

Not sure what do you mean by "replay of existing data".


>
>You added code to allow drivers to register for events in a specific
>namespace.

For some drivers, like "mlxsw" this is just enough as it does not
support move of netdevices to different namespaces and the namespace of
all netdevices is taken according to namespace of parent devlink
instance.


>
>Now you are saying that is not enough as devices can be moved from one
>namespace to another and you want core code to automagically register a
>driver for events as its devices are moved.
>
>My point is if a driver is trying to be efficient and not handle events
>in namespaces it does not care about (the argument for per-namespace
>notifiers) then something needs to track that a driver no longer cares
>about events in a given namespace once all devices are moved out of Only
>the driver knows that and IMHO the driver should be the one managing
>what namespaces it wants events.

Definitelly. This would be the case for per-driver notifiers.
However, the ones in mlx5 I'm taking care of are per-netdevice
notifiers. Each netdev registers a separate notifier.


>
>Example:
>driver A has 2 devices eth0, eth1. It registers for events ONLY in
>init_net. eth0 is moved to ns0. eth1 is moved to ns1. On the move, core
>code registers driver A for events in ns0 and ns1.
>
>Driver A now no longer cares about events in init_net, yet it still
>receives them. If this is not a big concern for driver A, then why the
>namespace only registration? ie., just use the global and move on. If it
>is a concern (your point in this thread), you have not solved the
>unregister problem.

Wait, why do you think that there is a "unregister problem"?
move_netdevice_notifiers_dev_net() unregisters from the original netns.


>
>ie., I don't like the automagic registration in the new namespace.
>drivers should be explicit about what it wants.
>
>> 
>> 
>>> driver cares for granularity, it can deal with namespace changes on its
>>> own. If that's too much, use the global registration.
>