[rfc | patch 0/6] netpoll: add support for the bonding driver

[rfc | patch 0/6] netpoll: add support for the bonding driver

[Jeff Moyer]

Hi,

The following patch series provides netpoll support for the bonding driver.
The best way to describe the approach taken is to walk through how it
works.  We'll use netconsole as our example.

Netconsole registers a console, and so release_console_sem will trigger the
write_msg implementation in netconsole.ko.  This, in turn, calls
netpoll_send_udp.  netpoll_send_udp will then call netpoll_send_skb, which
then calls into the network drivers hard_start_xmit routine.  It is
important to note that netpoll_send_skb takes care to not call the
hard_start_xmit routine if netif_queue_stopped(dev) returns true.  In this
case, it instead calls netpoll_poll, to hopefully free up some tx
descriptors.  After this is done, and the queue is woken up, the
hard_start_xmit routine can and will be called.

For all network drivers up to this point, this was the end of the line.
Netpoll would no longer have to be concerned with the skb.  It will be
sent off to the network by the driver.  However, this is not the case with
the bonding driver.

In the case of the bonding driver, we have now called its hard_start_xmit
routine, which can be one of many, depending on the configured bonging_mode
(specified at module load time).  For all of my testing, I have used
dynamic link aggregation (802.3ad).  So, in this case, we will call
bond_3ad_xmit_xor.  In all cases, the hard_start_xmit routine selects the
proper slave device over which to send the packet, and fills in the
skb->dev pointer to point at that device.  Then, they all send the skb off
to bond_dev_queue_xmit.

bond_dev_queue_xmit will then send the skb off to the networking layer via
dev_queue_xmit.  dev_queue_xmit is then responsible for calling the
underlying network device's hard_start_xmit routine.  Note that
dev_queue_xmit cannot be called with interrupts disabled.

And so, we bring our discussion back to netpoll.  In the case of netpoll,
we cannot have the bonding driver call into dev_queue_xmit.  For
netconsole, we have disabled irqs before calling netpoll_send_skb (and we
may even be called from interrupt context, in the case of sysrq-X).  This
is not the only problem with allowing the bonding driver to call
dev_queue_xmit.  We also have to deal with the fact that the underlying
network device may not be ready to send.  As in the case described above,
we may be short on tx descriptors.  In such a case, we need to be able to
call the driver's poll_controller routine to clear them out.  And so, we
have to modify bond_dev_queue_xmit to call *back* into the netpoll code.

The call chain will look something like this:

printk
  release_console_sem
    call_console_drivers
      write_msg
        netpoll_send_udp <----- skb->dev points to bond0
          netpoll_send_skb
            bond_3ad_xmit_xor
              bond_dev_queue_xmit
                netpoll_send_skb <------ skb->dev now points to real_dev

                  (may have to call dev->poll_controller, and perform a
                   napi poll here)

                  dev->hard_start_xmit

I wrote a test module to exercise the receive code path, as well.  The
module registers a struct netpoll with an rx_hook.  It then responds to
each packet it receives with a "PONG".  For this case, our call path now
looks as follows:

net_rx
  dev->poll
    netif_receive_skb
      netpoll_rx
        __netpoll_rx
          module_rx_routine
            netpoll_send_udp
              netpoll_send_skb <-- skb->dev points at bond0
                bond_3ad_xmit_xor
                  bond_dev_queue_xmit
                    netpoll_send_skb <-- skb->dev points at real_dev
                    
And here it gets tricky.  If we are sending out over the same interface
on which the packet arrived, then we will have to either queue the packet,
or drop it.  This functionality is already implemented in netpoll, so if we
filled in our queue routine to point at netpoll_queue, then at this point
we will call that via the ->drop pointer in our netpoll structure.  So,
continuing along, we have:

                      netpoll_queue
                        schedule_work

and we are done.  If the packet was schedule to head out a different
device, then we would continue with the send path:

                      dev->hard_start_xmit


New netpoll function implemented by the network drivers:

net_device->netpoll_setup
  This is required, since the bonding device has to walk through each slave
  and point its slave_dev->npinfo at the npinfo for the master device.  The
  reason for this is so that when we're doing the napi polling, we can set
  the rx_flags appropriately.

