* [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 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
@ 2013-08-08 10:21 ` Veaceslav Falico
2013-08-08 10:39 ` [PATCH v2 " Veaceslav Falico
2013-08-08 10:21 ` [PATCH v1 net-next 2/6] vlan: add __vlan_find_dev_next() Veaceslav Falico
` (6 subsequent siblings)
7 siblings, 1 reply; 13+ 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] 13+ messages in thread* [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
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 ` Veaceslav Falico
0 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:39 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
v1 -> v2: remove the ASSERT_RTNL(), cause we can now be called under rcu.
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 | 5 ++---
4 files changed, 12 insertions(+), 7 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..361c97b 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -387,13 +387,12 @@ 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] 13+ messages in thread
* [PATCH v1 net-next 2/6] vlan: add __vlan_find_dev_next()
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
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:21 ` Veaceslav Falico
2013-08-08 10:21 ` [PATCH v1 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list Veaceslav Falico
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Patrick McHardy, David S. Miller
RFC -> v1: make the function accept/return vlan's net_device, this way we
won't have troubles with VLAN 0, and the user code will be
cleaner and faster.
Add a new exported function __vlan_find_dev_next(dev, vlan_dev), which
returns the a vlan's net_device that is used by the dev and is its id is
greater or equal to vlan_dev's vlan id. If vlan_dev is NULL, return first
vlan, if nothing is found return NULL.
This function must be under rcu_read_lock(), is aware of master devices and
doesn't guarantee that, once it returns, the vlan dev will still be used by
dev as its vlan.
It's basically a helper for "for_each_vlan_in_dev(dev, vlan_dev)" logic,
and is supposed to be used like this:
vlan_dev = NULL;
while ((vlan_dev = __vlan_find_dev_next(dev, vlan_dev))) {
if (!vlan_dev)
continue;
do_work(vlan_dev);
}
In that case we're sure that vlan_dev at least was used as a vlan, and won't
go away while we're holding rcu_read_lock().
However, if we don't hold rtnl_lock(), we can't be sure that that vlan_dev
is still in dev's vlan_list.
CC: Patrick McHardy <kaber@trash.net>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
include/linux/if_vlan.h | 8 ++++++++
net/8021q/vlan.h | 6 ++++--
net/8021q/vlan_core.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 715c343..1cfc201 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -86,6 +86,8 @@ static inline int is_vlan_dev(struct net_device *dev)
extern struct net_device *__vlan_find_dev_deep(struct net_device *real_dev,
__be16 vlan_proto, u16 vlan_id);
+extern struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev);
extern struct net_device *vlan_dev_real_dev(const struct net_device *dev);
extern u16 vlan_dev_vlan_id(const struct net_device *dev);
@@ -109,6 +111,12 @@ __vlan_find_dev_deep(struct net_device *real_dev,
return NULL;
}
+static struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev)
+{
+ return NULL;
+}
+
static inline struct net_device *vlan_dev_real_dev(const struct net_device *dev)
{
BUG();
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index ba5983f..e13aeac 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -168,10 +168,12 @@ static inline struct net_device *vlan_find_dev(struct net_device *real_dev,
return NULL;
}
-#define vlan_group_for_each_dev(grp, i, dev) \
- for ((i) = 0; i < VLAN_PROTO_NUM * VLAN_N_VID; i++) \
+#define vlan_group_for_each_dev_from(grp, i, dev, from) \
+ for ((i) = from; i < VLAN_PROTO_NUM * VLAN_N_VID; i++) \
if (((dev) = __vlan_group_get_device((grp), (i) / VLAN_N_VID, \
(i) % VLAN_N_VID)))
+#define vlan_group_for_each_dev(grp, i, dev) \
+ vlan_group_for_each_dev_from(grp, i, dev, 0)
/* found in vlan_dev.c */
void vlan_dev_set_ingress_priority(const struct net_device *dev,
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 12e1606..20e1f92 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -89,6 +89,39 @@ struct net_device *__vlan_find_dev_deep(struct net_device *dev,
}
EXPORT_SYMBOL(__vlan_find_dev_deep);
+/* Must be called under rcu_read_lock(), returns next vlan_id used by a
+ * vlan device on dev, starting with vlan_id, or 0 if no vlan_id found.
+ */
+struct net_device *__vlan_find_dev_next(struct net_device *dev,
+ struct net_device *vlan_dev)
+{
+ struct net_device *real_dev = dev, *master_dev, *ret;
+ struct vlan_info *vlan_info;
+ struct vlan_group *grp;
+ u16 vlan_id = 0, i;
+
+ if (vlan_dev)
+ vlan_id = vlan_dev_priv(vlan_dev)->vlan_id + 1;
+
+ master_dev = netdev_master_upper_dev_get_rcu(real_dev);
+
+ if (master_dev)
+ real_dev = master_dev;
+
+ vlan_info = rcu_dereference(real_dev->vlan_info);
+
+ if (!vlan_info)
+ return NULL;
+
+ grp = &vlan_info->grp;
+
+ vlan_group_for_each_dev_from(grp, i, ret, vlan_id)
+ return ret;
+
+ return NULL;
+}
+EXPORT_SYMBOL(__vlan_find_dev_next);
+
struct net_device *vlan_dev_real_dev(const struct net_device *dev)
{
return vlan_dev_priv(dev)->real_dev;
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v1 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
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:21 ` [PATCH v1 net-next 2/6] vlan: add __vlan_find_dev_next() Veaceslav Falico
@ 2013-08-08 10:21 ` Veaceslav Falico
2013-08-08 10:21 ` [PATCH v1 net-next 4/6] bonding: convert bond_has_this_ip to use bond->dev->vlan_info Veaceslav Falico
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
RFC -> v1: use the changed __vlan_find_dev_next, which now works with
vlan's net_device instead of vlan's id. Also, fix a subtle race
condition if we remove the only vlan while looping through
MAX_LP_BURST - we end up with using the old vlan_id, so set it
to 0 if we don't have vlans.
In alb mode, we only need each vlan's id (that is on top of bond) to tag
learning packets, so get them via __vlan_find_dev_next(bond->dev, last_dev).
We must also find *any* vlan (including last id stored in current_alb_vlan)
if we can't find anything >= current_alb_vlan id.
For that, we verify if bond has any vlans at all, and if yes - find the
next vlan id after current_alb_vlan id. So, if vlan id is not 0, we tag the
skb.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_alb.c | 47 ++++++++++++++++++++++++++++-----------
drivers/net/bonding/bond_alb.h | 2 +-
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 2684329..ced5753 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -974,8 +974,9 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
{
struct bonding *bond = bond_get_bond_by_slave(slave);
struct learning_pkt pkt;
+ struct net_device *vlan_dev;
int size = sizeof(struct learning_pkt);
- int i;
+ int i, vlan_id;
memset(&pkt, 0, size);
memcpy(pkt.mac_dst, mac_addr, ETH_ALEN);
@@ -1000,22 +1001,42 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
skb->priority = TC_PRIO_CONTROL;
skb->dev = slave->dev;
+ rcu_read_lock();
if (bond_vlan_used(bond)) {
- struct vlan_entry *vlan;
-
- vlan = bond_next_vlan(bond,
- bond->alb_info.current_alb_vlan);
-
- bond->alb_info.current_alb_vlan = vlan;
- if (!vlan) {
+ /* first try to find the previously used vlan by
+ * id, which might have gone away already
+ */
+ vlan_id = bond->alb_info.current_alb_vlan;
+ vlan_dev = __vlan_find_dev_deep(bond->dev,
+ htons(ETH_P_8021Q),
+ vlan_id);
+
+ /* search for the next one, if not found - for any */
+ if (vlan_dev)
+ vlan_dev = __vlan_find_dev_next(bond->dev,
+ vlan_dev);
+ if (!vlan_dev)
+ vlan_dev = __vlan_find_dev_next(bond->dev,
+ NULL);
+
+ if (vlan_dev) {
+ vlan_id = vlan_dev_vlan_id(vlan_dev);
+ bond->alb_info.current_alb_vlan = vlan_id;
+ } else {
+ bond->alb_info.current_alb_vlan = 0;
+ rcu_read_unlock();
kfree_skb(skb);
continue;
}
+ } else
+ vlan_id = 0;
+ rcu_read_unlock();
- skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vlan->vlan_id);
+ if (vlan_id) {
+ skb = vlan_put_tag(skb, htons(ETH_P_8021Q), vlan_id);
if (!skb) {
- pr_err("%s: Error: failed to insert VLAN tag\n",
- bond->dev->name);
+ pr_err("%s: Error: failed to insert VLAN tag %d\n",
+ bond->dev->name, vlan_id);
continue;
}
}
@@ -1759,8 +1780,8 @@ int bond_alb_set_mac_address(struct net_device *bond_dev, void *addr)
void bond_alb_clear_vlan(struct bonding *bond, unsigned short vlan_id)
{
if (bond->alb_info.current_alb_vlan &&
- (bond->alb_info.current_alb_vlan->vlan_id == vlan_id)) {
- bond->alb_info.current_alb_vlan = NULL;
+ (bond->alb_info.current_alb_vlan == vlan_id)) {
+ bond->alb_info.current_alb_vlan = 0;
}
if (bond->alb_info.rlb_enabled) {
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index e7a5b8b..b2452aa 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -170,7 +170,7 @@ struct alb_bond_info {
* rx traffic should be
* rebalanced
*/
- struct vlan_entry *current_alb_vlan;
+ u16 current_alb_vlan;
};
int bond_alb_initialize(struct bonding *bond, int rlb_enabled);
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v1 net-next 4/6] bonding: convert bond_has_this_ip to use bond->dev->vlan_info
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
` (2 preceding siblings ...)
2013-08-08 10:21 ` [PATCH v1 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list Veaceslav Falico
@ 2013-08-08 10:21 ` Veaceslav Falico
2013-08-08 10:21 ` [PATCH v1 net-next 5/6] bonding: convert bond_arp_send_all " Veaceslav Falico
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
RFC -> v1: use the new __vlan_find_dev_next(), which simplifies the code
and omits issues with vlan id 0.
Use __vlan_find_dev_next() to loop through dev's vlan devices and verify if
the ip matches.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 14 ++++++--------
1 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9d1045d..14447a6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2392,20 +2392,18 @@ re_arm:
static int bond_has_this_ip(struct bonding *bond, __be32 ip)
{
- struct vlan_entry *vlan;
struct net_device *vlan_dev;
if (ip == bond_confirm_addr(bond->dev, 0, ip))
return 1;
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- rcu_read_lock();
- vlan_dev = __vlan_find_dev_deep(bond->dev, htons(ETH_P_8021Q),
- vlan->vlan_id);
- rcu_read_unlock();
- if (vlan_dev && ip == bond_confirm_addr(vlan_dev, 0, ip))
+ rcu_read_lock();
+ while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
+ if (ip == bond_confirm_addr(vlan_dev, 0, ip)) {
+ rcu_read_unlock();
return 1;
- }
+ }
+ rcu_read_unlock();
return 0;
}
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v1 net-next 5/6] bonding: convert bond_arp_send_all to use bond->dev->vlan_info
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
` (3 preceding siblings ...)
2013-08-08 10:21 ` [PATCH v1 net-next 4/6] bonding: convert bond_has_this_ip to use bond->dev->vlan_info Veaceslav Falico
@ 2013-08-08 10:21 ` Veaceslav Falico
2013-08-08 10:21 ` [PATCH v1 net-next 6/6] bonding: remove unused bond->vlan_list Veaceslav Falico
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
RFC -> v1: use the new __vlan_find_dev_next(), also release rcu_read_lock()
only after we stop using the vlan_dev.
Instead of looping through bond->vlan_list, loop through
bond->dev->vlan_info via __vlan_find_dev_next() under rcu_read_lock().
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 29 +++++++++++++----------------
1 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 14447a6..215c497 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2440,11 +2440,10 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
{
- int i, vlan_id;
- __be32 *targets = bond->params.arp_targets;
- struct vlan_entry *vlan;
- struct net_device *vlan_dev = NULL;
+ struct net_device *vlan_dev;
struct rtable *rt;
+ __be32 *targets = bond->params.arp_targets;
+ int i;
for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
__be32 addr;
@@ -2486,28 +2485,26 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
continue;
}
- vlan_id = 0;
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- rcu_read_lock();
- vlan_dev = __vlan_find_dev_deep(bond->dev,
- htons(ETH_P_8021Q),
- vlan->vlan_id);
- rcu_read_unlock();
+ vlan_dev = NULL;
+
+ rcu_read_lock();
+ while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
if (vlan_dev == rt->dst.dev) {
- vlan_id = vlan->vlan_id;
pr_debug("basa: vlan match on %s %d\n",
- vlan_dev->name, vlan_id);
+ vlan_dev->name,
+ vlan_dev_vlan_id(vlan_dev));
break;
}
- }
- if (vlan_id && vlan_dev) {
+ if (vlan_dev) {
ip_rt_put(rt);
addr = bond_confirm_addr(vlan_dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
- addr, vlan_id);
+ addr, vlan_dev_vlan_id(vlan_dev));
+ rcu_read_unlock();
continue;
}
+ rcu_read_unlock();
if (net_ratelimit()) {
pr_warning("%s: no path to arp_ip_target %pI4 via rt.dev %s\n",
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v1 net-next 6/6] bonding: remove unused bond->vlan_list
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
` (4 preceding siblings ...)
2013-08-08 10:21 ` [PATCH v1 net-next 5/6] bonding: convert bond_arp_send_all " Veaceslav Falico
@ 2013-08-08 10:21 ` Veaceslav Falico
2013-08-08 10:39 ` [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
2013-08-08 16:49 ` Veaceslav Falico
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:21 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
RFC -> v1: No changes.
There are currently no users of bond->vlan_list (other than the maintaining
functions add/remove) - so remove it and every unneeded helper.
In this patch we remove:
vlan_list from struct bonding
bond_next_vlan - we don't need it anymore
struct vlan_entry - it was a helper struct for bond->vlan_list
some bits from bond_vlan_rx_add/kill_vid() - which were related to
bond->vlan_list
(de)initialization of vlan_list from bond_setup/uninit
bond_add_vlan - its only scope was to maintain bond->vlan_list
We don't fully remove bond_del_vlan() - bond_alb_clear_vlan() still needs
to be called when a vlan disappears. And we make bond_del_vlan() to not
return anything cause it cannot fail already.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 117 +-------------------------------------
drivers/net/bonding/bonding.h | 6 --
2 files changed, 4 insertions(+), 119 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 215c497..49bdda2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -282,113 +282,24 @@ const char *bond_mode_name(int mode)
/*---------------------------------- VLAN -----------------------------------*/
/**
- * bond_add_vlan - add a new vlan id on bond
- * @bond: bond that got the notification
- * @vlan_id: the vlan id to add
- *
- * Returns -ENOMEM if allocation failed.
- */
-static int bond_add_vlan(struct bonding *bond, unsigned short vlan_id)
-{
- struct vlan_entry *vlan;
-
- pr_debug("bond: %s, vlan id %d\n",
- (bond ? bond->dev->name : "None"), vlan_id);
-
- vlan = kzalloc(sizeof(struct vlan_entry), GFP_KERNEL);
- if (!vlan)
- return -ENOMEM;
-
- INIT_LIST_HEAD(&vlan->vlan_list);
- vlan->vlan_id = vlan_id;
-
- write_lock_bh(&bond->lock);
-
- list_add_tail(&vlan->vlan_list, &bond->vlan_list);
-
- write_unlock_bh(&bond->lock);
-
- pr_debug("added VLAN ID %d on bond %s\n", vlan_id, bond->dev->name);
-
- return 0;
-}
-
-/**
* bond_del_vlan - delete a vlan id from bond
* @bond: bond that got the notification
* @vlan_id: the vlan id to delete
*
* returns -ENODEV if @vlan_id was not found in @bond.
*/
-static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
+static void bond_del_vlan(struct bonding *bond, unsigned short vlan_id)
{
- struct vlan_entry *vlan;
- int res = -ENODEV;
-
pr_debug("bond: %s, vlan id %d\n", bond->dev->name, vlan_id);
block_netpoll_tx();
write_lock_bh(&bond->lock);
- list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
- if (vlan->vlan_id == vlan_id) {
- list_del(&vlan->vlan_list);
-
- if (bond_is_lb(bond))
- bond_alb_clear_vlan(bond, vlan_id);
-
- pr_debug("removed VLAN ID %d from bond %s\n",
- vlan_id, bond->dev->name);
+ if (bond_is_lb(bond))
+ bond_alb_clear_vlan(bond, vlan_id);
- kfree(vlan);
-
- res = 0;
- goto out;
- }
- }
-
- pr_debug("couldn't find VLAN ID %d in bond %s\n",
- vlan_id, bond->dev->name);
-
-out:
write_unlock_bh(&bond->lock);
unblock_netpoll_tx();
- return res;
-}
-
-/**
- * bond_next_vlan - safely skip to the next item in the vlans list.
- * @bond: the bond we're working on
- * @curr: item we're advancing from
- *
- * Returns %NULL if list is empty, bond->next_vlan if @curr is %NULL,
- * or @curr->next otherwise (even if it is @curr itself again).
- *
- * Caller must hold bond->lock
- */
-struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr)
-{
- struct vlan_entry *next, *last;
-
- if (list_empty(&bond->vlan_list))
- return NULL;
-
- if (!curr) {
- next = list_entry(bond->vlan_list.next,
- struct vlan_entry, vlan_list);
- } else {
- last = list_entry(bond->vlan_list.prev,
- struct vlan_entry, vlan_list);
- if (last == curr) {
- next = list_entry(bond->vlan_list.next,
- struct vlan_entry, vlan_list);
- } else {
- next = list_entry(curr->vlan_list.next,
- struct vlan_entry, vlan_list);
- }
- }
-
- return next;
}
/**
@@ -450,13 +361,6 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
goto unwind;
}
- res = bond_add_vlan(bond, vid);
- if (res) {
- pr_err("%s: Error: Failed to add vlan id %d\n",
- bond_dev->name, vid);
- goto unwind;
- }
-
return 0;
unwind:
@@ -477,17 +381,11 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave;
- int res;
bond_for_each_slave(bond, slave)
vlan_vid_del(slave->dev, proto, vid);
- res = bond_del_vlan(bond, vid);
- if (res) {
- pr_err("%s: Error: Failed to remove vlan id %d\n",
- bond_dev->name, vid);
- return res;
- }
+ bond_del_vlan(bond, vid);
return 0;
}
@@ -4135,7 +4033,6 @@ static void bond_setup(struct net_device *bond_dev)
/* Initialize pointers */
bond->dev = bond_dev;
- INIT_LIST_HEAD(&bond->vlan_list);
/* Initialize the device entry points */
ether_setup(bond_dev);
@@ -4188,7 +4085,6 @@ static void bond_uninit(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave, *tmp_slave;
- struct vlan_entry *vlan, *tmp;
bond_netpoll_cleanup(bond_dev);
@@ -4200,11 +4096,6 @@ static void bond_uninit(struct net_device *bond_dev)
list_del(&bond->bond_list);
bond_debug_unregister(bond);
-
- list_for_each_entry_safe(vlan, tmp, &bond->vlan_list, vlan_list) {
- list_del(&vlan->vlan_list);
- kfree(vlan);
- }
}
/*------------------------- Module initialization ---------------------------*/
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9c4539e..bfa33c4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -186,11 +186,6 @@ struct bond_parm_tbl {
#define BOND_MAX_MODENAME_LEN 20
-struct vlan_entry {
- struct list_head vlan_list;
- unsigned short vlan_id;
-};
-
struct slave {
struct net_device *dev; /* first - useful for panic debug */
struct list_head list;
@@ -255,7 +250,6 @@ struct bonding {
struct ad_bond_info ad_info;
struct alb_bond_info alb_info;
struct bond_params params;
- struct list_head vlan_list;
struct workqueue_struct *wq;
struct delayed_work mii_work;
struct delayed_work arp_work;
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH net-next v1 0/6] bonding: remove bond->vlan_list
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
` (5 preceding siblings ...)
2013-08-08 10:21 ` [PATCH v1 net-next 6/6] bonding: remove unused bond->vlan_list Veaceslav Falico
@ 2013-08-08 10:39 ` Veaceslav Falico
2013-08-08 16:49 ` Veaceslav Falico
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 10:39 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Andy Gospodarek, Patrick McHardy, David S. Miller,
Nikolay Aleksandrov
On Thu, Aug 08, 2013 at 12:21:03PM +0200, Veaceslav Falico wrote:
>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.
I've re-sent the 1/6 (with v2), got the old version from the wrong git
branch.
>
>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] 13+ messages in thread* Re: [PATCH net-next v1 0/6] bonding: remove bond->vlan_list
2013-08-08 10:21 [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
` (6 preceding siblings ...)
2013-08-08 10:39 ` [PATCH net-next v1 0/6] bonding: remove bond->vlan_list Veaceslav Falico
@ 2013-08-08 16:49 ` Veaceslav Falico
7 siblings, 0 replies; 13+ messages in thread
From: Veaceslav Falico @ 2013-08-08 16:49 UTC (permalink / raw)
To: netdev
Cc: Jay Vosburgh, Andy Gospodarek, Patrick McHardy, David S. Miller,
Nikolay Aleksandrov
On Thu, Aug 08, 2013 at 12:21:03PM +0200, Veaceslav Falico wrote:
>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.
Got some more feedback and spotted some bugs, will send a full v2 shortly.
^ permalink raw reply [flat|nested] 13+ messages in thread