linux-s390.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net-next 00/16] net: introduce upper device lists and remove dev->master
@ 2012-08-13 15:26 Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 02/16] macvlan: add link to upper device Jiri Pirko
                   ` (15 more replies)
  0 siblings, 16 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:26 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Hi all.

Recent discussion around
"[net-next] bonding: don't allow the master to become its slave"
forced me to think about upper<->lower device connections.

This patchset adds a possibility to record upper device linkage.
All upper<->lower devices are converted to use this mechanism right after.
That leads to dev->master removal because this info becomes redundant since
"unique links" have the same value.

After all changes, there is no longer possible to do:
"bond->someotherdevice->samebond"

Also I think that drivers like cxgb3, qlcnic, qeth would benefit by this
in future by being able to get more appropriate info about l3 addresses.

Jiri Pirko (16):
  net: introduce upper device lists
  macvlan: add link to upper device
  vlan: add link to upper device
  rtnetlink: remove usage of dev->master
  team: remove usage of netdev_set_master()
  bridge: remove usage of netdev_set_master()
  netpoll: remove usage of dev->master
  cxgb3: remove usage of dev->master
  qlcnic: guard __vlan_find_dev_deep() by rcu_read_lock
  qeth: ensure that __vlan_find_dev_deep() is called with rcu_read_lock
  vlan: remove usage of dev->master in __vlan_find_dev_deep()
  nes: remove usage of dev->master
  bonding: remove usage of dev->master
  net: remove no longer used netdev_set_bond_master() and
    netdev_set_master()
  net: remove usage of dev->master
  net: kill dev->master

 drivers/infiniband/hw/nes/nes.c                    |    8 +-
 drivers/infiniband/hw/nes/nes_cm.c                 |    2 +-
 drivers/net/bonding/bond_3ad.c                     |   30 +--
 drivers/net/bonding/bond_alb.c                     |    6 +-
 drivers/net/bonding/bond_main.c                    |   94 ++++----
 drivers/net/bonding/bonding.h                      |   14 +-
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c |   11 +-
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c   |    2 +
 drivers/net/macvlan.c                              |    9 +-
 drivers/net/team/team.c                            |   13 +-
 drivers/s390/net/qeth_l3_main.c                    |   21 +-
 include/linux/netdevice.h                          |   22 +-
 net/8021q/vlan.c                                   |   10 +-
 net/8021q/vlan_core.c                              |   18 +-
 net/bridge/br_if.c                                 |    6 +-
 net/core/dev.c                                     |  241 +++++++++++++++++---
 net/core/netpoll.c                                 |    8 +-
 net/core/rtnetlink.c                               |   45 ++--
 18 files changed, 392 insertions(+), 168 deletions(-)

-- 
1.7.10.4

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

* [patch net-next 02/16] macvlan: add link to upper device
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 03/16] vlan: " Jiri Pirko
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/macvlan.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 66a9bfe..f5bd8a2 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -765,16 +765,22 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 		memcpy(dev->dev_addr, lowerdev->dev_addr, ETH_ALEN);
 	}
 
+	err = netdev_upper_dev_link(lowerdev, dev);
+	if (err)
+		goto destroy_port;
+
 	port->count += 1;
 	err = register_netdevice(dev);
 	if (err < 0)
-		goto destroy_port;
+		goto upper_dev_unlink;
 
 	list_add_tail(&vlan->list, &port->vlans);
 	netif_stacked_transfer_operstate(lowerdev, dev);
 
 	return 0;
 
+upper_dev_unlink:
+	netdev_upper_dev_unlink(lowerdev, dev);
 destroy_port:
 	port->count -= 1;
 	if (!port->count)
@@ -798,6 +804,7 @@ void macvlan_dellink(struct net_device *dev, struct list_head *head)
 
 	list_del(&vlan->list);
 	unregister_netdevice_queue(dev, head);
+	netdev_upper_dev_unlink(vlan->lowerdev, dev);
 }
 EXPORT_SYMBOL_GPL(macvlan_dellink);
 
-- 
1.7.10.4

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

* [patch net-next 03/16] vlan: add link to upper device
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 02/16] macvlan: add link to upper device Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 19:04   ` Flavio Leitner
  2012-08-13 15:27 ` [patch net-next 04/16] rtnetlink: remove usage of dev->master Jiri Pirko
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/8021q/vlan.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 9096bcb..739665e 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -105,6 +105,8 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
 	 */
 	unregister_netdevice_queue(dev, head);
 
+	netdev_upper_dev_unlink(real_dev, dev);
+
 	if (grp->nr_vlan_devs == 0)
 		vlan_gvrp_uninit_applicant(real_dev);
 
@@ -162,9 +164,13 @@ int register_vlan_dev(struct net_device *dev)
 	if (err < 0)
 		goto out_uninit_applicant;
 
+	err = netdev_upper_dev_link(real_dev, dev);
+	if (err)
+		goto out_uninit_applicant;
+
 	err = register_netdevice(dev);
 	if (err < 0)
-		goto out_uninit_applicant;
+		goto out_upper_dev_unlink;
 
 	/* Account for reference in struct vlan_dev_priv */
 	dev_hold(real_dev);
@@ -180,6 +186,8 @@ int register_vlan_dev(struct net_device *dev)
 
 	return 0;
 
+upper_dev_unlink:
+	netdev_upper_dev_unlink(real_dev, dev);
 out_uninit_applicant:
 	if (grp->nr_vlan_devs == 0)
 		vlan_gvrp_uninit_applicant(real_dev);
-- 
1.7.10.4

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

* [patch net-next 04/16] rtnetlink: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 02/16] macvlan: add link to upper device Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 03/16] vlan: " Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 05/16] team: remove usage of netdev_set_master() Jiri Pirko
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/core/rtnetlink.c |   44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 34d975b..c341eb7 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -879,6 +879,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	const struct rtnl_link_stats64 *stats;
 	struct nlattr *attr, *af_spec;
 	struct rtnl_af_ops *af_ops;
+	struct net_device *upper_dev = netdev_unique_upper_dev_get(dev);
 
 	ASSERT_RTNL();
 	nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
@@ -907,8 +908,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 #endif
 	    (dev->ifindex != dev->iflink &&
 	     nla_put_u32(skb, IFLA_LINK, dev->iflink)) ||
-	    (dev->master &&
-	     nla_put_u32(skb, IFLA_MASTER, dev->master->ifindex)) ||
+	    (upper_dev &&
+	     nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex)) ||
 	    (dev->qdisc &&
 	     nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
 	    (dev->ifalias &&
@@ -1270,16 +1271,16 @@ static int do_setvfinfo(struct net_device *dev, struct nlattr *attr)
 
 static int do_set_master(struct net_device *dev, int ifindex)
 {
-	struct net_device *master_dev;
+	struct net_device *upper_dev = netdev_unique_upper_dev_get(dev);
 	const struct net_device_ops *ops;
 	int err;
 
-	if (dev->master) {
-		if (dev->master->ifindex == ifindex)
+	if (upper_dev) {
+		if (upper_dev->ifindex == ifindex)
 			return 0;
-		ops = dev->master->netdev_ops;
+		ops = upper_dev->netdev_ops;
 		if (ops->ndo_del_slave) {
-			err = ops->ndo_del_slave(dev->master, dev);
+			err = ops->ndo_del_slave(upper_dev, dev);
 			if (err)
 				return err;
 		} else {
@@ -1288,12 +1289,12 @@ static int do_set_master(struct net_device *dev, int ifindex)
 	}
 
 	if (ifindex) {
-		master_dev = __dev_get_by_index(dev_net(dev), ifindex);
-		if (!master_dev)
+		upper_dev = __dev_get_by_index(dev_net(dev), ifindex);
+		if (!upper_dev)
 			return -EINVAL;
-		ops = master_dev->netdev_ops;
+		ops = upper_dev->netdev_ops;
 		if (ops->ndo_add_slave) {
-			err = ops->ndo_add_slave(master_dev, dev);
+			err = ops->ndo_add_slave(upper_dev, dev);
 			if (err)
 				return err;
 		} else {
@@ -2050,7 +2051,6 @@ errout:
 static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 {
 	struct net *net = sock_net(skb->sk);
-	struct net_device *master = NULL;
 	struct ndmsg *ndm;
 	struct nlattr *tb[NDA_MAX+1];
 	struct net_device *dev;
@@ -2089,9 +2089,10 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	/* Support fdb on master device the net/bridge default case */
 	if ((!ndm->ndm_flags || ndm->ndm_flags & NTF_MASTER) &&
 	    (dev->priv_flags & IFF_BRIDGE_PORT)) {
-		master = dev->master;
-		err = master->netdev_ops->ndo_fdb_add(ndm, dev, addr,
-						      nlh->nlmsg_flags);
+		struct net_device *br_dev = netdev_unique_upper_dev_get(dev);
+		const struct net_device_ops *ops = br_dev->netdev_ops;
+
+		err = ops->ndo_fdb_add(ndm, dev, addr, nlh->nlmsg_flags);
 		if (err)
 			goto out;
 		else
@@ -2148,10 +2149,11 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 	/* Support fdb on master device the net/bridge default case */
 	if ((!ndm->ndm_flags || ndm->ndm_flags & NTF_MASTER) &&
 	    (dev->priv_flags & IFF_BRIDGE_PORT)) {
-		struct net_device *master = dev->master;
+		struct net_device *br_dev = netdev_unique_upper_dev_get(dev);
+		const struct net_device_ops *ops = br_dev->netdev_ops;
 
-		if (master->netdev_ops->ndo_fdb_del)
-			err = master->netdev_ops->ndo_fdb_del(ndm, dev, addr);
+		if (ops->ndo_fdb_del)
+			err = ops->ndo_fdb_del(ndm, dev, addr);
 
 		if (err)
 			goto out;
@@ -2234,9 +2236,11 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb)
 	rcu_read_lock();
 	for_each_netdev_rcu(net, dev) {
 		if (dev->priv_flags & IFF_BRIDGE_PORT) {
-			struct net_device *master = dev->master;
-			const struct net_device_ops *ops = master->netdev_ops;
+			struct net_device *br_dev;
+			const struct net_device_ops *ops;
 
+			br_dev = netdev_unique_upper_dev_get(dev);
+			ops = br_dev->netdev_ops;
 			if (ops->ndo_fdb_dump)
 				idx = ops->ndo_fdb_dump(skb, cb, dev, idx);
 		}
-- 
1.7.10.4

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

* [patch net-next 05/16] team: remove usage of netdev_set_master()
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (2 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 04/16] rtnetlink: remove usage of dev->master Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 06/16] bridge: " Jiri Pirko
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index ba10c46..0533cbf 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1040,10 +1040,11 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
 		}
 	}
 
-	err = netdev_set_master(port_dev, dev);
+	err = netdev_unique_upper_dev_link(port_dev, dev);
 	if (err) {
-		netdev_err(dev, "Device %s failed to set master\n", portname);
-		goto err_set_master;
+		netdev_err(dev, "Device %s failed to set upper link\n",
+			   portname);
+		goto err_set_upper_link;
 	}
 
 	err = netdev_rx_handler_register(port_dev, team_handle_frame,
@@ -1076,9 +1077,9 @@ err_option_port_add:
 	netdev_rx_handler_unregister(port_dev);
 
 err_handler_register:
-	netdev_set_master(port_dev, NULL);
+	netdev_upper_dev_unlink(port_dev, dev);
 
-err_set_master:
+err_set_upper_link:
 	team_port_disable_netpoll(port);
 
 err_enable_netpoll:
@@ -1121,7 +1122,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 	team_port_disable(team, port);
 	list_del_rcu(&port->list);
 	netdev_rx_handler_unregister(port_dev);
-	netdev_set_master(port_dev, NULL);
+	netdev_upper_dev_unlink(port_dev, dev);
 	team_port_disable_netpoll(port);
 	vlan_vids_del_by_dev(port_dev, dev);
 	dev_close(port_dev);
-- 
1.7.10.4

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

* [patch net-next 06/16] bridge: remove usage of netdev_set_master()
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (3 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 05/16] team: remove usage of netdev_set_master() Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 07/16] netpoll: remove usage of dev->master Jiri Pirko
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/bridge/br_if.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index e1144e1..5933900 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -148,7 +148,7 @@ static void del_nbp(struct net_bridge_port *p)
 	netdev_rx_handler_unregister(dev);
 	synchronize_net();
 
-	netdev_set_master(dev, NULL);
+	netdev_upper_dev_unlink(dev, br->dev);
 
 	br_multicast_del_port(p);
 
@@ -364,7 +364,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 	if (br_netpoll_info(br) && ((err = br_netpoll_enable(p))))
 		goto err3;
 
-	err = netdev_set_master(dev, br->dev);
+	err = netdev_unique_upper_dev_link(dev, br->dev);
 	if (err)
 		goto err3;
 
@@ -403,7 +403,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
 	return 0;
 
 err4:
-	netdev_set_master(dev, NULL);
+	netdev_upper_dev_unlink(dev, br->dev);
 err3:
 	sysfs_remove_link(br->ifobj, p->dev->name);
 err2:
-- 
1.7.10.4

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

* [patch net-next 07/16] netpoll: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (4 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 06/16] bridge: " Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 08/16] cxgb3: " Jiri Pirko
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/core/netpoll.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index b4c90e4..5c8e560 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -207,12 +207,16 @@ static void netpoll_poll_dev(struct net_device *dev)
 
 	if (dev->flags & IFF_SLAVE) {
 		if (dev->npinfo) {
-			struct net_device *bond_dev = dev->master;
+			struct net_device *bond_dev;
 			struct sk_buff *skb;
+
+			rcu_read_lock();
+			bond_dev = netdev_unique_upper_dev_get_rcu(dev);
 			while ((skb = skb_dequeue(&dev->npinfo->arp_tx))) {
 				skb->dev = bond_dev;
 				skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
 			}
+			rcu_read_unlock();
 		}
 	}
 
@@ -795,7 +799,7 @@ int netpoll_setup(struct netpoll *np)
 		return -ENODEV;
 	}
 
-	if (ndev->master) {
+	if (netdev_unique_upper_dev_get(ndev)) {
 		np_err(np, "%s is a slave device, aborting\n", np->dev_name);
 		err = -EBUSY;
 		goto put;
-- 
1.7.10.4

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

* [patch net-next 08/16] cxgb3: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (5 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 07/16] netpoll: remove usage of dev->master Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 09/16] qlcnic: guard __vlan_find_dev_deep() by rcu_read_lock Jiri Pirko
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
index 2dbbcbb..28365db 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
@@ -182,14 +182,17 @@ static struct net_device *get_iff_from_mac(struct adapter *adapter,
 		struct net_device *dev = adapter->port[i];
 
 		if (!memcmp(dev->dev_addr, mac, ETH_ALEN)) {
+			rcu_read_lock();
 			if (vlan && vlan != VLAN_VID_MASK) {
-				rcu_read_lock();
 				dev = __vlan_find_dev_deep(dev, vlan);
-				rcu_read_unlock();
 			} else if (netif_is_bond_slave(dev)) {
-				while (dev->master)
-					dev = dev->master;
+				struct net_device *upper_dev;
+
+				while ((upper_dev =
+					netdev_unique_upper_dev_get_rcu(dev)))
+					dev = upper_dev;
 			}
+			rcu_read_unlock();
 			return dev;
 		}
 	}
-- 
1.7.10.4

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

* [patch net-next 09/16] qlcnic: guard __vlan_find_dev_deep() by rcu_read_lock
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (6 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 08/16] cxgb3: " Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 10/16] qeth: ensure that __vlan_find_dev_deep() is called with rcu_read_lock Jiri Pirko
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

rcu_read_lock was missing here

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 212c121..9eadf17 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -4425,12 +4425,14 @@ qlcnic_restore_indev_addr(struct net_device *netdev, unsigned long event)
 
 	qlcnic_config_indev_addr(adapter, netdev, event);
 
+	rcu_read_lock();
 	for_each_set_bit(vid, adapter->vlans, VLAN_N_VID) {
 		dev = __vlan_find_dev_deep(netdev, vid);
 		if (!dev)
 			continue;
 		qlcnic_config_indev_addr(adapter, dev, event);
 	}
+	rcu_read_unlock();
 }
 
 static int qlcnic_netdev_event(struct notifier_block *this,
-- 
1.7.10.4

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

* [patch net-next 10/16] qeth: ensure that __vlan_find_dev_deep() is called with rcu_read_lock
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (7 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 09/16] qlcnic: guard __vlan_find_dev_deep() by rcu_read_lock Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 11/16] vlan: remove usage of dev->master in __vlan_find_dev_deep() Jiri Pirko
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Also benefit from rcu_read_lock held and use __in_dev_get_rcu() in ipv4 case.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/s390/net/qeth_l3_main.c |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index c5f03fa..6b3988b 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -1638,6 +1638,7 @@ static void qeth_l3_add_mc(struct qeth_card *card, struct in_device *in4_dev)
 	}
 }
 
+/* called with rcu_read_lock */
 static void qeth_l3_add_vlan_mc(struct qeth_card *card)
 {
 	struct in_device *in_dev;
@@ -1650,19 +1651,14 @@ static void qeth_l3_add_vlan_mc(struct qeth_card *card)
 	for_each_set_bit(vid, card->active_vlans, VLAN_N_VID) {
 		struct net_device *netdev;
 
-		rcu_read_lock();
 		netdev = __vlan_find_dev_deep(card->dev, vid);
-		rcu_read_unlock();
 		if (netdev == NULL ||
 		    !(netdev->flags & IFF_UP))
 			continue;
-		in_dev = in_dev_get(netdev);
+		in_dev = __in_dev_get_rcu(netdev);
 		if (!in_dev)
 			continue;
-		rcu_read_lock();
 		qeth_l3_add_mc(card, in_dev);
-		rcu_read_unlock();
-		in_dev_put(in_dev);
 	}
 }
 
@@ -1671,14 +1667,14 @@ static void qeth_l3_add_multicast_ipv4(struct qeth_card *card)
 	struct in_device *in4_dev;
 
 	QETH_CARD_TEXT(card, 4, "chkmcv4");
-	in4_dev = in_dev_get(card->dev);
-	if (in4_dev == NULL)
-		return;
 	rcu_read_lock();
+	in4_dev = __in_dev_get_rcu(card->dev);
+	if (in4_dev == NULL)
+		goto unlock;
 	qeth_l3_add_mc(card, in4_dev);
 	qeth_l3_add_vlan_mc(card);
+unlock:
 	rcu_read_unlock();
-	in_dev_put(in4_dev);
 }
 
 #ifdef CONFIG_QETH_IPV6
@@ -1703,6 +1699,7 @@ static void qeth_l3_add_mc6(struct qeth_card *card, struct inet6_dev *in6_dev)
 	}
 }
 
+/* called with rcu_read_lock */
 static void qeth_l3_add_vlan_mc6(struct qeth_card *card)
 {
 	struct inet6_dev *in_dev;
@@ -1739,10 +1736,12 @@ static void qeth_l3_add_multicast_ipv6(struct qeth_card *card)
 	in6_dev = in6_dev_get(card->dev);
 	if (in6_dev == NULL)
 		return;
+	rcu_read_lock();
 	read_lock_bh(&in6_dev->lock);
 	qeth_l3_add_mc6(card, in6_dev);
 	qeth_l3_add_vlan_mc6(card);
 	read_unlock_bh(&in6_dev->lock);
+	rcu_read_unlock();
 	in6_dev_put(in6_dev);
 }
 #endif /* CONFIG_QETH_IPV6 */
@@ -1811,8 +1810,10 @@ static void qeth_l3_free_vlan_addresses6(struct qeth_card *card,
 static void qeth_l3_free_vlan_addresses(struct qeth_card *card,
 			unsigned short vid)
 {
+	rcu_read_lock();
 	qeth_l3_free_vlan_addresses4(card, vid);
 	qeth_l3_free_vlan_addresses6(card, vid);
+	rcu_read_unlock();
 }
 
 static int qeth_l3_vlan_rx_add_vid(struct net_device *dev, unsigned short vid)
-- 
1.7.10.4

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

* [patch net-next 11/16] vlan: remove usage of dev->master in __vlan_find_dev_deep()
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (8 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 10/16] qeth: ensure that __vlan_find_dev_deep() is called with rcu_read_lock Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 12/16] nes: remove usage of dev->master Jiri Pirko
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Also, since all users call __vlan_find_dev_deep() with rcu_read_lock,
make no possibility to call this with rtnl mutex held only.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/8021q/vlan_core.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 8ca533c..f9518ad 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -66,21 +66,25 @@ bool vlan_do_receive(struct sk_buff **skbp, bool last_handler)
 	return true;
 }
 
-/* Must be invoked with rcu_read_lock or with RTNL. */
-struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
+/* Must be invoked with rcu_read_lock. */
+struct net_device *__vlan_find_dev_deep(struct net_device *dev,
 					u16 vlan_id)
 {
-	struct vlan_info *vlan_info = rcu_dereference_rtnl(real_dev->vlan_info);
+	struct vlan_info *vlan_info = rcu_dereference(dev->vlan_info);
 
 	if (vlan_info) {
 		return vlan_group_get_device(&vlan_info->grp, vlan_id);
 	} else {
 		/*
-		 * Bonding slaves do not have grp assigned to themselves.
-		 * Grp is assigned to bonding master instead.
+		 * Lower devices of unique uppers (bonding, team) do not have
+		 * grp assigned to themselves. Grp is assigned to upper device
+		 * instead.
 		 */
-		if (netif_is_bond_slave(real_dev))
-			return __vlan_find_dev_deep(real_dev->master, vlan_id);
+		struct net_device *upper_dev;
+
+		upper_dev = netdev_unique_upper_dev_get_rcu(dev);
+		if (upper_dev)
+			return __vlan_find_dev_deep(upper_dev, vlan_id);
 	}
 
 	return NULL;
-- 
1.7.10.4

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

* [patch net-next 12/16] nes: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (9 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 11/16] vlan: remove usage of dev->master in __vlan_find_dev_deep() Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 13/16] bonding: " Jiri Pirko
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/infiniband/hw/nes/nes.c    |    8 +++++---
 drivers/infiniband/hw/nes/nes_cm.c |    2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes.c b/drivers/infiniband/hw/nes/nes.c
index 7140199..e58e9af 100644
--- a/drivers/infiniband/hw/nes/nes.c
+++ b/drivers/infiniband/hw/nes/nes.c
@@ -140,6 +140,7 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 	struct net_device *event_netdev = ifa->ifa_dev->dev;
 	struct nes_device *nesdev;
 	struct net_device *netdev;
+	struct net_device *upper_dev;
 	struct nes_vnic *nesvnic;
 	unsigned int is_bonded;
 
@@ -150,8 +151,9 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 				nesdev, nesdev->netdev[0]->name);
 		netdev = nesdev->netdev[0];
 		nesvnic = netdev_priv(netdev);
+		upper_dev = netdev_unique_upper_dev_get(netdev);
 		is_bonded = netif_is_bond_slave(netdev) &&
-			    (netdev->master == event_netdev);
+			    (upper_dev == event_netdev);
 		if ((netdev == event_netdev) || is_bonded) {
 			if (nesvnic->rdma_enabled == 0) {
 				nes_debug(NES_DBG_NETDEV, "Returning without processing event for %s since"
@@ -184,9 +186,9 @@ static int nes_inetaddr_event(struct notifier_block *notifier,
 					/* fall through */
 				case NETDEV_CHANGEADDR:
 					/* Add the address to the IP table */
-					if (netdev->master)
+					if (upper_dev)
 						nesvnic->local_ipaddr =
-							((struct in_device *)netdev->master->ip_ptr)->ifa_list->ifa_address;
+							((struct in_device *)upper_dev->ip_ptr)->ifa_list->ifa_address;
 					else
 						nesvnic->local_ipaddr = ifa->ifa_address;
 
diff --git a/drivers/infiniband/hw/nes/nes_cm.c b/drivers/infiniband/hw/nes/nes_cm.c
index 020e95c..15e6340 100644
--- a/drivers/infiniband/hw/nes/nes_cm.c
+++ b/drivers/infiniband/hw/nes/nes_cm.c
@@ -1352,7 +1352,7 @@ static int nes_addr_resolve_neigh(struct nes_vnic *nesvnic, u32 dst_ip, int arpi
 	}
 
 	if (netif_is_bond_slave(nesvnic->netdev))
-		netdev = nesvnic->netdev->master;
+		netdev = netdev_unique_upper_dev_get(nesvnic->netdev);
 	else
 		netdev = nesvnic->netdev;
 
-- 
1.7.10.4

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

* [patch net-next 13/16] bonding: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (10 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 12/16] nes: remove usage of dev->master Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 14/16] net: remove no longer used netdev_set_bond_master() and netdev_set_master() Jiri Pirko
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Benefit from new upper dev list and free bonding from dev->master usage.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/bonding/bond_3ad.c  |   30 ++++++-------
 drivers/net/bonding/bond_alb.c  |    6 +--
 drivers/net/bonding/bond_main.c |   94 +++++++++++++++++++++++----------------
 drivers/net/bonding/bonding.h   |   14 +++---
 net/core/rtnetlink.c            |    1 +
 5 files changed, 81 insertions(+), 64 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index a030e63..84fabd6 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1127,7 +1127,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 				// INFO_RECEIVED_LOOPBACK_FRAMES
 				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
 				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->dev->master->name, port->slave->dev->name);
+				       port->slave->bond->dev->name, port->slave->dev->name);
 				return;
 			}
 			__update_selected(lacpdu, port);
@@ -1306,7 +1306,7 @@ static void ad_port_selection_logic(struct port *port)
 		}
 		if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
 			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
-				   port->slave->dev->master->name,
+				   port->slave->bond->dev->name,
 				   port->actor_port_number,
 				   port->slave->dev->name,
 				   port->aggregator->aggregator_identifier);
@@ -1386,7 +1386,7 @@ static void ad_port_selection_logic(struct port *port)
 				 port->aggregator->aggregator_identifier);
 		} else {
 			pr_err("%s: Port %d (on %s) did not find a suitable aggregator\n",
-			       port->slave->dev->master->name,
+			       port->slave->bond->dev->name,
 			       port->actor_port_number, port->slave->dev->name);
 		}
 	}
@@ -1463,7 +1463,7 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 
 	default:
 		pr_warning("%s: Impossible agg select mode %d\n",
-			   curr->slave->dev->master->name,
+			   curr->slave->bond->dev->name,
 			   __get_agg_selection_mode(curr->lag_ports));
 		break;
 	}
@@ -1571,7 +1571,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		// check if any partner replys
 		if (best->is_individual) {
 			pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
-				   best->slave ? best->slave->dev->master->name : "NULL");
+				   best->slave ? best->slave->bond->dev->name : "NULL");
 		}
 
 		best->is_active = 1;
@@ -1898,7 +1898,7 @@ int bond_3ad_bind_slave(struct slave *slave)
 
 	if (bond == NULL) {
 		pr_err("%s: The slave %s is not attached to its bond\n",
-		       slave->dev->master->name, slave->dev->name);
+		       slave->bond->dev->name, slave->dev->name);
 		return -1;
 	}
 
@@ -1973,7 +1973,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2009,7 +2009,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 
 				if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
 					pr_info("%s: Removing an active aggregator\n",
-						aggregator->slave->dev->master->name);
+						aggregator->slave->bond->dev->name);
 					// select new active aggregator
 					 select_new_active_agg = 1;
 				}
@@ -2040,7 +2040,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					ad_agg_selection_logic(__get_first_agg(port));
 			} else {
 				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
-					   slave->dev->master->name);
+					   slave->bond->dev->name);
 			}
 		} else { // in case that the only port related to this aggregator is the one we want to remove
 			select_new_active_agg = aggregator->is_active;
@@ -2048,7 +2048,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
 				pr_info("%s: Removing an active aggregator\n",
-					slave->dev->master->name);
+					slave->bond->dev->name);
 				// select new active aggregator
 				ad_agg_selection_logic(__get_first_agg(port));
 			}
@@ -2076,7 +2076,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
 						pr_info("%s: Removing an active aggregator\n",
-							slave->dev->master->name);
+							slave->bond->dev->name);
 						// select new active aggregator
 						ad_agg_selection_logic(__get_first_agg(port));
 					}
@@ -2184,7 +2184,7 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u1
 
 		if (!port->slave) {
 			pr_warning("%s: Warning: port of slave %s is uninitialized\n",
-				   slave->dev->name, slave->dev->master->name);
+				   slave->dev->name, slave->bond->dev->name);
 			return ret;
 		}
 
@@ -2240,7 +2240,7 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2268,7 +2268,7 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2297,7 +2297,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
 		pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
-			   slave->dev->master->name, slave->dev->name);
+			   slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index e15cc11..6dc3625 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -491,7 +491,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 				 client_info->mac_dst);
 		if (!skb) {
 			pr_err("%s: Error: failed to create an ARP packet\n",
-			       client_info->slave->dev->master->name);
+			       client_info->slave->bond->dev->name);
 			continue;
 		}
 
@@ -501,7 +501,7 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 			skb = vlan_put_tag(skb, client_info->vlan_id);
 			if (!skb) {
 				pr_err("%s: Error: failed to insert VLAN tag\n",
-				       client_info->slave->dev->master->name);
+				       client_info->slave->bond->dev->name);
 				continue;
 			}
 		}
@@ -914,7 +914,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[])
 	if (dev_set_mac_address(dev, &s_addr)) {
 		pr_err("%s: Error: dev_set_mac_address of dev %s failed!\n"
 		       "ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n",
-		       dev->master->name, dev->name);
+		       slave->bond->dev->name, dev->name);
 		return -EOPNOTSUPP;
 	}
 	return 0;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6fae5f3..de75541 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -752,11 +752,9 @@ static void __bond_resend_igmp_join_requests(struct net_device *dev)
 {
 	struct in_device *in_dev;
 
-	rcu_read_lock();
 	in_dev = __in_dev_get_rcu(dev);
 	if (in_dev)
 		ip_mc_rejoin_groups(in_dev);
-	rcu_read_unlock();
 }
 
 /*
@@ -766,9 +764,10 @@ static void __bond_resend_igmp_join_requests(struct net_device *dev)
  */
 static void bond_resend_igmp_join_requests(struct bonding *bond)
 {
-	struct net_device *bond_dev, *vlan_dev, *master_dev;
+	struct net_device *bond_dev, *vlan_dev, *upper_dev;
 	struct vlan_entry *vlan;
 
+	rcu_read_lock();
 	read_lock(&bond->lock);
 
 	bond_dev = bond->dev;
@@ -780,18 +779,14 @@ static void bond_resend_igmp_join_requests(struct bonding *bond)
 	 * if bond is enslaved to a bridge,
 	 * then rejoin all groups on its master
 	 */
-	master_dev = bond_dev->master;
-	if (master_dev)
-		if ((master_dev->priv_flags & IFF_EBRIDGE)
-			&& (bond_dev->priv_flags & IFF_BRIDGE_PORT))
-			__bond_resend_igmp_join_requests(master_dev);
+	upper_dev = netdev_unique_upper_dev_get_rcu(bond_dev);
+	if (upper_dev && upper_dev->priv_flags & IFF_EBRIDGE)
+		__bond_resend_igmp_join_requests(upper_dev);
 
 	/* rejoin all groups on vlan devices */
 	list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
-		rcu_read_lock();
 		vlan_dev = __vlan_find_dev_deep(bond_dev,
 						vlan->vlan_id);
-		rcu_read_unlock();
 		if (vlan_dev)
 			__bond_resend_igmp_join_requests(vlan_dev);
 	}
@@ -800,13 +795,16 @@ static void bond_resend_igmp_join_requests(struct bonding *bond)
 		queue_delayed_work(bond->wq, &bond->mcast_work, HZ/5);
 
 	read_unlock(&bond->lock);
+	rcu_read_unlock();
 }
 
 static void bond_resend_igmp_join_requests_delayed(struct work_struct *work)
 {
 	struct bonding *bond = container_of(work, struct bonding,
 					    mcast_work.work);
+	rcu_read_lock();
 	bond_resend_igmp_join_requests(bond);
+	rcu_read_unlock();
 }
 
 /*
@@ -1494,6 +1492,27 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 	return ret;
 }
 
+static int bond_unique_upper_dev_link(struct net_device *bond_dev,
+				      struct net_device *slave_dev)
+{
+	int err;
+
+	err = netdev_unique_upper_dev_link(slave_dev, bond_dev);
+	if (err)
+		return err;
+	slave_dev->flags |= IFF_SLAVE;
+	rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE);
+	return 0;
+}
+
+static void bond_upper_dev_unlink(struct net_device *bond_dev,
+				  struct net_device *slave_dev)
+{
+	netdev_upper_dev_unlink(slave_dev, bond_dev);
+	slave_dev->flags &= ~IFF_SLAVE;
+	rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE);
+}
+
 /* enslave device <slave> to bond device <master> */
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 {
@@ -1655,9 +1674,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		}
 	}
 
-	res = netdev_set_bond_master(slave_dev, bond_dev);
+	res = bond_unique_upper_dev_link(bond_dev, slave_dev);
 	if (res) {
-		pr_debug("Error %d calling netdev_set_bond_master\n", res);
+		pr_debug("Error %d calling bond_unique_upper_dev_link\n", res);
 		goto err_restore_mac;
 	}
 
@@ -1891,7 +1910,7 @@ err_close:
 	dev_close(slave_dev);
 
 err_unset_master:
-	netdev_set_bond_master(slave_dev, NULL);
+	bond_upper_dev_unlink(bond_dev, slave_dev);
 
 err_restore_mac:
 	if (!bond->params.fail_over_mac) {
@@ -1936,7 +1955,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	/* slave is not a slave or master is not master of this slave */
 	if (!(slave_dev->flags & IFF_SLAVE) ||
-	    (slave_dev->master != bond_dev)) {
+	    !netdev_has_upper_dev(slave_dev, bond_dev)) {
 		pr_err("%s: Error: cannot release %s.\n",
 		       bond_dev->name, slave_dev->name);
 		return -EINVAL;
@@ -2080,7 +2099,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 		netif_addr_unlock_bh(bond_dev);
 	}
 
-	netdev_set_bond_master(slave_dev, NULL);
+	bond_upper_dev_unlink(bond_dev, slave_dev);
 
 	slave_disable_netpoll(slave);
 
@@ -2195,7 +2214,7 @@ static int bond_release_all(struct net_device *bond_dev)
 			netif_addr_unlock_bh(bond_dev);
 		}
 
-		netdev_set_bond_master(slave_dev, NULL);
+		bond_upper_dev_unlink(bond_dev, slave_dev);
 
 		slave_disable_netpoll(slave);
 
@@ -2259,8 +2278,9 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi
 	if (!USES_PRIMARY(bond->params.mode))
 		return -EINVAL;
 
-	/* Verify that master_dev is indeed the master of slave_dev */
-	if (!(slave_dev->flags & IFF_SLAVE) || (slave_dev->master != bond_dev))
+	/* Verify that bond_dev is indeed the master of slave_dev */
+	if (!(slave_dev->flags & IFF_SLAVE) ||
+	    !netdev_has_upper_dev(slave_dev, bond_dev))
 		return -EINVAL;
 
 	read_lock(&bond->lock);
@@ -3249,36 +3269,32 @@ static int bond_master_netdev_event(unsigned long event,
 static int bond_slave_netdev_event(unsigned long event,
 				   struct net_device *slave_dev)
 {
-	struct net_device *bond_dev = slave_dev->master;
-	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave = NULL;
+	struct slave *slave = bond_slave_get_rtnl(slave_dev);
+	struct bonding *bond = slave->bond;
+	struct net_device *bond_dev = slave->bond->dev;
+	u32 old_speed;
+	u8 old_duplex;
 
 	switch (event) {
 	case NETDEV_UNREGISTER:
-		if (bond_dev) {
-			if (bond->setup_by_slave)
-				bond_release_and_destroy(bond_dev, slave_dev);
-			else
-				bond_release(bond_dev, slave_dev);
-		}
+		if (bond->setup_by_slave)
+			bond_release_and_destroy(bond_dev, slave_dev);
+		else
+			bond_release(bond_dev, slave_dev);
 		break;
 	case NETDEV_UP:
 	case NETDEV_CHANGE:
-		slave = bond_get_slave_by_dev(bond, slave_dev);
-		if (slave) {
-			u32 old_speed = slave->speed;
-			u8  old_duplex = slave->duplex;
+		old_speed = slave->speed;
+		old_duplex = slave->duplex;
 
-			bond_update_speed_duplex(slave);
+		bond_update_speed_duplex(slave);
 
-			if (bond->params.mode == BOND_MODE_8023AD) {
-				if (old_speed != slave->speed)
-					bond_3ad_adapter_speed_changed(slave);
-				if (old_duplex != slave->duplex)
-					bond_3ad_adapter_duplex_changed(slave);
-			}
+		if (bond->params.mode == BOND_MODE_8023AD) {
+			if (old_speed != slave->speed)
+				bond_3ad_adapter_speed_changed(slave);
+			if (old_duplex != slave->duplex)
+				bond_3ad_adapter_duplex_changed(slave);
 		}
-
 		break;
 	case NETDEV_DOWN:
 		/*
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index f8af2fc..c9cd1c0 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -257,6 +257,9 @@ static inline bool bond_vlan_used(struct bonding *bond)
 #define bond_slave_get_rcu(dev) \
 	((struct slave *) rcu_dereference(dev->rx_handler_data))
 
+#define bond_slave_get_rtnl(dev) \
+	((struct slave *) rtnl_dereference(dev->rx_handler_data))
+
 /**
  * Returns NULL if the net_device does not belong to any of the bond's slaves
  *
@@ -279,11 +282,9 @@ static inline struct slave *bond_get_slave_by_dev(struct bonding *bond,
 
 static inline struct bonding *bond_get_bond_by_slave(struct slave *slave)
 {
-	if (!slave || !slave->dev->master) {
+	if (!slave || !slave->bond)
 		return NULL;
-	}
-
-	return netdev_priv(slave->dev->master);
+	return slave->bond;
 }
 
 static inline bool bond_is_lb(const struct bonding *bond)
@@ -359,10 +360,9 @@ static inline void bond_netpoll_send_skb(const struct slave *slave,
 
 static inline void bond_set_slave_inactive_flags(struct slave *slave)
 {
-	struct bonding *bond = netdev_priv(slave->dev->master);
-	if (!bond_is_lb(bond))
+	if (!bond_is_lb(slave->bond))
 		bond_set_backup_slave(slave);
-	if (!bond->params.all_slaves_active)
+	if (!slave->bond->params.all_slaves_active)
 		slave->inactive = 1;
 }
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c341eb7..df16052 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1989,6 +1989,7 @@ errout:
 	if (err < 0)
 		rtnl_set_sk_err(net, RTNLGRP_LINK, err);
 }
+EXPORT_SYMBOL(rtmsg_ifinfo);
 
 static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
 				   struct net_device *dev,
-- 
1.7.10.4

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

* [patch net-next 14/16] net: remove no longer used netdev_set_bond_master() and netdev_set_master()
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (11 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 13/16] bonding: " Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 15/16] net: remove usage of dev->master Jiri Pirko
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h |    6 +----
 net/core/dev.c            |   63 ---------------------------------------------
 2 files changed, 1 insertion(+), 68 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e7a07f8..757f627 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -886,8 +886,7 @@ struct netdev_fcoe_hbainfo {
  *	flow_id is a flow ID to be passed to rps_may_expire_flow() later.
  *	Return the filter ID on success, or a negative error code.
  *
- *	Slave management functions (for bridge, bonding, etc). User should
- *	call netdev_set_master() to set dev->master properly.
+ *	Slave management functions (for bridge, bonding, etc).
  * int (*ndo_add_slave)(struct net_device *dev, struct net_device *slave_dev);
  *	Called to make another netdev an underling.
  *
@@ -2625,9 +2624,6 @@ extern int netdev_unique_upper_dev_link(struct net_device *dev,
 					struct net_device *upper_dev);
 extern void netdev_upper_dev_unlink(struct net_device *dev,
 				    struct net_device *upper_dev);
-extern int		netdev_set_master(struct net_device *dev, struct net_device *master);
-extern int netdev_set_bond_master(struct net_device *dev,
-				  struct net_device *master);
 extern int skb_checksum_help(struct sk_buff *skb);
 extern struct sk_buff *skb_gso_segment(struct sk_buff *skb,
 	netdev_features_t features);
diff --git a/net/core/dev.c b/net/core/dev.c
index 68db1ac..c0f9adb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4648,69 +4648,6 @@ void netdev_upper_dev_unlink(struct net_device *dev,
 }
 EXPORT_SYMBOL(netdev_upper_dev_unlink);
 
-/**
- *	netdev_set_master	-	set up master pointer
- *	@slave: slave device
- *	@master: new master device
- *
- *	Changes the master device of the slave. Pass %NULL to break the
- *	bonding. The caller must hold the RTNL semaphore. On a failure
- *	a negative errno code is returned. On success the reference counts
- *	are adjusted and the function returns zero.
- */
-int netdev_set_master(struct net_device *slave, struct net_device *master)
-{
-	struct net_device *old = slave->master;
-	int err;
-
-	ASSERT_RTNL();
-
-	if (master) {
-		if (old)
-			return -EBUSY;
-		err = netdev_unique_upper_dev_link(slave, master);
-		if (err)
-			return err;
-	}
-
-	slave->master = master;
-
-	if (old)
-		netdev_upper_dev_unlink(slave, master);
-
-	return 0;
-}
-EXPORT_SYMBOL(netdev_set_master);
-
-/**
- *	netdev_set_bond_master	-	set up bonding master/slave pair
- *	@slave: slave device
- *	@master: new master device
- *
- *	Changes the master device of the slave. Pass %NULL to break the
- *	bonding. The caller must hold the RTNL semaphore. On a failure
- *	a negative errno code is returned. On success %RTM_NEWLINK is sent
- *	to the routing socket and the function returns zero.
- */
-int netdev_set_bond_master(struct net_device *slave, struct net_device *master)
-{
-	int err;
-
-	ASSERT_RTNL();
-
-	err = netdev_set_master(slave, master);
-	if (err)
-		return err;
-	if (master)
-		slave->flags |= IFF_SLAVE;
-	else
-		slave->flags &= ~IFF_SLAVE;
-
-	rtmsg_ifinfo(RTM_NEWLINK, slave, IFF_SLAVE);
-	return 0;
-}
-EXPORT_SYMBOL(netdev_set_bond_master);
-
 static void dev_change_rx_flags(struct net_device *dev, int flags)
 {
 	const struct net_device_ops *ops = dev->netdev_ops;
-- 
1.7.10.4

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

* [patch net-next 15/16] net: remove usage of dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (12 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 14/16] net: remove no longer used netdev_set_bond_master() and netdev_set_master() Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
  2012-08-13 15:27 ` [patch net-next 16/16] net: kill dev->master Jiri Pirko
       [not found] ` <1344871635-1052-2-git-send-email-jiri@resnulli.us>
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/core/dev.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index c0f9adb..8977404 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5461,8 +5461,8 @@ static void rollback_registered_many(struct list_head *head)
 		if (dev->netdev_ops->ndo_uninit)
 			dev->netdev_ops->ndo_uninit(dev);
 
-		/* Notifier chain MUST detach us from master device. */
-		WARN_ON(dev->master);
+		/* Notifier chain MUST detach us all upper devices. */
+		WARN_ON(netdev_has_any_upper_dev(dev));
 
 		/* Remove entries from kobject tree */
 		netdev_unregister_kobject(dev);
-- 
1.7.10.4

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

* [patch net-next 16/16] net: kill dev->master
  2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
                   ` (13 preceding siblings ...)
  2012-08-13 15:27 ` [patch net-next 15/16] net: remove usage of dev->master Jiri Pirko
@ 2012-08-13 15:27 ` Jiri Pirko
       [not found] ` <1344871635-1052-2-git-send-email-jiri@resnulli.us>
  15 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 15:27 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, faisal.latif, roland, sean.hefty, hal.rosenstock,
	fubar, andy, divy, jitendra.kalsaria, sony.chacko, linux-driver,
	kaber, ursula.braun, blaschka, linux390, shemminger, bhutchings,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Nobody uses this now. Remove it.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 757f627..28c5ef0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1168,10 +1168,6 @@ struct net_device {
 						 * avoid dirtying this cache line.
 						 */
 
-	struct net_device	*master; /* Pointer to master device of a group,
-					  * which this device is member of.
-					  */
-
 	struct list_head	upper_dev_list; /* List of upper devices */
 
 	/* Interface address info used in eth_type_trans() */
-- 
1.7.10.4

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

* Re: [patch net-next 01/16] net: introduce upper device lists
       [not found]   ` <1344877451.2733.26.camel@bwh-desktop.uk.solarflarecom.com>
