Skip to contentSkip to conversation

bonding: sync netpoll code with bridge

PATCH Applied Amerigo Wang 3 participants 11 messages Feb 18, 2011 – Feb 28, 2011

Subsystems (3)
Networking Network drivers Bonding driver
lore
Share & tools
lore

Amerigo Wang

author
#0 lore raw

PATCH v4 1/3 bonding: sync netpoll code with bridge

V4: rebase to net-next-2.6
V3: remove an useless #ifdef.

This patch unifies the netpoll code in bonding with netpoll code in bridge, thanks to Herbert that code is much cleaner now.

Signed-off-by WANG Cong <[email protected]>
Cc Neil Horman <[email protected]>
Cc Herbert Xu <[email protected]>
---
 drivers/net/bonding/bond_main.c |  155 ++++++++++++++++++++++++--------------
 drivers/net/bonding/bonding.h   |   20 +++++
 2 files changed, 118 insertions(+), 57 deletions(-)
drivers/net/bonding/bond_main.c +98 -57
index 77e3c6a..2ed6624 100644
@@ -59,7 +59,6 @@
 #include <linux/uaccess.h>
 #include <linux/errno.h>
 #include <linux/netdevice.h>
-#include <linux/netpoll.h>
 #include <linux/inetdevice.h>
 #include <linux/igmp.h>
 #include <linux/etherdevice.h>

