netdev - Re: [PATCH net-next v3 7/7] bnxt_en: add support for device memory tcp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241009170102.1980ed1d@kernel.org>
Date: Wed, 9 Oct 2024 17:01:02 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Taehee Yoo <ap420073@...il.com>, Mina Almasry <almasrymina@...gle.com>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
 netdev@...r.kernel.org, linux-doc@...r.kernel.org, donald.hunter@...il.com,
 corbet@....net, michael.chan@...adcom.com, kory.maincent@...tlin.com,
 andrew@...n.ch, maxime.chevallier@...tlin.com, danieller@...dia.com,
 hengqi@...ux.alibaba.com, ecree.xilinx@...il.com,
 przemyslaw.kitszel@...el.com, hkallweit1@...il.com, ahmed.zaki@...el.com,
 paul.greenwalt@...el.com, rrameshbabu@...dia.com, idosch@...dia.com,
 asml.silence@...il.com, kaiyuanz@...gle.com, willemb@...gle.com,
 aleksander.lobakin@...el.com, dw@...idwei.uk, sridhar.samudrala@...el.com,
 bcreeley@....com
Subject: Re: [PATCH net-next v3 7/7] bnxt_en: add support for device memory
 tcp

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__,