@ 2012-08-13 17:31     ` Jiri Pirko
  2012-08-15 20:33       ` Nicolas de Pesloüan
  0 siblings, 1 reply; 24+ messages in thread
From: Jiri Pirko @ 2012-08-13 17:31 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	therbert, xiyou.wangcong, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Mon, Aug 13, 2012 at 07:04:11PM CEST, bhutchings@solarflare.com wrote:
>On Mon, 2012-08-13 at 17:27 +0200, Jiri Pirko wrote:
>> This lists are supposed to serve for storing pointers to all upper devices.
>> Eventually it will replace dev->master pointer which is used for
>> bonding, bridge, team but it cannot be used for vlan, macvlan where
>> there might be multiple "masters" present.
>> 
>> New upper device list resolves this limitation. Also, the information
>> stored in lists is used for preventing looping setups like
>> "bond->somethingelse->samebond"
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>[...]
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4425,6 +4425,229 @@ static int __init dev_proc_init(void)
>>  #endif	/* CONFIG_PROC_FS */
>>  
>> 
>> +struct netdev_upper {
>> +	struct net_device *dev;
>> +	bool unique;
>
>This needs a better name.  It doesn't really have anything to do with
>uniqueness and doesn't ensure exclusivity.  I think that it would be
>fine to keep the 'master' term.

Hmm. I admit that "unique" I do not like too much as well. But "master"
I like even less.

This flag should ensure exclusivity. Only one upper device with this
flag can be present at a time.

>
>> +	struct list_head list;
>> +	struct rcu_head rcu;
>> +};
>[...]
>> +static int __netdev_upper_dev_link(struct net_device *dev,
>> +				   struct net_device *upper_dev, bool unique)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	ASSERT_RTNL();
>> +
>> +	if (dev == upper_dev)
>> +		return -EBUSY;
>> +	/*
>> +	 * To prevent loops, check if dev is not upper device to upper_dev.
>> +	 */
>> +	if (__netdev_has_upper_dev(upper_dev, dev, true))
>> +		return -EBUSY;
>> +
>> +	if (__netdev_find_upper(dev, upper_dev))
>> +		return -EEXIST;
>> +
>> +	if (unique && netdev_unique_upper_dev_get(dev))
>> +		return -EBUSY;
>> +
>> +	upper = kmalloc(sizeof(*upper), GFP_KERNEL);
>> +	if (!upper)
>> +		return -ENOMEM;
>> +
>> +	upper->dev = upper_dev;
>> +	upper->unique = unique;
>> +
>> +	/*
>> +	 * Ensure that unique upper link is always the first item in the list.
>> +	 */
>> +	if (unique)
>> +		list_add_rcu(&upper->list, &dev->upper_dev_list);
>> +	else
>> +		list_add_tail_rcu(&upper->list, &dev->upper_dev_list);
>> +	dev_hold(upper_dev);
>
>This behaviour (calling dev_hold()) matches netdev_set_master().  But
>it's oddly asymmetric: generally the administrator can remove either the
>upper device or the lower device (rtnl_link_ops or unbinding a physical
>device) and the upper device driver must then unlink itself from the
>lower device (using a notifier to catch lower device removal).
>
>If the upper device driver fails to unlink when the upper device is
>unregistered, then this extra reference causes netdev_wait_allrefs() to
>hang... is that the intent?  Or should there be a more explicit counter
>and check on unregistration, e.g. WARN_ON(dev->num_lower_devs != 0)?
>

