All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/2] macsec: fix lockdep splats when nesting devices
@ 2016-08-12 14:10 Sabrina Dubroca
  2016-08-12 14:10 ` [PATCH net 2/2] net: remove type_check from dev_get_nest_level() Sabrina Dubroca
  2016-08-13 22:16 ` [PATCH net 1/2] macsec: fix lockdep splats when nesting devices David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Sabrina Dubroca @ 2016-08-12 14:10 UTC (permalink / raw)
  To: netdev; +Cc: Vlad Yasevich, Eric Dumazet, Hannes Frederic Sowa, Sabrina Dubroca

Currently, trying to setup a vlan over a macsec device, or other
combinations of devices, triggers a lockdep warning.

Use netdev_lockdep_set_classes and ndo_get_lock_subclass, similar to
what macvlan does.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index dbd590a8177f..2043e8c97a81 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -270,6 +270,7 @@ struct macsec_dev {
 	struct pcpu_secy_stats __percpu *stats;
 	struct list_head secys;
 	struct gro_cells gro_cells;
+	unsigned int nest_level;
 };
 
 /**
@@ -2699,6 +2700,8 @@ static netdev_tx_t macsec_start_xmit(struct sk_buff *skb,
 
 #define MACSEC_FEATURES \
 	(NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST)
+static struct lock_class_key macsec_netdev_addr_lock_key;
+
 static int macsec_dev_init(struct net_device *dev)
 {
 	struct macsec_dev *macsec = macsec_priv(dev);
@@ -2910,6 +2913,13 @@ static int macsec_get_iflink(const struct net_device *dev)
 	return macsec_priv(dev)->real_dev->ifindex;
 }
 
+
+static int macsec_get_nest_level(struct net_device *dev)
+{
+	return macsec_priv(dev)->nest_level;
+}
+
+
 static const struct net_device_ops macsec_netdev_ops = {
 	.ndo_init		= macsec_dev_init,
 	.ndo_uninit		= macsec_dev_uninit,
@@ -2923,6 +2933,7 @@ static const struct net_device_ops macsec_netdev_ops = {
 	.ndo_start_xmit		= macsec_start_xmit,
 	.ndo_get_stats64	= macsec_get_stats64,
 	.ndo_get_iflink		= macsec_get_iflink,
+	.ndo_get_lock_subclass  = macsec_get_nest_level,
 };
 
 static const struct device_type macsec_type = {
@@ -3050,10 +3061,12 @@ static void macsec_del_dev(struct macsec_dev *macsec)
 static void macsec_common_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct macsec_dev *macsec = macsec_priv(dev);
+	struct net_device *real_dev = macsec->real_dev;
 
 	unregister_netdevice_queue(dev, head);
 	list_del_rcu(&macsec->secys);
 	macsec_del_dev(macsec);
+	netdev_upper_dev_unlink(real_dev, dev);
 
 	macsec_generation++;
 }
@@ -3188,6 +3201,16 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 
 	dev_hold(real_dev);
 
+	macsec->nest_level = dev_get_nest_level(real_dev, netif_is_macsec) + 1;
+	netdev_lockdep_set_classes(dev);
+	lockdep_set_class_and_subclass(&dev->addr_list_lock,
+				       &macsec_netdev_addr_lock_key,
+				       macsec_get_nest_level(dev));
+
+	err = netdev_upper_dev_link(real_dev, dev);
+	if (err < 0)
+		goto unregister;
+
 	/* need to be already registered so that ->init has run and
 	 * the MAC addr is set
 	 */
@@ -3200,12 +3223,12 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 
 	if (rx_handler && sci_exists(real_dev, sci)) {
 		err = -EBUSY;
-		goto unregister;
+		goto unlink;
 	}
 
 	err = macsec_add_dev(dev, sci, icv_len);
 	if (err)
-		goto unregister;
+		goto unlink;
 
 	if (data)
 		macsec_changelink_common(dev, data);
