linux-kernel - Re: [PATCH net-next 04/12] net: lan966x: use the FDMA library for allocation of rx buffers

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <f8b58d30-cd45-4cb6-b6ca-ac076f072688@redhat.com>
Date: Tue, 10 Sep 2024 11:15:44 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Daniel Machon <daniel.machon@...rochip.com>,
 Horatiu Vultur <horatiu.vultur@...rochip.com>, UNGLinuxDriver@...rochip.com,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH net-next 04/12] net: lan966x: use the FDMA library for
 allocation of rx buffers

On 9/5/24 10:06, Daniel Machon wrote:
> Use the two functions: fdma_alloc_phys() and fdma_dcb_init() for rx
> buffer allocation and use the new buffers throughout.
> 
> In order to replace the old buffers with the new ones, we have to do the
> following refactoring:
> 
>      - use fdma_alloc_phys() and fdma_dcb_init()
> 
>      - replace the variables: rx->dma, rx->dcbs and rx->last_entry
>        with the equivalents from the FDMA struct.
> 
>      - make use of fdma->db_size for rx buffer size.
> 
>      - add lan966x_fdma_rx_dataptr_cb callback for obtaining the dataptr.
> 
>      - Initialize FDMA struct values.
> 
> Signed-off-by: Daniel Machon <daniel.machon@...rochip.com>
> Reviewed-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> ---
>   .../net/ethernet/microchip/lan966x/lan966x_fdma.c  | 116 ++++++++++-----------
>   .../net/ethernet/microchip/lan966x/lan966x_main.h  |  15 ---
>   2 files changed, 55 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index b64f04ff99a8..99d09c97737e 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> @@ -6,13 +6,30 @@
>   
>   #include "lan966x_main.h"
>   
> +static int lan966x_fdma_rx_dataptr_cb(struct fdma *fdma, int dcb, int db,
> +				      u64 *dataptr)
> +{
> +	struct lan966x *lan966x = (struct lan966x *)fdma->priv;
> +	struct lan966x_rx *rx = &lan966x->rx;
> +	struct page *page;
> +
> +	page = page_pool_dev_alloc_pages(rx->page_pool);
> +	if (unlikely(!page))
> +		return -ENOMEM;
> +
> +	rx->page[dcb][db] = page;
> +	*dataptr = page_pool_get_dma_addr(page) + XDP_PACKET_HEADROOM;
> +
> +	return 0;
> +}

Very nice cleanup indeed!

Out of ENOMEM I can't recall if the following was already discussed, but 
looking at this cb, I'm wondering if a possible follow-up could replace 
the dataptr_cb() and nextptr_cb() with lib functions i.e. operating on 
page pool or doing netdev allocations according to some fdma lib flags.

Cheers,

Paolo