I'm not sure I understand you. I believe that upper device notifier
should take care of the unlink. This behaviour is unchanged by the
patch.



>If it fails to unlink when the lower device is removed, this warning in
>rollback_registered_many() may be triggered:
>
>		/* Notifier chain MUST detach us from master device. */
>		WARN_ON(dev->master);
>
>I think that needs to become WARN_ON(netdev_has_upper_dev(dev)).

Patch 15

>
>> +	return 0;
>> +}
>[...] 
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

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

* Re: [patch net-next 01/16] net: introduce upper device lists
       [not found] ` <1344871635-1052-2-git-send-email-jiri@resnulli.us>
       [not found]   ` <1344877451.2733.26.camel@bwh-desktop.uk.solarflarecom.com>
@ 2012-08-13 17:52   ` Flavio Leitner
  2012-08-14 12:24     ` Jiri Pirko
       [not found]   ` <502A1429.9060005@gmail.com>
  2 siblings, 1 reply; 24+ messages in thread
From: Flavio Leitner @ 2012-08-13 17:52 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
	john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge

On Mon, 13 Aug 2012 17:27:00 +0200
Jiri Pirko <jiri@resnulli.us> wrote:

> This lists are supposed to serve for storing pointers to all upper devices.
> Eventually it will replace dev->master pointer which is used for
> bonding, bridge, team but it cannot be used for vlan, macvlan where
> there might be multiple "masters" present.
> 
> New upper device list resolves this limitation. Also, the information
> stored in lists is used for preventing looping setups like
> "bond->somethingelse->samebond"
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  include/linux/netdevice.h |   14 +++
>  net/core/dev.c            |  232 ++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 244 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index a9db4f3..e7a07f8 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1173,6 +1173,8 @@ struct net_device {
>  					  * which this device is member of.
>  					  */
>  
> +	struct list_head	upper_dev_list; /* List of upper devices */
> +
>  	/* Interface address info used in eth_type_trans() */
>  	unsigned char		*dev_addr;	/* hw address, (before bcast
>  						   because most packets are
> @@ -2611,6 +2613,18 @@ extern int		netdev_max_backlog;
>  extern int		netdev_tstamp_prequeue;
>  extern int		weight_p;
>  extern int		bpf_jit_enable;
> +
> +extern bool netdev_has_upper_dev(struct net_device *dev,
> +				 struct net_device *upper_dev);
> +extern bool netdev_has_any_upper_dev(struct net_device *dev);
> +extern struct net_device *netdev_unique_upper_dev_get(struct net_device *dev);
> +extern struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev);
> +extern int netdev_upper_dev_link(struct net_device *dev,
> +				 struct net_device *upper_dev);
> +extern int netdev_unique_upper_dev_link(struct net_device *dev,
> +					struct net_device *upper_dev);
> +extern void netdev_upper_dev_unlink(struct net_device *dev,
> +				    struct net_device *upper_dev);
>  extern int		netdev_set_master(struct net_device *dev, struct net_device *master);
>  extern int netdev_set_bond_master(struct net_device *dev,
>  				  struct net_device *master);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 1f06df8..68db1ac 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4425,6 +4425,229 @@ static int __init dev_proc_init(void)
>  #endif	/* CONFIG_PROC_FS */
>  
>  
> +struct netdev_upper {
> +	struct net_device *dev;
> +	bool unique;

unique is quite confusing here. I see that it is possible to have 
one unique and many non-unique linked in the list, so maybe rename
to 'main_dev', 'master' or 'principal'...


> +	struct list_head list;
> +	struct rcu_head rcu;
> +};
> +
> +static bool __netdev_has_upper_dev(struct net_device *dev,
> +				   struct net_device *upper_dev,
> +				   bool deep)
> +{
> +	struct netdev_upper *upper;
> +
> +	list_for_each_entry(upper, &dev->upper_dev_list, list) {
> +		if (upper->dev == upper_dev)
> +			return true;
> +		if (deep && __netdev_has_upper_dev(upper->dev, upper_dev, deep))
> +			return true;
> +	}
> +	return false;
> +}
> +
> +static struct netdev_upper *__netdev_find_upper(struct net_device *dev,
> +						struct net_device *upper_dev)
> +{
> +	struct netdev_upper *upper;
> +
> +	list_for_each_entry(upper, &dev->upper_dev_list, list) {
> +		if (upper->dev == upper_dev)
> +			return upper;
> +	}
> +	return NULL;
> +}
> +
> +/**
> + * netdev_has_upper_dev - Check if device is linked to an upper device
> + * @dev: device
> + * @upper_dev: upper device to check
> + *
> + * Find out if a device is linked to specified upper device and return true
> + * in case it is. The caller must hold the RTNL semaphore.
> + */
> +bool netdev_has_upper_dev(struct net_device *dev,
> +			  struct net_device *upper_dev)
> +{
> +	ASSERT_RTNL();
> +
> +	return __netdev_has_upper_dev(dev, upper_dev, false);
> +}
> +EXPORT_SYMBOL(netdev_has_upper_dev);
> +
> +/**
> + * netdev_has_any_upper_dev - Check if device is linked to some device
> + * @dev: device
> + *
> + * Find out if a device is linked to an upper device and return true in case
> + * it is. The caller must hold the RTNL semaphore.
> + */
> +bool netdev_has_any_upper_dev(struct net_device *dev)
> +{
> +	ASSERT_RTNL();
> +
> +	return !list_empty(&dev->upper_dev_list);
> +}
> +EXPORT_SYMBOL(netdev_has_any_upper_dev);
> +
> +/**
> + * netdev_unique_upper_dev_get - Get unique upper device
> + * @dev: device
> + *
> + * Find a unique upper device and return pointer to it or NULL in case
> + * it's not there. The caller must hold the RTNL semaphore.
> + */
> +struct net_device *netdev_unique_upper_dev_get(struct net_device *dev)
> +{
> +	struct netdev_upper *upper;
> +
> +	ASSERT_RTNL();
> +
> +	if (list_empty(&dev->upper_dev_list))
> +		return NULL;
> +
> +	upper = list_first_entry(&dev->upper_dev_list,
> +				 struct netdev_upper, list);
> +	if (likely(upper->unique))
> +		return upper->dev;
> +	return NULL;
> +}
> +EXPORT_SYMBOL(netdev_unique_upper_dev_get);
> +
> +/**
> + * netdev_unique_upper_dev_get_rcu - Get unique upper device
> + * @dev: device
> + *
> + * Find a unique upper device and return pointer to it or NULL in case
> + * it's not there. The caller must hold the RCU read lock.
> + */
> +struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev)
> +{
> +	struct netdev_upper *upper;
> +
> +	upper = list_first_or_null_rcu(&dev->upper_dev_list,
> +				       struct netdev_upper, list);
> +	if (likely(upper->unique))

It will oopses here if 'upper' is NULL (i.e. no upper devices).


> +		return upper->dev;
> +	return NULL;
> +}
> +EXPORT_SYMBOL(netdev_unique_upper_dev_get_rcu);
> +
> +static int __netdev_upper_dev_link(struct net_device *dev,
> +				   struct net_device *upper_dev, bool unique)
> +{
> +	struct netdev_upper *upper;
> +
> +	ASSERT_RTNL();
> +
> +	if (dev == upper_dev)
> +		return -EBUSY;
> +	/*
> +	 * To prevent loops, check if dev is not upper device to upper_dev.
> +	 */
> +	if (__netdev_has_upper_dev(upper_dev, dev, true))
> +		return -EBUSY;
> +
> +	if (__netdev_find_upper(dev, upper_dev))
> +		return -EEXIST;

__netdev_has_upper_dev() can go all the way up finding the device and
the __netdev_find_upper() just check the first level.

I think it would be better to use:
__netdev_find_upper_dev(,,deep=true/false)
__netdev_has_upper(,)

thanks,
fbl

> +	if (unique && netdev_unique_upper_dev_get(dev))
> +		return -EBUSY;
> +
> +	upper = kmalloc(sizeof(*upper), GFP_KERNEL);
> +	if (!upper)
> +		return -ENOMEM;
> +
> +	upper->dev = upper_dev;
> +	upper->unique = unique;
> +
> +	/*
> +	 * Ensure that unique upper link is always the first item in the list.
> +	 */
> +	if (unique)
> +		list_add_rcu(&upper->list, &dev->upper_dev_list);
> +	else
> +		list_add_tail_rcu(&upper->list, &dev->upper_dev_list);
> +	dev_hold(upper_dev);
> +
> +	return 0;
> +}
> +/**
> + * netdev_upper_dev_link - Add a link to the upper device
> + * @dev: device
> + * @upper_dev: new upper device
> + *
> + * Adds a link to device which is upper to this one. The caller must hold
> + * the RTNL semaphore. On a failure a negative errno code is returned.
> + * On success the reference counts are adjusted and the function
> + * returns zero.
> + */
> +int netdev_upper_dev_link(struct net_device *dev,
> +			  struct net_device *upper_dev)
> +{
> +	return __netdev_upper_dev_link(dev, upper_dev, false);
> +}
> +EXPORT_SYMBOL(netdev_upper_dev_link);
> +
> +/**
> + * netdev_unique_upper_dev_link - Add a unique link to the upper device
> + * @dev: device
> + * @upper_dev: new upper device
> + *
> + * Adds a link to device which is upper to this one. In this case, only
> + * one unique upper device can be linked, although other non-unique devices
> + * might be linked as well. The caller must hold the RTNL semaphore.
> + * On a failure a negative errno code is returned. On success the reference
> + * counts are adjusted and the function returns zero.
> + */
> +int netdev_unique_upper_dev_link(struct net_device *dev,
> +				 struct net_device *upper_dev)
> +{
> +	return __netdev_upper_dev_link(dev, upper_dev, true);
> +}
> +EXPORT_SYMBOL(netdev_unique_upper_dev_link);
> +
> +/**
> + * netdev_upper_free_rcu - Frees a upper device list item via the RCU pointer
> + * @entry: the entry's RCU field
> + *
> + * This function is designed to be used as a callback to the call_rcu()
> + * function so that the memory allocated to the netdev upper device list item
> + * can be released safely.
> + */
> +static void netdev_upper_free_rcu(struct rcu_head *entry)
> +{
> +	struct netdev_upper *upper;
> +
> +	upper = container_of(entry, struct netdev_upper, rcu);
> +	kfree(upper);
> +}
> +
> +/**
> + * netdev_upper_dev_unlink - Removes a link to upper device
> + * @dev: device
> + * @upper_dev: new upper device
> + *
> + * Removes a link to device which is upper to this one. The caller must hold
> + * the RTNL semaphore.
> + */
> +void netdev_upper_dev_unlink(struct net_device *dev,
> +			     struct net_device *upper_dev)
> +{
> +	struct netdev_upper *upper;
> +
> +	ASSERT_RTNL();
> +
> +	upper = __netdev_find_upper(dev, upper_dev);
> +	if (!upper)
> +		return;
> +	list_del_rcu(&upper->list);
> +	dev_put(upper_dev);
> +	call_rcu(&upper->rcu, netdev_upper_free_rcu);
> +}
> +EXPORT_SYMBOL(netdev_upper_dev_unlink);
> +
>  /**
>   *	netdev_set_master	-	set up master pointer
>   *	@slave: slave device
> @@ -4438,19 +4661,23 @@ static int __init dev_proc_init(void)
>  int netdev_set_master(struct net_device *slave, struct net_device *master)
>  {
>  	struct net_device *old = slave->master;
> +	int err;
>  
>  	ASSERT_RTNL();
>  
>  	if (master) {
>  		if (old)
>  			return -EBUSY;
> -		dev_hold(master);
> +		err = netdev_unique_upper_dev_link(slave, master);
> +		if (err)
> +			return err;
>  	}
>  
>  	slave->master = master;
>  
>  	if (old)
> -		dev_put(old);
> +		netdev_upper_dev_unlink(slave, master);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(netdev_set_master);
> @@ -5999,6 +6226,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>  	INIT_LIST_HEAD(&dev->napi_list);
>  	INIT_LIST_HEAD(&dev->unreg_list);
>  	INIT_LIST_HEAD(&dev->link_watch_list);
> +	INIT_LIST_HEAD(&dev->upper_dev_list);
>  	dev->priv_flags = IFF_XMIT_DST_RELEASE;
>  	setup(dev);
>  

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

* Re: [patch net-next 03/16] vlan: add link to upper device
  2012-08-13 15:27 ` [patch net-next 03/16] vlan: " Jiri Pirko
