ifb: add multi-queue support

ifb: add multi-queue support

Add multi-queue support, and one kernel thread is created for per queue. It can used to emulate multi-queue NIC in software, and distribute work among CPUs.
gentux linux # modprobe ifb numtxqs=2
gentux linux # ifconfig ifb0 up
gentux linux # pgrep ifb0
18508
18509
gentux linux # taskset -p 1 18508

pid 18508's current affinity mask: 3
pid 18508's new affinity mask: 1
gentux linux # taskset -p 2 18509
pid 18509's current affinity mask: 3
pid 18509's new affinity mask: 2
gentux linux # tc qdisc add dev br0 ingress
gentux linux # tc filter add dev br0 parent ffff: protocol ip basic action mirred egress redirect dev ifb0

Signed-off-by Changli Gao <[email protected]>

----
drivers/net/ifb.c | 309
++++++++++++++++++++++++++++++++----------------------

1 file changed, 186 insertions(+), 123 deletions(-)

drivers/net/ifb.c +7 -12

index 030913f..6e04188 100644
@@ -33,139 +33,101 @@
 #include <linux/etherdevice.h>
 #include <linux/init.h>
 #include <linux/moduleparam.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+#include <linux/kthread.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/ip.h>
 #include <net/pkt_sched.h>
 #include <net/net_namespace.h>
 
-#define TX_TIMEOUT  (2*HZ)
-
 #define TX_Q_LIMIT    32
+
 struct ifb_private {
-	struct tasklet_struct   ifb_tasklet;
-	int     tasklet_pending;
-	/* mostly debug stats leave in for now */
-	unsigned long   st_task_enter; /* tasklet entered */
-	unsigned long   st_txq_refl_try; /* transmit queue refill attempt */
-	unsigned long   st_rxq_enter; /* receive queue entered */
-	unsigned long   st_rx2tx_tran; /* receive to trasmit transfers */
-	unsigned long   st_rxq_notenter; /*receiveQ not entered, resched */
-	unsigned long   st_rx_frm_egr; /* received from egress path */
-	unsigned long   st_rx_frm_ing; /* received from ingress path */

drivers/net/ifb.c +7 -4

+/* Number of ifb devices to be set up by this module. */
 static int numifbs = 2;
+module_param(numifbs, int, 0444);
+MODULE_PARM_DESC(numifbs, "Number of ifb devices");
 
-static void ri_tasklet(unsigned long dev);
-static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev);
-static int ifb_open(struct net_device *dev);
-static int ifb_close(struct net_device *dev);
+/* Number of TX queues per ifb */
+static int numtxqs = 1;
+module_param(numtxqs, int, 0444);
+MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
 

drivers/net/ifb.c +18 -51

+	unsigned int num = dp - (struct ifb_private*)netdev_priv(dev);
+	struct netdev_queue *txq = netdev_get_tx_queue(dev, num);
 	struct sk_buff *skb;
