loopback: optimization

RFC In review Stephen Hemminger 3 participants 6 messages Nov 4, 2008 – Nov 6, 2008

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Stephen Hemminger

author

2008-11-04 05:38 UTC #0 lore raw

RFC 1/1 loopback: optimization

This is something I was trying out to try improving loopback performance. It moves loopback processing from the generic backlog per-cpu queues, to its own set of queues. I thought this might help the cache locality and the loopback doesn't have to do relatively expensive local_irq_disable() operations.

BUT it didn't seem to help with tbench, but it might help others who want to go farther with it. Code runs but treat it ASIS at this point.

Signed-off-by Stephen Hemminger <[email protected]>

drivers/net/loopback.c +73 ~~-22~~

@@ -59,51 +59,79 @@
 #include <linux/percpu.h>
 #include <net/net_namespace.h>
 
-struct pcpu_lstats {
+struct loopback_per_cpu {
+	struct sk_buff_head rxq;
+	struct napi_struct napi;
+
 	unsigned long packets;
 	unsigned long bytes;
 };
 
+
 /*
  * The higher levels take care of making this non-reentrant (it's
  * called with bh's disabled).
  */
 static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct pcpu_lstats *pcpu_lstats, *lb_stats;
+	struct loopback_per_cpu *lo = dev->ml_priv, *pcpu;
 
 	skb_orphan(skb);
 
-	skb->protocol = eth_type_trans(skb,dev);
+	skb->protocol = eth_type_trans(skb, dev);
 
 	dev->last_rx = jiffies;
 
 	/* it's OK to use per_cpu_ptr() because BHs are off */
-	pcpu_lstats = dev->ml_priv;
-	lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
-	lb_stats->bytes += skb->len;
-	lb_stats->packets++;
+	pcpu = per_cpu_ptr(lo, smp_processor_id());
 
-	netif_rx(skb);
+	if (likely(pcpu->rxq.qlen < netdev_max_backlog)) {
+		if (pcpu->rxq.qlen == 0)
+			__napi_schedule(&lo->napi);
+
+		__skb_queue_tail(&pcpu->rxq, skb);
+		pcpu->bytes += skb->len;
+		pcpu->packets++;
+
+		return NET_XMIT_SUCCESS;
+	} else {
+		dev->stats.rx_dropped++;
+		dev_kfree_skb_any(skb);
+		return NET_XMIT_DROP;
+	}
+}
 
-	return 0;
+static int loopback_poll(struct napi_struct *napi, int work_limit)
+{
+	struct loopback_per_cpu *pcpu
+		= container_of(napi, struct loopback_per_cpu, napi);
+	struct sk_buff *skb;
+	int work_done = 0;
+
+	while ( (skb = __skb_dequeue(&pcpu->rxq)) ) {
+		netif_receive_skb(skb);
+
+		if (++work_done >= work_limit)
+			goto done;
+	}
+
+	__napi_complete(napi);
+done:
+	return work_done;
 }
 
 static struct net_device_stats *get_stats(struct net_device *dev)
 {
-	const struct pcpu_lstats *pcpu_lstats;
+	const struct loopback_per_cpu *lo = dev->ml_priv;
 	struct net_device_stats *stats = &dev->stats;
 	unsigned long bytes = 0;
 	unsigned long packets = 0;
 	int i;
 
-	pcpu_lstats = dev->ml_priv;
 	for_each_possible_cpu(i) {
-		const struct pcpu_lstats *lb_stats;
-
-		lb_stats = per_cpu_ptr(pcpu_lstats, i);
-		bytes   += lb_stats->bytes;
-		packets += lb_stats->packets;
+		const struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+		bytes   += pcpu->bytes;
+		packets += pcpu->packets;
 	}
 	stats->rx_packets = packets;
 	stats->tx_packets = packets;

@@ -127,21 +155,43 @@ static const struct ethtool_ops loopback
 
 static int loopback_dev_init(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats;
+	struct loopback_per_cpu *lo;
+	int i;
 
-	lstats = alloc_percpu(struct pcpu_lstats);
-	if (!lstats)
+	lo = alloc_percpu(struct loopback_per_cpu);
+	if (!lo)
 		return -ENOMEM;
 
-	dev->ml_priv = lstats;
+	dev->ml_priv = lo;
+
+	for_each_possible_cpu(i) {
+		struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+		skb_queue_head_init(&pcpu->rxq);
+		netif_napi_add(dev, &pcpu->napi, loopback_poll, 64);
+	}
+
+	return 0;
+}
+
+static int loopback_dev_stop(struct net_device *dev)
+{
+	struct loopback_per_cpu *lo = dev->ml_priv;
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+
+		napi_synchronize(&pcpu->napi);
+		__skb_queue_purge(&pcpu->rxq);
+	}
 	return 0;
 }
 
 static void loopback_dev_free(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats = dev->ml_priv;
+	struct loopback_per_cpu *lo = dev->ml_priv;
 
-	free_percpu(lstats);
+	free_percpu(lo);
 	free_netdev(dev);
 }

