net: mitigating kmem_cache slowpath for network stack in NAPI context

It have been a long road. Back in July 2014 I realized that network stack were hitting the kmem_cache/SLUB slowpath when freeing SKBs, but had no solution. In Dec 2014 I had implemented a solution called qmempool[1], that showed it was possible to improve this, but got rejected due to being a cache on top of kmem_cache. In July 2015 improvements to kmem_cache were proposed, and recently Oct 2015 my kmem_cache (SLAB+SLUB) patches for bulk alloc and free have been accepted into the AKPM quilt tree.

This patchset is the first real use-case kmem_cache bulk alloc and free. And is joint work with Alexander Duyck while still at Red Hat.

Using bulk free to avoid the SLUB slowpath shows the full potential. In this patchset it is realized in NAPI/softirq context. 1. During DMA TX completion bulk free is optimal and does not introduce any added latency. 2. bulk free of SKBs delay free'ed due to IRQ context in net_tx_action softirq completion queue.

Using bulk alloc is showing minor improvements for SLUB(+0.9%), but a very slight slowdown for SLAB(-0.1%).

[1] http://thread.gmane.org/gmane.linux.network/342347/focus=126138

This patchset is based on net-next (commit 26440c835), BUT I've applied several patches from AKPMs MM-tree.

Cherrypick some commits from MMOTM tree on branch/tag mmotm-2015-10-06-16-30 from git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git
(Below commit IDs are obviously not stable)

Pickup my own MM-changes:
b0aa3e95ce82 ("slub: mark the dangling ifdef #else of CONFIG_SLUB_DEBUG") 114a2b37847c ("slab: implement bulking for SLAB allocator") 606397476e8b ("slub: support for bulk free with SLUB freelists") ee29cd6a570c ("slub: optimize bulk slowpath free by detached freelist") 491c6e0ca89d ("slub-optimize-bulk-slowpath-free-by-detached-freelist-fix")

Pickup slab.h changes:
d9a47e0b776b ("compiler.h: add support for function attribute assume_aligned") 1c3a5c789b4f ("slab.h: sprinkle __assume_aligned attributes")

Wanted Kirill A. Shutemov's changes as they change virt_to_head_page(), had to apply patches manually from http://ozlabs.org/~akpm/mmotm/
(stamp-2015-10-20-16-33) as AKPM made several small fixes.

---

Jesper Dangaard Brouer (4):
net: bulk free infrastructure for NAPI context, use napi_consume_skb
net: bulk free SKBs that were delay free'ed due to IRQ context
ixgbe: bulk free SKBs during TX completion cleanup cycle
net: bulk alloc and reuse of SKBs in NAPI context

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 +
 include/linux/skbuff.h                        |    4 +
 net/core/dev.c                                |    9 ++
 net/core/skbuff.c                             |  122 +++++++++++++++++++++++--
 4 files changed, 127 insertions(+), 14 deletions(-)

--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer

Discovered that network stack were hitting the kmem_cache/SLUB slowpath when freeing SKBs. Doing bulk free with kmem_cache_free_bulk can speedup this slowpath.

NAPI context is a bit special, lets take advantage of that for bulk free'ing SKBs.

In NAPI context we are running in softirq, which gives us certain protection. A softirq can run on several CPUs at once. BUT the important part is a softirq will never preempt another softirq running on the same CPU. This gives us the opportunity to access per-cpu variables in softirq context.

Extend napi_alloc_cache (before only contained page_frag_cache) to be a struct with a small array based stack for holding SKBs. Introduce a SKB defer and flush API for accessing this.

Introduce napi_consume_skb() as replacement for e.g. dev_consume_skb_any() when running in NAPI context. A small trick to handle/detect if we are called from netpoll is to see if budget is 0. In that case, we need to invoke dev_consume_skb_irq().

Joint work with Alexander Duyck.

---
 include/linux/skbuff.h |    3 ++
 net/core/dev.c         |    1 +
 net/core/skbuff.c      |   83 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 6 deletions(-)

index 4398411236f1..a3dec82e0e2c 100644
@@ -2308,6 +2308,9 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 {
 	return __napi_alloc_skb(napi, length, GFP_ATOMIC);
 }
+void napi_consume_skb(struct sk_buff *skb, int budget);
+
+void __kfree_skb_flush(void);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index 1225b4be8ed6..204059f67154 100644
@@ -4841,6 +4841,7 @@ static void net_rx_action(struct softirq_action *h)
 		}
 	}
 
+	__kfree_skb_flush();
 	local_irq_disable();
 
 	list_splice_tail_init(&sd->poll_list, &list);

The network stack defers SKBs free, in-case free happens in IRQ or when IRQs are disabled. This happens in __dev_kfree_skb_irq() that writes SKBs that were free'ed during IRQ to the softirq completion queue (softnet_data.completion_queue).

These SKBs are naturally delayed, and cleaned up during NET_TX_SOFTIRQ in function net_tx_action(). Take advantage of this a use the skb defer and flush API, as we are already in softirq context.

For modern drivers this rarely happens. Although most drivers do call dev_kfree_skb_any(), which detects the situation and calls __dev_kfree_skb_irq() when needed. This due to netpoll can call from IRQ context.

---
 include/linux/skbuff.h |    1 +
 net/core/dev.c         |    8 +++++++-
 net/core/skbuff.c      |    8 ++++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

index a3dec82e0e2c..f314abff2cbd 100644
@@ -2311,6 +2311,7 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 void napi_consume_skb(struct sk_buff *skb, int budget);
 
 void __kfree_skb_flush(void);
+void __kfree_skb_defer(struct sk_buff *skb);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index 204059f67154..0e88397db1fa 100644
@@ -3588,8 +3588,14 @@ static void net_tx_action(struct softirq_action *h)
 				trace_consume_skb(skb);
 			else
 				trace_kfree_skb(skb, net_tx_action);
-			__kfree_skb(skb);
+
+			if (skb->fclone != SKB_FCLONE_UNAVAILABLE)
+				__kfree_skb(skb);
+			else
+				__kfree_skb_defer(skb);
 		}
+
+		__kfree_skb_flush();
 	}
 
 	if (sd->output_queue) {

index 2682ac46d640..2ffcb014e00b 100644
@@ -767,7 +767,7 @@ void __kfree_skb_flush(void)
 	}
 }
 
-static void __kfree_skb_defer(struct sk_buff *skb)
+static inline void _kfree_skb_defer(struct sk_buff *skb)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
 

@@ -789,6 +789,10 @@ static void __kfree_skb_defer(struct sk_buff *skb)
 		nc->skb_count = 0;
 	}
 }
+void __kfree_skb_defer(struct sk_buff *skb)
+{
+	_kfree_skb_defer(skb);
+}
 
 void napi_consume_skb(struct sk_buff *skb, int budget)
 {

@@ -814,7 +818,7 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 		return;
 	}
 
-	__kfree_skb_defer(skb);
+	_kfree_skb_defer(skb);
 }
 EXPORT_SYMBOL(napi_consume_skb);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

There is an opportunity to bulk free SKBs during reclaiming of resources after DMA transmit completes in ixgbe_clean_tx_irq. Thus, bulk freeing at this point does not introduce any added latency.