@ 2012-08-13 19:04   ` Flavio Leitner
  0 siblings, 0 replies; 24+ messages in thread
From: Flavio Leitner @ 2012-08-13 19:04 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
	john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge

On Mon, 13 Aug 2012 17:27:02 +0200
Jiri Pirko <jiri@resnulli.us> wrote:

> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  net/8021q/vlan.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index 9096bcb..739665e 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -105,6 +105,8 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
>  	 */
>  	unregister_netdevice_queue(dev, head);
>  
> +	netdev_upper_dev_unlink(real_dev, dev);
> +
>  	if (grp->nr_vlan_devs == 0)
>  		vlan_gvrp_uninit_applicant(real_dev);
>  
> @@ -162,9 +164,13 @@ int register_vlan_dev(struct net_device *dev)
>  	if (err < 0)
>  		goto out_uninit_applicant;
>  
> +	err = netdev_upper_dev_link(real_dev, dev);
> +	if (err)
> +		goto out_uninit_applicant;
> +
>  	err = register_netdevice(dev);
>  	if (err < 0)
> -		goto out_uninit_applicant;
> +		goto out_upper_dev_unlink;
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
see below:

>  
>  	/* Account for reference in struct vlan_dev_priv */
>  	dev_hold(real_dev);
> @@ -180,6 +186,8 @@ int register_vlan_dev(struct net_device *dev)
>  
>  	return 0;
>  
> +upper_dev_unlink:
^^^^^^^^^^^^^^^^^^^
should be out_upper_dev_unlink:

fbl

> +	netdev_upper_dev_unlink(real_dev, dev);
>  out_uninit_applicant:
>  	if (grp->nr_vlan_devs == 0)
>  		vlan_gvrp_uninit_applicant(real_dev);

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

* Re: [patch net-next 01/16] net: introduce upper device lists
       [not found]   ` <502A1429.9060005@gmail.com>
