* [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev
@ 2024-02-28 11:05 Yunjian Wang
2024-02-29 10:43 ` Paolo Abeni
0 siblings, 1 reply; 4+ messages in thread
From: Yunjian Wang @ 2024-02-28 11:05 UTC (permalink / raw
To: mst, willemdebruijn.kernel, jasowang, kuba, bjorn,
magnus.karlsson, maciej.fijalkowski, jonathan.lemon, davem
Cc: bpf, netdev, linux-kernel, kvm, virtualization, xudingke,
liwei395, Yunjian Wang
Now dma mappings are used by the physical NICs. However the vNIC
maybe do not need them. So remove non-zero 'dma_page' check in
xp_assign_dev.
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
net/xdp/xsk_buff_pool.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index ce60ecd48a4d..a5af75b1f43c 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -219,16 +219,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
if (err)
goto err_unreg_pool;
- if (!pool->dma_pages) {
- WARN(1, "Driver did not DMA map zero-copy buffers");
- err = -EINVAL;
- goto err_unreg_xsk;
- }
pool->umem->zc = true;
return 0;
-err_unreg_xsk:
- xp_disable_drv_zc(pool);
err_unreg_pool:
if (!force_zc)
err = 0; /* fallback to copy mode */
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev
2024-02-28 11:05 [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev Yunjian Wang
@ 2024-02-29 10:43 ` Paolo Abeni
2024-02-29 12:52 ` wangyunjian
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Abeni @ 2024-02-29 10:43 UTC (permalink / raw
To: Yunjian Wang, mst, willemdebruijn.kernel, jasowang, kuba, bjorn,
magnus.karlsson, maciej.fijalkowski, jonathan.lemon, davem
Cc: bpf, netdev, linux-kernel, kvm, virtualization, xudingke,
liwei395
On Wed, 2024-02-28 at 19:05 +0800, Yunjian Wang wrote:
> Now dma mappings are used by the physical NICs. However the vNIC
> maybe do not need them. So remove non-zero 'dma_page' check in
> xp_assign_dev.
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
> net/xdp/xsk_buff_pool.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index ce60ecd48a4d..a5af75b1f43c 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -219,16 +219,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> if (err)
> goto err_unreg_pool;
>
> - if (!pool->dma_pages) {
> - WARN(1, "Driver did not DMA map zero-copy buffers");
> - err = -EINVAL;
> - goto err_unreg_xsk;
> - }
This would unconditionally remove an otherwise valid check for most
NIC. What about let the driver declare it wont need DMA map with a
(pool?) flag.
Cheers,
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev
2024-02-29 10:43 ` Paolo Abeni
@ 2024-02-29 12:52 ` wangyunjian
2024-03-04 13:58 ` Magnus Karlsson
0 siblings, 1 reply; 4+ messages in thread
From: wangyunjian @ 2024-02-29 12:52 UTC (permalink / raw
To: Paolo Abeni, mst@redhat.com, willemdebruijn.kernel@gmail.com,
jasowang@redhat.com, kuba@kernel.org, bjorn@kernel.org,
magnus.karlsson@intel.com, maciej.fijalkowski@intel.com,
jonathan.lemon@gmail.com, davem@davemloft.net
Cc: bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
virtualization@lists.linux.dev, xudingke, liwei (DT)
> -----Original Message-----
> From: Paolo Abeni [mailto:pabeni@redhat.com]
> Sent: Thursday, February 29, 2024 6:43 PM
> To: wangyunjian <wangyunjian@huawei.com>; mst@redhat.com;
> willemdebruijn.kernel@gmail.com; jasowang@redhat.com; kuba@kernel.org;
> bjorn@kernel.org; magnus.karlsson@intel.com; maciej.fijalkowski@intel.com;
> jonathan.lemon@gmail.com; davem@davemloft.net
> Cc: bpf@vger.kernel.org; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; kvm@vger.kernel.org;
> virtualization@lists.linux.dev; xudingke <xudingke@huawei.com>; liwei (DT)
> <liwei395@huawei.com>
> Subject: Re: [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in
> xp_assign_dev
>
> On Wed, 2024-02-28 at 19:05 +0800, Yunjian Wang wrote:
> > Now dma mappings are used by the physical NICs. However the vNIC maybe
> > do not need them. So remove non-zero 'dma_page' check in
> > xp_assign_dev.
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> > net/xdp/xsk_buff_pool.c | 7 -------
> > 1 file changed, 7 deletions(-)
> >
> > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c index
> > ce60ecd48a4d..a5af75b1f43c 100644
> > --- a/net/xdp/xsk_buff_pool.c
> > +++ b/net/xdp/xsk_buff_pool.c
> > @@ -219,16 +219,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> > if (err)
> > goto err_unreg_pool;
> >
> > - if (!pool->dma_pages) {
> > - WARN(1, "Driver did not DMA map zero-copy buffers");
> > - err = -EINVAL;
> > - goto err_unreg_xsk;
> > - }
>
> This would unconditionally remove an otherwise valid check for most NIC. What
> about let the driver declare it wont need DMA map with a
> (pool?) flag.
This check is redundant. The NIC's driver determines whether a DMA map is required.
If the NIC'driver requires the DMA map, it uses the xsk_pool_dma_map function, which
initializes the DMA map and performs a check.
Thanks
>
> Cheers,
>
> Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev
2024-02-29 12:52 ` wangyunjian
@ 2024-03-04 13:58 ` Magnus Karlsson
0 siblings, 0 replies; 4+ messages in thread
From: Magnus Karlsson @ 2024-03-04 13:58 UTC (permalink / raw
To: wangyunjian
Cc: Paolo Abeni, mst@redhat.com, willemdebruijn.kernel@gmail.com,
jasowang@redhat.com, kuba@kernel.org, bjorn@kernel.org,
magnus.karlsson@intel.com, maciej.fijalkowski@intel.com,
jonathan.lemon@gmail.com, davem@davemloft.net,
bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
virtualization@lists.linux.dev, xudingke, liwei (DT)
On Thu, 29 Feb 2024 at 13:52, wangyunjian <wangyunjian@huawei.com> wrote:
>
> > -----Original Message-----
> > From: Paolo Abeni [mailto:pabeni@redhat.com]
> > Sent: Thursday, February 29, 2024 6:43 PM
> > To: wangyunjian <wangyunjian@huawei.com>; mst@redhat.com;
> > willemdebruijn.kernel@gmail.com; jasowang@redhat.com; kuba@kernel.org;
> > bjorn@kernel.org; magnus.karlsson@intel.com; maciej.fijalkowski@intel.com;
> > jonathan.lemon@gmail.com; davem@davemloft.net
> > Cc: bpf@vger.kernel.org; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org; kvm@vger.kernel.org;
> > virtualization@lists.linux.dev; xudingke <xudingke@huawei.com>; liwei (DT)
> > <liwei395@huawei.com>
> > Subject: Re: [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in
> > xp_assign_dev
> >
> > On Wed, 2024-02-28 at 19:05 +0800, Yunjian Wang wrote:
> > > Now dma mappings are used by the physical NICs. However the vNIC maybe
> > > do not need them. So remove non-zero 'dma_page' check in
> > > xp_assign_dev.
> > >
> > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > ---
> > > net/xdp/xsk_buff_pool.c | 7 -------
> > > 1 file changed, 7 deletions(-)
> > >
> > > diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c index
> > > ce60ecd48a4d..a5af75b1f43c 100644
> > > --- a/net/xdp/xsk_buff_pool.c
> > > +++ b/net/xdp/xsk_buff_pool.c
> > > @@ -219,16 +219,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
> > > if (err)
> > > goto err_unreg_pool;
> > >
> > > - if (!pool->dma_pages) {
> > > - WARN(1, "Driver did not DMA map zero-copy buffers");
> > > - err = -EINVAL;
> > > - goto err_unreg_xsk;
> > > - }
> >
> > This would unconditionally remove an otherwise valid check for most NIC. What
> > about let the driver declare it wont need DMA map with a
> > (pool?) flag.
>
> This check is redundant. The NIC's driver determines whether a DMA map is required.
> If the NIC'driver requires the DMA map, it uses the xsk_pool_dma_map function, which
> initializes the DMA map and performs a check.
Just to provide some context: I put this check there many years ago to
guard against a zero-copy driver writer forgetting to call
xsk_pool_dma_map() during the implementation phase. A working driver
will always have pool->dma_pages != NULL. If you both think that this
check is too much of a precaution, then I have no problem getting rid
of it. Just thought that a text warning would be nicer than a crash
later.
Thanks: Magnus
> Thanks
>
> >
> > Cheers,
> >
> > Paolo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-04 13:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-28 11:05 [PATCH net-next v2 1/3] xsk: Remove non-zero 'dma_page' check in xp_assign_dev Yunjian Wang
2024-02-29 10:43 ` Paolo Abeni
2024-02-29 12:52 ` wangyunjian
2024-03-04 13:58 ` Magnus Karlsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).