On Fri, Nov 10, 2023 at 3:19 PM Jakub Kicinski kuba@kernel.org wrote:
On Sun, 5 Nov 2023 18:44:01 -0800 Mina Almasry wrote:
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 6fc5134095ed..d4bea053bb7e 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -60,6 +60,8 @@ struct page_pool_params { int nid; struct device *dev; struct napi_struct *napi;
u8 memory_provider;void *mp_priv; enum dma_data_direction dma_dir; unsigned int max_len; unsigned int offset;you should rebase on top of net-next
More importantly I was expecting those fields to be gone from params. The fact that the page pool is configured to a specific provider should be fully transparent to the driver, driver should just tell the core what queue its creating the pool from and if there's a dmabuf bound for that queue - out pops a pp backed by the dmabuf.
My issue with this is that if the driver doesn't support dmabuf then the driver will accidentally use the pp backed by the dmabuf, allocate a page from it, then call page_address() on it or something, and crash.
Currently I avoid that by having the driver be responsible for picking up the dmabuf from the netdev_rx_queue and giving it to the page pool. What would be the appropriate way to check for driver support in the netlink API? Perhaps adding something to ndo_features_check?