@ 2012-08-14 10:18     ` Jiri Pirko
  0 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-14 10:18 UTC (permalink / raw)
  To: Cong Wang
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, joe, gregory.v.rose, john.r.fastabend,
	linux-rdma, linux-kernel, linux-s390, bridge, fbl

Tue, Aug 14, 2012 at 11:02:33AM CEST, xiyou.wangcong@gmail.com wrote:
>On 08/13/2012 11:27 PM, Jiri Pirko wrote:
>>This lists are supposed to serve for storing pointers to all upper devices.
>>Eventually it will replace dev->master pointer which is used for
>>bonding, bridge, team but it cannot be used for vlan, macvlan where
>>there might be multiple "masters" present.
>>
>>New upper device list resolves this limitation. Also, the information
>>stored in lists is used for preventing looping setups like
>>"bond->somethingelse->samebond"
>
>Hi, Jiri,
>
>I have some difficulty to understand this patch description, so take
>bridged bonding as an example, IIUC, both the bridge dev and bonding
>dev will be in the upper dev list of eth0 and eth1? And br0 will be
>in the upper dev list of bond0 too?

No. Only direct upper device is in the list.

>
>I think it would be nice to describe the hierarchy after your patches.

Hierarchy is the same after.

>
>Thanks!
>

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

* Re: [patch net-next 01/16] net: introduce upper device lists
  2012-08-13 17:52   ` Flavio Leitner