Simply use napi_consume_skb() which were recently introduced. The napi_budget parameter is needed by napi_consume_skb() to detect if it is called from netpoll.

Benchmarking IPv4-forwarding, on CPU i7-4790K @4.2GHz (no turbo boost)
Single CPU/flow numbers: before: 1982144 pps -> after : 2064446 pps
Improvement: +82302 pps, -20 nanosec, +4.1%
(SLUB and GCC version 5.1.1 20150618 (Red Hat 5.1.1-4))

Joint work with Alexander Duyck.

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 9f8a7fd7a195..4614dfb3febd 100644
@@ -1090,7 +1090,7 @@ static void ixgbe_tx_timeout_reset(struct ixgbe_adapter *adapter)
  * @tx_ring: tx ring to clean
  **/
 static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
-			       struct ixgbe_ring *tx_ring)
+			       struct ixgbe_ring *tx_ring, int napi_budget)
 {
 	struct ixgbe_adapter *adapter = q_vector->adapter;
 	struct ixgbe_tx_buffer *tx_buffer;

@@ -1128,7 +1128,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 		total_packets += tx_buffer->gso_segs;
 
 		/* free the skb */
-		dev_consume_skb_any(tx_buffer->skb);
+		napi_consume_skb(tx_buffer->skb, napi_budget);
 
 		/* unmap skb header data */
 		dma_unmap_single(tx_ring->dev,

@@ -2784,7 +2784,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
 #endif
 
 	ixgbe_for_each_ring(ring, q_vector->tx)
-		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring);
+		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring, budget);
 
 	if (!ixgbe_qv_lock_napi(q_vector))
 		return budget;

Think twice before applying
- This patch can potentially introduce added latency in some workloads

This patch introduce bulk alloc of SKBs and allow reuse of SKBs free'ed in same softirq cycle. SKBs are normally free'ed during TX completion, but most high speed drivers also cleanup TX ring during NAPI RX poll cycle. Thus, if using napi_consume_skb/__kfree_skb_defer, SKBs will be avail in the napi_alloc_cache->skb_cache.

If no SKBs are avail for reuse, then only bulk alloc 8 SKBs, to limit the potential overshooting unused SKBs needed to free'ed when NAPI cycle ends (flushed in net_rx_action via __kfree_skb_flush()).

Benchmarking IPv4-forwarding, on CPU i7-4790K @4.2GHz (no turbo boost) (GCC version 5.1.1 20150618 (Red Hat 5.1.1-4))
Allocator SLUB:
Single CPU/flow numbers: before: 2064446 pps -> after: 2083031 pps
Improvement: +18585 pps, -4.3 nanosec, +0.9%
Allocator SLAB:
Single CPU/flow numbers: before: 2035949 pps -> after: 2033567 pps
Regression: -2382 pps, +0.57 nanosec, -0.1 %

Even-though benchmarking does show an improvement for SLUB(+0.9%), I'm not convinced bulk alloc will be a win in all situations:
* I see stalls on walking the SLUB freelist (normal hidden by prefetch)
* In case RX queue is not full, alloc and free more SKBs than needed

More testing is needed with more real life benchmarks.

Joint work with Alexander Duyck.

---
 net/core/skbuff.c |   35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

index 2ffcb014e00b..d0e0ccccfb11 100644
@@ -483,13 +483,14 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 				 gfp_t gfp_mask)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
+	struct skb_shared_info *shinfo;
 	struct sk_buff *skb;
 	void *data;
 
 	len += NET_SKB_PAD + NET_IP_ALIGN;
 