-
-	txq = netdev_get_tx_queue(_dev, 0);
-	dp->st_task_enter++;
-	if ((skb = skb_peek(&dp->tq)) == NULL) {
-		dp->st_txq_refl_try++;
-		if (__netif_tx_trylock(txq)) {
-			dp->st_rxq_enter++;
-			while ((skb = skb_dequeue(&dp->rq)) != NULL) {
+	DEFINE_WAIT(wait);
+
+	while (1) {
+		/* move skb from rq to tq */
+		while (1) {
+			prepare_to_wait(&dp->wq, &wait, TASK_UNINTERRUPTIBLE);
+			while (!__netif_tx_trylock(txq))
+				yield();
+			while ((skb = skb_dequeue(&dp->rq)) != NULL)
 				skb_queue_tail(&dp->tq, skb);
-				dp->st_rx2tx_tran++;
-			}
+			if (netif_queue_stopped(dev))
+				netif_wake_queue(dev);
 			__netif_tx_unlock(txq);
-		} else {
-			/* reschedule */
-			dp->st_rxq_notenter++;
-			goto resched;
+			if (kthread_should_stop() || !skb_queue_empty(&dp->tq))
+				break;
+			schedule();
 		}
-	}
-
-	while ((skb = skb_dequeue(&dp->tq)) != NULL) {
-		u32 from = G_TC_FROM(skb->tc_verd);
-
-		skb->tc_verd = 0;
-		skb->tc_verd = SET_TC_NCLS(skb->tc_verd);
-		stats->tx_packets++;
-		stats->tx_bytes +=skb->len;
-
-		skb->dev = dev_get_by_index(&init_net, skb->iif);
-		if (!skb->dev) {
-			dev_kfree_skb(skb);
-			stats->tx_dropped++;
+		finish_wait(&dp->wq, &wait);
+		if (kthread_should_stop())
 			break;
-		}
-		dev_put(skb->dev);
-		skb->iif = _dev->ifindex;
-
-		if (from & AT_EGRESS) {
-			dp->st_rx_frm_egr++;
-			dev_queue_xmit(skb);
-		} else if (from & AT_INGRESS) {
-			dp->st_rx_frm_ing++;
-			skb_pull(skb, skb->dev->hard_header_len);
-			netif_rx(skb);
-		} else
-			BUG();
-	}
 
-	if (__netif_tx_trylock(txq)) {
-		dp->st_rxq_check++;
-		if ((skb = skb_peek(&dp->rq)) == NULL) {
-			dp->tasklet_pending = 0;
-			if (netif_queue_stopped(_dev))
-				netif_wake_queue(_dev);
-		} else {
-			dp->st_rxq_rsch++;
-			__netif_tx_unlock(txq);

drivers/net/ifb.c +13 -28

+				break;
+			}
+			dev_put(skb->dev);
+			skb->iif = dev->ifindex;
+	
+			if (from & AT_EGRESS) {
+				dev_queue_xmit(skb);
+			} else if (from & AT_INGRESS) {
+				skb_pull(skb, skb->dev->hard_header_len);
+				netif_rx_ni(skb);
+			} else
+				BUG();
 		}
-		__netif_tx_unlock(txq);
-	} else {
-resched:
-		dp->tasklet_pending = 1;
-		tasklet_schedule(&dp->ifb_tasklet);
 	}
 
-}
-
-static const struct net_device_ops ifb_netdev_ops = {
-	.ndo_open	= ifb_open,
-	.ndo_stop	= ifb_close,
-	.ndo_start_xmit	= ifb_xmit,
-	.ndo_validate_addr = eth_validate_addr,
-};
-
-static void ifb_setup(struct net_device *dev)
-{
-	/* Initialize the device structure. */
-	dev->destructor = free_netdev;
-	dev->netdev_ops = &ifb_netdev_ops;
-
-	/* Fill in device structure with ethernet-generic values. */
-	ether_setup(dev);
-	dev->tx_queue_len = TX_Q_LIMIT;
-
-	dev->flags |= IFF_NOARP;
-	dev->flags &= ~IFF_MULTICAST;
-	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
-	random_ether_addr(dev->dev_addr);
+	return 0;
 }
 
 static netdev_tx_t ifb_xmit(struct sk_buff *skb, struct net_device *dev)

Not sure how to solve this problem (several cpus can updates counter in //)

2009/11/10 Eric Dumazet <[email protected]>:

Quoted 2 lines

Not sure how to solve this problem (several cpus can updates counter in //)

Thanks, and follow your suggestions. I can maintain the counter per TX queue, and update it in a timer handler like ixgbe or implement our own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev), and update the counters when it gets called.

Quoted 7 lines · Changli Gao

2009/11/10 Eric Dumazet <[email protected]>:

Quoted 2 lines

Not sure how to solve this problem (several cpus can updates counter in //)

Thanks, and follow your suggestions. I can maintain the counter per TX queue, and update it in a timer handler like ixgbe or implement our own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev), and update the counters when it gets called.

Please no timer stuff :)

2009/11/10 Eric Dumazet <[email protected]>:

Quoted 8 lines

Quoted 7 lines · Changli Gao

2009/11/10 Eric Dumazet <[email protected]>:

Quoted 2 lines

