net: stmmac: RX performance improvement
Subsystems (5)
cover net: stmmac: RX performance improvement
This series improves RX performance a lot, ~34% TCP RX throughput boost has been observed with DWXGMAC CORE 3.20a running on Cortex-A65 CPUs:
from 2.18 Gbits/sec increased to 2.92 Gbits/sec.
Furong Xu (3):
net: stmmac: Switch to zero-copy in non-XDP RX path
net: stmmac: Set page_pool_params.max_len to a precise size
net: stmmac: Optimize cache prefetch in RX path
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 32 +++++++++++-------- .../net/ethernet/stmicro/stmmac/stmmac_xdp.h | 1 - 3 files changed, 19 insertions(+), 15 deletions(-)
Signature · 1 line
2.34.1
PATCH 1/3 net: stmmac: Switch to zero-copy in non-XDP RX path
Avoid memcpy in non-XDP RX path by marking all allocated SKBs to be recycled in the upper network stack.
This patch brings ~11.5% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, from 2.18 Gbits/sec increased to 2.43 Gbits/sec.
--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +1 -0
@@ -126,6 +126,7 @@ struct stmmac_rx_queue {unsigned int cur_rx;unsigned int dirty_rx;unsigned int buf_alloc_num; +unsigned int napi_skb_frag_size;dma_addr_t dma_rx_phy;u32 rx_tail_addr;unsigned int state_saved;
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +14 -9
@@ -1320,7 +1320,7 @@ static unsigned int stmmac_rx_offset(struct stmmac_priv *priv)if (stmmac_xdp_is_enabled (priv))return XDP_PACKET_HEADROOM ; -return 0 ; +return NET_SKB_PAD ; }static int stmmac_set_bfsize (int mtu,int bufsize)
@@ -2019,17 +2019,21 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv,struct stmmac_channel *ch = &priv->channel[queue];bool xdp_prog =stmmac_xdp_is_enabled (priv);struct page_pool_params pp_params = {0 }; -unsigned int num_pages; +unsigned int dma_buf_sz_pad, num_pages;unsigned int napi_id;int ret; + dma_buf_sz_pad =stmmac_rx_offset (priv) + dma_conf->dma_buf_sz + +SKB_DATA_ALIGN (sizeof (struct skb_shared_info )); + num_pages =DIV_ROUND_UP (dma_buf_sz_pad,PAGE_SIZE ); + rx_q->queue_index = queue; rx_q->priv_data = priv; + rx_q->napi_skb_frag_size = num_pages *PAGE_SIZE ; pp_params.flags =PP_FLAG_DMA_MAP |PP_FLAG_DMA_SYNC_DEV ; pp_params.pool_size = dma_conf->dma_rx_size; - num_pages =DIV_ROUND_UP (dma_conf->dma_buf_sz,PAGE_SIZE ); - pp_params.order =ilog2 (num_pages); + pp_params.order =order_base_2 (num_pages); pp_params.nid =dev_to_node (priv->device); pp_params.dev = priv->device; pp_params.dma_dir = xdp_prog ?DMA_BIDIRECTIONAL :DMA_FROM_DEVICE ;
@@ -5574,19 +5578,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)/* XDP program may expand or reduce tail */ buf1_len = ctx.xdp.data_end - ctx.xdp.data; - skb =napi_alloc_skb (&ch->rx_napi, buf1_len); + skb =napi_build_skb (page_address (buf->page), + rx_q->napi_skb_frag_size);if (!skb) { +page_pool_recycle_direct (rx_q->page_pool, + buf->page); rx_dropped++; count++; goto drain_data; }/* XDP program may adjust header */ -skb_copy_to_linear_data (skb, ctx.xdp.data, buf1_len); +skb_reserve (skb, ctx.xdp.data - ctx.xdp.data_hard_start);skb_put (skb, buf1_len); - -/* Data payload copied into SKB, page ready for recycle */ -page_pool_recycle_direct (rx_q->page_pool, buf->page); +skb_mark_for_recycle (skb); buf->page =NULL ; }else if (buf1_len) { dma_sync_single_for_cpu(priv->device, buf->addr,
Signature · 1 line
2.34.1
PATCH 2/3 net: stmmac: Set page_pool_params.max_len to a precise size
In reply to Furong XuDMA engine will always write no more than dma_buf_sz bytes of a received frame into a page buffer, the remaining spaces are unused or used by CPU exclusively.
Setting page_pool_params.max_len to almost the full size of page(s) helps nothing more, but wastes more CPU cycles on cache maintenance.
For a standard MTU of 1500, then dma_buf_sz is assigned to 1536, and this patch brings ~16.9% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, from 2.43 Gbits/sec increased to 2.84 Gbits/sec.
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1 -1
@@ -2038,7 +2038,7 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv, pp_params.dev = priv->device; pp_params.dma_dir = xdp_prog ?DMA_BIDIRECTIONAL :DMA_FROM_DEVICE ; pp_params.offset =stmmac_rx_offset (priv); - pp_params.max_len =STMMAC_MAX_RX_BUF_SIZE (num_pages); + pp_params.max_len = dma_conf->dma_buf_sz; rx_q->page_pool =page_pool_create (&pp_params);if (IS_ERR (rx_q->page_pool)) {
drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.h +0 -1
@@ -4,7 +4,6 @@#ifndef _STMMAC_XDP_H_#define _STMMAC_XDP_H_ -#define STMMAC_MAX_RX_BUF_SIZE (num) (((num) * PAGE_SIZE) - XDP_PACKET_HEADROOM)#define STMMAC_RX_DMA_ATTR (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) int stmmac_xdp_setup_pool(struct stmmac_priv *priv, struct xsk_buff_pool *pool,
Signature · 1 line
2.34.1
PATCH 3/3 net: stmmac: Optimize cache prefetch in RX path
In reply to Furong XuCurrent code prefetches cache lines for the received frame first, and then dma_sync_single_for_cpu() against this frame, this is wrong. Cache prefetch should be triggered after dma_sync_single_for_cpu().
This patch brings ~2.8% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, 2.84 Gbits/sec increased to 2.92 Gbits/sec.
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3 -4
@@ -5497,10 +5497,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)/* Buffer is good. Go on. */ -prefetch (page_address (buf->page) + buf->page_offset); -if (buf->sec_page) -prefetch (page_address (buf->sec_page)); - buf1_len =stmmac_rx_buf1_len (priv, p, status, len); len += buf1_len; buf2_len =stmmac_rx_buf2_len (priv, p, status, len);
@@ -5522,6 +5518,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)dma_sync_single_for_cpu (priv->device, buf->addr, buf1_len, dma_dir); +prefetch (page_address (buf->page) + buf->page_offset);xdp_init_buff (&ctx.xdp, buf_sz, &rx_q->xdp_rxq); xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
@@ -5596,6 +5593,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) }else if (buf1_len) {dma_sync_single_for_cpu (priv->device, buf->addr, buf1_len, dma_dir); +prefetch (page_address (buf->page) + buf->page_offset);skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, buf->page, buf->page_offset, buf1_len, priv->dma_conf.dma_buf_sz);
@@ -5608,6 +5606,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)if (buf2_len) {dma_sync_single_for_cpu (priv->device, buf->sec_addr, buf2_len, dma_dir); +prefetch (page_address (buf->sec_page));skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, buf->sec_page,0 , buf2_len, priv->dma_conf.dma_buf_sz);
Signature · 1 line
2.34.1
Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in non-XDP RX path
In reply to Furong XuQuoted 71 lines · Furong Xu
Avoid memcpy in non-XDP RX path by marking all allocated SKBs to be recycled in the upper network stack.
This patch brings ~11.5% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, from 2.18 Gbits/sec increased to 2.43 Gbits/sec.
Signed-off-by Furong Xu <[email protected]>--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-)drivers/net/ethernet/stmicro/stmmac/stmmac.h +1
-0@@ -126,6 +126,7 @@ struct stmmac_rx_queue {unsigned int cur_rx;unsigned int dirty_rx;unsigned int buf_alloc_num; +unsigned int napi_skb_frag_size;dma_addr_t dma_rx_phy;u32 rx_tail_addr;unsigned int state_saved;drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +10
-5@@ -1320,7 +1320,7 @@ static unsigned int stmmac_rx_offset(struct stmmac_priv *priv)if (stmmac_xdp_is_enabled (priv))return XDP_PACKET_HEADROOM ; -return 0 ; +return NET_SKB_PAD ; }static int stmmac_set_bfsize (int mtu,int bufsize)@@ -2019,17 +2019,21 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv,struct stmmac_channel *ch = &priv->channel[queue];bool xdp_prog =stmmac_xdp_is_enabled (priv);struct page_pool_params pp_params = {0 }; -unsigned int num_pages; +unsigned int dma_buf_sz_pad, num_pages;unsigned int napi_id;int ret; + dma_buf_sz_pad =stmmac_rx_offset (priv) + dma_conf->dma_buf_sz + +SKB_DATA_ALIGN (sizeof (struct skb_shared_info )); + num_pages =DIV_ROUND_UP (dma_buf_sz_pad,PAGE_SIZE ); + rx_q->queue_index = queue; rx_q->priv_data = priv; + rx_q->napi_skb_frag_size = num_pages *PAGE_SIZE ; pp_params.flags =PP_FLAG_DMA_MAP |PP_FLAG_DMA_SYNC_DEV ; pp_params.pool_size = dma_conf->dma_rx_size; - num_pages =DIV_ROUND_UP (dma_conf->dma_buf_sz,PAGE_SIZE ); - pp_params.order =ilog2 (num_pages); + pp_params.order =order_base_2 (num_pages); pp_params.nid =dev_to_node (priv->device); pp_params.dev = priv->device; pp_params.dma_dir = xdp_prog ?DMA_BIDIRECTIONAL :DMA_FROM_DEVICE ;@@ -5574,19 +5578,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)/* XDP program may expand or reduce tail */ buf1_len = ctx.xdp.data_end - ctx.xdp.data; - skb =napi_alloc_skb (&ch->rx_napi, buf1_len); + skb =napi_build_skb (page_address (buf->page), + rx_q->napi_skb_frag_size);if (!skb) {
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) · line 5592
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3
-1+page_pool_recycle_direct (rx_q->page_pool, + buf->page); rx_dropped++; count++; goto drain_data; }/* XDP program may adjust header */ -skb_copy_to_linear_data (skb, ctx.xdp.data, buf1_len); +skb_reserve (skb, ctx.xdp.data - ctx.xdp.data_hard_start);
The network subsystem still requires that the length of each line of code should not exceed 80 characters. So let's silence the warning:
WARNING: line length of 81 exceeds 80 columns
#87: FILE: drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5592:
+ skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);
Thanks,
Yanteng
Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in non-XDP RX path
From: Yanteng Si <[email protected]>
Date: Mon, 13 Jan 2025 17:41:41 +0800
Quoted 85 lines · Furong Xu
Quoted 70 lines
Avoid memcpy in non-XDP RX path by marking all allocated SKBs to be recycled in the upper network stack.
This patch brings ~11.5% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, from 2.18 Gbits/sec increased to 2.43 Gbits/sec.
Signed-off-by Furong Xu <[email protected]>---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +.../net/ethernet/stmicro/stmmac/stmmac_main.c| 23 +++++++++++--------2 files changed, 15 insertions(+), 9 deletions(-)drivers/ renamed from drivers/net/ethernet/stmicro/stmmac/stmmac.h +0
-0
net/ethernet/stmicro/stmmac/stmmac.h
index548b28fed9b6..5c39292313de100644drivers/net/ethernet/stmicro/stmmac/stmmac.h +1
-0@@ -126,6 +126,7 @@ struct stmmac_rx_queue { unsigned int cur_rx; unsigned int dirty_rx; unsigned int buf_alloc_num; + unsigned int napi_skb_frag_size; dma_addr_t dma_rx_phy; u32 rx_tail_addr; unsigned int state_saved;(quoted diff) renamed from drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0
-0
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index038df1b2bb58..43125a6f8f6b100644drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2
-1@@ -1320,7 +1320,7 @@ static unsigned int stmmac_rx_offset(struct stmmac_priv *priv) if (stmmac_xdp_is_enabled(priv)) return XDP_PACKET_HEADROOM; - return 0; + return NET_SKB_PAD; } static int stmmac_set_bfsize(int mtu, int bufsize)@@ -2019,17 +2019,21 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv, struct stmmac_channel *ch = &priv->channel[queue]; bool xdp_prog = stmmac_xdp_is_enabled(priv); struct page_pool_params pp_params = { 0 }; - unsigned int num_pages; + unsigned int dma_buf_sz_pad, num_pages; unsigned int napi_id; int ret;+
dma_buf_sz_pad=stmmac_rx_offset(priv)+dma_conf->dma_buf_sz+ +SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); +num_pages=DIV_ROUND_UP(dma_buf_sz_pad, PAGE_SIZE);
+
rx_q->queue_index= queue;
rx_q->priv_data= priv;
+rx_q->napi_skb_frag_size=num_pages*PAGE_SIZE; pp_params.flags =PP_FLAG_DMA_MAP|PP_FLAG_DMA_SYNC_DEV; pp_params.pool_size =dma_conf->dma_rx_size;
-num_pages=DIV_ROUND_UP(dma_conf->dma_buf_sz, PAGE_SIZE); - pp_params.order =ilog2(num_pages);
+ pp_params.order =order_base_2(num_pages);
pp_params.nid =dev_to_node(priv->device); pp_params.dev =priv->device;
pp_params.dma_dir =xdp_prog?DMA_BIDIRECTIONAL:DMA_FROM_DEVICE;drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2
-0@@ -5574,19 +5578,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) /* XDP program may expand or reduce tail */ buf1_len = ctx.xdp.data_end - ctx.xdp.data; - skb = napi_alloc_skb(&ch->rx_napi, buf1_len); + skb = napi_build_skb(page_address(buf->page), + rx_q->napi_skb_frag_size);drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, · line 5592
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3
-1if (!skb) { + page_pool_recycle_direct(rx_q->page_pool, + buf->page); rx_dropped++; count++; goto drain_data; } /* XDP program may adjust header */ - skb_copy_to_linear_data(skb, ctx.xdp.data, buf1_len); + skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);The network subsystem still requires that the length of each line of code should not exceed 80 characters. So let's silence the warning:
Quoted 3 lines
WARNING: line length of 81 exceeds 80 columns
#87: FILE:drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5592:
+skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);
I agree that &ctx.xdp could be made an onstack pointer to shorten these lines, but please don't spam with the checkpatch output.
1. It's author's responsibility to read netdev CI output on Patchwork, reviewers shouldn't copy its logs.
2. The only alternative without making a shortcut for &ctx.xdp is
skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);
This looks really ugly and does more harm than good.
If you really want to help, pls come with good propositions how to avoid such warnings in an elegant way.
Quoted 3 lines
Thanks,
Yanteng
Thanks,
Olek
Re: [PATCH net-next v1 3/3] net: stmmac: Optimize cache prefetch in RX path
In reply to Furong XuFrom: Furong Xu <[email protected]>
Date: Fri, 10 Jan 2025 17:53:59 +0800
Quoted 37 lines
Current code prefetches cache lines for the received frame first, and then
dma_sync_single_for_cpu()against this frame, this is wrong. Cache prefetch should be triggered afterdma_sync_single_for_cpu().This patch brings ~2.8% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, 2.84 Gbits/sec increased to 2.92 Gbits/sec.
Signed-off-by Furong Xu <[email protected]>--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-4@@ -5497,10 +5497,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)/* Buffer is good. Go on. */ -prefetch (page_address (buf->page) + buf->page_offset); -if (buf->sec_page) -prefetch (page_address (buf->sec_page)); - buf1_len =stmmac_rx_buf1_len (priv, p, status, len); len += buf1_len; buf2_len =stmmac_rx_buf2_len (priv, p, status, len);@@ -5522,6 +5518,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)dma_sync_single_for_cpu (priv->device, buf->addr, buf1_len, dma_dir); +prefetch (page_address (buf->page) + buf->page_offset);xdp_init_buff (&ctx.xdp, buf_sz, &rx_q->xdp_rxq); xdp_prepare_buff(&ctx.xdp, page_address(buf->page),
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) · line 5599
Quoted 7 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-0@@ -5596,6 +5593,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) }else if (buf1_len) {dma_sync_single_for_cpu (priv->device, buf->addr, buf1_len, dma_dir); +prefetch (page_address (buf->page) + buf->page_offset);skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, buf->page, buf->page_offset, buf1_len, priv->dma_conf.dma_buf_sz);
Are you sure you need to prefetch frags as well? I'd say this is a waste of cycles, as the kernel core stack barely looks at payload... Probably prefetching only header buffers would be enough.
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) · line 5612
Quoted 8 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-0@@ -5608,6 +5606,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)if (buf2_len) {dma_sync_single_for_cpu (priv->device, buf->sec_addr, buf2_len, dma_dir); +prefetch (page_address (buf->sec_page));skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, buf->sec_page,0 , buf2_len, priv->dma_conf.dma_buf_sz);
Thanks,
Olek
Re: [PATCH net-next v1 3/3] net: stmmac: Optimize cache prefetch in RX path
Quoted 4 lines · Alexander Lobakin
Quoted 4 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0
-0@@ -5596,6 +5593,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) }else if (buf1_len) {dma_sync_single_for_cpu (priv->device, buf->addr, buf1_len, dma_dir);
Quoted 9 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) · line 5599
Quoted 4 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +1
-0+prefetch (page_address (buf->page) + buf->page_offset);skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, buf->page, buf->page_offset, buf1_len, priv->dma_conf.dma_buf_sz);Are you sure you need to prefetch frags as well? I'd say this is a waste of cycles, as the kernel core stack barely looks at payload... Probably prefetching only header buffers would be enough.
Yes, do not prefetch for frags is more reasonable. Thanks!
pw-bot: changes-requested
Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in non-XDP RX path
In reply to Alexander LobakinQuoted 88 lines · Alexander Lobakin
From: Yanteng Si <[email protected]>
Date: Mon, 13 Jan 2025 17:41:41 +0800Quoted 85 lines · Furong Xu
Quoted 70 lines
Avoid memcpy in non-XDP RX path by marking all allocated SKBs to be recycled in the upper network stack.
This patch brings ~11.5% driver performance improvement in a TCP RX throughput test with iPerf tool on a single isolated Cortex-A65 CPU core, from 2.18 Gbits/sec increased to 2.43 Gbits/sec.
Signed-off-by Furong Xu <[email protected]>--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 + .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +++++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-)drivers/ renamed from drivers/net/ethernet/stmicro/stmmac/stmmac.h +0
-0
net/ethernet/stmicro/stmmac/stmmac.h
index548b28fed9b6..5c39292313de100644drivers/net/ethernet/stmicro/stmmac/stmmac.h +1
-0@@ -126,6 +126,7 @@ struct stmmac_rx_queue { unsigned int cur_rx; unsigned int dirty_rx; unsigned int buf_alloc_num; + unsigned int napi_skb_frag_size; dma_addr_t dma_rx_phy; u32 rx_tail_addr; unsigned int state_saved;(quoted diff) renamed from drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +0
-0
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index038df1b2bb58..43125a6f8f6b100644drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +9
-3@@ -1320,7 +1320,7 @@ static unsigned int stmmac_rx_offset(struct stmmac_priv *priv) if (stmmac_xdp_is_enabled(priv)) return XDP_PACKET_HEADROOM; - return 0; + return NET_SKB_PAD; } static int stmmac_set_bfsize(int mtu, int bufsize)@@ -2019,17 +2019,21 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv, struct stmmac_channel *ch = &priv->channel[queue]; bool xdp_prog = stmmac_xdp_is_enabled(priv); struct page_pool_params pp_params = { 0 }; - unsigned int num_pages; + unsigned int dma_buf_sz_pad, num_pages; unsigned int napi_id; int ret; + dma_buf_sz_pad = stmmac_rx_offset(priv) + dma_conf->dma_buf_sz + + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); + num_pages = DIV_ROUND_UP(dma_buf_sz_pad, PAGE_SIZE); + rx_q->queue_index = queue; rx_q->priv_data = priv; + rx_q->napi_skb_frag_size = num_pages * PAGE_SIZE; pp_params.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV; pp_params.pool_size = dma_conf->dma_rx_size; - num_pages = DIV_ROUND_UP(dma_conf->dma_buf_sz, PAGE_SIZE); - pp_params.order = ilog2(num_pages); + pp_params.order = order_base_2(num_pages); pp_params.nid = dev_to_node(priv->device); pp_params.dev = priv->device; pp_params.dma_dir = xdp_prog ? DMA_BIDIRECTIONAL : DMA_FROM_DEVICE;@@ -5574,19 +5578,20 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) /* XDP program may expand or reduce tail */ buf1_len = ctx.xdp.data_end - ctx.xdp.data; - skb = napi_alloc_skb(&ch->rx_napi, buf1_len); + skb = napi_build_skb(page_address(buf->page), + rx_q->napi_skb_frag_size);drivers/net/ethernet/stmicro/stmmac/stmmac_main.c static int stmmac_rx(struct stmmac_priv *priv, · line 5592
Quoted 10 lines
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +3
-1if (!skb) { + page_pool_recycle_direct(rx_q->page_pool, + buf->page); rx_dropped++; count++; goto drain_data; } /* XDP program may adjust header */ - skb_copy_to_linear_data(skb, ctx.xdp.data, buf1_len); + skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);The network subsystem still requires that the length of each line of code should not exceed 80 characters. So let's silence the warning:
Quoted 8 lines
Quoted 3 lines
WARNING: line length of 81 exceeds 80 columns
#87: FILE:drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:5592:
+skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);I agree that &ctx.xdp could be made an onstack pointer to shorten these lines, but please don't spam with the checkpatch output.
1. It's author's responsibility to read netdev CI output on Patchwork, reviewers shouldn't copy its logs.
Sorry, I shouldn't have made noise on the mailing list.
Quoted 6 lines
2. The only alternative without making a shortcut for &ctx.xdp is
skb_reserve(skb, ctx.xdp.data - ctx.xdp.data_hard_start);This looks really ugly and does more harm than good.
Agree!
Quoted 3 lines
If you really want to help, pls come with good propositions how to avoid such warnings in an elegant way.
Simple. Just do as with buf1_len.
Thanks,
Yanteng
Quoted 5 lines
Quoted 3 lines
Thanks,
YantengThanks,
Olek
Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in non-XDP RX path
In reply to Alexander LobakinQuoted 2 lines
1. It's author's responsibility to read netdev CI output on Patchwork, reviewers shouldn't copy its logs.
I somewhat disagree with that. We want the author to of already run all the static analysers before they post code. We don't want the mailing list abused as a CI system.
So rather than pointing out a specific problem, it can be better to say that static analysers XZY is not happy with this patch, please run it and fix the issues it reports.
Andrew
Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in non-XDP RX path
From: Andrew Lunn <[email protected]>
Date: Mon, 13 Jan 2025 17:48:22 +0100
Quoted 6 lines
Quoted 2 lines
1. It's author's responsibility to read netdev CI output on Patchwork, reviewers shouldn't copy its logs.
I somewhat disagree with that. We want the author to of already run all the static analysers before they post code. We don't want the mailing list abused as a CI system.
Sure. Maybe I wasn't clear enough, but I don't encourage using our MLs/CIs to test stuff :D What I meant is that reviewers shouldn't copy stuff from the Patchwork output. The authors themselves should track their series there, but only to make sure everything is fine, not to "let's see if I need to fix anything else".
Quoted 4 lines
So rather than pointing out a specific problem, it can be better to say that static analysers XZY is not happy with this patch, please run it and fix the issues it reports.
Right, probably the best way.
Quoted 2 lines
Andrew
Thanks,
Olek