On 1/6/25 21:44, Mina Almasry wrote: ...
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index a473ea0c48c4..140fec6857c6 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -152,12 +152,15 @@ struct page_pool_stats { */ #define PAGE_POOL_FRAG_GROUP_ALIGN (4 * sizeof(long)) +struct netdev_rx_queue; + 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); int (*nl_report)(const struct page_pool *pool, struct sk_buff *rsp); + void (*uninstall)(void *mp_priv, struct netdev_rx_queue *rxq);nit: the other params take struct page_pool *pool as input, and find the mp_priv in there if they need, it may be nice for consistency to continue to pass the entire pool to these ops.
I think so as well, but there is simply no page pool to pass from where it's called.
AFAIU this is the first added non-mandatory op, right? Please specify it as so, maybe something like: /* optional: called when the memory provider is uninstalled from the netdev_rx_queue due to the interface going down or otherwise. The memory provider may perform whatever cleanup necessary here if it needs to. */}; struct pp_memory_provider_params { diff --git a/net/core/dev.c b/net/core/dev.c index c7f3dea3e0eb..aa082770ab1c 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -11464,6 +11464,19 @@ void unregister_netdevice_queue(struct net_device *dev, struct list_head *head) } EXPORT_SYMBOL(unregister_netdevice_queue); +static void dev_memory_provider_uninstall(struct net_device *dev) +{ + unsigned int i; + + for (i = 0; i < dev->real_num_rx_queues; i++) { + struct netdev_rx_queue *rxq = &dev->_rx[i]; + struct pp_memory_provider_params *p = &rxq->mp_params; + + if (p->mp_ops && p->mp_ops->uninstall)Previous versions of the code checked p->mp_priv to check if the queue is mp enabled or not, I guess you check mp_ops and that is set/cleared
The patchset converts it all to mp_ops (v9 missed one place), which is the right thing to do as only ops are typed and strictly defined. -- Pavel Begunkov
