All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/6] bonding: remove bond->vlan_list
@ 2013-08-08 16:57 Veaceslav Falico
  2013-08-08 16:57 ` [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it Veaceslav Falico
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Veaceslav Falico @ 2013-08-08 16:57 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andy Gospodarek, Patrick McHardy, David S. Miller,
	Nikolay Aleksandrov, Veaceslav Falico

RFC -> v1: Got some feedback from Nikolay Aleksandrov (privately), tried to
	   address it, also fixed some bugs that I've found on the way. I
	   think it's ready to be considered a patchset for
	   review/inclusion in net-next.

v1  -> v2: Remove ASSERT_RTNL() from vlan_uses_dev(), cause it can be
	   already called under rcu, without rtnl. Don't check for master
	   device in __vlan_find_dev_next(), otherwise we won't be able to
	   work in situations when a device has both vlans and master
	   device. Properly init vlan_dev in bond_has_this_ip() before
	   using (sigh). There was a proposal of making a macro
	   "dev_for_each_vlan_from(dev, vlan_dev, i, from)", which would
	   use __vlan_find_dev_deep() inside, with its strong and weak
	   parts, but I've decided to stick to the "while (dev = next())"
	   scheme currently - it might be added anytime, and now the only
	   user (bonding) doesn't really need it.

The aim of this patchset is to remove bond->vlan_list completely, and use
8021q's standard functions instead of it.

The patchset is on top of Nik's latest two patches:
[net-next,v2,1/2] bonding: change the bond's vlan syncing functions with
			   the standard ones
[net-next,v2,2/2] bonding: unwind on bond_add_vlan failure


First two patches add two helper functions to vlan code:

bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it

	Here we change vlan_uses_dev() to be able to work under both rtnl
	and rcu, and use it under rcu_read_lock() in bond_vlan_used().

vlan: add __vlan_find_dev_next()

	This function takes dev and vlan_dev and returns the next vlan dev
	that uses this dev. It can be used to cycle through the vlan list,
	and not only by bonding - but by any network driver that uses its
	private vlan list.

Next four patches actually convert bonding to use the new
functions/approach and remove the vlan_list completely.

This patchset solves several issues with bonding, simplify it overall,
RCUify further and add infrastructure to anyone else who'd like to use
8021q standard functions instead of their own vlan_list, which is quite
common amongst network drivers currently.

I'm testing it continuously currently, no issues found, will update on
anything.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 drivers/net/bonding/bond_alb.c  |   48 ++++++++----
 drivers/net/bonding/bond_alb.h  |    2 +-
 drivers/net/bonding/bond_main.c |  163 ++++++---------------------------------
 drivers/net/bonding/bonding.h   |   16 ++--
 include/linux/if_vlan.h         |    8 ++
 net/8021q/vlan.h                |    6 +-
 net/8021q/vlan_core.c           |   36 ++++++++-
 7 files changed, 115 insertions(+), 164 deletions(-)

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH v1 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
@ 2013-08-08 10:21 Veaceslav Falico
  2013-08-08 10:39 ` [PATCH v2 " Veaceslav Falico
  0 siblings, 1 reply; 18+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
  To: netdev
  Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek, Patrick McHardy,
	David S. Miller, Nikolay Aleksandrov

RFC -> v1: don't add vlan_uses_dev_rcu() and change vlan_uses_dev() instead

Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
id 0, and always returns true if 8021q is loaded. This creates several bad
situations - some warnings in __bond_release_one() because it thinks that
we still have vlans while removing, sending LB packets with vlan id 0 and,
possibly, other caused by vlan id 0.

Fix it by changing vlan_uses_dev() to use rcu_dereference_rtnl() instead of
rtnl_dereference(), and thus it can already be used in bond_vlan_used()
under rcu_read_lock().

By the time vlan_uses_dev() returns we cannot be sure if there were no
vlans added/removed, so it's basicly an optimization function.

Also, use the vlan_uses_dev() in __bond_release_one() cause the rtnl lock
is held there.

For this call to be visible in bonding.h, add include <linux/if_vlan.h>,
and also remove it from any other bonding file, cause they all include
bonding.h, and thus linux/if_vlan.h.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_alb.c  |    1 -
 drivers/net/bonding/bond_main.c |    3 +--
 drivers/net/bonding/bonding.h   |   10 +++++++++-
 net/8021q/vlan_core.c           |    3 ++-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 3a5db7b..2684329 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -34,7 +34,6 @@
 #include <linux/if_arp.h>
 #include <linux/if_ether.h>
 #include <linux/if_bonding.h>
-#include <linux/if_vlan.h>
 #include <linux/in.h>
 #include <net/ipx.h>
 #include <net/arp.h>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 4264a76..9d1045d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -69,7 +69,6 @@
 #include <net/arp.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
-#include <linux/if_vlan.h>
 #include <linux/if_bonding.h>
 #include <linux/jiffies.h>
 #include <linux/preempt.h>
@@ -1953,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 		bond_set_carrier(bond);
 		eth_hw_addr_random(bond_dev);
 
-		if (bond_vlan_used(bond)) {
+		if (vlan_uses_dev(bond_dev)) {
 			pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
 				   bond_dev->name, bond_dev->name);
 			pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4bf52d5..9c4539e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -23,6 +23,7 @@
 #include <linux/netpoll.h>
 #include <linux/inetdevice.h>
 #include <linux/etherdevice.h>
+#include <linux/if_vlan.h>
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
@@ -267,9 +268,16 @@ struct bonding {
 #endif /* CONFIG_DEBUG_FS */
 };
 
+/* use vlan_uses_dev() if under rtnl */
 static inline bool bond_vlan_used(struct bonding *bond)
 {
-	return !list_empty(&bond->vlan_list);
+	bool ret;
+
+	rcu_read_lock();
+	ret = vlan_uses_dev(bond->dev);
+	rcu_read_unlock();
+
+	return ret;
 }
 
 #define bond_slave_get_rcu(dev) \
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 4a78c4d..12e1606 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -387,13 +387,14 @@ void vlan_vids_del_by_dev(struct net_device *dev,
 }
 EXPORT_SYMBOL(vlan_vids_del_by_dev);
 
+/* Must hold either rtnl or rcu_read_lock */
 bool vlan_uses_dev(const struct net_device *dev)
 {
 	struct vlan_info *vlan_info;
 
 	ASSERT_RTNL();
 
-	vlan_info = rtnl_dereference(dev->vlan_info);
+	vlan_info = rcu_dereference_rtnl(dev->vlan_info);
 	if (!vlan_info)
 		return false;
 	return vlan_info->grp.nr_vlan_devs ? true : false;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2013-08-26 16:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-08 16:57 [PATCH net-next v2 0/6] bonding: remove bond->vlan_list Veaceslav Falico
2013-08-08 16:57 ` [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it Veaceslav Falico
2013-08-09 11:06   ` Nikolay Aleksandrov
2013-08-09 11:11     ` Veaceslav Falico
2013-08-08 16:57 ` [PATCH v2 net-next 2/6] vlan: add __vlan_find_dev_next() Veaceslav Falico
2013-08-09  7:30   ` Veaceslav Falico
2013-08-09 11:07   ` Nikolay Aleksandrov
2013-08-14 15:28     ` Veaceslav Falico
2013-08-08 16:57 ` [PATCH v2 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list Veaceslav Falico
2013-08-09 11:13   ` Nikolay Aleksandrov
2013-08-09 11:24     ` Veaceslav Falico
2013-08-08 16:57 ` [PATCH v2 net-next 4/6] bonding: convert bond_has_this_ip to use bond->dev->vlan_info Veaceslav Falico
2013-08-08 16:57 ` [PATCH v2 net-next 5/6] bonding: convert bond_arp_send_all " Veaceslav Falico
2013-08-09 11:42   ` Nikolay Aleksandrov
2013-08-08 16:57 ` [PATCH v2 net-next 6/6] bonding: remove unused bond->vlan_list Veaceslav Falico
2013-08-09 11:44   ` Nikolay Aleksandrov
2013-08-26 16:31 ` [PATCH net-next v2 0/6] bonding: remove bond->vlan_list Veaceslav Falico
  -- strict thread matches above, loose matches on Subject: below --
2013-08-08 10:21 [PATCH v1 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it Veaceslav Falico
2013-08-08 10:39 ` [PATCH v2 " Veaceslav Falico

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