-	if ((len > SKB_WITH_OVERHEAD(PAGE_SIZE)) ||
-	    (gfp_mask & (__GFP_WAIT | GFP_DMA))) {
+	if (unlikely((len > SKB_WITH_OVERHEAD(PAGE_SIZE)) ||
+		     (gfp_mask & (__GFP_WAIT | GFP_DMA)))) {
 		skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
 		if (!skb)
 			goto skb_fail;

@@ -506,12 +507,38 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 	if (unlikely(!data))
 		return NULL;
 
-	skb = __build_skb(data, len);
-	if (unlikely(!skb)) {
+#define BULK_ALLOC_SIZE 8
+	if (!nc->skb_count &&
+	    kmem_cache_alloc_bulk(skbuff_head_cache, gfp_mask,
+				  BULK_ALLOC_SIZE, nc->skb_cache)) {
+		nc->skb_count = BULK_ALLOC_SIZE;
+	}
+	if (likely(nc->skb_count)) {
+		skb = (struct sk_buff *)nc->skb_cache[--nc->skb_count];
+	} else {
+		/* alloc bulk failed */
 		skb_free_frag(data);
 		return NULL;
 	}
 
+	len -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+	memset(skb, 0, offsetof(struct sk_buff, tail));
+	skb->truesize = SKB_TRUESIZE(len);
+	atomic_set(&skb->users, 1);
+	skb->head = data;
+	skb->data = data;
+	skb_reset_tail_pointer(skb);
+	skb->end = skb->tail + len;
+	skb->mac_header = (typeof(skb->mac_header))~0U;
+	skb->transport_header = (typeof(skb->transport_header))~0U;
+
+	/* make sure we initialize shinfo sequentially */
+	shinfo = skb_shinfo(skb);
+	memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
+	atomic_set(&shinfo->dataref, 1);
+	kmemcheck_annotate_variable(shinfo->destructor_arg);
+
 	/* use OR instead of assignment to avoid clearing of bits in mask */
 	if (nc->page.pfmemalloc)
 		skb->pfmemalloc = 1;

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

From: Jesper Dangaard Brouer <[email protected]>
Date: Fri, 23 Oct 2015 14:46:01 +0200

Logically I'm fine with this series, but as you mention there are dependencies that need to hit upstream before I can merge any of this stuff into my tree.

I also think that patch #4 is a net-win, and also will expose the bulking code to more testing since it will be used more often.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

This patchset is relevant for my NetDev 1.1 "Network Performance BoF" [1].

The first 4 patches, is a repost[2], for the first real use-case of kmem_cache bulk alloc and free API. They were adjusted slightly to accomodate my last slab API changes. They should be ready for inclusion in net-next, as the needed MM tree are avail in net-next.

Patch 5 is also enabling the SKB bulk API for mlx5.

Thus, patches 1-5 should be ready for net-next.

After patch 5, the experimental patches begin, which is Prove-of-Concept code for what we will be discussing during the Network Performance BoF [1]

[1] http://netdevconf.org/1.1/bof-network-performance-bof-jesper-dangaard-brouer.html
[2] http://thread.gmane.org/gmane.linux.network/384302/

---

Jesper Dangaard Brouer (11):
net: bulk free infrastructure for NAPI context, use napi_consume_skb
net: bulk free SKBs that were delay free'ed due to IRQ context
ixgbe: bulk free SKBs during TX completion cleanup cycle
net: bulk alloc and reuse of SKBs in NAPI context
mlx5: use napi_*_skb APIs to get bulk alloc and free
mlx5: RX bulking or bundling of packets before calling network stack
net: introduce napi_alloc_skb_bulk() for more use-cases
mlx5: hint the NAPI alloc skb API about the expected bulk size
EXPERIMENT: dummy: bulk free
net: API for RX handover of multiple SKBs to stack
net: RPS bulk enqueue to backlog

 drivers/net/dummy.c                               |    3 
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |    6 -
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    5 -
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c   |   24 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 -
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    4 -
 include/linux/netdevice.h                         |   13 ++
 include/linux/skbuff.h                            |   23 ++-
 net/core/dev.c                                    |  160 ++++++++++++++++++++-
 net/core/skbuff.c                                 |  122 +++++++++++++++-
 10 files changed, 327 insertions(+), 37 deletions(-)

Discovered that network stack were hitting the kmem_cache/SLUB slowpath when freeing SKBs. Doing bulk free with kmem_cache_free_bulk can speedup this slowpath.

NAPI context is a bit special, lets take advantage of that for bulk free'ing SKBs.

In NAPI context we are running in softirq, which gives us certain protection. A softirq can run on several CPUs at once. BUT the important part is a softirq will never preempt another softirq running on the same CPU. This gives us the opportunity to access per-cpu variables in softirq context.

Extend napi_alloc_cache (before only contained page_frag_cache) to be a struct with a small array based stack for holding SKBs. Introduce a SKB defer and flush API for accessing this.

Introduce napi_consume_skb() as replacement for e.g. dev_consume_skb_any() when running in NAPI context. A small trick to handle/detect if we are called from netpoll is to see if budget is 0. In that case, we need to invoke dev_consume_skb_irq().

Joint work with Alexander Duyck.

---
 include/linux/skbuff.h |    3 ++
 net/core/dev.c         |    1 +
 net/core/skbuff.c      |   83 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 6 deletions(-)

index 11f935c1a090..3c8d348223d7 100644
@@ -2399,6 +2399,9 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 {
 	return __napi_alloc_skb(napi, length, GFP_ATOMIC);
 }
+void napi_consume_skb(struct sk_buff *skb, int budget);
+
+void __kfree_skb_flush(void);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index cc9e3652cf93..73e6cbc10ac6 100644
@@ -5149,6 +5149,7 @@ static void net_rx_action(struct softirq_action *h)
 		}
 	}
 
+	__kfree_skb_flush();
 	local_irq_disable();
 
 	list_splice_tail_init(&sd->poll_list, &list);

The network stack defers SKBs free, in-case free happens in IRQ or when IRQs are disabled. This happens in __dev_kfree_skb_irq() that writes SKBs that were free'ed during IRQ to the softirq completion queue (softnet_data.completion_queue).

These SKBs are naturally delayed, and cleaned up during NET_TX_SOFTIRQ in function net_tx_action(). Take advantage of this a use the skb defer and flush API, as we are already in softirq context.

For modern drivers this rarely happens. Although most drivers do call dev_kfree_skb_any(), which detects the situation and calls __dev_kfree_skb_irq() when needed. This due to netpoll can call from IRQ context.

---
 include/linux/skbuff.h |    1 +
 net/core/dev.c         |    8 +++++++-
 net/core/skbuff.c      |    8 ++++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

index 3c8d348223d7..b06ba2e07c89 100644
@@ -2402,6 +2402,7 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 void napi_consume_skb(struct sk_buff *skb, int budget);
 
 void __kfree_skb_flush(void);
+void __kfree_skb_defer(struct sk_buff *skb);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index 73e6cbc10ac6..24be1d07d854 100644
@@ -3829,8 +3829,14 @@ static void net_tx_action(struct softirq_action *h)
 				trace_consume_skb(skb);
 			else
 				trace_kfree_skb(skb, net_tx_action);
-			__kfree_skb(skb);
+
+			if (skb->fclone != SKB_FCLONE_UNAVAILABLE)
+				__kfree_skb(skb);
+			else
+				__kfree_skb_defer(skb);
 		}
+
+		__kfree_skb_flush();
 	}
 
 	if (sd->output_queue) {

index e26bb2b1dba4..d278e51789e9 100644
@@ -767,7 +767,7 @@ void __kfree_skb_flush(void)
 	}
 }
 
-static void __kfree_skb_defer(struct sk_buff *skb)
+static inline void _kfree_skb_defer(struct sk_buff *skb)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
 

@@ -789,6 +789,10 @@ static void __kfree_skb_defer(struct sk_buff *skb)
 		nc->skb_count = 0;
 	}
 }
+void __kfree_skb_defer(struct sk_buff *skb)
+{
+	_kfree_skb_defer(skb);
+}
 
 void napi_consume_skb(struct sk_buff *skb, int budget)
 {

@@ -814,7 +818,7 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 		return;
 	}
 
-	__kfree_skb_defer(skb);
+	_kfree_skb_defer(skb);
 }
 EXPORT_SYMBOL(napi_consume_skb);
 

There is an opportunity to bulk free SKBs during reclaiming of resources after DMA transmit completes in ixgbe_clean_tx_irq. Thus, bulk freeing at this point does not introduce any added latency.

Simply use napi_consume_skb() which were recently introduced. The napi_budget parameter is needed by napi_consume_skb() to detect if it is called from netpoll.

Benchmarking IPv4-forwarding, on CPU i7-4790K @4.2GHz (no turbo boost)
Single CPU/flow numbers: before: 1982144 pps -> after : 2064446 pps
Improvement: +82302 pps, -20 nanosec, +4.1%
(SLUB and GCC version 5.1.1 20150618 (Red Hat 5.1.1-4))

Joint work with Alexander Duyck.

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index c4003a88bbf6..0c701b8438b6 100644
@@ -1089,7 +1089,7 @@ static void ixgbe_tx_timeout_reset(struct ixgbe_adapter *adapter)
  * @tx_ring: tx ring to clean
  **/
 static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
-			       struct ixgbe_ring *tx_ring)
+			       struct ixgbe_ring *tx_ring, int napi_budget)
 {
 	struct ixgbe_adapter *adapter = q_vector->adapter;
 	struct ixgbe_tx_buffer *tx_buffer;

@@ -1127,7 +1127,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 		total_packets += tx_buffer->gso_segs;
 
 		/* free the skb */
-		dev_consume_skb_any(tx_buffer->skb);
+		napi_consume_skb(tx_buffer->skb, napi_budget);
 
 		/* unmap skb header data */
 		dma_unmap_single(tx_ring->dev,

@@ -2784,7 +2784,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
 #endif
 
 	ixgbe_for_each_ring(ring, q_vector->tx)
-		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring);
+		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring, budget);
 
 	/* Exit if we are called by netpoll or busy polling is active */
 	if ((budget <= 0) || !ixgbe_qv_lock_napi(q_vector))

Think twice before applying
- This patch can potentially introduce added latency in some workloads

This patch introduce bulk alloc of SKBs and allow reuse of SKBs free'ed in same softirq cycle. SKBs are normally free'ed during TX completion, but most high speed drivers also cleanup TX ring during NAPI RX poll cycle. Thus, if using napi_consume_skb/__kfree_skb_defer, SKBs will be avail in the napi_alloc_cache->skb_cache.

