On Thu, 10 Oct 2024 00:37:49 +0900 Taehee Yoo wrote:
> > Yes, but netmem_ref can be either a net_iov or a normal page,
> > and skb_add_rx_frag_netmem() and similar helpers should automatically
> > set skb->unreadable or not.
> >
> > IOW you should be able to always use netmem-aware APIs, no?
>
> I'm not sure the update skb->unreadable flag is possible because
> frag API like skb_add_rx_frag_netmem(), receives only frag, not skb.
> How about an additional API to update skb->unreadable flag?
> skb_update_unreadable() or skb_update_netmem()?
Ah, the case where we don't get skb is because we're just building XDP
frame at that stage. And XDP can't be netmem.
In that case switching to skb_frag_fill_netmem_desc() should be enough.
> > > The reason why the branch exists here is the PP_FLAG_ALLOW_UNREADABLE_NETMEM
> > > flag can't be used with PP_FLAG_DMA_SYNC_DEV.
> >
> > Hm. Isn't the existing check the wrong way around? Is the driver
> > supposed to sync the buffers for device before passing them down?
>
> I haven't thought the failure of PP_FLAG_DMA_SYNC_DEV
> for dmabuf may be wrong.
> I think device memory TCP is not related to this flag.
> So device memory TCP core API should not return failure when
> PP_FLAG_DMA_SYNC_DEV flag is set.
> How about removing this condition check code in device memory TCP core?
I think we need to invert the check..
Mina, WDYT?
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 11b91c12ee11..c5cace3f9831 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -331,12 +331,6 @@ int mp_dmabuf_devmem_init(struct page_pool *pool)
if (!binding)
return -EINVAL;
- if (!pool->dma_map)
- return -EOPNOTSUPP;
-
- if (pool->dma_sync)
- return -EOPNOTSUPP;
-
if (pool->p.order != 0)
return -E2BIG;
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index a813d30d2135..c8dbbf262de3 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -287,6 +287,12 @@ static int page_pool_init(struct page_pool *pool,
}
if (pool->mp_priv) {
+ if (!pool->dma_map || !pool->dma_sync)
+ return -EOPNOTSUPP;
+
+ /* Memory provider is responsible for syncing the pages. */
+ pool->dma_sync = 0;
+
err = mp_dmabuf_devmem_init(pool);
if (err) {
pr_warn("%s() mem-provider init failed %d\n", __func__,