@@ -169,6 +219,7 @@ static void loopback_setup(struct net_de
 	dev->header_ops		= &eth_header_ops;
 	dev->init = loopback_dev_init;
 	dev->destructor = loopback_dev_free;
+	dev->stop = loopback_dev_stop;
 }

Eric Dumazet

2008-11-04 06:37 UTC #1 lore raw

Quoted 46 lines · Stephen Hemminger

This is something I was trying out to try improving loopback performance. It moves loopback processing from the generic backlog per-cpu queues, to its own set of queues. I thought this might help the cache locality and the loopback doesn't have to do relatively expensive local_irq_disable() operations.
BUT it didn't seem to help with tbench, but it might help others who want to go farther with it. Code runs but treat it ASIS at this point.
Signed-off-by Stephen Hemminger <[email protected]>
drivers/net/loopback.c +7 -4
@@ -59,51 +59,79 @@
 #include <linux/percpu.h>
 #include <net/net_namespace.h>
 
-struct pcpu_lstats {
+struct loopback_per_cpu {
+	struct sk_buff_head rxq;
+	struct napi_struct napi;
+
 	unsigned long packets;
 	unsigned long bytes;
 };
 
+
 /*
  * The higher levels take care of making this non-reentrant (it's
  * called with bh's disabled).
  */
 static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct pcpu_lstats *pcpu_lstats, *lb_stats;
+	struct loopback_per_cpu *lo = dev->ml_priv, *pcpu;
 
 	skb_orphan(skb);
 
-	skb->protocol = eth_type_trans(skb,dev);
+	skb->protocol = eth_type_trans(skb, dev);
 
 	dev->last_rx = jiffies;
 
 	/* it's OK to use per_cpu_ptr() because BHs are off */
-	pcpu_lstats = dev->ml_priv;

drivers/net/loopback.c line 91

Quoted 10 lines

drivers/net/loopback.c +4 -4

-	lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
-	lb_stats->bytes += skb->len;
-	lb_stats->packets++;
+	pcpu = per_cpu_ptr(lo, smp_processor_id());
 
-	netif_rx(skb);
+	if (likely(pcpu->rxq.qlen < netdev_max_backlog)) {
 
+		if (pcpu->rxq.qlen == 0)
+			__napi_schedule(&lo->napi);

Hum...

Most of the time I suspect we take this __napi_schedule() thing.

So are you sure you use the right one (&lo->napi, not &pcpu->napi) ?

This is the where point we still do a cache line ping-pong.

Quoted 118 lines

drivers/net/loopback.c +33 -8

+
+		__skb_queue_tail(&pcpu->rxq, skb);
+		pcpu->bytes += skb->len;
+		pcpu->packets++;
+
+		return NET_XMIT_SUCCESS;
+	} else {
+		dev->stats.rx_dropped++;
+		dev_kfree_skb_any(skb);
+		return NET_XMIT_DROP;
+	}
+}
 