If no SKBs are avail for reuse, then only bulk alloc 8 SKBs, to limit the potential overshooting unused SKBs needed to free'ed when NAPI cycle ends (flushed in net_rx_action via __kfree_skb_flush()).

Benchmarking IPv4-forwarding, on CPU i7-4790K @4.2GHz (no turbo boost) (GCC version 5.1.1 20150618 (Red Hat 5.1.1-4))
Allocator SLUB:
Single CPU/flow numbers: before: 2064446 pps -> after: 2083031 pps
Improvement: +18585 pps, -4.3 nanosec, +0.9%
Allocator SLAB:
Single CPU/flow numbers: before: 2035949 pps -> after: 2033567 pps
Regression: -2382 pps, +0.57 nanosec, -0.1 %

Even-though benchmarking does show an improvement for SLUB(+0.9%), I'm not convinced bulk alloc will be a win in all situations:
* I see stalls on walking the SLUB freelist (normal hidden by prefetch)
* In case RX queue is not full, alloc and free more SKBs than needed

More testing is needed with more real life benchmarks.

Joint work with Alexander Duyck.

---
 net/core/skbuff.c |   35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

index d278e51789e9..ae8cdbec90ee 100644
@@ -483,13 +483,14 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 				 gfp_t gfp_mask)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
+	struct skb_shared_info *shinfo;
 	struct sk_buff *skb;
 	void *data;
 
 	len += NET_SKB_PAD + NET_IP_ALIGN;
 
-	if ((len > SKB_WITH_OVERHEAD(PAGE_SIZE)) ||
-	    (gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA))) {
+	if (unlikely((len > SKB_WITH_OVERHEAD(PAGE_SIZE)) ||
+		     (gfp_mask & (__GFP_DIRECT_RECLAIM | GFP_DMA)))) {
 		skb = __alloc_skb(len, gfp_mask, SKB_ALLOC_RX, NUMA_NO_NODE);
 		if (!skb)
 			goto skb_fail;

@@ -506,12 +507,38 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 	if (unlikely(!data))
 		return NULL;
 
-	skb = __build_skb(data, len);
-	if (unlikely(!skb)) {
+#define BULK_ALLOC_SIZE 8
+	if (!nc->skb_count) {
+		nc->skb_count = kmem_cache_alloc_bulk(skbuff_head_cache,
+						      gfp_mask, BULK_ALLOC_SIZE,
+						      nc->skb_cache);
+	}
+	if (likely(nc->skb_count)) {
+		skb = (struct sk_buff *)nc->skb_cache[--nc->skb_count];
+	} else {
+		/* alloc bulk failed */
 		skb_free_frag(data);
 		return NULL;
 	}
 
+	len -= SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+	memset(skb, 0, offsetof(struct sk_buff, tail));
+	skb->truesize = SKB_TRUESIZE(len);
+	atomic_set(&skb->users, 1);
+	skb->head = data;
+	skb->data = data;
+	skb_reset_tail_pointer(skb);
+	skb->end = skb->tail + len;
+	skb->mac_header = (typeof(skb->mac_header))~0U;
+	skb->transport_header = (typeof(skb->transport_header))~0U;
+
+	/* make sure we initialize shinfo sequentially */
+	shinfo = skb_shinfo(skb);
+	memset(shinfo, 0, offsetof(struct skb_shared_info, dataref));
+	atomic_set(&shinfo->dataref, 1);
+	kmemcheck_annotate_variable(shinfo->destructor_arg);
+
 	/* use OR instead of assignment to avoid clearing of bits in mask */
 	if (nc->page.pfmemalloc)
 		skb->pfmemalloc = 1;

...

Guess should have create a helper for the basic skb setup. I just kept it here, as I was planning to do trick like removing the memset, as discussed below.

Great, hope they can go into net-next soon.

I have many consideration on the SKB memset, as it shows up quite high in profiles, how we can either 1) avoid clearing this much, or 2) speed up clearing by clearing more (full pages in allocator).

My idea (2) is about clearing larger memory chunks, as the memset/rep-stos operation have a fixed startup cost. But do it inside the slab allocator's bulk alloc call. During bulk alloc we can identify objects that are "next-to-each-other" and exec one rep-stos operation. My measurements show we need 3x 256-byte object's cleared together before it is a win.

*BUT* I actually like your idea better, below, of delaying the init ... more comments below.

I could prefetch the next elem in skb_cache[]. I have been playing with that. It didn't have much effect, as the bulk alloc (at least for SLUB) have already walked the memory. It helped a little to prefetch the 3rd cache-line, on the memset speed... (but so small that is was difficult to measure with enough confidence).

I actually like this idea better than my own. Of delaying memset init of the SKB. (I didn't understood it, until after you rephrased it ;-))

The drivers also need some extra fields, like hash and some flags. But we could easily organize sk_buff, to account for this.

I also like it because, delaying it until napi_gro_receive, we might have a chance to avoid the clearing, e.g. if GRO can merge it, or for other fastpath forwarding, we have not invented yet. (Thinking out loud, maybe even RPS could delay the memset, until it reach the remote CPU, saving may cache-line transfers between the CPUs).

I think I like yours delayed memset idea the best. Lets see if others shoot it down? ;-)

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

Bulk alloc and free of SKBs happen transparently by the API calls napi_alloc_skb() and napi_consume_skb().

The mlx5 driver have an extra high benefit of these changes, because it already have a loop refilling its RX ring queue. I considered if the alloc API should be allowed to request larger allocation, knowing the size of objects it need to refill. For now, just use the default bulk size hidden inside napi_alloc_skb().

---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c   |    9 +++++----
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c   |    4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    4 ++--
 4 files changed, 11 insertions(+), 10 deletions(-)

index 9ea49a893323..3e531fae9ed3 100644
@@ -591,9 +591,9 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 void mlx5e_completion_event(struct mlx5_core_cq *mcq);
 void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq);
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
-bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq);
+bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
 
 void mlx5e_update_stats(struct mlx5e_priv *priv);

index dd959d929aad..e923f4adc0f8 100644
@@ -42,12 +42,13 @@ static inline bool mlx5e_rx_hw_stamp(struct mlx5e_tstamp *tstamp)
 }
 
 static inline int mlx5e_alloc_rx_wqe(struct mlx5e_rq *rq,
-				     struct mlx5e_rx_wqe *wqe, u16 ix)
+				     struct mlx5e_rx_wqe *wqe, u16 ix,
+				     struct napi_struct *napi)
 {
 	struct sk_buff *skb;
 	dma_addr_t dma_addr;
 
-	skb = netdev_alloc_skb(rq->netdev, rq->wqe_sz);
+	skb = napi_alloc_skb(napi, rq->wqe_sz);
 	if (unlikely(!skb))
 		return -ENOMEM;
 

@@ -76,7 +77,7 @@ err_free_skb:
 	return -ENOMEM;
 }
 
-bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq)
+bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi)
 {
 	struct mlx5_wq_ll *wq = &rq->wq;
 

@@ -86,7 +87,7 @@ bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq)
 	while (!mlx5_wq_ll_is_full(wq)) {
 		struct mlx5e_rx_wqe *wqe = mlx5_wq_ll_get_wqe(wq, wq->head);
 
-		if (unlikely(mlx5e_alloc_rx_wqe(rq, wqe, wq->head)))
+		if (unlikely(mlx5e_alloc_rx_wqe(rq, wqe, wq->head, napi)))
 			break;
 
 		mlx5_wq_ll_push(wq, be16_to_cpu(wqe->next.next_wqe_index));

index 2c3fba0fff54..06a29c8f5712 100644
@@ -326,7 +326,7 @@ netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev)
 	return mlx5e_sq_xmit(sq, skb);
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq *sq;
 	u32 dma_fifo_cc;

@@ -402,7 +402,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq)
 			npkts++;
 			nbytes += wi->num_bytes;
 			sqcc += wi->num_wqebbs;
-			dev_kfree_skb(skb);
+			napi_consume_skb(skb, napi_budget);
 		} while (!last_wqe);
 	}
 

index 4ac8d716dbdd..8fd07c8087e3 100644
@@ -60,11 +60,11 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	clear_bit(MLX5E_CHANNEL_NAPI_SCHED, &c->flags);
 
 	for (i = 0; i < c->num_tc; i++)
-		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq);
+		busy |= mlx5e_poll_tx_cq(&c->sq[i].cq, budget);
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;
-	busy |= mlx5e_post_rx_wqes(&c->rq);
+	busy |= mlx5e_post_rx_wqes(&c->rq, napi);
 
 	if (busy)
 		return budget;

There are several techniques/concepts combined in this optimization. It is both a data-cache and instruction-cache optimization.

First of all, this is primarily about delaying touching packet-data, which happend in eth_type_trans, until the prefetch have had time to fetch. Thus, hopefully avoiding a cache-miss on packet data.

Secondly, the instruction-cache optimization is about, not calling the network stack for every packet, which is pulled out of the RX ring. Calling the full stack likely removes/flushes the instruction cache every time.

Thus, have two loops, one loop pulling out packet from the RX ring and starting the prefetching, and the second loop calling eth_type_trans() and invoking the stack via napi_gro_receive().

Notes:
This is the patch that gave a speed up of 6.2Mpps to 12Mpps, when trying to measure lowest RX level, by dropping the packets in the driver itself (marked drop point as comment).

For now, the ring is emptied upto the budget. I don't know if it would be better to chunk it up more?

In the future, I imagine the we can call the stack with the full SKB-list instead of this local loop. But then it would look a bit strange, to call eth_type_trans() as the only function...

---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

index e923f4adc0f8..5d96d6682db0 100644
@@ -214,8 +214,6 @@ static inline void mlx5e_build_rx_skb(struct mlx5_cqe64 *cqe,
 
 	mlx5e_handle_csum(netdev, cqe, rq, skb);
 
-	skb->protocol = eth_type_trans(skb, netdev);
-
 	skb_record_rx_queue(skb, rq->ix);
 
 	if (likely(netdev->features & NETIF_F_RXHASH))

@@ -229,8 +227,15 @@ static inline void mlx5e_build_rx_skb(struct mlx5_cqe64 *cqe,
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 {
 	struct mlx5e_rq *rq = container_of(cq, struct mlx5e_rq, cq);
+	struct sk_buff_head rx_skb_list;
+	struct sk_buff *rx_skb;
 	int work_done;
 
+	/* Using SKB list infrastructure, even-though some instructions
+	 * could be saved by open-coding it on skb->next directly.
+	 */
+	__skb_queue_head_init(&rx_skb_list);
+
 	/* avoid accessing cq (dma coherent memory) if not needed */
 	if (!test_and_clear_bit(MLX5E_CQ_HAS_CQES, &cq->flags))
 		return 0;

@@ -252,7 +257,6 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 		wqe_counter    = be16_to_cpu(wqe_counter_be);
 		wqe            = mlx5_wq_ll_get_wqe(&rq->wq, wqe_counter);
 		skb            = rq->skb[wqe_counter];
-		prefetch(skb->data);
 		rq->skb[wqe_counter] = NULL;
 
 		dma_unmap_single(rq->pdev,

@@ -265,16 +269,27 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 			dev_kfree_skb(skb);
 			goto wq_ll_pop;
 		}
+		prefetch(skb->data);
 
 		mlx5e_build_rx_skb(cqe, rq, skb);
 		rq->stats.packets++;
-		napi_gro_receive(cq->napi, skb);
+		__skb_queue_tail(&rx_skb_list, skb);
 
 wq_ll_pop:
 		mlx5_wq_ll_pop(&rq->wq, wqe_counter_be,
 			       &wqe->next.next_wqe_index);
 	}
 
+	while ((rx_skb = __skb_dequeue(&rx_skb_list)) != NULL) {
+		rx_skb->protocol = eth_type_trans(rx_skb, rq->netdev);
+		napi_gro_receive(cq->napi, rx_skb);
+
+		/* NOT FOR UPSTREAM INCLUSION:
+		 * How I did isolated testing of driver RX, I here called:
+		 *  napi_consume_skb(rx_skb, budget);
+		 */
+	}
+
 	mlx5_cqwq_update_db_record(&cq->wq);
 
 	/* ensure cq space is freed before enabling more cqes */

Indeed looks very promising in respect of instruction-cache optimization, but i have some doubts regarding the data-cache optimizations (prefetch), please see my below questions.

We will take this patch and test it in house.

Not sure, according to netdevice.h :

/* Default NAPI poll() weight
* Device drivers are strongly advised to not use bigger value */
#define NAPI_POLL_WEIGHT 64

we will also compare different budget values with your approach, but I doubt it will be accepted to increase the NAPI_POLL_WEIGHT for mlx5 drivers.
furthermore increasing NAPI poll budget might cause cache overflow with this approach since you are chunking up all "prefetch(skb->data)" (I didn't do the math yet in regards of cache utilization with this approach).

You misunderstood me... I don't want to increase the NAPI_POLL_WEIGHT. I want to keep the 64, but sort of split it up, and e.g. call the stack for each 16 packets. Due to cache-size limits...

One approach could be to compare the HW skb->hash to prev packet, and exit loop if they don't match (and call netstack with this bundle).

Okay, understood. We should look into this too, but not as top priority. We can simply move mlx5e_handle_csum() like eth_type_trans().

For some CPU ARCHs the prefetch is compile time removed.

That is not the problem, using 64 cache-lines * 64 = 4096 bytes. The BIOS/HW sometime also take next cache line => 8092 bytes. The problem is also SKB 4x cache-lines clearing = 16384 bytes.

We should of-cause keep this CPU independent, but for Intel SandyBridge CPUs the optimal prefetch loop size is likely 10.

Quote from Intels optimization manual:
The L1 DCache can handle multiple outstanding cache misses and continue to service incoming stores and loads. Up to 10 requests of missing cache lines can be managed simultaneously using the LFB (Line File Buffers).

Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

I see.

Sorry i am failing to see how this could help, either way you need an inner budget of 16 as you said before.

No, it is not that simple. mlx5e_handle_csum needs the cqe form the first loop, referencing back to the cqe in the second loop will might introduce new cache misses as the cqe is already "cold", what i like in your approach is that you separated between two different flows (read from device & create SKBs bundle --> pass bundle to netstack), now we don't want the "pass bundle to netstack" flow to look back at device's (cqes/wqes etc..).

Again this is not the main issue for now as it is not the common case, but we are preparing a patch that fixes the mlx5e_handle_csum to not look at skb->data at all, we will share it once it is ready.

The default bulk alloc size arbitrarily choosen (to be 8) might not suit all use-cases, this introduce a function napi_alloc_skb_hint() that allow the caller to specify a bulk size hint they are expecting. It is a hint because __napi_alloc_skb() limits the bulk size to the array size.

One user is the mlx5 driver, which bulk re-populate it's RX ring with both SKBs and pages. Thus, it would like to work with bigger bulk alloc chunks.

---
 include/linux/skbuff.h |   19 +++++++++++++++----
 net/core/skbuff.c      |    8 +++-----
 2 files changed, 18 insertions(+), 9 deletions(-)

index b06ba2e07c89..4d0c0eacbc34 100644
@@ -2391,14 +2391,25 @@ static inline void skb_free_frag(void *addr)
 	__free_page_frag(addr);
 }
 
+#define NAPI_SKB_CACHE_SIZE	64U /* Used in struct napi_alloc_cache */
+#define NAPI_SKB_BULK_ALLOC	 8U /* Default slab bulk alloc in NAPI */
+
 void *napi_alloc_frag(unsigned int fragsz);
-struct sk_buff *__napi_alloc_skb(struct napi_struct *napi,
-				 unsigned int length, gfp_t gfp_mask);
+struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
+				 unsigned int bulk_hint, gfp_t gfp_mask);
 static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
-					     unsigned int length)
+					     unsigned int len)
+{
+	return __napi_alloc_skb(napi, len, NAPI_SKB_BULK_ALLOC, GFP_ATOMIC);
+}
+static inline struct sk_buff *napi_alloc_skb_hint(struct napi_struct *napi,
+						  unsigned int len,
+						  unsigned int bulk_hint)
 {
-	return __napi_alloc_skb(napi, length, GFP_ATOMIC);
+	bulk_hint = bulk_hint ? : 1;
+	return __napi_alloc_skb(napi, len, bulk_hint, GFP_ATOMIC);
 }