@ 2012-08-14 12:24     ` Jiri Pirko
  2012-08-14 13:14       ` Flavio Leitner
  0 siblings, 1 reply; 24+ messages in thread
From: Jiri Pirko @ 2012-08-14 12:24 UTC (permalink / raw)
  To: Flavio Leitner
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
	john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge

Mon, Aug 13, 2012 at 07:52:17PM CEST, fbl@redhat.com wrote:
>On Mon, 13 Aug 2012 17:27:00 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> This lists are supposed to serve for storing pointers to all upper devices.
>> Eventually it will replace dev->master pointer which is used for
>> bonding, bridge, team but it cannot be used for vlan, macvlan where
>> there might be multiple "masters" present.
>> 
>> New upper device list resolves this limitation. Also, the information
>> stored in lists is used for preventing looping setups like
>> "bond->somethingelse->samebond"
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  include/linux/netdevice.h |   14 +++
>>  net/core/dev.c            |  232 ++++++++++++++++++++++++++++++++++++++++++++-
>>  2 files changed, 244 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index a9db4f3..e7a07f8 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1173,6 +1173,8 @@ struct net_device {
>>  					  * which this device is member of.
>>  					  */
>>  
>> +	struct list_head	upper_dev_list; /* List of upper devices */
>> +
>>  	/* Interface address info used in eth_type_trans() */
>>  	unsigned char		*dev_addr;	/* hw address, (before bcast
>>  						   because most packets are
>> @@ -2611,6 +2613,18 @@ extern int		netdev_max_backlog;
>>  extern int		netdev_tstamp_prequeue;
>>  extern int		weight_p;
>>  extern int		bpf_jit_enable;
>> +
>> +extern bool netdev_has_upper_dev(struct net_device *dev,
>> +				 struct net_device *upper_dev);
>> +extern bool netdev_has_any_upper_dev(struct net_device *dev);
>> +extern struct net_device *netdev_unique_upper_dev_get(struct net_device *dev);
>> +extern struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev);
>> +extern int netdev_upper_dev_link(struct net_device *dev,
>> +				 struct net_device *upper_dev);
>> +extern int netdev_unique_upper_dev_link(struct net_device *dev,
>> +					struct net_device *upper_dev);
>> +extern void netdev_upper_dev_unlink(struct net_device *dev,
>> +				    struct net_device *upper_dev);
>>  extern int		netdev_set_master(struct net_device *dev, struct net_device *master);
>>  extern int netdev_set_bond_master(struct net_device *dev,
>>  				  struct net_device *master);
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 1f06df8..68db1ac 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4425,6 +4425,229 @@ static int __init dev_proc_init(void)
>>  #endif	/* CONFIG_PROC_FS */
>>  
>>  
>> +struct netdev_upper {
>> +	struct net_device *dev;
>> +	bool unique;
>
>unique is quite confusing here. I see that it is possible to have 
>one unique and many non-unique linked in the list, so maybe rename
>to 'main_dev', 'master' or 'principal'...
>
>
>> +	struct list_head list;
>> +	struct rcu_head rcu;
>> +};
>> +
>> +static bool __netdev_has_upper_dev(struct net_device *dev,
>> +				   struct net_device *upper_dev,
>> +				   bool deep)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	list_for_each_entry(upper, &dev->upper_dev_list, list) {
>> +		if (upper->dev == upper_dev)
>> +			return true;
>> +		if (deep && __netdev_has_upper_dev(upper->dev, upper_dev, deep))
>> +			return true;
>> +	}
>> +	return false;
>> +}
>> +
>> +static struct netdev_upper *__netdev_find_upper(struct net_device *dev,
>> +						struct net_device *upper_dev)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	list_for_each_entry(upper, &dev->upper_dev_list, list) {
>> +		if (upper->dev == upper_dev)
>> +			return upper;
>> +	}
>> +	return NULL;
>> +}
>> +
>> +/**
>> + * netdev_has_upper_dev - Check if device is linked to an upper device
>> + * @dev: device
>> + * @upper_dev: upper device to check
>> + *
>> + * Find out if a device is linked to specified upper device and return true
>> + * in case it is. The caller must hold the RTNL semaphore.
>> + */
>> +bool netdev_has_upper_dev(struct net_device *dev,
>> +			  struct net_device *upper_dev)
>> +{
>> +	ASSERT_RTNL();
>> +
>> +	return __netdev_has_upper_dev(dev, upper_dev, false);
>> +}
>> +EXPORT_SYMBOL(netdev_has_upper_dev);
>> +
>> +/**
>> + * netdev_has_any_upper_dev - Check if device is linked to some device
>> + * @dev: device
>> + *
>> + * Find out if a device is linked to an upper device and return true in case
>> + * it is. The caller must hold the RTNL semaphore.
>> + */
>> +bool netdev_has_any_upper_dev(struct net_device *dev)
>> +{
>> +	ASSERT_RTNL();
>> +
>> +	return !list_empty(&dev->upper_dev_list);
>> +}
>> +EXPORT_SYMBOL(netdev_has_any_upper_dev);
>> +
>> +/**
>> + * netdev_unique_upper_dev_get - Get unique upper device
>> + * @dev: device
>> + *
>> + * Find a unique upper device and return pointer to it or NULL in case
>> + * it's not there. The caller must hold the RTNL semaphore.
>> + */
>> +struct net_device *netdev_unique_upper_dev_get(struct net_device *dev)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	ASSERT_RTNL();
>> +
>> +	if (list_empty(&dev->upper_dev_list))
>> +		return NULL;
>> +
>> +	upper = list_first_entry(&dev->upper_dev_list,
>> +				 struct netdev_upper, list);
>> +	if (likely(upper->unique))
>> +		return upper->dev;
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_get);
>> +
>> +/**
>> + * netdev_unique_upper_dev_get_rcu - Get unique upper device
>> + * @dev: device
>> + *
>> + * Find a unique upper device and return pointer to it or NULL in case
>> + * it's not there. The caller must hold the RCU read lock.
>> + */
>> +struct net_device *netdev_unique_upper_dev_get_rcu(struct net_device *dev)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	upper = list_first_or_null_rcu(&dev->upper_dev_list,
>> +				       struct netdev_upper, list);
>> +	if (likely(upper->unique))
>
>It will oopses here if 'upper' is NULL (i.e. no upper devices).


