diff mbox series

[net-next,v9,08/20] net: expose page_pool_{set,clear}_pp_info

Message ID 20241218003748.796939-9-dw@davidwei.uk (mailing list archive)
State New
Headers show
Series [net-next,v9,01/20] net: page_pool: don't cast mp param to devmem | expand

Commit Message

David Wei Dec. 18, 2024, 12:37 a.m. UTC 
From: Pavel Begunkov <asml.silence@gmail.com>

Memory providers need to set page pool to its net_iovs on allocation, so
expose page_pool_{set,clear}_pp_info to providers outside net/.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: David Wei <dw@davidwei.uk>
---
 include/net/page_pool/helpers.h | 13 +++++++++++++
 net/core/page_pool_priv.h       |  9 ---------
 2 files changed, 13 insertions(+), 9 deletions(-)

Comments

Jakub Kicinski Dec. 20, 2024, 10:31 p.m. UTC | #1 
On Tue, 17 Dec 2024 16:37:34 -0800 David Wei wrote:
> From: Pavel Begunkov <asml.silence@gmail.com>
> 
> Memory providers need to set page pool to its net_iovs on allocation, so
> expose page_pool_{set,clear}_pp_info to providers outside net/.

I'd really rather not expose such low level functions in a header
included by every single user of the page pool API.
Pavel Begunkov Dec. 21, 2024, 1:07 a.m. UTC | #2 
On 12/20/24 22:31, Jakub Kicinski wrote:
> On Tue, 17 Dec 2024 16:37:34 -0800 David Wei wrote:
>> From: Pavel Begunkov <asml.silence@gmail.com>
>>
>> Memory providers need to set page pool to its net_iovs on allocation, so
>> expose page_pool_{set,clear}_pp_info to providers outside net/.
> 
> I'd really rather not expose such low level functions in a header
> included by every single user of the page pool API.

Are you fine if it's exposed in a new header file?
Jakub Kicinski Dec. 21, 2024, 2:23 a.m. UTC | #3 
On Sat, 21 Dec 2024 01:07:44 +0000 Pavel Begunkov wrote:
> >> Memory providers need to set page pool to its net_iovs on allocation, so
> >> expose page_pool_{set,clear}_pp_info to providers outside net/.  
> > 
> > I'd really rather not expose such low level functions in a header
> > included by every single user of the page pool API.  
> 
> Are you fine if it's exposed in a new header file?

I guess.

I'm uncomfortable with the "outside net/" phrasing of the commit
message. Nothing outside net should used this stuff. Next we'll have
a four letter subsystem abusing it and claiming that it's in a header
so it's a public.

Maybe we should have a conversation about whether io_uring/zcrx.c 
is considered part of networking, whether all patches will get cross
posted and need to get acks from both sides etc. Save ourselves
unpleasant misunderstandings.
diff mbox series

Patch

diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index e555921e5233..872947179bae 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -483,4 +483,17 @@  static inline void page_pool_nid_changed(struct page_pool *pool, int new_nid)
 		page_pool_update_nid(pool, new_nid);
 }
 
+#if defined(CONFIG_PAGE_POOL)
+void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem);
+void page_pool_clear_pp_info(netmem_ref netmem);
+#else
+static inline void page_pool_set_pp_info(struct page_pool *pool,
+					 netmem_ref netmem)
+{
+}
+static inline void page_pool_clear_pp_info(netmem_ref netmem)
+{
+}
+#endif
+
 #endif /* _NET_PAGE_POOL_HELPERS_H */
diff --git a/net/core/page_pool_priv.h b/net/core/page_pool_priv.h
index 57439787b9c2..11a45a5f3c9c 100644
--- a/net/core/page_pool_priv.h
+++ b/net/core/page_pool_priv.h
@@ -36,18 +36,9 @@  static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
 }
 
 #if defined(CONFIG_PAGE_POOL)
-void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem);
-void page_pool_clear_pp_info(netmem_ref netmem);
 int page_pool_check_memory_provider(struct net_device *dev,
 				    struct netdev_rx_queue *rxq);
 #else
-static inline void page_pool_set_pp_info(struct page_pool *pool,
-					 netmem_ref netmem)
-{
-}
-static inline void page_pool_clear_pp_info(netmem_ref netmem)
-{
-}
 static inline int page_pool_check_memory_provider(struct net_device *dev,
 						  struct netdev_rx_queue *rxq)
 {