+
 void napi_consume_skb(struct sk_buff *skb, int budget);
 
 void __kfree_skb_flush(void);

index ae8cdbec90ee..f77209fb5361 100644
@@ -347,8 +347,6 @@ struct sk_buff *build_skb(void *data, unsigned int frag_size)
 }
 EXPORT_SYMBOL(build_skb);
 
-#define NAPI_SKB_CACHE_SIZE	64
-
 struct napi_alloc_cache {
 	struct page_frag_cache page;
 	size_t skb_count;

@@ -480,9 +478,10 @@ EXPORT_SYMBOL(__netdev_alloc_skb);
  *	%NULL is returned if there is no free memory.
  */
 struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
-				 gfp_t gfp_mask)
+				 unsigned int bulk_hint, gfp_t gfp_mask)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
+	unsigned int bulk_sz = min(bulk_hint, NAPI_SKB_CACHE_SIZE);
 	struct skb_shared_info *shinfo;
 	struct sk_buff *skb;
 	void *data;

@@ -507,10 +506,9 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 	if (unlikely(!data))
 		return NULL;
 
-#define BULK_ALLOC_SIZE 8
 	if (!nc->skb_count) {
 		nc->skb_count = kmem_cache_alloc_bulk(skbuff_head_cache,
-						      gfp_mask, BULK_ALLOC_SIZE,
+						      gfp_mask, bulk_sz,
 						      nc->skb_cache);
 	}
 	if (likely(nc->skb_count)) {

Use the newly introduced napi_alloc_skb_hint() API, to get the underlying slab bulk allocation sizes to align with what mlx5 driver need for refilling its RX ring queue.

---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      |    3 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c   |   10 ++++++----
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c |    2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

index 3e531fae9ed3..b2aba498e8d4 100644
@@ -593,7 +593,8 @@ void mlx5e_cq_error_event(struct mlx5_core_cq *mcq, enum mlx5_event event);
 int mlx5e_napi_poll(struct napi_struct *napi, int budget);
 bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
 int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
-bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi);
+bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi,
+			unsigned int bulk_hint);
 struct mlx5_cqe64 *mlx5e_get_cqe(struct mlx5e_cq *cq);
 
 void mlx5e_update_stats(struct mlx5e_priv *priv);

index 5d96d6682db0..88f88d354abc 100644
@@ -43,12 +43,13 @@ static inline bool mlx5e_rx_hw_stamp(struct mlx5e_tstamp *tstamp)
 
 static inline int mlx5e_alloc_rx_wqe(struct mlx5e_rq *rq,
 				     struct mlx5e_rx_wqe *wqe, u16 ix,
-				     struct napi_struct *napi)
+				     struct napi_struct *napi,
+				     unsigned int bulk_hint)
 {
 	struct sk_buff *skb;
 	dma_addr_t dma_addr;
 
-	skb = napi_alloc_skb(napi, rq->wqe_sz);
+	skb = napi_alloc_skb_hint(napi, rq->wqe_sz, bulk_hint);
 	if (unlikely(!skb))
 		return -ENOMEM;
 

@@ -77,7 +78,8 @@ err_free_skb:
 	return -ENOMEM;
 }
 
-bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi)
+bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi,
+			unsigned int hint)
 {
 	struct mlx5_wq_ll *wq = &rq->wq;
 

@@ -87,7 +89,7 @@ bool mlx5e_post_rx_wqes(struct mlx5e_rq *rq, struct napi_struct *napi)
 	while (!mlx5_wq_ll_is_full(wq)) {
 		struct mlx5e_rx_wqe *wqe = mlx5_wq_ll_get_wqe(wq, wq->head);
 
-		if (unlikely(mlx5e_alloc_rx_wqe(rq, wqe, wq->head, napi)))
+		if (unlikely(mlx5e_alloc_rx_wqe(rq, wqe, wq->head, napi, hint)))
 			break;
 
 		mlx5_wq_ll_push(wq, be16_to_cpu(wqe->next.next_wqe_index));

index 8fd07c8087e3..6488404edff6 100644
@@ -64,7 +64,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 
 	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
 	busy |= work_done == budget;
-	busy |= mlx5e_post_rx_wqes(&c->rq, napi);
+	busy |= mlx5e_post_rx_wqes(&c->rq, napi, work_done);
 
 	if (busy)
 		return budget;

Normal TX completion uses napi_consume_skb(), thus also make dummy driver use this, as it make it easier to see the effect of bulk freeing SKBs.

---
 drivers/net/dummy.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 69fc8409a973..985565ec5d60 100644
@@ -85,7 +85,8 @@ static netdev_tx_t dummy_xmit(struct sk_buff *skb, struct net_device *dev)
 	dstats->tx_bytes += skb->len;
 	u64_stats_update_end(&dstats->syncp);
 
-	dev_kfree_skb(skb);
+	//dev_kfree_skb(skb);
+	napi_consume_skb(skb, 1);
 	return NETDEV_TX_OK;
 }
 

Introduce napi_gro_receive_list() which takes a full SKB-list for processing by the stack.

It also take over invoking eth_type_trans().

One purpose is to disconnect the icache usage/sharing between driver level RX (NAPI loop) and upper RX network stack.

Another advantage is that the stack now knows how many packets it received, and can do the appropiate packet bundling inside the stack. E.g. flush/process these bundles when the skb list is empty.

PITFALLS: Slightly overkill to use a struct sk_buff_head (24 bytes), to handover packets. Which is allocated on callers stack. It also maintains a qlen, which is unnecessary in this hotpath code. A simple list within the first SKB could be a minimum solution.

---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c |   12 +-----------
 include/linux/netdevice.h                       |    3 +++
 net/core/dev.c                                  |   18 ++++++++++++++++++
 3 files changed, 22 insertions(+), 11 deletions(-)

index 88f88d354abc..b6e7cc29f02c 100644
@@ -230,7 +230,6 @@ int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
 {
 	struct mlx5e_rq *rq = container_of(cq, struct mlx5e_rq, cq);
 	struct sk_buff_head rx_skb_list;
-	struct sk_buff *rx_skb;
 	int work_done;
 
 	/* Using SKB list infrastructure, even-though some instructions

@@ -281,16 +280,7 @@ wq_ll_pop:
 		mlx5_wq_ll_pop(&rq->wq, wqe_counter_be,
 			       &wqe->next.next_wqe_index);
 	}
-
-	while ((rx_skb = __skb_dequeue(&rx_skb_list)) != NULL) {
-		rx_skb->protocol = eth_type_trans(rx_skb, rq->netdev);
-		napi_gro_receive(cq->napi, rx_skb);
-
-		/* NOT FOR UPSTREAM INCLUSION:
-		 * How I did isolated testing of driver RX, I here called:
-		 *  napi_consume_skb(rx_skb, budget);
-		 */
-	}
+	napi_gro_receive_list(cq->napi, &rx_skb_list, rq->netdev);
 
 	mlx5_cqwq_update_db_record(&cq->wq);
 

index 5ac140dcb789..11df9af41a3c 100644
@@ -3142,6 +3142,9 @@ int netif_rx(struct sk_buff *skb);
 int netif_rx_ni(struct sk_buff *skb);
 int netif_receive_skb(struct sk_buff *skb);
 gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
+void napi_gro_receive_list(struct napi_struct *napi,
+			   struct sk_buff_head *skb_list,
+			   struct net_device *netdev);
 void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
 gro_result_t napi_gro_frags(struct napi_struct *napi);

index 24be1d07d854..35c92a968937 100644
@@ -4579,6 +4579,24 @@ gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(napi_gro_receive);
 
+void napi_gro_receive_list(struct napi_struct *napi,
+			   struct sk_buff_head *skb_list,
+			   struct net_device *netdev)
+{
+	struct sk_buff *skb;
+
+	while ((skb = __skb_dequeue(skb_list)) != NULL) {
+		skb->protocol = eth_type_trans(skb, netdev);
+
+		skb_mark_napi_id(skb, napi);
+		trace_napi_gro_receive_entry(skb);
+
+		skb_gro_reset_offset(skb);
+		napi_skb_finish(dev_gro_receive(napi, skb), skb);
+	}
+}
+EXPORT_SYMBOL(napi_gro_receive_list);
+
 static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
 {
 	if (unlikely(skb->pfmemalloc)) {

NEED TO CLEAN UP PATCH (likely still contains bugs...)

When enabling Receive Packet Steering (RPS) like :

echo 32768 > /proc/sys/net/core/rps_sock_flow_entries

for N in $(seq 0 7) ; do

    echo 4096 > /sys/class/net/${DEV}/queues/rx-$N/rps_flow_cnt
    echo f >  /sys/class/net/${DEV}/queues/rx-$N/rps_cpus
    grep -H . /sys/class/net/${DEV}/queues/rx-$N/rps_cpus

done

I noticed high contention on enqueue_to_backlog(). To mitigate this introduce enqueue_list_to_backlog(), which allow to enqueue an entire skb list, instead of having to take the per packet cost.

The skb list's needed for bulk enqueing are constructed in the per CPU area of softnet_data. And thus, can be constructed without heavy CPU synchronization.

I'm excited about the performance improvement of this patch:

Before:
Ethtool(mlx5p2 ) stat: 7246630 ( 7,246,630) <= rx3_packets /sec
After:
Ethtool(mlx5p2 ) stat: 9182886 ( 9,182,886) <= rx3_packets /sec
Improvement:
(1/9182886-1/7246630)*10^9 = saving -29.0 ns

The benchmark is a single pktgen flow, which is RPS directed to another CPU, for further processing via process_backlog(). The remote CPU cannot handle the load and this CPU drops packets when it cannot get them enqueued.

---
 include/linux/netdevice.h |   10 +++
 net/core/dev.c            |  133 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 140 insertions(+), 3 deletions(-)

index 11df9af41a3c..dc5baef95d27 100644
@@ -633,6 +633,15 @@ struct rps_dev_flow {
 };
 #define RPS_NO_FILTER 0xffff
 
+struct rps_cpu_queue {
+	struct sk_buff_head skb_list;
+	int to_cpu;
+	struct rps_dev_flow *rflow;
+	struct net_device *dev;
+};
+#define RPS_CPU_QUEUES		2 /* Must be power of 2 */
+#define RPS_CPU_QUEUES_MASK	(RPS_CPU_QUEUES - 1)
+
 /*
  * The rps_dev_flow_table structure contains a table of flow mappings.
  */

@@ -2662,6 +2671,7 @@ struct softnet_data {
 	unsigned int		received_rps;
 #ifdef CONFIG_RPS
 	struct softnet_data	*rps_ipi_list;
+	struct rps_cpu_queue	local_rps_queue[RPS_CPU_QUEUES];
 #endif
 #ifdef CONFIG_NET_FLOW_LIMIT
 	struct sd_flow_limit __rcu *flow_limit;

From: Jesper Dangaard Brouer <[email protected]>
Date: Tue, 02 Feb 2016 22:11:28 +0100

Very exciting work, indeed!

Jesper, why don't you resubmit just the first three patches as a group? They seem a lot less controversial and as Alexei said, stand on their own.

Thanks.

This patchset is the first real use-case for kmem_cache bulk _free_. The use of bulk _alloc_ is NOT included in this patchset. The full use have previously been posted here [1].

The bulk free side have the largest benefit for the network stack use-case, because network stack is hitting the kmem_cache/SLUB slowpath when freeing SKBs, due to the amount of outstanding SKBs. This is solved by using the new API kmem_cache_free_bulk().

Introduce new API napi_consume_skb(), that hides/handles bulk freeing for the caller. The drivers simply need to use this call when freeing SKBs in NAPI context, e.g. replacing their calles to dev_kfree_skb() / dev_consume_skb_any().

Driver ixgbe is the first user of this new API.

[1] http://thread.gmane.org/gmane.linux.network/384302/focus=397373

---

Jesper Dangaard Brouer (3):
net: bulk free infrastructure for NAPI context, use napi_consume_skb
net: bulk free SKBs that were delay free'ed due to IRQ context
ixgbe: bulk free SKBs during TX completion cleanup cycle

 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 +-
 include/linux/skbuff.h                        |    4 +
 net/core/dev.c                                |    9 ++-
 net/core/skbuff.c                             |   87 +++++++++++++++++++++++--
 4 files changed, 96 insertions(+), 10 deletions(-)

Discovered that network stack were hitting the kmem_cache/SLUB slowpath when freeing SKBs. Doing bulk free with kmem_cache_free_bulk can speedup this slowpath.

NAPI context is a bit special, lets take advantage of that for bulk free'ing SKBs.

In NAPI context we are running in softirq, which gives us certain protection. A softirq can run on several CPUs at once. BUT the important part is a softirq will never preempt another softirq running on the same CPU. This gives us the opportunity to access per-cpu variables in softirq context.

Extend napi_alloc_cache (before only contained page_frag_cache) to be a struct with a small array based stack for holding SKBs. Introduce a SKB defer and flush API for accessing this.

Introduce napi_consume_skb() as replacement for e.g. dev_consume_skb_any() when running in NAPI context. A small trick to handle/detect if we are called from netpoll is to see if budget is 0. In that case, we need to invoke dev_consume_skb_irq().

Joint work with Alexander Duyck.

---
 include/linux/skbuff.h |    3 ++
 net/core/dev.c         |    1 +
 net/core/skbuff.c      |   83 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 6 deletions(-)

index 11f935c1a090..3c8d348223d7 100644
@@ -2399,6 +2399,9 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 {
 	return __napi_alloc_skb(napi, length, GFP_ATOMIC);
 }
+void napi_consume_skb(struct sk_buff *skb, int budget);
+
+void __kfree_skb_flush(void);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index 8cba3d852f25..44384a8c9613 100644
@@ -5152,6 +5152,7 @@ static void net_rx_action(struct softirq_action *h)
 		}
 	}
 
+	__kfree_skb_flush();
 	local_irq_disable();
 
 	list_splice_tail_init(&sd->poll_list, &list);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

The network stack defers SKBs free, in-case free happens in IRQ or when IRQs are disabled. This happens in __dev_kfree_skb_irq() that writes SKBs that were free'ed during IRQ to the softirq completion queue (softnet_data.completion_queue).

These SKBs are naturally delayed, and cleaned up during NET_TX_SOFTIRQ in function net_tx_action(). Take advantage of this a use the skb defer and flush API, as we are already in softirq context.

For modern drivers this rarely happens. Although most drivers do call dev_kfree_skb_any(), which detects the situation and calls __dev_kfree_skb_irq() when needed. This due to netpoll can call from IRQ context.

---
 include/linux/skbuff.h |    1 +
 net/core/dev.c         |    8 +++++++-
 net/core/skbuff.c      |    8 ++++++--
 3 files changed, 14 insertions(+), 3 deletions(-)

index 3c8d348223d7..b06ba2e07c89 100644
@@ -2402,6 +2402,7 @@ static inline struct sk_buff *napi_alloc_skb(struct napi_struct *napi,
 void napi_consume_skb(struct sk_buff *skb, int budget);
 
 void __kfree_skb_flush(void);
+void __kfree_skb_defer(struct sk_buff *skb);
 
 /**
  * __dev_alloc_pages - allocate page for network Rx

index 44384a8c9613..b185d7eaa2e4 100644
@@ -3829,8 +3829,14 @@ static void net_tx_action(struct softirq_action *h)
 				trace_consume_skb(skb);
 			else
 				trace_kfree_skb(skb, net_tx_action);
-			__kfree_skb(skb);
+
+			if (skb->fclone != SKB_FCLONE_UNAVAILABLE)
+				__kfree_skb(skb);
+			else
+				__kfree_skb_defer(skb);
 		}
+
+		__kfree_skb_flush();
 	}
 
 	if (sd->output_queue) {

index e26bb2b1dba4..d278e51789e9 100644
@@ -767,7 +767,7 @@ void __kfree_skb_flush(void)
 	}
 }
 
-static void __kfree_skb_defer(struct sk_buff *skb)
+static inline void _kfree_skb_defer(struct sk_buff *skb)
 {
 	struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
 

@@ -789,6 +789,10 @@ static void __kfree_skb_defer(struct sk_buff *skb)
 		nc->skb_count = 0;
 	}
 }
+void __kfree_skb_defer(struct sk_buff *skb)
+{
+	_kfree_skb_defer(skb);
+}
 
 void napi_consume_skb(struct sk_buff *skb, int budget)
 {

@@ -814,7 +818,7 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
 		return;
 	}
 
-	__kfree_skb_defer(skb);
+	_kfree_skb_defer(skb);
 }
 EXPORT_SYMBOL(napi_consume_skb);
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

There is an opportunity to bulk free SKBs during reclaiming of resources after DMA transmit completes in ixgbe_clean_tx_irq. Thus, bulk freeing at this point does not introduce any added latency.

Simply use napi_consume_skb() which were recently introduced. The napi_budget parameter is needed by napi_consume_skb() to detect if it is called from netpoll.

Benchmarking IPv4-forwarding, on CPU i7-4790K @4.2GHz (no turbo boost)
Single CPU/flow numbers: before: 1982144 pps -> after : 2064446 pps
Improvement: +82302 pps, -20 nanosec, +4.1%
(SLUB and GCC version 5.1.1 20150618 (Red Hat 5.1.1-4))

Joint work with Alexander Duyck.

---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index c4003a88bbf6..0c701b8438b6 100644
@@ -1089,7 +1089,7 @@ static void ixgbe_tx_timeout_reset(struct ixgbe_adapter *adapter)
  * @tx_ring: tx ring to clean
  **/
 static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
-			       struct ixgbe_ring *tx_ring)
+			       struct ixgbe_ring *tx_ring, int napi_budget)
 {
 	struct ixgbe_adapter *adapter = q_vector->adapter;
 	struct ixgbe_tx_buffer *tx_buffer;

@@ -1127,7 +1127,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector,
 		total_packets += tx_buffer->gso_segs;
 
 		/* free the skb */
-		dev_consume_skb_any(tx_buffer->skb);
+		napi_consume_skb(tx_buffer->skb, napi_budget);
 
 		/* unmap skb header data */
 		dma_unmap_single(tx_ring->dev,

@@ -2784,7 +2784,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
 #endif
 
 	ixgbe_for_each_ring(ring, q_vector->tx)
-		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring);
+		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring, budget);
 
 	/* Exit if we are called by netpoll or busy polling is active */
 	if ((budget <= 0) || !ixgbe_qv_lock_napi(q_vector))

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

From: Jesper Dangaard Brouer <[email protected]>
Date: Mon, 08 Feb 2016 13:14:54 +0100

Series applied, thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi Jesper,

Am 08.02.2016 um 13:14 schrieb Jesper Dangaard Brouer:

Would you mind adding a kerneldoc comment for the new API function?

Thanks,
Tilman

Tilman Schmidt                              E-Mail: [email protected]
Bonn, Germany
Nous, on a des fleurs et des bougies pour nous protéger.