@@ -3220,6 +3243,8 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 
 del_dev:
 	macsec_del_dev(macsec);
+unlink:
+	netdev_upper_dev_unlink(real_dev, dev);
 unregister:
 	unregister_netdevice(dev);
 	return err;
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 2/2] net: remove type_check from dev_get_nest_level()
  2016-08-12 14:10 [PATCH net 1/2] macsec: fix lockdep splats when nesting devices Sabrina Dubroca
@ 2016-08-12 14:10 ` Sabrina Dubroca
  2016-08-13 22:16   ` David Miller
  2016-08-13 22:16 ` [PATCH net 1/2] macsec: fix lockdep splats when nesting devices David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Sabrina Dubroca @ 2016-08-12 14:10 UTC (permalink / raw)
  To: netdev; +Cc: Vlad Yasevich, Eric Dumazet, Hannes Frederic Sowa, Sabrina Dubroca

The idea for type_check in dev_get_nest_level() was to count the number
of nested devices of the same type (currently, only macvlan or vlan
devices).
This prevented the false positive lockdep warning on configurations such
as:

eth0 <--- macvlan0 <--- vlan0 <--- macvlan1

However, this doesn't prevent a warning on a configuration such as:

eth0 <--- macvlan0 <--- vlan0
eth1 <--- vlan1 <--- macvlan1

In this case, all the locks end up with a nesting subclass of 1, so
lockdep thinks that there is still a deadlock:

- in the first case we have (macvlan_netdev_addr_lock_key, 1) and then
  take (vlan_netdev_xmit_lock_key, 1)
- in the second case, we have (vlan_netdev_xmit_lock_key, 1) and then
  take (macvlan_netdev_addr_lock_key, 1)

By removing the linktype check in dev_get_nest_level() and always
incrementing the nesting depth, lockdep considers this configuration
valid.

Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 drivers/net/macsec.c      |  2 +-
 drivers/net/macvlan.c     |  2 +-
 include/linux/netdevice.h |  3 +--
 net/8021q/vlan.c          |  2 +-
 net/core/dev.c            | 10 +++-------
 5 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 2043e8c97a81..351e701eb043 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3201,7 +3201,7 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
 
 	dev_hold(real_dev);
 
-	macsec->nest_level = dev_get_nest_level(real_dev, netif_is_macsec) + 1;
+	macsec->nest_level = dev_get_nest_level(real_dev) + 1;
 	netdev_lockdep_set_classes(dev);
 	lockdep_set_class_and_subclass(&dev->addr_list_lock,
 				       &macsec_netdev_addr_lock_key,
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index cd9b53834bf6..3234fcdea317 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1315,7 +1315,7 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	vlan->dev      = dev;
 	vlan->port     = port;
 	vlan->set_features = MACVLAN_FEATURES;
-	vlan->nest_level = dev_get_nest_level(lowerdev, netif_is_macvlan) + 1;
+	vlan->nest_level = dev_get_nest_level(lowerdev) + 1;
 
 	vlan->mode     = MACVLAN_MODE_VEPA;
 	if (data && data[IFLA_MACVLAN_MODE])
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 076df5360ba5..3a788bf0affd 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3891,8 +3891,7 @@ void netdev_default_l2upper_neigh_destroy(struct net_device *dev,
 extern u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
 void netdev_rss_key_fill(void *buffer, size_t len);
 
-int dev_get_nest_level(struct net_device *dev,
-		       bool (*type_check)(const struct net_device *dev));
+int dev_get_nest_level(struct net_device *dev);
 int skb_checksum_help(struct sk_buff *skb);
 struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 				  netdev_features_t features, bool tx_path);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 82a116ba590e..8de138d3306b 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -169,7 +169,7 @@ int register_vlan_dev(struct net_device *dev)
 	if (err < 0)
 		goto out_uninit_mvrp;
 
-	vlan->nest_level = dev_get_nest_level(real_dev, is_vlan_dev) + 1;
+	vlan->nest_level = dev_get_nest_level(real_dev) + 1;
 	err = register_netdevice(dev);
 	if (err < 0)
 		goto out_uninit_mvrp;
diff --git a/net/core/dev.c b/net/core/dev.c
index 4ce07dc25573..dd6ce598de89 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6045,8 +6045,7 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
 EXPORT_SYMBOL(netdev_lower_dev_get_private);
 
 
-int dev_get_nest_level(struct net_device *dev,
-		       bool (*type_check)(const struct net_device *dev))
+int dev_get_nest_level(struct net_device *dev)
 {
 	struct net_device *lower = NULL;
 	struct list_head *iter;
@@ -6056,15 +6055,12 @@ int dev_get_nest_level(struct net_device *dev,
 	ASSERT_RTNL();
 
 	netdev_for_each_lower_dev(dev, lower, iter) {
-		nest = dev_get_nest_level(lower, type_check);
+		nest = dev_get_nest_level(lower);
 		if (max_nest < nest)
 			max_nest = nest;
 	}
 
-	if (type_check(dev))
-		max_nest++;
-
-	return max_nest;
+	return max_nest + 1;
 }
 EXPORT_SYMBOL(dev_get_nest_level);
 
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net 1/2] macsec: fix lockdep splats when nesting devices
  2016-08-12 14:10 [PATCH net 1/2] macsec: fix lockdep splats when nesting devices Sabrina Dubroca
  2016-08-12 14:10 ` [PATCH net 2/2] net: remove type_check from dev_get_nest_level() Sabrina Dubroca
