netdev - Re: [net-next 07/11] net/mlx5e: Convert over to netmem

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHS8izOfkLtFzqsfnacQrVaiW0ZkHRoeZwNK4FVV7j3yR1T_vQ@mail.gmail.com>
Date: Wed, 5 Feb 2025 12:14:08 -0800
From: Mina Almasry <almasrymina@...gle.com>
To: Saeed Mahameed <saeed@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, 
	Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org, 
	Tariq Toukan <tariqt@...dia.com>, Gal Pressman <gal@...dia.com>, Leon Romanovsky <leonro@...dia.com>, 
	Dragos Tatulea <dtatulea@...dia.com>
Subject: Re: [net-next 07/11] net/mlx5e: Convert over to netmem

On Thu, Jan 16, 2025 at 1:56 PM Saeed Mahameed <saeed@...nel.org> wrote:
>
> From: Saeed Mahameed <saeedm@...dia.com>
>
> mlx5e_page_frag holds the physical page itself, to naturally support
> zc page pools, remove physical page reference from mlx5 and replace it
> with netmem_ref, to avoid internal handling in mlx5 for net_iov backed
> pages.
>
> No performance degradation observed.
>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@...dia.com>
> Reviewed-by: Tariq Toukan <tariqt@...dia.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
>  .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 80 ++++++++++---------
>  2 files changed, 43 insertions(+), 39 deletions(-)
>
...
> @@ -514,9 +514,9 @@ mlx5e_add_skb_shared_info_frag(struct mlx5e_rq *rq, struct skb_shared_info *sinf
>         }
>
>         frag = &sinfo->frags[sinfo->nr_frags++];
> -       skb_frag_fill_page_desc(frag, frag_page->page, frag_offset, len);
> +       skb_frag_fill_netmem_desc(frag, netmem, frag_offset, len);
>
> -       if (page_is_pfmemalloc(frag_page->page))
> +       if (!netmem_is_net_iov(netmem) && page_is_pfmemalloc(netmem_to_page(netmem)))
>                 xdp_buff_set_frag_pfmemalloc(xdp);

Consider using:

netmem_is_pfmemalloc(netmem_ref netmem)

In general we try to avoid netmem_to_page() casts in the driver. These
assumptions may break in the future.

>         sinfo->xdp_frags_size += len;
>  }
> @@ -527,27 +527,29 @@ mlx5e_add_skb_frag(struct mlx5e_rq *rq, struct sk_buff *skb,
>                    u32 frag_offset, u32 len,
>                    unsigned int truesize)
>  {
> -       dma_addr_t addr = page_pool_get_dma_addr(frag_page->page);
> +       dma_addr_t addr = page_pool_get_dma_addr_netmem(frag_page->netmem);
> +       struct page *page = netmem_to_page(frag_page->netmem);
>         u8 next_frag = skb_shinfo(skb)->nr_frags;
>
>         dma_sync_single_for_cpu(rq->pdev, addr + frag_offset, len,
>                                 rq->buff.map_dir);
>
> -       if (skb_can_coalesce(skb, next_frag, frag_page->page, frag_offset)) {
> +       if (skb_can_coalesce(skb, next_frag, page, frag_offset)) {

Similarly here, consider adding skb_can_coalesce_netmem() that handles
this correctly in core code (which future drivers can reuse) rather
than doing 1-off handling in the driver.

Also, from a quick look at skb_can_coalesce(), I think it can work
fine with netmems? Because it just needs to be converted to use
skb_frag_netmem istead of skb_frag_page() inside of the function, but
otherwise the function looks applicable to netmem for me.

-- 
Thanks,
Mina