net_device->netpoll_start_xmit
  This routine is required since, otherwise, there is no way to intercept
  packets bound for interfaces that are not ready for them.  Of course, it
  requires further logic in the bonding driver to then call into the
  netpoll_send_skb routine (which is a new export).

Note that neither of these pointers has to be filled in by the driver.
These functions should only be implemented where needed, and to date, that
is only in the bonding driver.

Newly exported are:

netpoll_send_skb
  This is exported so that the bonding driver can queue a packet to be sent
  via the real ethernet device it has chosen.

netpoll_poll_dev
  This is a new routine that was created and exported so that the
  poll_controller implementation in the bonding driver could poll each of
  the underlying real devices without duplicating all of the logic that
  exists internally to netpoll already.


To test this, as I mentioned above, I wrote a simple module which, upon
receipt of any packet, sends out a packet with the message "PONG".  I fired
up netcat to send test packets, and receive the responses.  I also loaded
the netconsole module for the very same interface, bond0, and issue a
series of sysrq-X's, both via sysrq-trigger and via the keyboard.  I did
this while simultaneously testing the PING server on an SMP machine.  As
things stand, it is very stable in my environment.

And so, the patch set follows.  Any and all comments are appreciated.

Thanks,

Jeff

Re: [rfc | patch 0/6] netpoll: add support for the bonding driver

[Jeff Moyer]

Initialize npinfo->rx_flags.  The way it stands now, this will have random
garbage, and so will incur a locking penalty even when an rx_hook isn't
registered and we are not active in the netpoll polling code.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---

--- linux-2.6.12/net/core/netpoll.c.orig	2005-07-01 14:02:56.039174635 -0400
+++ linux-2.6.12/net/core/netpoll.c	2005-07-01 14:03:16.688739508 -0400
@@ -639,6 +639,7 @@ int netpoll_setup(struct netpoll *np)
 		if (!npinfo)
 			goto release;
 
+		npinfo->rx_flags = 0;
 		npinfo->rx_np = NULL;
 		npinfo->poll_lock = SPIN_LOCK_UNLOCKED;
 		npinfo->poll_owner = -1;

Re: [rfc | patch 0/6] netpoll: add support for the bonding driver

[Jeff Moyer]

Move the poll_lock and poll_owner to the struct net_device.  This change is
important for the bonding driver support, as multiple real devices will
point at the same netpoll_info.  In such cases, we would be artificially
limiting ourselves to calling the polling routine of only one member of a
bond at a time.

To understand how this benefits the bonding driver, let's consider a
netpoll client that sends packets in response to receiving packets.  Let's
assume a packet comes in on eth0, and the response is to be delivered over
eth1.  If the poll_lock lived in the npinfo (and hence, there was only one
per bond), then we would have to queue the packet.  With this patch in
place, we can send the outbound packet immediately.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---