@ 2016-08-13 22:16 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2016-08-13 22:16 UTC (permalink / raw)
  To: sd; +Cc: netdev, vyasevich, eric.dumazet, hannes

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 12 Aug 2016 16:10:32 +0200

> Currently, trying to setup a vlan over a macsec device, or other
> combinations of devices, triggers a lockdep warning.
> 
> Use netdev_lockdep_set_classes and ndo_get_lock_subclass, similar to
> what macvlan does.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 2/2] net: remove type_check from dev_get_nest_level()
  2016-08-12 14:10 ` [PATCH net 2/2] net: remove type_check from dev_get_nest_level() Sabrina Dubroca
@ 2016-08-13 22:16   ` David Miller
  0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2016-08-13 22:16 UTC (permalink / raw)
  To: sd; +Cc: netdev, vyasevich, eric.dumazet, hannes

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Fri, 12 Aug 2016 16:10:33 +0200

> The idea for type_check in dev_get_nest_level() was to count the number
> of nested devices of the same type (currently, only macvlan or vlan
> devices).
> This prevented the false positive lockdep warning on configurations such
> as:
> 
> eth0 <--- macvlan0 <--- vlan0 <--- macvlan1
> 
> However, this doesn't prevent a warning on a configuration such as:
> 
> eth0 <--- macvlan0 <--- vlan0
> eth1 <--- vlan1 <--- macvlan1
> 
> In this case, all the locks end up with a nesting subclass of 1, so
> lockdep thinks that there is still a deadlock:
> 
> - in the first case we have (macvlan_netdev_addr_lock_key, 1) and then
>   take (vlan_netdev_xmit_lock_key, 1)
> - in the second case, we have (vlan_netdev_xmit_lock_key, 1) and then
>   take (macvlan_netdev_addr_lock_key, 1)
> 
> By removing the linktype check in dev_get_nest_level() and always
> incrementing the nesting depth, lockdep considers this configuration
> valid.
> 
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>

Applied.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-08-14  8:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-12 14:10 [PATCH net 1/2] macsec: fix lockdep splats when nesting devices Sabrina Dubroca
2016-08-12 14:10 ` [PATCH net 2/2] net: remove type_check from dev_get_nest_level() Sabrina Dubroca
2016-08-13 22:16   ` David Miller
2016-08-13 22:16 ` [PATCH net 1/2] macsec: fix lockdep splats when nesting devices David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.