diff mbox

geneve: Use GRO cells infrastructure.

Message ID 1440806080-126757-1-git-send-email-jesse@nicira.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Jesse Gross Aug. 28, 2015, 11:54 p.m. UTC
Geneve can benefit from GRO at the device level in a manner similar
to other tunnels, especially as hardware offloads are still emerging.

After this patch, aggregated frames are seen on the tunnel interface.
Single stream throughput nearly doubles in ideal circumstances (on
old hardware).

Signed-off-by: Jesse Gross <jesse@nicira.com>
---
 drivers/net/geneve.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

David Miller Aug. 29, 2015, 8:45 p.m. UTC | #1
From: Jesse Gross <jesse@nicira.com>
Date: Fri, 28 Aug 2015 16:54:40 -0700

> Geneve can benefit from GRO at the device level in a manner similar
> to other tunnels, especially as hardware offloads are still emerging.
> 
> After this patch, aggregated frames are seen on the tunnel interface.
> Single stream throughput nearly doubles in ideal circumstances (on
> old hardware).
> 
> Signed-off-by: Jesse Gross <jesse@nicira.com>

Applied, thanks Jesse.

While reviewing this I noticed that the gro cells code goes through all
of the trouble of allocating full per-cpu objects to manage the packet
queuing and processing, but then it uses a full spinlock for protection.

It ought to be sufficient to just block out BH processing or, at worst,
local cpu interrupts, to protect the individual per-cpu cells.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet Aug. 30, 2015, 3:55 p.m. UTC | #2
On Sat, 2015-08-29 at 13:45 -0700, David Miller wrote:

> 
> While reviewing this I noticed that the gro cells code goes through all
> of the trouble of allocating full per-cpu objects to manage the packet
> queuing and processing, but then it uses a full spinlock for protection.
> 
> It ought to be sufficient to just block out BH processing or, at worst,
> local cpu interrupts, to protect the individual per-cpu cells.

Original commit envisioned a possible improvement of gro_cells in case a
NIC receives all GRE traffic on a single RX queue.

(If for example Toeplitz hash RSS looks at outer IP header, and all
traffic is encapsulated with a constant tuple)

Idea was to add a mode where we could select a gro cell not based on
current cpu number but a hash based on flow software flow dissection.

If we do not plan to add this feature, we certainly can remove the
spinlocks now we have per cpu cells.






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

Patch

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 4357bae..cbb30fc 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -16,6 +16,7 @@ 
 #include <linux/etherdevice.h>
 #include <linux/hash.h>
 #include <net/dst_metadata.h>
+#include <net/gro_cells.h>
 #include <net/rtnetlink.h>
 #include <net/geneve.h>
 #include <net/protocol.h>
@@ -58,6 +59,7 @@  struct geneve_dev {
 	struct list_head   next;	/* geneve's per namespace list */
 	__be16		   dst_port;
 	bool		   collect_md;
+	struct gro_cells   gro_cells;
 };
 
 struct geneve_sock {
@@ -199,7 +201,7 @@  static void geneve_rx(struct geneve_sock *gs, struct sk_buff *skb)
 	stats->rx_bytes += skb->len;
 	u64_stats_update_end(&stats->syncp);
 
-	netif_rx(skb);
+	gro_cells_receive(&geneve->gro_cells, skb);
 	return;
 drop:
 	/* Consume bad packet */
@@ -209,14 +211,27 @@  drop:
 /* Setup stats when device is created */
 static int geneve_init(struct net_device *dev)
 {
+	struct geneve_dev *geneve = netdev_priv(dev);
+	int err;
+
 	dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
 	if (!dev->tstats)
 		return -ENOMEM;
+
+	err = gro_cells_init(&geneve->gro_cells, dev);
+	if (err) {
+		free_percpu(dev->tstats);
+		return err;
+	}
+
 	return 0;
 }
 
 static void geneve_uninit(struct net_device *dev)
 {
+	struct geneve_dev *geneve = netdev_priv(dev);
+
+	gro_cells_destroy(&geneve->gro_cells);
 	free_percpu(dev->tstats);
 }