--- linux-2.6.12/net/core/netpoll.c.orig	2005-07-01 14:52:11.932101811 -0400
+++ linux-2.6.12/net/core/netpoll.c	2005-07-01 14:53:59.322183590 -0400
@@ -131,19 +131,20 @@ static int checksum_udp(struct sk_buff *
 static void poll_napi(struct netpoll *np)
 {
 	struct netpoll_info *npinfo = np->dev->npinfo;
+	struct net_device *dev = np->dev;
 	int budget = 16;
 
-	if (test_bit(__LINK_STATE_RX_SCHED, &np->dev->state) &&
-	    npinfo->poll_owner != smp_processor_id() &&
-	    spin_trylock(&npinfo->poll_lock)) {
+	if (test_bit(__LINK_STATE_RX_SCHED, &dev->state) &&
+	    dev->poll_owner != smp_processor_id() &&
+	    spin_trylock(&dev->poll_lock)) {
 		npinfo->rx_flags |= NETPOLL_RX_DROP;
 		atomic_inc(&trapped);
 
-		np->dev->poll(np->dev, &budget);
+		dev->poll(dev, &budget);
 
 		atomic_dec(&trapped);
 		npinfo->rx_flags &= ~NETPOLL_RX_DROP;
-		spin_unlock(&npinfo->poll_lock);
+		spin_unlock(&dev->poll_lock);
 	}
 }
 
@@ -246,7 +247,6 @@ repeat:
 static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 {
 	int status;
-	struct netpoll_info *npinfo;
 
 repeat:
 	if(!np || !np->dev || !netif_running(np->dev)) {
@@ -255,8 +255,7 @@ repeat:
 	}
 
 	/* avoid recursion */
-	npinfo = np->dev->npinfo;
-	if (npinfo->poll_owner == smp_processor_id() ||
+	if (dev->poll_owner == smp_processor_id() ||
 	    np->dev->xmit_lock_owner == smp_processor_id()) {
 		if (np->drop)
 			np->drop(skb);
@@ -641,8 +640,6 @@ int netpoll_setup(struct netpoll *np)
 
 		npinfo->rx_flags = 0;
 		npinfo->rx_np = NULL;
-		npinfo->poll_lock = SPIN_LOCK_UNLOCKED;
-		npinfo->poll_owner = -1;
 		npinfo->rx_lock = SPIN_LOCK_UNLOCKED;
 	} else
 		npinfo = ndev->npinfo;
--- linux-2.6.12/net/core/dev.c.orig	2005-07-01 14:52:08.613655521 -0400
+++ linux-2.6.12/net/core/dev.c	2005-07-01 14:52:38.495669512 -0400
@@ -3069,6 +3069,10 @@ struct net_device *alloc_netdev(int size
 
 	setup(dev);
 	strcpy(dev->name, name);
+#ifdef CONFIG_NETPOLL
+	dev->poll_owner = -1;
+	dev->poll_lock = SPIN_LOCK_UNLOCKED;
+#endif
 	return dev;
 }
 EXPORT_SYMBOL(alloc_netdev);
--- linux-2.6.12/include/linux/netpoll.h.orig	2005-07-01 14:52:16.016420312 -0400
+++ linux-2.6.12/include/linux/netpoll.h	2005-07-01 14:52:38.495669512 -0400
@@ -24,8 +24,6 @@ struct netpoll {
 };
 
 struct netpoll_info {
-	spinlock_t poll_lock;
-	int poll_owner;
 	int rx_flags;
 	spinlock_t rx_lock;
 	struct netpoll *rx_np; /* netpoll that registered an rx_hook */
@@ -63,16 +61,16 @@ static inline int netpoll_rx(struct sk_b
 static inline void netpoll_poll_lock(struct net_device *dev)
 {
 	if (dev->npinfo) {
-		spin_lock(&dev->npinfo->poll_lock);
-		dev->npinfo->poll_owner = smp_processor_id();
+		spin_lock(&dev->poll_lock);
+		dev->poll_owner = smp_processor_id();
 	}
 }
 
 static inline void netpoll_poll_unlock(struct net_device *dev)
 {
 	if (dev->npinfo) {
-		dev->npinfo->poll_owner = -1;
-		spin_unlock(&dev->npinfo->poll_lock);
+		dev->poll_owner = -1;
+		spin_unlock(&dev->poll_lock);
 	}
 }
 
--- linux-2.6.12/include/linux/netdevice.h.orig	2005-07-01 14:52:22.666310731 -0400
+++ linux-2.6.12/include/linux/netdevice.h	2005-07-01 14:52:38.496669345 -0400
@@ -469,6 +469,8 @@ struct net_device
 	int			(*neigh_setup)(struct net_device *dev, struct neigh_parms *);
 #ifdef CONFIG_NETPOLL
 	struct netpoll_info	*npinfo;
+	spinlock_t		poll_lock;
+	int			poll_owner;
 #endif
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	void                    (*poll_controller)(struct net_device *dev);

[rfc | patch 3/6] netpoll: change poll_napi to take a net_device

[Jeff Moyer]

(sorry for munging the subject of the prior two patches)

The poll_napi routine is not specific to a netpoll.  It really should be
able to be called with a struct net_device.  Changing the routine to take a
struct net_device makes it easier to support the bonding driver, since we
will be implementing a netpoll_poll_dev routine, that will not have a
struct netpoll associated with it.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---

--- linux-2.6.12/net/core/netpoll.c.orig	2005-07-01 14:12:33.986337259 -0400
+++ linux-2.6.12/net/core/netpoll.c	2005-07-01 14:20:45.848186887 -0400
@@ -128,10 +128,9 @@ static int checksum_udp(struct sk_buff *
  * network adapter, forcing superfluous retries and possibly timeouts.
  * Thus, we set our budget to greater than 1.
  */
-static void poll_napi(struct netpoll *np)
+static void poll_napi(struct net_device *dev)
 {
-	struct netpoll_info *npinfo = np->dev->npinfo;
-	struct net_device *dev = np->dev;
+	struct netpoll_info *npinfo = dev->npinfo;
 	int budget = 16;
 
 	if (test_bit(__LINK_STATE_RX_SCHED, &dev->state) &&
@@ -156,7 +155,7 @@ void netpoll_poll(struct netpoll *np)
 	/* Process pending work on NIC */
 	np->dev->poll_controller(np->dev);
 	if (np->dev->poll)
-		poll_napi(np);
+		poll_napi(np->dev);
 
 	zap_completion_queue();
 }

[rfc | patch 4/6] netpoll: add netpoll hooks to support the bonding driver

[Jeff Moyer]

Implement hooks in the netpoll code to support the bonding driver (and
hopefully other virtual device drivers, as well).  The key additions here
are netpoll_poll_dev(new) and netpoll_send_skb(newly exported).

netpoll_send_skb
  This is exported so that the bonding driver can queue a packet to be sent
  via the real ethernet device it has chosen.

netpoll_poll_dev
  This is a new routine that was created and exported so that the
  poll_controller implementation in the bonding driver could poll each of
  the underlying real devices without duplicating all of the logic that
  exists internally to netpoll already.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---

--- linux-2.6.12/net/core/netpoll.c.orig	2005-07-01 15:02:05.503951082 -0400
+++ linux-2.6.12/net/core/netpoll.c	2005-07-01 15:02:36.718705979 -0400
@@ -147,19 +147,27 @@ static void poll_napi(struct net_device 
 	}
 }
 
-void netpoll_poll(struct netpoll *np)
+void netpoll_poll_dev(struct net_device *dev)
 {
-	if(!np->dev || !netif_running(np->dev) || !np->dev->poll_controller)
+	if(!netif_running(dev) || !dev->poll_controller)
 		return;
 
 	/* Process pending work on NIC */
-	np->dev->poll_controller(np->dev);
-	if (np->dev->poll)
-		poll_napi(np->dev);
+	dev->poll_controller(dev);
+	if (dev->poll)
+		poll_napi(dev);
 
 	zap_completion_queue();
 }
 
+void netpoll_poll(struct netpoll *np)
+{
+	if(!np->dev)
+		return;
+
+	netpoll_poll_dev(np->dev);
+}
+
 static void refill_skbs(void)
 {
 	struct sk_buff *skb;
@@ -243,48 +251,59 @@ repeat:
 	return skb;
 }
 
-static void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
+/*
+ *  This function can be called from virtual device drivers, such as the
+ *  bonding driver.  In this case, skb->dev is filled in with the real
+ *  device over which the packet needs to be sent.  For this reason, we use
+ *  the skb->dev instead of np->dev.  Essentially, np->dev can be, for
+ *  example, bond0, while we actually need to send the packet out over eth0.
+ */
+void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb)
 {
 	int status;
+	struct net_device *dev = skb->dev;
 
 repeat:
-	if(!np || !np->dev || !netif_running(np->dev)) {
+	if(!np || !dev || !netif_running(dev)) {
 		__kfree_skb(skb);
 		return;
 	}
 
 	/* avoid recursion */
 	if (dev->poll_owner == smp_processor_id() ||
-	    np->dev->xmit_lock_owner == smp_processor_id()) {
-		if (np->drop)
+	    dev->xmit_lock_owner == smp_processor_id()) {
+		if (np && np->drop)
 			np->drop(skb);
 		else
 			__kfree_skb(skb);
 		return;
 	}
 
-	spin_lock(&np->dev->xmit_lock);
-	np->dev->xmit_lock_owner = smp_processor_id();
+	spin_lock(&dev->xmit_lock);
+	dev->xmit_lock_owner = smp_processor_id();
 
 	/*
 	 * network drivers do not expect to be called if the queue is
 	 * stopped.
 	 */
-	if (netif_queue_stopped(np->dev)) {
-		np->dev->xmit_lock_owner = -1;
-		spin_unlock(&np->dev->xmit_lock);
+	if (netif_queue_stopped(dev)) {
+		dev->xmit_lock_owner = -1;
+		spin_unlock(&dev->xmit_lock);
 
-		netpoll_poll(np);
+		netpoll_poll_dev(dev);
 		goto repeat;
 	}
 
-	status = np->dev->hard_start_xmit(skb, np->dev);
-	np->dev->xmit_lock_owner = -1;
-	spin_unlock(&np->dev->xmit_lock);
+	if (dev->netpoll_start_xmit)
+		status = dev->netpoll_start_xmit(np, skb, dev);
+	else
+		status = dev->hard_start_xmit(skb, dev);
+	dev->xmit_lock_owner = -1;
+	spin_unlock(&dev->xmit_lock);
 
 	/* transmit busy */
 	if(status) {
-		netpoll_poll(np);
+		netpoll_poll_dev(dev);
 		goto repeat;
 	}
 }
@@ -715,6 +734,11 @@ int netpoll_setup(struct netpoll *np)
 		npinfo->rx_np = np;
 		spin_unlock_irqrestore(&npinfo->rx_lock, flags);
 	}
+
+	/* Call the device specific netpoll initialization routine. */
+	if (ndev->netpoll_setup)
+		ndev->netpoll_setup(ndev, npinfo);
+
 	/* last thing to do is link it to the net device structure */
 	ndev->npinfo = npinfo;
 
@@ -766,5 +790,7 @@ EXPORT_SYMBOL(netpoll_parse_options);
 EXPORT_SYMBOL(netpoll_setup);
 EXPORT_SYMBOL(netpoll_cleanup);
 EXPORT_SYMBOL(netpoll_send_udp);
+EXPORT_SYMBOL(netpoll_send_skb);
 EXPORT_SYMBOL(netpoll_poll);
+EXPORT_SYMBOL(netpoll_poll_dev);
 EXPORT_SYMBOL(netpoll_queue);
--- linux-2.6.12/net/core/dev.c.orig	2005-07-01 14:59:50.161654219 -0400
+++ linux-2.6.12/net/core/dev.c	2005-07-01 15:00:23.437103656 -0400
@@ -1655,15 +1655,15 @@ int netif_receive_skb(struct sk_buff *sk
 	int ret = NET_RX_DROP;
 	unsigned short type;
 
-	/* if we've gotten here through NAPI, check netpoll */
-	if (skb->dev->poll && netpoll_rx(skb))
+	skb_bond(skb);
+
+	/* if there is a netpoll client registered, check netpoll */
+	if (skb->dev->npinfo && netpoll_rx(skb))
 		return NET_RX_DROP;
 
 	if (!skb->stamp.tv_sec)
 		net_timestamp(&skb->stamp);
 
-	skb_bond(skb);
-
 	__get_cpu_var(netdev_rx_stat).total++;
 
 	skb->h.raw = skb->nh.raw = skb->data;
--- linux-2.6.12/include/linux/netpoll.h.orig	2005-07-01 14:59:56.135657708 -0400
+++ linux-2.6.12/include/linux/netpoll.h	2005-07-01 15:00:23.437103656 -0400
@@ -30,7 +30,9 @@ struct netpoll_info {
 };
 
 void netpoll_poll(struct netpoll *np);
+void netpoll_poll_dev(struct net_device *dev);
 void netpoll_send_udp(struct netpoll *np, const char *msg, int len);
+void netpoll_send_skb(struct netpoll *np, struct sk_buff *skb);
 int netpoll_parse_options(struct netpoll *np, char *opt);
 int netpoll_setup(struct netpoll *np);
 int netpoll_trap(void);
--- linux-2.6.12/include/linux/netdevice.h.orig	2005-07-01 14:59:59.835040624 -0400
+++ linux-2.6.12/include/linux/netdevice.h	2005-07-01 15:00:23.438103489 -0400
@@ -41,6 +41,7 @@
 struct divert_blk;
 struct vlan_group;
 struct ethtool_ops;
+struct netpoll;
 struct netpoll_info;
 					/* source back-compat hooks */
 #define SET_ETHTOOL_OPS(netdev,ops) \
@@ -473,7 +474,12 @@ struct net_device
 	int			poll_owner;
 #endif
 #ifdef CONFIG_NET_POLL_CONTROLLER
+	void			(*netpoll_setup)(struct net_device *dev,
+						 struct netpoll_info *npinfo);
 	void                    (*poll_controller)(struct net_device *dev);
+	int			(*netpoll_start_xmit)(struct netpoll *np,
+						      struct sk_buff *skb,
+						      struct net_device *dev);
 #endif
 
 	/* bridge stuff */

[rfc | patch 5/6] netpoll: modify bonding driver to support netpoll

[Jeff Moyer]

Implement netpoll hooks in the bonding driver.  We register the following
netpoll specific routines:

bond_netpoll_setup
  This routine associates the struct netpoll_info of the master device with
  each of the slave devices.

bond_xmit_netpoll
  This routine sets bonding->netpoll to the struct netpoll provided by
  netpoll_send_skb.  This pointer is then passed along to netpoll_send_skb
  in bond_dev_queue_xmit, when we are ready to send the skb out over the
  real network device.

bond_poll_controller
  This routine calls netpoll_poll_dev for each of the slaves in the bond.

When a new slave is added to the bond, if it does not support netpoll, then
netpoll is disabled for the bond.  When a slave is released, that slave's
npinfo pointer is cleared.

I have tested this code extensively, and it works well in my environment.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---

--- linux-2.6.12/drivers/net/bonding/bond_main.c.orig	2005-06-29 14:28:08.000000000 -0400
+++ linux-2.6.12/drivers/net/bonding/bond_main.c	2005-06-30 19:33:50.641009622 -0400
@@ -821,8 +821,12 @@ int bond_dev_queue_xmit(struct bonding *
 	}
 
 	skb->priority = 1;
-	dev_queue_xmit(skb);
-
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	if (bond->netpoll)
+		netpoll_send_skb(bond->netpoll, skb);
+	else
+#endif
+		dev_queue_xmit(skb);
 	return 0;
 }
 
@@ -1567,6 +1571,45 @@ static void bond_detach_slave(struct bon
 	bond->slave_cnt--;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+static int slaves_support_netpoll(struct net_device *bond_dev)
+{
+	struct bonding *bond = bond_dev->priv;
+	struct slave *slave;
+	int i;
+
+	bond_for_each_slave(bond, slave, i) {
+		if (!slave->dev->poll_controller)
+			return 0;
+	}
+
+	return 1;
+}
+
+static void bond_poll_controller(struct net_device *bond_dev)
+{
+	struct bonding *bond = bond_dev->priv;
+	struct slave *slave;
+	int i;
+
+	bond_for_each_slave(bond, slave, i) {
+		if (slave->dev->poll_controller)
+			netpoll_poll_dev(slave->dev);
+	}
+}
+
+static void bond_netpoll_setup(struct net_device *bond_dev,
+			       struct netpoll_info *npinfo)
+{
+	struct bonding *bond = bond_dev->priv;
+	struct slave *slave;
+	int i;
+
+	bond_for_each_slave(bond, slave, i)
+		slave->dev->npinfo = npinfo;
+}
+#endif
+
 /*---------------------------------- IOCTL ----------------------------------*/
 
 static int bond_sethwaddr(struct net_device *bond_dev, struct net_device *slave_dev)
@@ -1969,6 +2012,17 @@ static int bond_enslave(struct net_devic
 	       new_slave->state == BOND_STATE_ACTIVE ? "n active" : " backup",
 	       new_slave->link != BOND_LINK_DOWN ? "n up" : " down");
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	if (slaves_support_netpoll(bond_dev)) {
+		bond_dev->poll_controller = bond_poll_controller;
+		slave_dev->npinfo = bond_dev->npinfo;
+	} else if (bond_dev->poll_controller) {
+		bond_dev->poll_controller = NULL;
+		printk("New slave device %s does not support netpoll.\n",
+			slave_dev->name);
+		printk("netpoll disabled for %s.\n", bond_dev->name);
+	}
+#endif
 	/* enslave is successful */
 	return 0;
 
@@ -2173,6 +2227,9 @@ static int bond_release(struct net_devic
 		slave_dev->flags &= ~IFF_NOARP;
 	}
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	slave_dev->npinfo = NULL;
+#endif
 	kfree(slave);
 
 	return 0;  /* deletion OK */
@@ -4203,6 +4260,21 @@ out:
 	return 0;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+int bond_xmit_netpoll(struct netpoll *np, struct sk_buff *skb,
+		      struct net_device *bond_dev)
+{
+	struct bonding *bond = bond_dev->priv;
+	int ret;
+
+	bond->netpoll = np;
+	ret = bond_dev->hard_start_xmit(skb, bond_dev);
+	bond->netpoll = NULL;
+
+	return ret;
+}
+#endif
+
 /*------------------------- Device initialization ---------------------------*/
 
 /*
@@ -4277,6 +4349,10 @@ static int __init bond_init(struct net_d
 
 	bond_dev->destructor = free_netdev;
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	bond_dev->netpoll_setup = bond_netpoll_setup;
+	bond_dev->netpoll_start_xmit = bond_xmit_netpoll;
+#endif
 	/* Initialize the device options */
 	bond_dev->tx_queue_len = 0;
 	bond_dev->flags |= IFF_MASTER|IFF_MULTICAST;
--- linux-2.6.12/drivers/net/bonding/bonding.h.orig	2005-06-29 14:29:40.000000000 -0400
+++ linux-2.6.12/drivers/net/bonding/bonding.h	2005-06-30 19:02:54.000000000 -0400
@@ -33,6 +33,7 @@
 #include <linux/timer.h>
 #include <linux/proc_fs.h>
 #include <linux/if_bonding.h>
+#include <linux/netpoll.h>
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
@@ -203,6 +204,9 @@ struct bonding {
 	struct   bond_params params;
 	struct   list_head vlan_list;
 	struct   vlan_group *vlgrp;
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	struct   netpoll *netpoll;
+#endif
 };
 
 /**

[rfc | patch 6/6] netpoll: fix deadlock in arp_reply

[Jeff Moyer]

This fixes an obvious deadlock in the netpoll code.  netpoll_rx takes the
npinfo->rx_lock.  netpoll_rx is also the only caller of arp_reply (through
__netpoll_rx).  As such, it is not necessary to take this lock.

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

--- linux-2.6.12/net/core/netpoll.c.orig	2005-07-01 17:55:24.313339068 -0400
+++ linux-2.6.12/net/core/netpoll.c	2005-07-01 17:55:57.354863472 -0400
@@ -370,11 +370,8 @@ static void arp_reply(struct sk_buff *sk
 	struct sk_buff *send_skb;
 	struct netpoll *np = NULL;
 
-	spin_lock_irqsave(&npinfo->rx_lock, flags);
 	if (npinfo->rx_np && npinfo->rx_np->dev == skb->dev)
 		np = npinfo->rx_np;
-	spin_unlock_irqrestore(&npinfo->rx_lock, flags);
-
 	if (!np)
 		return;
 

Re: [rfc | patch 0/6] netpoll: add support for the bonding driver

[Matt Mackall]

On Fri, Jul 01, 2005 at 07:05:54PM -0400, Jeff Moyer wrote:
> New netpoll function implemented by the network drivers:
> 
> net_device->netpoll_setup
>   This is required, since the bonding device has to walk through each slave
>   and point its slave_dev->npinfo at the npinfo for the master device.  The
>   reason for this is so that when we're doing the napi polling, we can set
>   the rx_flags appropriately.
> 
> net_device->netpoll_start_xmit
>   This routine is required since, otherwise, there is no way to intercept
>   packets bound for interfaces that are not ready for them.  Of course, it
>   requires further logic in the bonding driver to then call into the
>   netpoll_send_skb routine (which is a new export).
> 
> Note that neither of these pointers has to be filled in by the driver.
> These functions should only be implemented where needed, and to date, that
> is only in the bonding driver.
> 
> Newly exported are:
> 
> netpoll_send_skb
>   This is exported so that the bonding driver can queue a packet to be sent
>   via the real ethernet device it has chosen.
> 
> netpoll_poll_dev
>   This is a new routine that was created and exported so that the
>   poll_controller implementation in the bonding driver could poll each of
>   the underlying real devices without duplicating all of the logic that
>   exists internally to netpoll already.
> 
> 
> To test this, as I mentioned above, I wrote a simple module which, upon
> receipt of any packet, sends out a packet with the message "PONG".  I fired
> up netcat to send test packets, and receive the responses.  I also loaded
> the netconsole module for the very same interface, bond0, and issue a
> series of sysrq-X's, both via sysrq-trigger and via the keyboard.  I did
> this while simultaneously testing the PING server on an SMP machine.  As
> things stand, it is very stable in my environment.
> 
> And so, the patch set follows.  Any and all comments are appreciated.

Patches 1, 3, and 6 are unrelated bugfixes and should just go in.

I don't like that we rely on queueing to process round trips for PONG.
Is this really unavoidable?

And I think the most controversial thing here is moving locks from npinfo
into the device.

Not really happy about how incestuous this makes the bonding driver
with netpoll. I'll try to think more about it over the weekend.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [rfc | patch 0/6] netpoll: add support for the bonding driver

[Jeff Moyer]

==> Regarding Re: [rfc | patch 0/6] netpoll: add support for the bonding driver; Matt Mackall <mpm@selenic.com> adds:

mpm> On Fri, Jul 01, 2005 at 07:05:54PM -0400, Jeff Moyer wrote:
>> New netpoll function implemented by the network drivers:
>> 
net_device-> netpoll_setup
>> This is required, since the bonding device has to walk through each
>> slave and point its slave_dev->npinfo at the npinfo for the master
>> device.  The reason for this is so that when we're doing the napi
>> polling, we can set the rx_flags appropriately.
>> 
net_device-> netpoll_start_xmit
>> This routine is required since, otherwise, there is no way to intercept
>> packets bound for interfaces that are not ready for them.  Of course, it
>> requires further logic in the bonding driver to then call into the
>> netpoll_send_skb routine (which is a new export).
>> 
>> Note that neither of these pointers has to be filled in by the driver.
>> These functions should only be implemented where needed, and to date,
>> that is only in the bonding driver.
>> 
>> Newly exported are:
>> 
>> netpoll_send_skb This is exported so that the bonding driver can queue a
>> packet to be sent via the real ethernet device it has chosen.
>> 
>> netpoll_poll_dev This is a new routine that was created and exported so
>> that the poll_controller implementation in the bonding driver could poll
>> each of the underlying real devices without duplicating all of the logic
>> that exists internally to netpoll already.
>> 
>> 
>> To test this, as I mentioned above, I wrote a simple module which, upon
>> receipt of any packet, sends out a packet with the message "PONG".  I
>> fired up netcat to send test packets, and receive the responses.  I also
>> loaded the netconsole module for the very same interface, bond0, and
>> issue a series of sysrq-X's, both via sysrq-trigger and via the
>> keyboard.  I did this while simultaneously testing the PING server on an
>> SMP machine.  As things stand, it is very stable in my environment.
>> 
>> And so, the patch set follows.  Any and all comments are appreciated.

mpm> Patches 1, 3, and 6 are unrelated bugfixes and should just go in.

Right.  Sorry I lumped them together.

mpm> I don't like that we rely on queueing to process round trips for PONG.
mpm> Is this really unavoidable?

That's how it works without these patches, too.

mpm> And I think the most controversial thing here is moving locks from
mpm> npinfo into the device.

Well, as I mentioned, that's a bit of an optimization, if you will.

mpm> Not really happy about how incestuous this makes the bonding driver
mpm> with netpoll. I'll try to think more about it over the weekend.

I'd be delighted if you came up with a better way to do things.  However, I
think you'll find that this is about as clean as it gets.

-Jeff