-	return 0;
+static int loopback_poll(struct napi_struct *napi, int work_limit)
+{
+	struct loopback_per_cpu *pcpu
+		= container_of(napi, struct loopback_per_cpu, napi);
+	struct sk_buff *skb;
+	int work_done = 0;
+
+	while ( (skb = __skb_dequeue(&pcpu->rxq)) ) {
+		netif_receive_skb(skb);
+
+		if (++work_done >= work_limit)
+			goto done;
+	}
+
+	__napi_complete(napi);
+done:
+	return work_done;
 }
 
 static struct net_device_stats *get_stats(struct net_device *dev)
 {
-	const struct pcpu_lstats *pcpu_lstats;
+	const struct loopback_per_cpu *lo = dev->ml_priv;
 	struct net_device_stats *stats = &dev->stats;
 	unsigned long bytes = 0;
 	unsigned long packets = 0;
 	int i;
 
-	pcpu_lstats = dev->ml_priv;
 	for_each_possible_cpu(i) {
-		const struct pcpu_lstats *lb_stats;
-
-		lb_stats = per_cpu_ptr(pcpu_lstats, i);
-		bytes   += lb_stats->bytes;
-		packets += lb_stats->packets;
+		const struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+		bytes   += pcpu->bytes;
+		packets += pcpu->packets;
 	}
 	stats->rx_packets = packets;

drivers/net/loopback.c +29 -7

 	stats->tx_packets = packets;

@@ -127,21 +155,43 @@ static const struct ethtool_ops loopback
 
 static int loopback_dev_init(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats;
+	struct loopback_per_cpu *lo;
+	int i;
 
-	lstats = alloc_percpu(struct pcpu_lstats);
-	if (!lstats)
+	lo = alloc_percpu(struct loopback_per_cpu);
+	if (!lo)
 		return -ENOMEM;
 
-	dev->ml_priv = lstats;
+	dev->ml_priv = lo;
+
+	for_each_possible_cpu(i) {
+		struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+		skb_queue_head_init(&pcpu->rxq);
+		netif_napi_add(dev, &pcpu->napi, loopback_poll, 64);
+	}
+
+	return 0;
+}
+
+static int loopback_dev_stop(struct net_device *dev)
+{
+	struct loopback_per_cpu *lo = dev->ml_priv;
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct loopback_per_cpu *pcpu = per_cpu_ptr(lo, i);
+
+		napi_synchronize(&pcpu->napi);
+		__skb_queue_purge(&pcpu->rxq);
+	}
 	return 0;
 }
 
 static void loopback_dev_free(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats = dev->ml_priv;
+	struct loopback_per_cpu *lo = dev->ml_priv;
 
-	free_percpu(lstats);
+	free_percpu(lo);
 	free_netdev(dev);
 }

@@ -169,6 +219,7 @@ static void loopback_setup(struct net_de
 	dev->header_ops		= &eth_header_ops;
 	dev->init = loopback_dev_init;
 	dev->destructor = loopback_dev_free;
+	dev->stop = loopback_dev_stop;
 }
 
--

To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

David Miller

maintainer

2008-11-05 09:49 UTC #2 lore raw

From: Eric Dumazet <[email protected]>
Date: Tue, 04 Nov 2008 07:36:48 +0100

Quoted 1 line

So are you sure you use the right one (&lo->napi, not &pcpu->napi) ?

Yes I think this logic error in Stephen's patch might explain why it had no effect.

Stephen please try with lo->napi replaced with pcpu->napi here.

Stephen Hemminger

author

2008-11-05 20:37 UTC #3 lore raw

RFC 1/1 loopback: optimization

In reply to Stephen Hemminger

Convert loopback device from using common network queues to a per-cpu receive queue with NAPI. This gives a small 1% performance gain when measured over 5 runs of tbench. Not sure if it's worth bothering though.

Signed-off-by Stephen Hemminger <[email protected]>

drivers/net/loopback.c +84 ~~-19~~

@@ -59,7 +59,10 @@
 #include <linux/percpu.h>
 #include <net/net_namespace.h>
 