Not sure how to solve this problem (several cpus can updates counter in //)

Thanks, and follow your suggestions. I can maintain the counter per TX queue, and update it in a timer handler like ixgbe or implement our own struct net_device_stats* (*ndo_get_stats)(struct net_device *dev), and update the counters when it gets called.

Quoted 1 line

Please no timer stuff :)

The whole ifb.c file is attached, please review and test it. Thanks!

/* Number of TX queues per ifb */
static int numtxqs = 1;
module_param(numtxqs, int, 0444); MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");

static int __init ifb_init_one(int index)

{
	struct net_device *dev_ifb;
	int err;

	dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d",
				  ifb_setup, numtxqs);

Quoted 3 lines · Changli Gao

2009/11/10 Eric Dumazet <[email protected]>:

The whole ifb.c file is attached, please review and test it. Thanks!

Quoted 4 lines

/* Number of TX queues per ifb */
static int numtxqs = 1;
module_param(numtxqs, int, 0444); MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");

Module parameters suck as a configuration API. The existing numifbs option exists purely for compatibility reasons, I'd prefer if you'd this to the netlink interface.

Quoted 6 lines

static int __init ifb_init_one(int index)
{
      struct net_device *dev_ifb;
      int err;

Quoted 3 lines

      dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d", ifb_setup, numtxqs);

This won't work for the rtnl_link setup since the size must be constant.

Quoted 8 lines · Patrick McHardy

Quoted 7 lines · Changli Gao

2009/11/10 Eric Dumazet <[email protected]>:

The whole ifb.c file is attached, please review and test it. Thanks! /* Number of TX queues per ifb */
static int numtxqs = 1;
module_param(numtxqs, int, 0444); MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");

Quoted 3 lines

Module parameters suck as a configuration API. The existing numifbs option exists purely for compatibility reasons, I'd prefer if you'd this to the netlink interface.

How to do that? I haven't found any examples. Is there a interface to config the number of the TX queues of a NIC.

Quoted 11 lines

Quoted 8 lines

static int __init ifb_init_one(int index)

{
      struct net_device *dev_ifb;
      int err;

      dev_ifb = alloc_netdev_mq(sizeof(struct ifb_private) * numtxqs, "ifb%d",
                                ifb_setup, numtxqs);

This won't work for the rtnl_link setup since the size must be constant.

Does this work?

        ifb_link_ops.priv_size = sizeof(struct ifb_private) * numtxqs;
        rtnl_lock();
        err = __rtnl_link_register(&ifb_link_ops);

You have to add a get_tx_queues() callback to the rtnl_link_ops.

Additionally you need a new attribute (IFLA_NTXQ or something like that) that contains the number of queues. The callback has to parse the attribute and set the number of queues accordingly.

Quoted 5 lines

Does this work?

        ifb_link_ops.priv_size = sizeof(struct ifb_private) * numtxqs; rtnl_lock();
        err = __rtnl_link_register(&ifb_link_ops);

Only for the module parameter. For rtnl_link you need to either allocate the private space seperately or turn priv_size into a callback that returns the required space based on the number of queues.

Quoted 2 lines · Patrick McHardy

You have to add a get_tx_queues() callback to the rtnl_link_ops.

It is used to get the current real_num_tx_queues and num_tx_queues, not to setting.

Quoted 4 lines

Additionally you need a new attribute (IFLA_NTXQ or something like that) that contains the number of queues. The callback has to parse the attribute and set the number of queues accordingly.

It seems another patch is needed first.

Quoted 6 lines

Quoted 6 lines

Does this work?

        ifb_link_ops.priv_size = sizeof(struct ifb_private) * numtxqs;
        rtnl_lock();
        err = __rtnl_link_register(&ifb_link_ops);

Quoted 5 lines

Only for the module parameter. For rtnl_link you need to either allocate the private space seperately or turn priv_size into a callback that returns the required space based on the number of queues.

Do you means that if module ifb is loaded automatically, parameters won't be set correctly?

Its a trivial change, you can put them in the same patch:

static int ifb_get_tx_queues(struct net *net, struct nlattr *tb[], unsigned int *num_tx_queues, unsigned int *real_num_tx_queues) {
   unsigned int n = 1;

   if (tb[IFLA_NTXQ])
      n = nla_get_u32(tb[IFLA_NTXQ]);

   *num_tx_queues = n;
   *real_num_tx_queues = n;
   return 0;
}

Quoted 11 lines · Patrick McHardy

Its a trivial change, you can put them in the same patch:

static int ifb_get_tx_queues(struct net *net, struct nlattr *tb[],
unsigned int *num_tx_queues, unsigned int *real_num_tx_queues)

{
   unsigned int n = 1;

   if (tb[IFLA_NTXQ])
      n = nla_get_u32(tb[IFLA_NTXQ]);

Quoted 5 lines

   *num_tx_queues = n;
   *real_num_tx_queues = n;
   return 0;
}

Is IFLA_NTXQ added? I didn't find it in any branch. And I think both num_tx_queueus and real_num_tx_queues are needed.

Changli Gao a écrit :

I believe Patrick was referring to vlan get_tx_queues() implementation, but it actually gets values from real device (tb[IFLA_LINK])

In your case you'll need to add a new IFLA_NTXQ attribute, and change iproute2 to pass this new attribute at link creation. (check include/linux/if_link.h)

ip link add link .....  ntxq 2

static int vlan_get_tx_queues(struct net *net, struct nlattr *tb[], unsigned int *num_tx_queues, unsigned int *real_num_tx_queues) {
       struct net_device *real_dev;

       if (!tb[IFLA_LINK])
               return -EINVAL;

       real_dev = __dev_get_by_index(net, nla_get_u32(tb[IFLA_LINK])); if (!real_dev)
               return -ENODEV;

       *num_tx_queues      = real_dev->num_tx_queues; *real_num_tx_queues = real_dev->real_num_tx_queues; return 0;
}

Quoted 21 lines · Eric Dumazet

Changli Gao a écrit :

I believe Patrick was referring to vlan get_tx_queues() implementation, but it actually gets values from real device (tb[IFLA_LINK])

In your case you'll need to add a new IFLA_NTXQ attribute, and change iproute2 to pass this new attribute at link creation. (check include/linux/if_link.h)

ip link add link .....  ntxq 2

static int vlan_get_tx_queues(struct net *net, struct nlattr *tb[], unsigned int *num_tx_queues, unsigned int *real_num_tx_queues) {
       struct net_device *real_dev;

       if (!tb[IFLA_LINK])
               return -EINVAL;

Quoted 11 lines

       real_dev = __dev_get_by_index(net, nla_get_u32(tb[IFLA_LINK])); if (!real_dev)
               return -ENODEV;

       *num_tx_queues      = real_dev->num_tx_queues; *real_num_tx_queues = real_dev->real_num_tx_queues; return 0;
}

got it. Thanks. BTW: why not merge iproute2 into linux, just like perf.

On Tue, 10 Nov 2009 21:49:31 +0800
Changli Gao <[email protected]> wrote:

Distro's would find that hard, also it would cause people to forgot binary API compatibility.

ifb: add multi-queue support

Add multi-queue support, and one kernel thread is created for per queue. It can used to emulate multi-queue NIC in software, and distribute work among CPUs.
gentux linux # modprobe ifb numtxqs=2
gentux linux # ifconfig ifb0 up
gentux linux # pgrep ifb0
18508
18509
gentux linux # taskset -p 1 18508

pid 18508's current affinity mask: 3
pid 18508's new affinity mask: 1
gentux linux # taskset -p 2 18509
pid 18509's current affinity mask: 3
pid 18509's new affinity mask: 2
gentux linux # tc qdisc add dev br0 ingress
gentux linux # tc filter add dev br0 parent ffff: protocol ip basic action mirred egress redirect dev ifb0

I'm not sure how this will help, ifb device transmission is still serialized by the ingress queue lock and the mirred lock.

Quoted 3 lines · Patrick McHardy

I'm not sure how this will help, ifb device transmission is still serialized by the ingress queue lock and the mirred lock.

But not in ifb_thread. For firewall the main routine is after netif_rx_ni(). For server applications, the packets belong to the same flow are always routed to the same CPU, and the applications can benefit with better localization.