From: Matteo Croce <[email protected]>
This is a respin of [1]
This patchset shows the plans for allowing page_pool to handle and
maintain DMA map/unmap of the pages it serves to the driver. For this
to work a return hook in the network core is introduced.
The overall purpose is to simplify drivers, by providing a page
allocation API that does recycling, such that each driver doesn't have
to reinvent its own recycling scheme. Using page_pool in a driver
does not require implementing XDP support, but it makes it trivially
easy to do so. Instead of allocating buffers specifically for SKBs
we now allocate a generic buffer and either wrap it on an SKB
(via build_skb) or create an XDP frame.
The recycling code leverages the XDP recycle APIs.
The Marvell mvpp2 and mvneta drivers are used in this patchset to
demonstrate how to use the API, and tested on a MacchiatoBIN
and EspressoBIN boards respectively.
[1] https://lore.kernel.org/netdev/154413868810.21735.572808840657728172.stgit@firesoul/
v1 -> v2:
- fix a commit message
- avoid setting pp_recycle multiple times on mvneta
- squash two patches to avoid breaking bisect
Ilias Apalodimas (1):
page_pool: Allow drivers to hint on SKB recycling
Jesper Dangaard Brouer (1):
xdp: reduce size of struct xdp_mem_info
Matteo Croce (3):
mm: add a signature in struct page
mvpp2: recycle buffers
mvneta: recycle buffers
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/marvell/mvneta.c | 7 ++-
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +++----
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
include/linux/mm_types.h | 1 +
include/linux/skbuff.h | 35 ++++++++++++--
include/net/page_pool.h | 15 ++++++
include/net/xdp.h | 5 +-
net/core/page_pool.c | 47 +++++++++++++++++++
net/core/skbuff.c | 20 +++++++-
net/core/xdp.c | 14 ++++--
net/tls/tls_device.c | 2 +-
13 files changed, 142 insertions(+), 27 deletions(-)
--
2.30.2
From: Ilias Apalodimas <[email protected]>
Up to now several high speed NICs have custom mechanisms of recycling
the allocated memory they use for their payloads.
Our page_pool API already has recycling capabilities that are always
used when we are running in 'XDP mode'. So let's tweak the API and the
kernel network stack slightly and allow the recycling to happen even
during the standard operation.
The API doesn't take into account 'split page' policies used by those
drivers currently, but can be extended once we have users for that.
The idea is to be able to intercept the packet on skb_release_data().
If it's a buffer coming from our page_pool API recycle it back to the
pool for further usage or just release the packet entirely.
To achieve that we introduce a bit in struct sk_buff (pp_recycle:1) and
store the xdp_mem_info in page->private. Storing the information in
page->private allows us to recycle both SKBs and their fragments.
The SKB bit is needed for a couple of reasons. First of all in an
effort to affect the free path as less as possible, reading a single bit,
is better that trying to derive identical information for the page stored
data. Moreover page->private is used by skb_copy_ubufs. We do have a
special mark in the page, that won't allow this to happen, but again
deciding without having to read the entire page is preferable.
The driver has to take care of the sync operations on it's own
during the buffer recycling since the buffer is, after opting-in to the
recycling, never unmapped.
Since the gain on the drivers depends on the architecture, we are not
enabling recycling by default if the page_pool API is used on a driver.
In order to enable recycling the driver must call skb_mark_for_recycle()
to store the information we need for recycling in page->private and
enabling the recycling bit, or page_pool_store_mem_info() for a fragment
Since we added an extra argument on __skb_frag_unref() to handle
recycling, update the current users of the function with that.
Co-developed-by: Jesper Dangaard Brouer <[email protected]>
Co-developed-by: Matteo Croce <[email protected]>
Signed-off-by: Ilias Apalodimas <[email protected]>
---
.../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 2 +-
drivers/net/ethernet/marvell/sky2.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_rx.c | 2 +-
include/linux/skbuff.h | 35 ++++++++++++---
include/net/page_pool.h | 13 ++++++
include/net/xdp.h | 1 +
net/core/page_pool.c | 43 +++++++++++++++++++
net/core/skbuff.c | 20 ++++++++-
net/core/xdp.c | 6 +++
net/tls/tls_device.c | 2 +-
10 files changed, 115 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index 1115b8f9ea4e..8f815ebb59ae 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -2125,7 +2125,7 @@ static int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
/* clear the frag ref count which increased locally before */
for (i = 0; i < record->num_frags; i++) {
/* clear the frag ref count */
- __skb_frag_unref(&record->frags[i]);
+ __skb_frag_unref(&record->frags[i], false);
}
/* if any failure, come out from the loop. */
if (ret) {
diff --git a/drivers/net/ethernet/marvell/sky2.c b/drivers/net/ethernet/marvell/sky2.c
index 68c154d715d6..9dc25c4fb359 100644
--- a/drivers/net/ethernet/marvell/sky2.c
+++ b/drivers/net/ethernet/marvell/sky2.c
@@ -2503,7 +2503,7 @@ static void skb_put_frags(struct sk_buff *skb, unsigned int hdr_space,
if (length == 0) {
/* don't need this page */
- __skb_frag_unref(frag);
+ __skb_frag_unref(frag, false);
--skb_shinfo(skb)->nr_frags;
} else {
size = min(length, (unsigned) PAGE_SIZE);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index e35e4d7ef4d1..cea62b8f554c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -526,7 +526,7 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
fail:
while (nr > 0) {
nr--;
- __skb_frag_unref(skb_shinfo(skb)->frags + nr);
+ __skb_frag_unref(skb_shinfo(skb)->frags + nr, false);
}
return 0;
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c8def85fcc22..fffa6b399995 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -40,6 +40,9 @@
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
#include <linux/netfilter/nf_conntrack_common.h>
#endif
+#if IS_BUILTIN(CONFIG_PAGE_POOL)
+#include <net/page_pool.h>
+#endif
/* The interface for checksum offload between the stack and networking drivers
* is as follows...
@@ -247,6 +250,7 @@ struct napi_struct;
struct bpf_prog;
union bpf_attr;
struct skb_ext;
+struct xdp_mem_info;
#if IS_ENABLED(CONFIG_BRIDGE_NETFILTER)
struct nf_bridge_info {
@@ -667,6 +671,8 @@ typedef unsigned char *sk_buff_data_t;
* @head_frag: skb was allocated from page fragments,
* not allocated by kmalloc() or vmalloc().
* @pfmemalloc: skbuff was allocated from PFMEMALLOC reserves
+ * @pp_recycle: mark the packet for recycling instead of freeing (implies
+ * page_pool support on driver)
* @active_extensions: active extensions (skb_ext_id types)
* @ndisc_nodetype: router type (from link layer)
* @ooo_okay: allow the mapping of a socket to a queue to be changed
@@ -791,10 +797,12 @@ struct sk_buff {
fclone:2,
peeked:1,
head_frag:1,
- pfmemalloc:1;
+ pfmemalloc:1,
+ pp_recycle:1; /* page_pool recycle indicator */
#ifdef CONFIG_SKB_EXTENSIONS
__u8 active_extensions;
#endif
+
/* fields enclosed in headers_start/headers_end are copied
* using a single memcpy() in __copy_skb_header()
*/
@@ -3081,12 +3089,20 @@ static inline void skb_frag_ref(struct sk_buff *skb, int f)
/**
* __skb_frag_unref - release a reference on a paged fragment.
* @frag: the paged fragment
+ * @recycle: recycle the page if allocated via page_pool
*
- * Releases a reference on the paged fragment @frag.
+ * Releases a reference on the paged fragment @frag
+ * or recycles the page via the page_pool API.
*/
-static inline void __skb_frag_unref(skb_frag_t *frag)
+static inline void __skb_frag_unref(skb_frag_t *frag, bool recycle)
{
- put_page(skb_frag_page(frag));
+ struct page *page = skb_frag_page(frag);
+
+#if IS_BUILTIN(CONFIG_PAGE_POOL)
+ if (recycle && page_pool_return_skb_page(page_address(page)))
+ return;
+#endif
+ put_page(page);
}
/**
@@ -3098,7 +3114,7 @@ static inline void __skb_frag_unref(skb_frag_t *frag)
*/
static inline void skb_frag_unref(struct sk_buff *skb, int f)
{
- __skb_frag_unref(&skb_shinfo(skb)->frags[f]);
+ __skb_frag_unref(&skb_shinfo(skb)->frags[f], skb->pp_recycle);
}
/**
@@ -4696,5 +4712,14 @@ static inline u64 skb_get_kcov_handle(struct sk_buff *skb)
#endif
}
+#if IS_BUILTIN(CONFIG_PAGE_POOL)
+static inline void skb_mark_for_recycle(struct sk_buff *skb, struct page *page,
+ struct xdp_mem_info *mem)
+{
+ skb->pp_recycle = 1;
+ page_pool_store_mem_info(page, mem);
+}
+#endif
+
#endif /* __KERNEL__ */
#endif /* _LINUX_SKBUFF_H */
diff --git a/include/net/page_pool.h b/include/net/page_pool.h
index b30405e84b5e..75fffc15788b 100644
--- a/include/net/page_pool.h
+++ b/include/net/page_pool.h
@@ -65,6 +65,8 @@
#define PP_ALLOC_CACHE_REFILL 64
#define PP_SIGNATURE 0x20210303
+struct xdp_mem_info;
+
struct pp_alloc_cache {
u32 count;
void *cache[PP_ALLOC_CACHE_SIZE];
@@ -148,6 +150,8 @@ inline enum dma_data_direction page_pool_get_dma_dir(struct page_pool *pool)
return pool->p.dma_dir;
}
+bool page_pool_return_skb_page(void *data);
+
struct page_pool *page_pool_create(const struct page_pool_params *params);
#ifdef CONFIG_PAGE_POOL
@@ -243,4 +247,13 @@ static inline void page_pool_ring_unlock(struct page_pool *pool)
spin_unlock_bh(&pool->ring.producer_lock);
}
+/* Store mem_info on struct page and use it while recycling skb frags */
+static inline
+void page_pool_store_mem_info(struct page *page, struct xdp_mem_info *mem)
+{
+ u32 *xmi = (u32 *)mem;
+
+ set_page_private(page, *xmi);
+}
+
#endif /* _NET_PAGE_POOL_H */
diff --git a/include/net/xdp.h b/include/net/xdp.h
index c35864d59113..5d7316f1f195 100644
--- a/include/net/xdp.h
+++ b/include/net/xdp.h
@@ -235,6 +235,7 @@ void xdp_return_buff(struct xdp_buff *xdp);
void xdp_flush_frame_bulk(struct xdp_frame_bulk *bq);
void xdp_return_frame_bulk(struct xdp_frame *xdpf,
struct xdp_frame_bulk *bq);
+void xdp_return_skb_frame(void *data, struct xdp_mem_info *mem);
/* When sending xdp_frame into the network stack, then there is no
* return point callback, which is needed to release e.g. DMA-mapping
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 2ae9b554ef98..43bfd2e3d8df 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/device.h>
+#include <linux/skbuff.h>
#include <net/page_pool.h>
#include <net/xdp.h>
@@ -17,12 +18,19 @@
#include <linux/dma-mapping.h>
#include <linux/page-flags.h>
#include <linux/mm.h> /* for __put_page() */
+#include <net/xdp.h>
#include <trace/events/page_pool.h>
#define DEFER_TIME (msecs_to_jiffies(1000))
#define DEFER_WARN_INTERVAL (60 * HZ)
+/* Used to store/retrieve hi/lo bytes from xdp_mem_info to page->private */
+union page_pool_xmi {
+ u32 raw;
+ struct xdp_mem_info mem_info;
+};
+
static int page_pool_init(struct page_pool *pool,
const struct page_pool_params *params)
{
@@ -587,3 +595,38 @@ void page_pool_update_nid(struct page_pool *pool, int new_nid)
}
}
EXPORT_SYMBOL(page_pool_update_nid);
+
+bool page_pool_return_skb_page(void *data)
+{
+ struct xdp_mem_info mem_info;
+ union page_pool_xmi info;
+ struct page *page;
+
+ page = virt_to_head_page(data);
+ if (unlikely(page->signature != PP_SIGNATURE))
+ return false;
+
+ info.raw = page_private(page);
+ mem_info = info.mem_info;
+
+ /* If a buffer is marked for recycle and does not belong to
+ * MEM_TYPE_PAGE_POOL, the buffers will be eventually freed from the
+ * network stack and kfree_skb, but the DMA region will *not* be
+ * correctly unmapped. WARN here for the recycling misusage
+ */
+ if (unlikely(mem_info.type != MEM_TYPE_PAGE_POOL)) {
+ WARN_ONCE(true, "Tried to recycle non MEM_TYPE_PAGE_POOL");
+ return false;
+ }
+
+ /* Driver set this to memory recycling info. Reset it on recycle
+ * This will *not* work for NIC using a split-page memory model.
+ * The page will be returned to the pool here regardless of the
+ * 'flipped' fragment being in use or not
+ */
+ set_page_private(page, 0);
+ xdp_return_skb_frame(data, &mem_info);
+
+ return true;
+}
+EXPORT_SYMBOL(page_pool_return_skb_page);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e8320b5d651a..7f5c02085438 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -69,6 +69,9 @@
#include <net/xfrm.h>
#include <net/mpls.h>
#include <net/mptcp.h>
+#if IS_BUILTIN(CONFIG_PAGE_POOL)
+#include <net/page_pool.h>
+#endif
#include <linux/uaccess.h>
#include <trace/events/skb.h>
@@ -644,6 +647,11 @@ static void skb_free_head(struct sk_buff *skb)
{
unsigned char *head = skb->head;
+#if IS_BUILTIN(CONFIG_PAGE_POOL)
+ if (skb->pp_recycle && page_pool_return_skb_page(head))
+ return;
+#endif
+
if (skb->head_frag)
skb_free_frag(head);
else
@@ -663,7 +671,7 @@ static void skb_release_data(struct sk_buff *skb)
skb_zcopy_clear(skb, true);
for (i = 0; i < shinfo->nr_frags; i++)
- __skb_frag_unref(&shinfo->frags[i]);
+ __skb_frag_unref(&shinfo->frags[i], skb->pp_recycle);
if (shinfo->frag_list)
kfree_skb_list(shinfo->frag_list);
@@ -1045,6 +1053,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
n->nohdr = 0;
n->peeked = 0;
C(pfmemalloc);
+ C(pp_recycle);
n->destructor = NULL;
C(tail);
C(end);
@@ -3453,7 +3462,7 @@ int skb_shift(struct sk_buff *tgt, struct sk_buff *skb, int shiftlen)
fragto = &skb_shinfo(tgt)->frags[merge];
skb_frag_size_add(fragto, skb_frag_size(fragfrom));
- __skb_frag_unref(fragfrom);
+ __skb_frag_unref(fragfrom, skb->pp_recycle);
}
/* Reposition in the original skb */
@@ -5234,6 +5243,13 @@ bool skb_try_coalesce(struct sk_buff *to, struct sk_buff *from,
if (skb_cloned(to))
return false;
+ /* We can't coalesce skb that are allocated from slab and page_pool
+ * The recycle mark is on the skb, so that might end up trying to
+ * recycle slab allocated skb->head
+ */
+ if (to->pp_recycle != from->pp_recycle)
+ return false;
+
if (len <= skb_tailroom(to)) {
if (len)
BUG_ON(skb_copy_bits(from, 0, skb_put(to, len), len));
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 3dd47ed83778..d89b827e54a9 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -372,6 +372,12 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
}
}
+void xdp_return_skb_frame(void *data, struct xdp_mem_info *mem)
+{
+ __xdp_return(data, mem, false, NULL);
+}
+EXPORT_SYMBOL_GPL(xdp_return_skb_frame);
+
void xdp_return_frame(struct xdp_frame *xdpf)
{
__xdp_return(xdpf->data, &xdpf->mem, false, NULL);
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 790c6b7ecb26..8027a58c76a2 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -127,7 +127,7 @@ static void destroy_record(struct tls_record_info *record)
int i;
for (i = 0; i < record->num_frags; i++)
- __skb_frag_unref(&record->frags[i]);
+ __skb_frag_unref(&record->frags[i], false);
kfree(record);
}
--
2.30.2
From: Matteo Croce <[email protected]>
Use the new recycling API for page_pool.
In a drop rate test, the packet rate is more than doubled,
from 962 Kpps to 2047 Kpps.
perf top on a stock system shows:
Overhead Shared Object Symbol
30.67% [kernel] [k] page_pool_release_page
8.37% [kernel] [k] get_page_from_freelist
7.34% [kernel] [k] free_unref_page
6.47% [mvpp2] [k] mvpp2_rx
4.69% [kernel] [k] eth_type_trans
4.55% [kernel] [k] __netif_receive_skb_core
4.40% [kernel] [k] build_skb
4.29% [kernel] [k] kmem_cache_free
4.00% [kernel] [k] kmem_cache_alloc
3.81% [kernel] [k] dev_gro_receive
With packet rate stable at 962 Kpps:
tx: 0 bps 0 pps rx: 477.4 Mbps 962.6 Kpps
tx: 0 bps 0 pps rx: 477.6 Mbps 962.8 Kpps
tx: 0 bps 0 pps rx: 477.6 Mbps 962.9 Kpps
tx: 0 bps 0 pps rx: 477.2 Mbps 962.1 Kpps
tx: 0 bps 0 pps rx: 477.5 Mbps 962.7 Kpps
And this is the same output with recycling enabled:
Overhead Shared Object Symbol
12.75% [mvpp2] [k] mvpp2_rx
9.56% [kernel] [k] __netif_receive_skb_core
9.29% [kernel] [k] build_skb
9.27% [kernel] [k] eth_type_trans
8.39% [kernel] [k] kmem_cache_alloc
7.85% [kernel] [k] kmem_cache_free
7.36% [kernel] [k] page_pool_put_page
6.45% [kernel] [k] dev_gro_receive
4.72% [kernel] [k] __xdp_return
3.06% [kernel] [k] page_pool_refill_alloc_cache
With packet rate above 2000 Kpps:
tx: 0 bps 0 pps rx: 1015 Mbps 2046 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
tx: 0 bps 0 pps rx: 1015 Mbps 2047 Kpps
The major performance increase is explained by the fact that the most CPU
consuming functions (page_pool_release_page, get_page_from_freelist
and free_unref_page) are no longer called on a per packet basis.
The test was done by sending to the macchiatobin 64 byte ethernet frames
with an invalid ethertype, so the packets are dropped early in the RX path.
Signed-off-by: Matteo Croce <[email protected]>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index ec706d614cac..633b39cfef65 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3847,6 +3847,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
struct mvpp2_pcpu_stats ps = {};
enum dma_data_direction dma_dir;
struct bpf_prog *xdp_prog;
+ struct xdp_rxq_info *rxqi;
struct xdp_buff xdp;
int rx_received;
int rx_done = 0;
@@ -3912,15 +3913,15 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
else
frag_size = bm_pool->frag_size;
- if (xdp_prog) {
- struct xdp_rxq_info *xdp_rxq;
+ if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
+ rxqi = &rxq->xdp_rxq_short;
+ else
+ rxqi = &rxq->xdp_rxq_long;
- if (bm_pool->pkt_size == MVPP2_BM_SHORT_PKT_SIZE)
- xdp_rxq = &rxq->xdp_rxq_short;
- else
- xdp_rxq = &rxq->xdp_rxq_long;
+ if (xdp_prog) {
+ xdp.rxq = rxqi;
- xdp_init_buff(&xdp, PAGE_SIZE, xdp_rxq);
+ xdp_init_buff(&xdp, PAGE_SIZE, rxqi);
xdp_prepare_buff(&xdp, data,
MVPP2_MH_SIZE + MVPP2_SKB_HEADROOM,
rx_bytes, false);
@@ -3964,7 +3965,7 @@ static int mvpp2_rx(struct mvpp2_port *port, struct napi_struct *napi,
}
if (pp)
- page_pool_release_page(pp, virt_to_page(data));
+ skb_mark_for_recycle(skb, virt_to_page(data), &rxqi->mem);
else
dma_unmap_single_attrs(dev->dev.parent, dma_addr,
bm_pool->buf_size, DMA_FROM_DEVICE,
--
2.30.2
From: Matteo Croce <[email protected]>
Use the new recycling API for page_pool.
In a drop rate test, the packet rate increased di 10%,
from 269 Kpps to 296 Kpps.
perf top on a stock system shows:
Overhead Shared Object Symbol
21.78% [kernel] [k] __pi___inval_dcache_area
21.66% [mvneta] [k] mvneta_rx_swbm
7.00% [kernel] [k] kmem_cache_alloc
6.05% [kernel] [k] eth_type_trans
4.44% [kernel] [k] kmem_cache_free.part.0
3.80% [kernel] [k] __netif_receive_skb_core
3.68% [kernel] [k] dev_gro_receive
3.65% [kernel] [k] get_page_from_freelist
3.43% [kernel] [k] page_pool_release_page
3.35% [kernel] [k] free_unref_page
And this is the same output with recycling enabled:
Overhead Shared Object Symbol
24.10% [kernel] [k] __pi___inval_dcache_area
23.02% [mvneta] [k] mvneta_rx_swbm
7.19% [kernel] [k] kmem_cache_alloc
6.50% [kernel] [k] eth_type_trans
4.93% [kernel] [k] __netif_receive_skb_core
4.77% [kernel] [k] kmem_cache_free.part.0
3.93% [kernel] [k] dev_gro_receive
3.03% [kernel] [k] build_skb
2.91% [kernel] [k] page_pool_put_page
2.85% [kernel] [k] __xdp_return
The test was done with mausezahn on the TX side with 64 byte raw
ethernet frames.
Signed-off-by: Matteo Croce <[email protected]>
---
drivers/net/ethernet/marvell/mvneta.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index f20dfd1d7a6b..f88c189b60a4 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -2331,7 +2331,7 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
if (!skb)
return ERR_PTR(-ENOMEM);
- page_pool_release_page(rxq->page_pool, virt_to_page(xdp->data));
+ skb_mark_for_recycle(skb, virt_to_page(xdp->data), &xdp->rxq->mem);
skb_reserve(skb, xdp->data - xdp->data_hard_start);
skb_put(skb, xdp->data_end - xdp->data);
@@ -2343,7 +2343,10 @@ mvneta_swbm_build_skb(struct mvneta_port *pp, struct mvneta_rx_queue *rxq,
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
skb_frag_page(frag), skb_frag_off(frag),
skb_frag_size(frag), PAGE_SIZE);
- page_pool_release_page(rxq->page_pool, skb_frag_page(frag));
+ /* We don't need to reset pp_recycle here. It's already set, so
+ * just mark fragments for recycling.
+ */
+ page_pool_store_mem_info(skb_frag_page(frag), &xdp->rxq->mem);
}
return skb;
--
2.30.2
On Fri, 2 Apr 2021 20:17:31 +0200 Matteo Croce wrote:
> Co-developed-by: Jesper Dangaard Brouer <[email protected]>
> Co-developed-by: Matteo Croce <[email protected]>
> Signed-off-by: Ilias Apalodimas <[email protected]>
Checkpatch says we need sign-offs from all authors.
Especially you since you're posting.
On Fri, Apr 09, 2021 at 11:56:48AM -0700, Jakub Kicinski wrote:
> On Fri, 2 Apr 2021 20:17:31 +0200 Matteo Croce wrote:
> > Co-developed-by: Jesper Dangaard Brouer <[email protected]>
> > Co-developed-by: Matteo Croce <[email protected]>
> > Signed-off-by: Ilias Apalodimas <[email protected]>
>
> Checkpatch says we need sign-offs from all authors.
> Especially you since you're posting.
Yes it does, we forgot that. Let me take a chance on this one.
The patch is changing the default skb return path and while we've done enough
testing, I would really prefer this going in on a future -rc1 (assuming we even
consider merging it), allowing enough time to have wider tests.
Regards
/Ilias
On Fri, 9 Apr 2021 22:01:51 +0300 Ilias Apalodimas wrote:
> On Fri, Apr 09, 2021 at 11:56:48AM -0700, Jakub Kicinski wrote:
> > On Fri, 2 Apr 2021 20:17:31 +0200 Matteo Croce wrote:
> > > Co-developed-by: Jesper Dangaard Brouer <[email protected]>
> > > Co-developed-by: Matteo Croce <[email protected]>
> > > Signed-off-by: Ilias Apalodimas <[email protected]>
> >
> > Checkpatch says we need sign-offs from all authors.
> > Especially you since you're posting.
>
> Yes it does, we forgot that. Let me take a chance on this one.
> The patch is changing the default skb return path and while we've done enough
> testing, I would really prefer this going in on a future -rc1 (assuming we even
> consider merging it), allowing enough time to have wider tests.
Up to you guys. FWIW if you decide to try for 5.13 the missing signoffs
can be posted in replies, no need to repost.
On Fri, Apr 09, 2021 at 12:29:29PM -0700, Jakub Kicinski wrote:
> On Fri, 9 Apr 2021 22:01:51 +0300 Ilias Apalodimas wrote:
> > On Fri, Apr 09, 2021 at 11:56:48AM -0700, Jakub Kicinski wrote:
> > > On Fri, 2 Apr 2021 20:17:31 +0200 Matteo Croce wrote:
> > > > Co-developed-by: Jesper Dangaard Brouer <[email protected]>
> > > > Co-developed-by: Matteo Croce <[email protected]>
> > > > Signed-off-by: Ilias Apalodimas <[email protected]>
> > >
> > > Checkpatch says we need sign-offs from all authors.
> > > Especially you since you're posting.
> >
> > Yes it does, we forgot that. Let me take a chance on this one.
> > The patch is changing the default skb return path and while we've done enough
> > testing, I would really prefer this going in on a future -rc1 (assuming we even
> > consider merging it), allowing enough time to have wider tests.
>
> Up to you guys. FWIW if you decide to try for 5.13 the missing signoffs
> can be posted in replies, no need to repost.
Thanks! but...
I think I prefer another repost, including mm-people on the list as well (and
fixing SoB's).
I just noticed noone is cc'ed and patch [2/5] adds a line in mm_types.h
/Ilias
On Fri, 9 Apr 2021 22:01:51 +0300
Ilias Apalodimas <[email protected]> wrote:
> On Fri, Apr 09, 2021 at 11:56:48AM -0700, Jakub Kicinski wrote:
> > On Fri, 2 Apr 2021 20:17:31 +0200 Matteo Croce wrote:
> > > Co-developed-by: Jesper Dangaard Brouer <[email protected]>
> > > Co-developed-by: Matteo Croce <[email protected]>
> > > Signed-off-by: Ilias Apalodimas <[email protected]>
> >
> > Checkpatch says we need sign-offs from all authors.
> > Especially you since you're posting.
>
> Yes it does, we forgot that. Let me take a chance on this one.
> The patch is changing the default skb return path and while we've done enough
> testing, I would really prefer this going in on a future -rc1 (assuming we even
> consider merging it), allowing enough time to have wider tests.
You can have my:
Signed-off-by: Jesper Dangaard Brouer <[email protected]>
But as Ilias suggested in IRC lets send a V3, and Cc the MM-people, as
this also dig into their area.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer