On Tue, Dec 17, 2024 at 4:38 PM David Wei <dw@xxxxxxxxxxx> wrote:
>
> From: Pavel Begunkov <asml.silence@xxxxxxxxx>
>
> Add a mandatory memory provider callback that prints information about
> the provider.
>
> Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> Signed-off-by: David Wei <dw@xxxxxxxxxxx>
Seems like a straightforward refactor to add the op that Jakub requested:
Reviewed-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> ---
> include/net/page_pool/types.h | 1 +
> net/core/devmem.c | 9 +++++++++
> net/core/page_pool_user.c | 3 +--
> 3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index d6241e8a5106..a473ea0c48c4 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -157,6 +157,7 @@ struct memory_provider_ops {
> bool (*release_netmem)(struct page_pool *pool, netmem_ref netmem);
> int (*init)(struct page_pool *pool);
> void (*destroy)(struct page_pool *pool);
> + int (*nl_report)(const struct page_pool *pool, struct sk_buff *rsp);
> };
>
> struct pp_memory_provider_params {
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 48903b7ab215..df51a6c312db 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -394,9 +394,18 @@ bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
> return false;
> }
>
> +static int mp_dmabuf_devmem_nl_report(const struct page_pool *pool,
> + struct sk_buff *rsp)
> +{
> + const struct net_devmem_dmabuf_binding *binding = pool->mp_priv;
> +
> + return nla_put_u32(rsp, NETDEV_A_PAGE_POOL_DMABUF, binding->id);
> +}
> +
> static const struct memory_provider_ops dmabuf_devmem_ops = {
> .init = mp_dmabuf_devmem_init,
> .destroy = mp_dmabuf_devmem_destroy,
> .alloc_netmems = mp_dmabuf_devmem_alloc_netmems,
> .release_netmem = mp_dmabuf_devmem_release_page,
> + .nl_report = mp_dmabuf_devmem_nl_report,
> };
> diff --git a/net/core/page_pool_user.c b/net/core/page_pool_user.c
> index 8d31c71bea1a..61212f388bc8 100644
> --- a/net/core/page_pool_user.c
> +++ b/net/core/page_pool_user.c
> @@ -214,7 +214,6 @@ static int
> page_pool_nl_fill(struct sk_buff *rsp, const struct page_pool *pool,
> const struct genl_info *info)
> {
> - struct net_devmem_dmabuf_binding *binding = pool->mp_priv;
> size_t inflight, refsz;
> void *hdr;
>
> @@ -244,7 +243,7 @@ page_pool_nl_fill(struct sk_buff *rsp, const struct page_pool *pool,
> pool->user.detach_time))
> goto err_cancel;
>
> - if (binding && nla_put_u32(rsp, NETDEV_A_PAGE_POOL_DMABUF, binding->id))
> + if (pool->mp_ops && pool->mp_ops->nl_report(pool, rsp))
> goto err_cancel;
>
I thought maybe you should check nl_report exists here so that we
don't crash if we accidentally merge a memory provider that doesn't
define this, but the commit message says it's mandatory and we don't
check the existence of the other ops anyway, so this is probably fine.
--
Thanks,
Mina