@@ -424,15 +423,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
 {
 	skb->dev = slave_dev;
 	skb->priority = 1;
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	if (unlikely(bond->dev->priv_flags & IFF_IN_NETPOLL)) {
-		struct netpoll *np = bond->dev->npinfo->netpoll;
-		slave_dev->npinfo = bond->dev->npinfo;
+	if (unlikely(netpoll_tx_running(slave_dev))) {
 		slave_dev->priv_flags |= IFF_IN_NETPOLL;
-		netpoll_send_skb_on_dev(np, skb, slave_dev);
+		bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
 		slave_dev->priv_flags &= ~IFF_IN_NETPOLL;
 	} else
-#endif
 		dev_queue_xmit(skb);
 
 	return 0;

@@ -1288,63 +1283,113 @@ static void bond_detach_slave(struct bonding *bond, struct slave *slave)
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-/*
- * You must hold read lock on bond->lock before calling this.
- */
-static bool slaves_support_netpoll(struct net_device *bond_dev)
+static inline int slave_enable_netpoll(struct slave *slave)
 {
-	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave;
-	int i = 0;
-	bool ret = true;
+	struct netpoll *np;
+	int err = 0;
 
-	bond_for_each_slave(bond, slave, i) {
-		if ((slave->dev->priv_flags & IFF_DISABLE_NETPOLL) ||
-		    !slave->dev->netdev_ops->ndo_poll_controller)
-			ret = false;
+	np = kzalloc(sizeof(*np), GFP_KERNEL);
+	err = -ENOMEM;
+	if (!np)
+		goto out;
+
+	np->dev = slave->dev;
+	err = __netpoll_setup(np);
+	if (err) {
+		kfree(np);
+		goto out;
 	}
-	return i != 0 && ret;
+	slave->np = np;
+out:
+	return err;
+}
+static inline void slave_disable_netpoll(struct slave *slave)
+{
+	struct netpoll *np = slave->np;
+
+	if (!np)
+		return;
+
+	slave->np = NULL;
+	synchronize_rcu_bh();
+	__netpoll_cleanup(np);
+	kfree(np);
+}
+static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
+{
+	if (slave_dev->priv_flags & IFF_DISABLE_NETPOLL)
+		return false;
+	if (!slave_dev->netdev_ops->ndo_poll_controller)
+		return false;
+	return true;
 }
 
 static void bond_poll_controller(struct net_device *bond_dev)
 {
-	struct bonding *bond = netdev_priv(bond_dev);
+}
+
+static void __bond_netpoll_cleanup(struct bonding *bond)
+{
 	struct slave *slave;
 	int i;
 
-	bond_for_each_slave(bond, slave, i) {
-		if (slave->dev && IS_UP(slave->dev))
-			netpoll_poll_dev(slave->dev);
-	}
+	bond_for_each_slave(bond, slave, i)
+		if (IS_UP(slave->dev))
+			slave_disable_netpoll(slave);
 }
-
 static void bond_netpoll_cleanup(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+
+	read_lock(&bond->lock);
+	__bond_netpoll_cleanup(bond);
+	read_unlock(&bond->lock);
+}
+
+static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
+{
+	struct bonding *bond = netdev_priv(dev);
 	struct slave *slave;
-	const struct net_device_ops *ops;
-	int i;
+	int i, err = 0;
 
 	read_lock(&bond->lock);
-	bond_dev->npinfo = NULL;
 	bond_for_each_slave(bond, slave, i) {
-		if (slave->dev) {
-			ops = slave->dev->netdev_ops;
-			if (ops->ndo_netpoll_cleanup)
-				ops->ndo_netpoll_cleanup(slave->dev);
-			else
-				slave->dev->npinfo = NULL;
+		if (!IS_UP(slave->dev))
+			continue;
+		err = slave_enable_netpoll(slave);
+		if (err) {
+			__bond_netpoll_cleanup(bond);
+			break;
 		}
 	}
 	read_unlock(&bond->lock);
+	return err;
 }
 
-#else
+static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
+{
+	return bond->dev->npinfo;
+}
 
+#else
+static inline int slave_enable_netpoll(struct slave *slave)
+{
+	return 0;
+}
+static inline void slave_disable_netpoll(struct slave *slave)
+{
+}
 static void bond_netpoll_cleanup(struct net_device *bond_dev)
 {
 }
-
+static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
+{
+	return 0;
+}
+static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
+{
+	return NULL;
+}
 #endif
 
 /*---------------------------------- IOCTL ----------------------------------*/

@@ -1782,17 +1827,19 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	bond_set_carrier(bond);
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-	if (slaves_support_netpoll(bond_dev)) {
-		bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
-		if (bond_dev->npinfo)
-			slave_dev->npinfo = bond_dev->npinfo;
-	} else if (!(bond_dev->priv_flags & IFF_DISABLE_NETPOLL)) {
-		bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
-		pr_info("New slave device %s does not support netpoll\n",
-			slave_dev->name);
-		pr_info("Disabling netpoll support for %s\n", bond_dev->name);
+	slave_dev->npinfo = bond_netpoll_info(bond);
+	if (slave_dev->npinfo) {
+		if (slave_enable_netpoll(new_slave)) {
+			read_unlock(&bond->lock);
+			pr_info("Error, %s: master_dev is using netpoll, "
+				 "but new slave device does not support netpoll.\n",
+				 bond_dev->name);
+			res = -EBUSY;
+			goto err_close;
+		}
 	}
 #endif
+
 	read_unlock(&bond->lock);
 
 	res = bond_create_slave_symlinks(bond_dev, slave_dev);

@@ -1994,17 +2041,7 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	netdev_set_bond_master(slave_dev, NULL);
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	read_lock_bh(&bond->lock);
-
-	if (slaves_support_netpoll(bond_dev))
-		bond_dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
-	read_unlock_bh(&bond->lock);
-	if (slave_dev->netdev_ops->ndo_netpoll_cleanup)
-		slave_dev->netdev_ops->ndo_netpoll_cleanup(slave_dev);
-	else
-		slave_dev->npinfo = NULL;
-#endif
+	slave_disable_netpoll(slave);
 
 	/* close slave before restoring its mac address */
 	dev_close(slave_dev);

@@ -2039,6 +2076,7 @@ static int  bond_release_and_destroy(struct net_device *bond_dev,
 
 	ret = bond_release(bond_dev, slave_dev);
 	if ((ret == 0) && (bond->slave_cnt == 0)) {
+		bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
 		pr_info("%s: destroying bond %s.\n",
 			bond_dev->name, bond_dev->name);
 		unregister_netdevice(bond_dev);

@@ -2116,6 +2154,8 @@ static int bond_release_all(struct net_device *bond_dev)
 
 		netdev_set_bond_master(slave_dev, NULL);
 
+		slave_disable_netpoll(slave);
+
 		/* close slave before restoring its mac address */
 		dev_close(slave_dev);
 

@@ -4654,6 +4694,7 @@ static const struct net_device_ops bond_netdev_ops = {
 	.ndo_vlan_rx_add_vid 	= bond_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid	= bond_vlan_rx_kill_vid,
 #ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_netpoll_setup	= bond_netpoll_setup,
 	.ndo_netpoll_cleanup	= bond_netpoll_cleanup,
 	.ndo_poll_controller	= bond_poll_controller,
 #endif
drivers/net/bonding/bonding.h +20 -0
index 31fe980..0a3e00b 100644
@@ -20,6 +20,7 @@
 #include <linux/if_bonding.h>
 #include <linux/cpumask.h>
 #include <linux/in6.h>
+#include <linux/netpoll.h>
 #include "bond_3ad.h"
 #include "bond_alb.h"
 

@@ -198,6 +199,9 @@ struct slave {
 	u16    queue_id;
 	struct ad_slave_info ad_info; /* HUGE - better to dynamically alloc */
 	struct tlb_slave_info tlb_info;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	struct netpoll *np;
+#endif
 };
 
 /*

@@ -323,6 +327,22 @@ static inline unsigned long slave_last_rx(struct bonding *bond,
 	return slave->dev->last_rx;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+static inline void bond_netpoll_send_skb(const struct slave *slave,
+					 struct sk_buff *skb)
+{
+	struct netpoll *np = slave->np;
+
+	if (np)
+		netpoll_send_skb(np, skb);
+}
+#else
+static inline void bond_netpoll_send_skb(const struct slave *slave,
+					 struct sk_buff *skb)
+{
+}
+#endif
+
 static inline void bond_set_slave_inactive_flags(struct slave *slave)
 {
 	struct bonding *bond = netdev_priv(slave->dev->master);
Signature · 1 line
1.7.1
author
#1 lore raw

PATCH v4 2/3 netpoll: remove IFF_IN_NETPOLL flag

V4: rebase to net-next-2.6

This patch removes the flag IFF_IN_NETPOLL, we don't need it any more since we have netpoll_tx_running() now.

Signed-off-by WANG Cong <[email protected]>
Cc Neil Horman <[email protected]>
Cc Herbert Xu <[email protected]>
---
 drivers/net/bonding/bond_main.c |    6 ++----
 drivers/net/bonding/bonding.h   |    2 +-
 include/linux/if.h              |    9 ++++-----
 net/core/netpoll.c              |    2 --
 4 files changed, 7 insertions(+), 12 deletions(-)
drivers/net/bonding/bond_main.c +2 -4
index 2ed6624..c75126d 100644
@@ -423,11 +423,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
 {
 	skb->dev = slave_dev;
 	skb->priority = 1;
-	if (unlikely(netpoll_tx_running(slave_dev))) {
-		slave_dev->priv_flags |= IFF_IN_NETPOLL;
+	if (unlikely(netpoll_tx_running(slave_dev)))
 		bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
-		slave_dev->priv_flags &= ~IFF_IN_NETPOLL;
-	} else
+	else
 		dev_queue_xmit(skb);
 
 	return 0;
drivers/net/bonding/bonding.h +1 -1
index 0a3e00b..a401b8d 100644
@@ -133,7 +133,7 @@ static inline void unblock_netpoll_tx(void)
 
 static inline int is_netpoll_tx_blocked(struct net_device *dev)
 {
-	if (unlikely(dev->priv_flags & IFF_IN_NETPOLL))
+	if (unlikely(netpoll_tx_running(dev)))
 		return atomic_read(&netpoll_block_tx);
 	return 0;
 }
include/linux/if.h +4 -5
index 1239599..3bc63e6 100644
@@ -71,11 +71,10 @@
 					 * release skb->dst
 					 */
 #define IFF_DONT_BRIDGE 0x800		/* disallow bridging this ether dev */
-#define IFF_IN_NETPOLL	0x1000		/* whether we are processing netpoll */
-#define IFF_DISABLE_NETPOLL	0x2000	/* disable netpoll at run-time */
-#define IFF_MACVLAN_PORT	0x4000	/* device used as macvlan port */
-#define IFF_BRIDGE_PORT	0x8000		/* device used as bridge port */
-#define IFF_OVS_DATAPATH	0x10000	/* device used as Open vSwitch
+#define IFF_DISABLE_NETPOLL	0x1000	/* disable netpoll at run-time */
+#define IFF_MACVLAN_PORT	0x2000	/* device used as macvlan port */
+#define IFF_BRIDGE_PORT	0x4000		/* device used as bridge port */
+#define IFF_OVS_DATAPATH	0x8000	/* device used as Open vSwitch
 					 * datapath port */
 
 #define IF_GET_IFACE	0x0001		/* for querying only */
net/core/netpoll.c +0 -2
index 02dc2cb..f68e694 100644
@@ -313,9 +313,7 @@ void netpoll_send_skb_on_dev(struct netpoll *np, struct sk_buff *skb,
 		     tries > 0; --tries) {
 			if (__netif_tx_trylock(txq)) {
 				if (!netif_tx_queue_stopped(txq)) {
-					dev->priv_flags |= IFF_IN_NETPOLL;
 					status = ops->ndo_start_xmit(skb, dev);
-					dev->priv_flags &= ~IFF_IN_NETPOLL;
 					if (status == NETDEV_TX_OK)
 						txq_trans_update(txq);
 				}
Signature · 1 line
1.7.1
maintainer Applied
#2 lore raw

Re: [V4 PATCH 2/3] netpoll: remove IFF_IN_NETPOLL flag

From: Amerigo Wang <[email protected]>
Date: Fri, 18 Feb 2011 17:43:33 +0800

Quoted 6 lines

V4: rebase to net-next-2.6

This patch removes the flag IFF_IN_NETPOLL, we don't need it any more since we have netpoll_tx_running() now.

Signed-off-by WANG Cong <[email protected]>

Applied.

author
#3 lore raw

PATCH v4 3/3 bond: service netpoll arp queue on master device

In reply to Amerigo Wang

Neil pointed out that we can't send ARP reply on behalf of slaves, we need to move the arp queue to their bond device.

Signed-off-by WANG Cong <[email protected]>
Cc Neil Horman <[email protected]>
---
 net/core/netpoll.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
net/core/netpoll.c +9 -0
index f68e694..013e04a 100644
@@ -193,6 +193,15 @@ void netpoll_poll_dev(struct net_device *dev)
 
 	poll_napi(dev);
 
+	if (dev->priv_flags & IFF_SLAVE) {
+		if (dev->npinfo) {
+			struct net_device *bond_dev = dev->master;
+			struct sk_buff *skb;
+			while ((skb = skb_dequeue(&dev->npinfo->arp_tx)))
+				skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+		}
+	}
+
 	service_arp_queue(dev->npinfo);
 
 	zap_completion_queue();
Signature · 1 line
1.7.1
author
#4 lore raw

Re: [V4 PATCH 3/3] bond: service netpoll arp queue on master device

Neil, ping. :)

Quoted 30 lines · Amerigo Wang

Neil pointed out that we can't send ARP reply on behalf of slaves, we need to move the arp queue to their bond device.

Signed-off-by WANG Cong<[email protected]>
Cc Neil Horman<[email protected]>
---
  net/core/netpoll.c |    9 +++++++++
  1 files changed, 9 insertions(+), 0 deletions(-)
net/core/netpoll.c +9 -0
index f68e694..013e04a 100644
@@ -193,6 +193,15 @@ void netpoll_poll_dev(struct net_device *dev)
 
  	poll_napi(dev);
 
+	if (dev->priv_flags&  IFF_SLAVE) {
+		if (dev->npinfo) {
+			struct net_device *bond_dev = dev->master;
+			struct sk_buff *skb;
+			while ((skb = skb_dequeue(&dev->npinfo->arp_tx)))
+				skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+		}
+	}
+
  	service_arp_queue(dev->npinfo);
 
  	zap_completion_queue();
Signature · 1 line
我认识的人越多，我就越喜欢狗。
Acked-by
#5 lore raw

Re: [V4 PATCH 3/3] bond: service netpoll arp queue on master device

Quoted 32 lines · Cong Wang

Neil, ping. :)

Quoted 29 lines · Amerigo Wang

Neil pointed out that we can't send ARP reply on behalf of slaves, we need to move the arp queue to their bond device.

Signed-off-by WANG Cong<[email protected]>
Cc Neil Horman<[email protected]>
---
 net/core/netpoll.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
net/core/netpoll.c +9 -0
index f68e694..013e04a 100644
@@ -193,6 +193,15 @@ void netpoll_poll_dev(struct net_device *dev)
 
 	poll_napi(dev);
 
+	if (dev->priv_flags&  IFF_SLAVE) {
+		if (dev->npinfo) {
+			struct net_device *bond_dev = dev->master;
+			struct sk_buff *skb;
+			while ((skb = skb_dequeue(&dev->npinfo->arp_tx)))
+				skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+		}
+	}
+
 	service_arp_queue(dev->npinfo);
Quoted 5 lines
Quoted 1 line
net/core/netpoll.c +0 -0
 	zap_completion_queue();
Signature · 1 line
我认识的人越多，我就越喜欢狗。

seems reasonable, thanks!

Acked-by Neil Horman <[email protected]>
author
#8 lore raw

Re: [V4 PATCH 3/3] bond: service netpoll arp queue on master device

Quoted 9 lines · David Miller

From: Amerigo Wang<[email protected]>
Date: Fri, 18 Feb 2011 17:43:34 +0800

Quoted 4 lines

Neil pointed out that we can't send ARP reply on behalf of slaves, we need to move the arp queue to their bond device.

Signed-off-by WANG Cong<[email protected]>

Applied.

Oops! Just found that this one I sent was not a refreshed patch. Please discard this one, and use the one below, that is the correct one in my git tree and the one that I tested.

Sorry for this.

----

Signed-off-by WANG Cong <[email protected]>
net/core/netpoll.c +11 -0
index f68e694..06be243 100644
@@ -193,6 +193,17 @@ void netpoll_poll_dev(struct net_device *dev)
 
 	poll_napi(dev);
 
+	if (dev->priv_flags & IFF_SLAVE) {
+		if (dev->npinfo) {
+			struct net_device *bond_dev = dev->master;
+			struct sk_buff *skb;
+			while ((skb = skb_dequeue(&dev->npinfo->arp_tx))) {
+				skb->dev = bond_dev;
+				skb_queue_tail(&bond_dev->npinfo->arp_tx, skb);
+			}
+		}
+	}
+
 	service_arp_queue(dev->npinfo);
 
 	zap_completion_queue();
maintainer
#9 lore raw

Re: [V4 PATCH 3/3] bond: service netpoll arp queue on master device

From: Cong Wang <[email protected]>
Date: Mon, 28 Feb 2011 11:32:18 +0800

Quoted 7 lines
Quoted 6 lines · David Miller

From: Amerigo Wang<[email protected]>
Date: Fri, 18 Feb 2011 17:43:34 +0800

Quoted 3 lines

Neil pointed out that we can't send ARP reply on behalf of slaves, we need to move the arp queue to their bond device.

Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by WANG Cong<[email protected]>

Applied.

Oops! Just found that this one I sent was not a refreshed patch. Please discard this one, and use the one below, that is the correct one in my git tree and the one that I tested.

Done.