Fixed

>
>
>> +		return upper->dev;
>> +	return NULL;
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_get_rcu);
>> +
>> +static int __netdev_upper_dev_link(struct net_device *dev,
>> +				   struct net_device *upper_dev, bool unique)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	ASSERT_RTNL();
>> +
>> +	if (dev == upper_dev)
>> +		return -EBUSY;
>> +	/*
>> +	 * To prevent loops, check if dev is not upper device to upper_dev.
>> +	 */
>> +	if (__netdev_has_upper_dev(upper_dev, dev, true))
>> +		return -EBUSY;
>> +
>> +	if (__netdev_find_upper(dev, upper_dev))
>> +		return -EEXIST;
>
>__netdev_has_upper_dev() can go all the way up finding the device and
>the __netdev_find_upper() just check the first level.


I do not think this ordering is somewhat inportant.

>
>I think it would be better to use:
>__netdev_find_upper_dev(,,deep=true/false)
>__netdev_has_upper(,)
>
>thanks,
>fbl
>
>> +	if (unique && netdev_unique_upper_dev_get(dev))
>> +		return -EBUSY;
>> +
>> +	upper = kmalloc(sizeof(*upper), GFP_KERNEL);
>> +	if (!upper)
>> +		return -ENOMEM;
>> +
>> +	upper->dev = upper_dev;
>> +	upper->unique = unique;
>> +
>> +	/*
>> +	 * Ensure that unique upper link is always the first item in the list.
>> +	 */
>> +	if (unique)
>> +		list_add_rcu(&upper->list, &dev->upper_dev_list);
>> +	else
>> +		list_add_tail_rcu(&upper->list, &dev->upper_dev_list);
>> +	dev_hold(upper_dev);
>> +
>> +	return 0;
>> +}
>> +/**
>> + * netdev_upper_dev_link - Add a link to the upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Adds a link to device which is upper to this one. The caller must hold
>> + * the RTNL semaphore. On a failure a negative errno code is returned.
>> + * On success the reference counts are adjusted and the function
>> + * returns zero.
>> + */
>> +int netdev_upper_dev_link(struct net_device *dev,
>> +			  struct net_device *upper_dev)
>> +{
>> +	return __netdev_upper_dev_link(dev, upper_dev, false);
>> +}
>> +EXPORT_SYMBOL(netdev_upper_dev_link);
>> +
>> +/**
>> + * netdev_unique_upper_dev_link - Add a unique link to the upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Adds a link to device which is upper to this one. In this case, only
>> + * one unique upper device can be linked, although other non-unique devices
>> + * might be linked as well. The caller must hold the RTNL semaphore.
>> + * On a failure a negative errno code is returned. On success the reference
>> + * counts are adjusted and the function returns zero.
>> + */
>> +int netdev_unique_upper_dev_link(struct net_device *dev,
>> +				 struct net_device *upper_dev)
>> +{
>> +	return __netdev_upper_dev_link(dev, upper_dev, true);
>> +}
>> +EXPORT_SYMBOL(netdev_unique_upper_dev_link);
>> +
>> +/**
>> + * netdev_upper_free_rcu - Frees a upper device list item via the RCU pointer
>> + * @entry: the entry's RCU field
>> + *
>> + * This function is designed to be used as a callback to the call_rcu()
>> + * function so that the memory allocated to the netdev upper device list item
>> + * can be released safely.
>> + */
>> +static void netdev_upper_free_rcu(struct rcu_head *entry)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	upper = container_of(entry, struct netdev_upper, rcu);
>> +	kfree(upper);
>> +}
>> +
>> +/**
>> + * netdev_upper_dev_unlink - Removes a link to upper device
>> + * @dev: device
>> + * @upper_dev: new upper device
>> + *
>> + * Removes a link to device which is upper to this one. The caller must hold
>> + * the RTNL semaphore.
>> + */
>> +void netdev_upper_dev_unlink(struct net_device *dev,
>> +			     struct net_device *upper_dev)
>> +{
>> +	struct netdev_upper *upper;
>> +
>> +	ASSERT_RTNL();
>> +
>> +	upper = __netdev_find_upper(dev, upper_dev);
>> +	if (!upper)
>> +		return;
>> +	list_del_rcu(&upper->list);
>> +	dev_put(upper_dev);
>> +	call_rcu(&upper->rcu, netdev_upper_free_rcu);
>> +}
>> +EXPORT_SYMBOL(netdev_upper_dev_unlink);
>> +
>>  /**
>>   *	netdev_set_master	-	set up master pointer
>>   *	@slave: slave device
>> @@ -4438,19 +4661,23 @@ static int __init dev_proc_init(void)
>>  int netdev_set_master(struct net_device *slave, struct net_device *master)
>>  {
>>  	struct net_device *old = slave->master;
>> +	int err;
>>  
>>  	ASSERT_RTNL();
>>  
>>  	if (master) {
>>  		if (old)
>>  			return -EBUSY;
>> -		dev_hold(master);
>> +		err = netdev_unique_upper_dev_link(slave, master);
>> +		if (err)
>> +			return err;
>>  	}
>>  
>>  	slave->master = master;
>>  
>>  	if (old)
>> -		dev_put(old);
>> +		netdev_upper_dev_unlink(slave, master);
>> +
>>  	return 0;
>>  }
>>  EXPORT_SYMBOL(netdev_set_master);
>> @@ -5999,6 +6226,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
>>  	INIT_LIST_HEAD(&dev->napi_list);
>>  	INIT_LIST_HEAD(&dev->unreg_list);
>>  	INIT_LIST_HEAD(&dev->link_watch_list);
>> +	INIT_LIST_HEAD(&dev->upper_dev_list);
>>  	dev->priv_flags = IFF_XMIT_DST_RELEASE;
>>  	setup(dev);
>>  
>

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

* Re: [patch net-next 01/16] net: introduce upper device lists
  2012-08-14 12:24     ` Jiri Pirko
@ 2012-08-14 13:14       ` Flavio Leitner
  2012-08-14 13:35         ` Jiri Pirko
  0 siblings, 1 reply; 24+ messages in thread
From: Flavio Leitner @ 2012-08-14 13:14 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
	john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge

On Tue, 14 Aug 2012 14:24:33 +0200
Jiri Pirko <jiri@resnulli.us> wrote:

> Mon, Aug 13, 2012 at 07:52:17PM CEST, fbl@redhat.com wrote:
> >On Mon, 13 Aug 2012 17:27:00 +0200
> >Jiri Pirko <jiri@resnulli.us> wrote:
> >> +	/*
> >> +	 * To prevent loops, check if dev is not upper device to upper_dev.
> >> +	 */
> >> +	if (__netdev_has_upper_dev(upper_dev, dev, true))
> >> +		return -EBUSY;
> >> +
> >> +	if (__netdev_find_upper(dev, upper_dev))
> >> +		return -EEXIST;
> >
> >__netdev_has_upper_dev() can go all the way up finding the device and
> >the __netdev_find_upper() just check the first level.
> 
> 
> I do not think this ordering is somewhat inportant.

