Re: [Patch] bonding: clean up netpoll code

Previous thread: [PATCH] ARM: module: fix 'mod' undeclared compile error by Axel Lin on Wednesday, December 1, 2010 - 12:21 am. (1 message)

Next thread: [PATCH v16 01/17] Add a new structure for skb buffer from external. by xiaohui.xin on Wednesday, December 1, 2010 - 1:08 am. (18 messages)
From: Amerigo Wang
Date: Wednesday, December 1, 2010 - 12:45 am

Against net-next-2.6.

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

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

It passes my basic testings.

Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: Neil Horman <nhorman@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Jiri Pirko <jpirko@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>


---
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0273ad0..0f04cd0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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>
@@ -450,13 +449,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
 
 	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;
-		slave_dev->priv_flags |= IFF_IN_NETPOLL;
-		netpoll_send_skb_on_dev(np, skb, slave_dev);
-		slave_dev->priv_flags &= ~IFF_IN_NETPOLL;
-	} else
+	if (unlikely(netpoll_tx_running(slave_dev)))
+		bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
+	else
 #endif
 		dev_queue_xmit(skb);
 
@@ -1310,63 +1305,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 ...
From: Eric Dumazet
Date: Wednesday, December 1, 2010 - 1:03 am

Sorry this NETPOLL patch is frightening...

Could you split it in several parts ?

The removal of IFF_IN_NETPOLL deserves a patch on its own, its not a
cleanup at all, if you ask me.

Thanks



--

From: Cong Wang
Date: Wednesday, December 1, 2010 - 1:26 am

Is this necessary?
It is just replacing checking IFF_IN_NETPOLL with netpoll_tx_running(),
you might need to take a look at the bridge code.
--

From: Herbert Xu
Date: Wednesday, December 1, 2010 - 2:06 am

I agree with Eric.  The patch does look pretty scary right now.

Each patch should do one thing and one thing only.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

From: Cong Wang
Date: Wednesday, December 1, 2010 - 4:15 am

Ok, I will separate that part out.

Thanks!
--

From: Stephen Hemminger
Date: Wednesday, December 1, 2010 - 9:14 am

On Wed, 1 Dec 2010 02:45:45 -0500

Split assignment and conditional into two lines


-- 
--

From: Cong Wang
Date: Wednesday, December 1, 2010 - 8:14 pm

Ok, will change this in the next update.

Thanks!
--

Previous thread: [PATCH] ARM: module: fix 'mod' undeclared compile error by Axel Lin on Wednesday, December 1, 2010 - 12:21 am. (1 message)

Next thread: [PATCH v16 01/17] Add a new structure for skb buffer from external. by xiaohui.xin on Wednesday, December 1, 2010 - 1:08 am. (18 messages)