| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8d6303f1-37f1-4298-b377-d2bd55ac01de@linux.dev>
Date: Mon, 13 Jan 2025 23:16:31 +0800
From: Yanteng Si <si.yanteng@...ux.dev>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Furong Xu <0x1207@...il.com>, netdev@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, xfr@...look.com
Subject: Re: [PATCH net-next v1 1/3] net: stmmac: Switch to zero-copy in
non-XDP RX path
在 1/13/25 20:03, Alexander Lobakin 写道:
> From: Yanteng Si <si.yanteng@...ux.dev>
> Date: Mon, 13 Jan 2025 17:41:41 +0800
>
>> 在 2025/1/10 17:53, 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 <0x1207@...il.com>
>>> ---
>>> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
>>> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 23 +++++++++++--------
>>> 2 files changed, 15 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/
>>> net/ethernet/stmicro/stmmac/stmmac.h
>>> index 548b28fed9b6..5c39292313de 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
>>> @@ -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;
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/
>>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> index 038df1b2bb58..43125a6f8f6b 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>>> @@ -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);
>> 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);
> 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.
> 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!
>
> 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
>
>> Thanks,
>> Yanteng
> Thanks,
> Olek
Powered by blists - more mailing lists