-struct pcpu_lstats {
+struct loopback_queue {
+	struct sk_buff_head rxq;
+	struct napi_struct napi;
+
 	unsigned long packets;
 	unsigned long bytes;
 };

@@ -70,36 +73,60 @@ struct pcpu_lstats {
  */
 static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct pcpu_lstats *pcpu_lstats, *lb_stats;
+	struct loopback_queue *pcpu;
 
 	skb_orphan(skb);
 
 	skb->protocol = eth_type_trans(skb,dev);
 
-	/* it's OK to use per_cpu_ptr() because BHs are off */
-	pcpu_lstats = dev->ml_priv;
-	lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
-	lb_stats->bytes += skb->len;
-	lb_stats->packets++;
-
-	netif_rx(skb);
+	pcpu = per_cpu_ptr(dev->ml_priv, smp_processor_id());
+	if (likely(pcpu->rxq.qlen <= netdev_max_backlog)) {
+		__skb_queue_tail(&pcpu->rxq, skb);
+		pcpu->bytes += skb->len;
+		pcpu->packets++;
+		napi_schedule_irq(&pcpu->napi);
+
+		return NET_XMIT_SUCCESS;
+	} else {
+		dev->stats.rx_dropped++;
+		dev_kfree_skb_any(skb);
+		return NET_XMIT_DROP;
+	}
 
 	return 0;
 }
 
+static int loopback_poll(struct napi_struct *arg, int quota)
+{
+	struct loopback_queue *pcpu = container_of(arg, struct loopback_queue, napi);
+	int work = 0;
+
+	do {
+		struct sk_buff *skb = __skb_dequeue(&pcpu->rxq);
+
+		if (!skb) {
+			__napi_complete(arg);
+			break;
+		}
+
+		netif_receive_skb(skb);
+	} while (++work < quota);
+
+	return work;
+}
+
+
 static struct net_device_stats *get_stats(struct net_device *dev)
 {
-	const struct pcpu_lstats *pcpu_lstats;
 	struct net_device_stats *stats = &dev->stats;
 	unsigned long bytes = 0;
 	unsigned long packets = 0;
 	int i;
 
-	pcpu_lstats = dev->ml_priv;
 	for_each_possible_cpu(i) {
-		const struct pcpu_lstats *lb_stats;
+		const struct loopback_queue *lb_stats;
 
-		lb_stats = per_cpu_ptr(pcpu_lstats, i);
+		lb_stats = per_cpu_ptr(dev->ml_priv, i);
 		bytes   += lb_stats->bytes;
 		packets += lb_stats->packets;
 	}

@@ -125,21 +152,57 @@ static const struct ethtool_ops loopback
 
 static int loopback_dev_init(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats;
+	void *p;
+	int i;
 
-	lstats = alloc_percpu(struct pcpu_lstats);
-	if (!lstats)
+	p = alloc_percpu(struct loopback_queue);
+	if (!p)
 		return -ENOMEM;
 
-	dev->ml_priv = lstats;
+	for_each_possible_cpu(i) {
+		struct loopback_queue *pcpu = per_cpu_ptr(p, i);
+		skb_queue_head_init(&pcpu->rxq);
+		netif_napi_add(dev, &pcpu->napi, loopback_poll, 64);
+	}
+
+	dev->ml_priv = p;
+
+	return 0;
+}
+
+static int loopback_dev_start(struct net_device *dev)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct loopback_queue *pcpu = per_cpu_ptr(dev->ml_priv, i);
+		napi_enable(&pcpu->napi);
+	}
+	return 0;
+}
+
+static int loopback_dev_stop(struct net_device *dev)
+{
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct loopback_queue *pcpu = per_cpu_ptr(dev->ml_priv, i);
+		napi_disable(&pcpu->napi);
+		__skb_queue_purge(&pcpu->rxq);
+	}
 	return 0;
 }
 
 static void loopback_dev_free(struct net_device *dev)
 {
-	struct pcpu_lstats *lstats = dev->ml_priv;
+	int i;
+
+	for_each_possible_cpu(i) {
+		struct loopback_queue *pcpu = per_cpu_ptr(dev->ml_priv, i);
+		napi_disable(&pcpu->napi);
+	}
 
-	free_percpu(lstats);
+	free_percpu(dev->ml_priv);
 	free_netdev(dev);
 }

@@ -166,6 +229,8 @@ static void loopback_setup(struct net_de
 	dev->ethtool_ops	= &loopback_ethtool_ops;
 	dev->header_ops		= &eth_header_ops;
 	dev->init = loopback_dev_init;
+	dev->open = loopback_dev_start;
+	dev->stop = loopback_dev_stop;
 	dev->destructor = loopback_dev_free;
 }

include/linux/netdevice.h +2 -0

@@ -366,6 +366,8 @@ static inline int napi_reschedule(struct
 	return 0;
 }
 