it's not the order, see below:

> >I think it would be better to use:
> >__netdev_find_upper_dev(,,deep=true/false)
> >__netdev_has_upper(,)

It's their names.  Currently, the function ..._find_... look at
one level only, while the function ..._has_... does one or more
levels.  I think it's better to swap 'has' and 'find' in their names:

__netdev_find_upper_dev(,,deep=true/false) <-- find in all levels
__netdev_has_upper(,)  <-- check only the one level.

fbl

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

* Re: [patch net-next 01/16] net: introduce upper device lists
  2012-08-14 13:14       ` Flavio Leitner
@ 2012-08-14 13:35         ` Jiri Pirko
  0 siblings, 0 replies; 24+ messages in thread
From: Jiri Pirko @ 2012-08-14 13:35 UTC (permalink / raw)
  To: Flavio Leitner
  Cc: netdev, davem, edumazet, faisal.latif, roland, sean.hefty,
	hal.rosenstock, fubar, andy, divy, jitendra.kalsaria, sony.chacko,
	linux-driver, kaber, ursula.braun, blaschka, linux390, shemminger,
	bhutchings, therbert, xiyou.wangcong, joe, gregory.v.rose,
	john.r.fastabend, linux-rdma, linux-kernel, linux-s390, bridge

Tue, Aug 14, 2012 at 03:14:00PM CEST, fbl@redhat.com wrote:
>On Tue, 14 Aug 2012 14:24:33 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Mon, Aug 13, 2012 at 07:52:17PM CEST, fbl@redhat.com wrote:
>> >On Mon, 13 Aug 2012 17:27:00 +0200
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >> +	/*
>> >> +	 * To prevent loops, check if dev is not upper device to upper_dev.
>> >> +	 */
>> >> +	if (__netdev_has_upper_dev(upper_dev, dev, true))
>> >> +		return -EBUSY;
>> >> +
>> >> +	if (__netdev_find_upper(dev, upper_dev))
>> >> +		return -EEXIST;
>> >
>> >__netdev_has_upper_dev() can go all the way up finding the device and
>> >the __netdev_find_upper() just check the first level.
>> 
>> 
>> I do not think this ordering is somewhat inportant.
>
>it's not the order, see below:
>
>> >I think it would be better to use:
>> >__netdev_find_upper_dev(,,deep=true/false)
>> >__netdev_has_upper(,)
>
>It's their names.  Currently, the function ..._find_... look at
>one level only, while the function ..._has_... does one or more
>levels.  I think it's better to swap 'has' and 'find' in their names:
>
>__netdev_find_upper_dev(,,deep=true/false) <-- find in all levels
>__netdev_has_upper(,)  <-- check only the one level.

Oh, now I think I see your point. But realise this:

The main reason for __netdev_find_upper() is to find "struct upper" for
netdev_upper_dev_unlink(). Therefore the name is not
"__netdev_find_upper_dev" and there's no need to go deep here.

On the orher hand, __netdev_has_upper_dev() only says whether device is lower
to specified upper device. In this case I think the name is quite
convenient as well.


>
>fbl

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

* Re: [patch net-next 01/16] net: introduce upper device lists
  2012-08-13 17:31     ` [patch net-next 01/16] net: introduce upper device lists Jiri Pirko
@ 2012-08-15 20:33       ` Nicolas de Pesloüan
  0 siblings, 0 replies; 24+ messages in thread
From: Nicolas de Pesloüan @ 2012-08-15 20:33 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: bridge, ursula.braun, john.r.fastabend, edumazet, shemminger,
	sean.hefty, therbert, roland, linux-s390, linux-rdma, fubar, fbl,
	hal.rosenstock, faisal.latif, blaschka, sony.chacko, linux-driver,
	Ben Hutchings, xiyou.wangcong, jitendra.kalsaria, divy,
	gregory.v.rose, netdev, linux-kernel, kaber, joe, linux390, davem

Le 13/08/2012 19:31, Jiri Pirko a écrit :
> Mon, Aug 13, 2012 at 07:04:11PM CEST, bhutchings@solarflare.com wrote:

>>> +struct netdev_upper {
>>> +	struct net_device *dev;
>>> +	bool unique;
>>
>> This needs a better name.  It doesn't really have anything to do with
>> uniqueness and doesn't ensure exclusivity.  I think that it would be
>> fine to keep the 'master' term.
>
> Hmm. I admit that "unique" I do not like too much as well. But "master"
> I like even less.
>
> This flag should ensure exclusivity. Only one upper device with this
> flag can be present at a time.

Well, can't we simply call it "upper_device"?

And as we only have a single field, this is exclusive by design.

	Nicolas.

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

end of thread, other threads:[~2012-08-15 20:33 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-13 15:26 [patch net-next 00/16] net: introduce upper device lists and remove dev->master Jiri Pirko
2012-08-13 15:27 ` [patch net-next 02/16] macvlan: add link to upper device Jiri Pirko
2012-08-13 15:27 ` [patch net-next 03/16] vlan: " Jiri Pirko
2012-08-13 19:04   ` Flavio Leitner
2012-08-13 15:27 ` [patch net-next 04/16] rtnetlink: remove usage of dev->master Jiri Pirko
2012-08-13 15:27 ` [patch net-next 05/16] team: remove usage of netdev_set_master() Jiri Pirko
2012-08-13 15:27 ` [patch net-next 06/16] bridge: " Jiri Pirko
2012-08-13 15:27 ` [patch net-next 07/16] netpoll: remove usage of dev->master Jiri Pirko
2012-08-13 15:27 ` [patch net-next 08/16] cxgb3: " Jiri Pirko
2012-08-13 15:27 ` [patch net-next 09/16] qlcnic: guard __vlan_find_dev_deep() by rcu_read_lock Jiri Pirko
2012-08-13 15:27 ` [patch net-next 10/16] qeth: ensure that __vlan_find_dev_deep() is called with rcu_read_lock Jiri Pirko
2012-08-13 15:27 ` [patch net-next 11/16] vlan: remove usage of dev->master in __vlan_find_dev_deep() Jiri Pirko
2012-08-13 15:27 ` [patch net-next 12/16] nes: remove usage of dev->master Jiri Pirko
2012-08-13 15:27 ` [patch net-next 13/16] bonding: " Jiri Pirko
2012-08-13 15:27 ` [patch net-next 14/16] net: remove no longer used netdev_set_bond_master() and netdev_set_master() Jiri Pirko
2012-08-13 15:27 ` [patch net-next 15/16] net: remove usage of dev->master Jiri Pirko
2012-08-13 15:27 ` [patch net-next 16/16] net: kill dev->master Jiri Pirko
     [not found] ` <1344871635-1052-2-git-send-email-jiri@resnulli.us>
     [not found]   ` <1344877451.2733.26.camel@bwh-desktop.uk.solarflarecom.com>
2012-08-13 17:31     ` [patch net-next 01/16] net: introduce upper device lists Jiri Pirko
2012-08-15 20:33       ` Nicolas de Pesloüan
2012-08-13 17:52   ` Flavio Leitner
2012-08-14 12:24     ` Jiri Pirko
2012-08-14 13:14       ` Flavio Leitner
2012-08-14 13:35         ` Jiri Pirko
     [not found]   ` <502A1429.9060005@gmail.com>
2012-08-14 10:18     ` Jiri Pirko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).