Re: [PATCH net-next v10 06/22] net: page_pool: create hooks for custom memory providers

[Jakub Kicinski <kuba@kernel.org>]

On Wed,  8 Jan 2025 14:06:27 -0800 David Wei wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> 
> A spin off from the original page pool memory providers patch by Jakub,
> which allows extending page pools with custom allocators. One of such
> providers is devmem TCP, and the other is io_uring zerocopy added in
> following patches.
> 
> Suggested-by: Jakub Kicinski <kuba@kernel.org>

Something odd with authorship here. You list me as author (From)
but didn't add my SoB. Maybe add something like "Based on 
earlier work by Jakub" to the commit and reset the tags?
Or the Suggested-by is just for the warning on ops not being built in?

> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
>  include/net/page_pool/memory_provider.h | 20 ++++++++++++++++++++
>  include/net/page_pool/types.h           |  4 ++++
>  net/core/devmem.c                       | 15 ++++++++++++++-
>  net/core/page_pool.c                    | 23 +++++++++++++++--------
>  4 files changed, 53 insertions(+), 9 deletions(-)
>  create mode 100644 include/net/page_pool/memory_provider.h
> 
> diff --git a/include/net/page_pool/memory_provider.h b/include/net/page_pool/memory_provider.h
> new file mode 100644
> index 000000000000..79412a8714fa
> --- /dev/null
> +++ b/include/net/page_pool/memory_provider.h
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * page_pool/memory_provider.h
> + *	Author:	Pavel Begunkov <asml.silence@gmail.com>
> + *	Author:	David Wei <dw@davidwei.uk>

Not a customary thing in networking to list authors in comments.

> + */
> +#ifndef _NET_PAGE_POOL_MEMORY_PROVIDER_H
> +#define _NET_PAGE_POOL_MEMORY_PROVIDER_H
> +
> +#include <net/netmem.h>
> +#include <net/page_pool/types.h>

No need? All you need is forward declarations for types at this stage. 

> +struct memory_provider_ops {
> +	netmem_ref (*alloc_netmems)(struct page_pool *pool, gfp_t gfp);
> +	bool (*release_netmem)(struct page_pool *pool, netmem_ref netmem);
> +	int (*init)(struct page_pool *pool);
> +	void (*destroy)(struct page_pool *pool);
> +};
> +
> +#endif

Rest LGTM.