+extern void napi_schedule_irq(struct napi_struct *n);
+
 /**
  *	napi_complete - NAPI processing complete
  *	@n: napi context

net/core/dev.c +9 -0

@@ -2369,6 +2369,15 @@ void __napi_schedule(struct napi_struct 
 }
 EXPORT_SYMBOL(__napi_schedule);
 
+/* Special case version of napi_schedule since loopback device has no hard irq */
+void napi_schedule_irq(struct napi_struct *n)
+{
+	if (napi_schedule_prep(n)) {
+		list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
+		__raise_softirq_irqoff(NET_RX_SOFTIRQ);
+	}
+}
+
 
 static void net_rx_action(struct softirq_action *h)
 {

Convert loopback device from using common network queues to a per-cpu receive queue with NAPI. This gives a small 1% performance gain when measured over 5 runs of tbench. It does make the code larger and more space needs to be allocated as well.

Signed-off-by Stephen Hemminger <[email protected]>

drivers/net/loopback.c +84 ~~-19~~

154 lines not on this page · raw message

include/linux/netdevice.h +2 -0

@@ -366,6 +366,8 @@ static inline int napi_reschedule(struct
 	return 0;
 }
 
+extern void napi_schedule_irq(struct napi_struct *n);
+
 /**
  *	napi_complete - NAPI processing complete
  *	@n: napi context

net/core/dev.c +9 -0

@@ -2369,6 +2369,15 @@ void __napi_schedule(struct napi_struct 
 }
 EXPORT_SYMBOL(__napi_schedule);
 
+/* Special case version of napi_schedule since loopback device has no hard irq */
+void napi_schedule_irq(struct napi_struct *n)
+{
+	if (napi_schedule_prep(n)) {
+		list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
+		__raise_softirq_irqoff(NET_RX_SOFTIRQ);
+	}
+}
+
 
 static void net_rx_action(struct softirq_action *h)
 {

Eric Dumazet

2008-11-05 23:14 UTC #4 lore raw

Quoted 11 lines · Stephen Hemminger

Convert loopback device from using common network queues to a per-cpu receive queue with NAPI. This gives a small 1% performance gain when measured over 5 runs of tbench. Not sure if it's worth bothering though.
Signed-off-by Stephen Hemminger <[email protected]>

drivers/net/loopback.c line 68

Quoted 10 lines

drivers/net/loopback.c +9 -0

@@ -59,7 +59,10 @@
 
+/* Special case version of napi_schedule since loopback device has no hard irq */
+void napi_schedule_irq(struct napi_struct *n)
+{
+	if (napi_schedule_prep(n)) {
+		list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
+		__raise_softirq_irqoff(NET_RX_SOFTIRQ);
+	}
+}
+

Stephen, I dont get it.

Sure loopback device cannot generate hard irqs, but what prevent's a real hardware interrupt to call NIC driver that can call napi_schedule() and corrupt softnet_data.poll_list ?

Why not using a queue dedicated on loopback directly in cpu_var(softnet_data) ?

(ie not using a napi structure for each cpu and each loopback dev)

This queue would be irq safe yes.

net_rx_action could handle this list without local_irq_disable()/local_irq_enable() games.

Hum, maybe complex for loopback_dev_stop() to purge all queues without interfering with other namespaces.

Stephen Hemminger

author

2008-11-06 00:42 UTC #5 lore raw

Quoted 30 lines · On Thu, 06 Nov 2008 00:14:16 +0100 Eric Dumazet <[email protected]> wrote:

Quoted 11 lines · Stephen Hemminger
Convert loopback device from using common network queues to a per-cpu receive queue with NAPI. This gives a small 1% performance gain when measured over 5 runs of tbench. Not sure if it's worth bothering though.
Signed-off-by Stephen Hemminger <[email protected]>
drivers/net/loopback.c line 68
Quoted 10 lines
drivers/net/loopback.c +9 -0
@@ -59,7 +59,10 @@
 
+/* Special case version of napi_schedule since loopback device has no hard irq */
+void napi_schedule_irq(struct napi_struct *n)
+{
+	if (napi_schedule_prep(n)) {
+		list_add_tail(&n->poll_list, &__get_cpu_var(softnet_data).poll_list);
+		__raise_softirq_irqoff(NET_RX_SOFTIRQ);
+	}
+}
+
Stephen, I dont get it.
Sure loopback device cannot generate hard irqs, but what prevent's a real hardware interrupt to call NIC driver that can call napi_schedule() and corrupt softnet_data.poll_list ?
Why not using a queue dedicated on loopback directly in cpu_var(softnet_data) ?

Quoted 7 lines

(ie not using a napi structure for each cpu and each loopback dev)
This queue would be irq safe yes.
net_rx_action could handle this list without local_irq_disable()/local_irq_enable() games.
Hum, maybe complex for loopback_dev_stop() to purge all queues without interfering with other namespaces.

I did try a workqueue and kthread version previously, but they both had much worse performance. Forgot that the NAPI schedule is shared, so yes that would have to locked.

Doing it purely for loopback would mean using a tasklet or another softirq.