diff mbox series

[vhost,v11,05/10] virtio_ring: introduce virtqueue_dma_dev()

Message ID 20230710034237.12391-6-xuanzhuo@linux.alibaba.com (mailing list archive)
State Superseded
Headers show
Series virtio core prepares for AF_XDP | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Xuan Zhuo July 10, 2023, 3:42 a.m. UTC
Added virtqueue_dma_dev() to get DMA device for virtio. Then the
caller can do dma operation in advance. The purpose is to keep memory
mapped across multiple add/get buf operations.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 drivers/virtio/virtio_ring.c | 17 +++++++++++++++++
 include/linux/virtio.h       |  2 ++
 2 files changed, 19 insertions(+)

Comments

Jason Wang July 12, 2023, 8:33 a.m. UTC | #1
On Mon, Jul 10, 2023 at 11:42 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> caller can do dma operation in advance. The purpose is to keep memory
> mapped across multiple add/get buf operations.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Acked-by: Jason Wang <jasowang@redhat.com>

Thanks

> ---
>  drivers/virtio/virtio_ring.c | 17 +++++++++++++++++
>  include/linux/virtio.h       |  2 ++
>  2 files changed, 19 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index d471dee3f4f7..1fb2c6dca9ea 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -2265,6 +2265,23 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
>
> +/**
> + * virtqueue_dma_dev - get the dma dev
> + * @_vq: the struct virtqueue we're talking about.
> + *
> + * Returns the dma dev. That can been used for dma api.
> + */
> +struct device *virtqueue_dma_dev(struct virtqueue *_vq)
> +{
> +       struct vring_virtqueue *vq = to_vvq(_vq);
> +
> +       if (vq->use_dma_api)
> +               return vring_dma_dev(vq);
> +       else
> +               return NULL;
> +}
> +EXPORT_SYMBOL_GPL(virtqueue_dma_dev);
> +
>  /**
>   * virtqueue_kick_prepare - first half of split virtqueue_kick call.
>   * @_vq: the struct virtqueue
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 2efd07b79ecf..35d175121cc6 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -61,6 +61,8 @@ int virtqueue_add_sgs(struct virtqueue *vq,
>                       void *data,
>                       gfp_t gfp);
>
> +struct device *virtqueue_dma_dev(struct virtqueue *vq);
> +
>  bool virtqueue_kick(struct virtqueue *vq);
>
>  bool virtqueue_kick_prepare(struct virtqueue *vq);
> --
> 2.32.0.3.g01195cf9f
>
Christoph Hellwig July 13, 2023, 11:15 a.m. UTC | #2
On Mon, Jul 10, 2023 at 11:42:32AM +0800, Xuan Zhuo wrote:
> Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> caller can do dma operation in advance. The purpose is to keep memory
> mapped across multiple add/get buf operations.

This is just poking holes into the abstraction..
Michael S. Tsirkin July 13, 2023, 2:51 p.m. UTC | #3
On Thu, Jul 13, 2023 at 04:15:16AM -0700, Christoph Hellwig wrote:
> On Mon, Jul 10, 2023 at 11:42:32AM +0800, Xuan Zhuo wrote:
> > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > caller can do dma operation in advance. The purpose is to keep memory
> > mapped across multiple add/get buf operations.
> 
> This is just poking holes into the abstraction..

More specifically?
Christoph Hellwig July 20, 2023, 6:22 a.m. UTC | #4
On Thu, Jul 13, 2023 at 10:51:59AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jul 13, 2023 at 04:15:16AM -0700, Christoph Hellwig wrote:
> > On Mon, Jul 10, 2023 at 11:42:32AM +0800, Xuan Zhuo wrote:
> > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > caller can do dma operation in advance. The purpose is to keep memory
> > > mapped across multiple add/get buf operations.
> > 
> > This is just poking holes into the abstraction..
> 
> More specifically?

Because now you expose a device that can't be used for the non-dma
mapping case and shoud be hidden.
Xuan Zhuo July 20, 2023, 6:45 a.m. UTC | #5
On Wed, 19 Jul 2023 23:22:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> On Thu, Jul 13, 2023 at 10:51:59AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jul 13, 2023 at 04:15:16AM -0700, Christoph Hellwig wrote:
> > > On Mon, Jul 10, 2023 at 11:42:32AM +0800, Xuan Zhuo wrote:
> > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > caller can do dma operation in advance. The purpose is to keep memory
> > > > mapped across multiple add/get buf operations.
> > >
> > > This is just poking holes into the abstraction..
> >
> > More specifically?
>
> Because now you expose a device that can't be used for the non-dma
> mapping case and shoud be hidden.

 Sorry I can not got.

 virtqueue_dma_dev() return the device that working with the DMA APIs.
 Then that can be used like other devices. So what is the problem.

 I always think the code path without the DMA APIs is the trouble for you.

 Thanks.
Christoph Hellwig July 20, 2023, 6:57 a.m. UTC | #6
On Thu, Jul 20, 2023 at 02:45:14PM +0800, Xuan Zhuo wrote:
>  virtqueue_dma_dev() return the device that working with the DMA APIs.
>  Then that can be used like other devices. So what is the problem.
> 
>  I always think the code path without the DMA APIs is the trouble for you.

Because we now have an API where the upper level drivers sometimes
see the dma device and sometimes not.  This will be abused and cause
trouble sooner than you can say "layering".
Xuan Zhuo July 20, 2023, 7:34 a.m. UTC | #7
On Wed, 19 Jul 2023 23:57:51 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> On Thu, Jul 20, 2023 at 02:45:14PM +0800, Xuan Zhuo wrote:
> >  virtqueue_dma_dev() return the device that working with the DMA APIs.
> >  Then that can be used like other devices. So what is the problem.
> >
> >  I always think the code path without the DMA APIs is the trouble for you.
>
> Because we now have an API where the upper level drivers sometimes
> see the dma device and sometimes not.

No dma device is just for the old devices.

The API without DMA dev are only compatible with older devices. We can't give up
these old devices, but we also have to embrace new features.

> This will be abused and cause
> trouble sooner than you can say "layering".

I don't understand what the possible trouble here is.

When no dma device, the driver just does the same thing as before.

Thanks.
Michael S. Tsirkin July 20, 2023, 5:21 p.m. UTC | #8
On Wed, Jul 19, 2023 at 11:22:42PM -0700, Christoph Hellwig wrote:
> On Thu, Jul 13, 2023 at 10:51:59AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Jul 13, 2023 at 04:15:16AM -0700, Christoph Hellwig wrote:
> > > On Mon, Jul 10, 2023 at 11:42:32AM +0800, Xuan Zhuo wrote:
> > > > Added virtqueue_dma_dev() to get DMA device for virtio. Then the
> > > > caller can do dma operation in advance. The purpose is to keep memory
> > > > mapped across multiple add/get buf operations.
> > > 
> > > This is just poking holes into the abstraction..
> > 
> > More specifically?
> 
> Because now you expose a device that can't be used for the non-dma
> mapping case and shoud be hidden.


Ah, ok.
Well I think we can add wrappers like virtio_dma_sync and so on.
There are NOP for non-dma so passing the dma device is harmless.
Christoph Hellwig July 24, 2023, 4:43 p.m. UTC | #9
On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> Well I think we can add wrappers like virtio_dma_sync and so on.
> There are NOP for non-dma so passing the dma device is harmless.

Yes, please.
Michael S. Tsirkin July 24, 2023, 8:05 p.m. UTC | #10
On Thu, Jul 20, 2023 at 03:34:01PM +0800, Xuan Zhuo wrote:
> On Wed, 19 Jul 2023 23:57:51 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > On Thu, Jul 20, 2023 at 02:45:14PM +0800, Xuan Zhuo wrote:
> > >  virtqueue_dma_dev() return the device that working with the DMA APIs.
> > >  Then that can be used like other devices. So what is the problem.
> > >
> > >  I always think the code path without the DMA APIs is the trouble for you.
> >
> > Because we now have an API where the upper level drivers sometimes
> > see the dma device and sometimes not.
> 
> No dma device is just for the old devices.
> 
> The API without DMA dev are only compatible with older devices. We can't give up
> these old devices, but we also have to embrace new features.
> 
> > This will be abused and cause
> > trouble sooner than you can say "layering".
> 
> I don't understand what the possible trouble here is.
> 
> When no dma device, the driver just does the same thing as before.
> 
> Thanks.

Instead of skipping operations, Christoph wants wrappers that
do nothing for non dma case.
Xuan Zhuo July 25, 2023, 2:13 a.m. UTC | #11
On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > Well I think we can add wrappers like virtio_dma_sync and so on.
> > There are NOP for non-dma so passing the dma device is harmless.
>
> Yes, please.


I am not sure I got this fully.

Are you mean this:
https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/

Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
No care the device is non-dma device or dma device.

Then the AF_XDP must use these virtio_dma_* APIs for virtio device.

Thanks
Michael S. Tsirkin July 25, 2023, 7:34 a.m. UTC | #12
On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > There are NOP for non-dma so passing the dma device is harmless.
> >
> > Yes, please.
> 
> 
> I am not sure I got this fully.
> 
> Are you mean this:
> https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> 
> Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> No care the device is non-dma device or dma device.

yes

> Then the AF_XDP must use these virtio_dma_* APIs for virtio device.

We'll worry about AF_XDP when the patch is posted.
Xuan Zhuo July 25, 2023, 11:07 a.m. UTC | #13
On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > There are NOP for non-dma so passing the dma device is harmless.
> > >
> > > Yes, please.
> >
> >
> > I am not sure I got this fully.
> >
> > Are you mean this:
> > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> >
> > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > No care the device is non-dma device or dma device.
>
> yes
>
> > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
>
> We'll worry about AF_XDP when the patch is posted.

YES.

We discussed it. They voted 'no'.

http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org

Thanks.


>
> --
> MST
>
Xuan Zhuo July 28, 2023, 6:02 a.m. UTC | #14
On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > >
> > > > Yes, please.
> > >
> > >
> > > I am not sure I got this fully.
> > >
> > > Are you mean this:
> > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > >
> > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > No care the device is non-dma device or dma device.
> >
> > yes
> >
> > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> >
> > We'll worry about AF_XDP when the patch is posted.
>
> YES.
>
> We discussed it. They voted 'no'.
>
> http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org


Hi guys, this topic is stuck again. How should I proceed with this work?

Let me briefly summarize:
1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
the driver layer, we need to support these APIs. The current conclusion of
AF_XDP is no.

2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
driver. This idea seems to be inconsistent with the framework design of DMA. The
conclusion is no.

3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
uses DMA API. And this type of device is the future direction, so we only
support DMA premapped for this type of virtio device. The problem with this
solution is that virtqueue_dma_dev() only returns dev in some cases, because
VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
This option is currently NO.

So I'm wondering what should I do, from a DMA point of view, is there any
solution in case of using DMA API?

Thank you



>
> Thanks.
>
>
> >
> > --
> > MST
> >
>
Jakub Kicinski July 28, 2023, 3:03 p.m. UTC | #15
On Fri, 28 Jul 2023 14:02:33 +0800 Xuan Zhuo wrote:
> Hi guys, this topic is stuck again. How should I proceed with this work?
> 
> Let me briefly summarize:
> 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> the driver layer, we need to support these APIs. The current conclusion of
> AF_XDP is no.
> 
> 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> driver. This idea seems to be inconsistent with the framework design of DMA. The
> conclusion is no.
> 
> 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> uses DMA API. And this type of device is the future direction, so we only
> support DMA premapped for this type of virtio device. The problem with this
> solution is that virtqueue_dma_dev() only returns dev in some cases, because
> VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> This option is currently NO.
> 
> So I'm wondering what should I do, from a DMA point of view, is there any
> solution in case of using DMA API?

I'd step back and ask you why do you want to use AF_XDP with virtio.
Instead of bifurcating one virtio instance into different queues why
not create a separate virtio instance?
Jason Wang July 31, 2023, 1:23 a.m. UTC | #16
On Fri, Jul 28, 2023 at 11:03 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 28 Jul 2023 14:02:33 +0800 Xuan Zhuo wrote:
> > Hi guys, this topic is stuck again. How should I proceed with this work?
> >
> > Let me briefly summarize:
> > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > the driver layer, we need to support these APIs. The current conclusion of
> > AF_XDP is no.
> >
> > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > conclusion is no.
> >
> > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > uses DMA API. And this type of device is the future direction, so we only
> > support DMA premapped for this type of virtio device. The problem with this
> > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> > This option is currently NO.
> >
> > So I'm wondering what should I do, from a DMA point of view, is there any
> > solution in case of using DMA API?
>
> I'd step back and ask you why do you want to use AF_XDP with virtio.
> Instead of bifurcating one virtio instance into different queues why
> not create a separate virtio instance?
>

I'm not sure I get this, but do you mean a separate virtio device that
owns AF_XDP queues only? If I understand it correctly, bifurcating is
one of the key advantages of AF_XDP. What's more, current virtio
doesn't support being split at queue (pair) level. And it may still
suffer from the yes/no DMA API issue.

Thanks
Xuan Zhuo July 31, 2023, 2:34 a.m. UTC | #17
On Fri, 28 Jul 2023 08:03:05 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Fri, 28 Jul 2023 14:02:33 +0800 Xuan Zhuo wrote:
> > Hi guys, this topic is stuck again. How should I proceed with this work?
> >
> > Let me briefly summarize:
> > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > the driver layer, we need to support these APIs. The current conclusion of
> > AF_XDP is no.
> >
> > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > conclusion is no.
> >
> > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > uses DMA API. And this type of device is the future direction, so we only
> > support DMA premapped for this type of virtio device. The problem with this
> > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> > This option is currently NO.
> >
> > So I'm wondering what should I do, from a DMA point of view, is there any
> > solution in case of using DMA API?
>
> I'd step back and ask you why do you want to use AF_XDP with virtio.

Or do you mean virtio vs virtio-net?
All I did with virtio was to get the virtio-net to support AF_XDP.

> Instead of bifurcating one virtio instance into different queues

That is not the key of our problem.

Even though we have a device that only works with AF_XDP,
it still has this DMA issues.

I think the current way(v11, v12) is a good solution, the only problem is that
if the device is old, we can not do dma with DMA APIs. Then we will not suppot
AF_XDP. I don't think it matters. But Christoph was a little worried.

Thanks.


> why
> not create a separate virtio instance?
Jakub Kicinski July 31, 2023, 3:46 p.m. UTC | #18
On Mon, 31 Jul 2023 09:23:29 +0800 Jason Wang wrote:
> > I'd step back and ask you why do you want to use AF_XDP with virtio.
> > Instead of bifurcating one virtio instance into different queues why
> > not create a separate virtio instance?
> 
> I'm not sure I get this, but do you mean a separate virtio device that
> owns AF_XDP queues only? If I understand it correctly, bifurcating is
> one of the key advantages of AF_XDP. What's more, current virtio
> doesn't support being split at queue (pair) level. And it may still
> suffer from the yes/no DMA API issue.

I guess we should step even further back and ask Xuan what the use case
is, because I'm not very sure. All we hear is "enable AF_XDP on virtio"
but AF_XDP is barely used on real HW, so why?

Bifurcating makes (used to make?) some sense in case of real HW when you
had only one PCI function and had to subdivide it. Virtio is either a SW
construct or offloaded to very capable HW, so either way cost of
creating an extra instance for DPDK or whatever else is very low.
Xuan Zhuo Aug. 1, 2023, 2:03 a.m. UTC | #19
On Mon, 31 Jul 2023 08:46:51 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Mon, 31 Jul 2023 09:23:29 +0800 Jason Wang wrote:
> > > I'd step back and ask you why do you want to use AF_XDP with virtio.
> > > Instead of bifurcating one virtio instance into different queues why
> > > not create a separate virtio instance?
> >
> > I'm not sure I get this, but do you mean a separate virtio device that
> > owns AF_XDP queues only? If I understand it correctly, bifurcating is
> > one of the key advantages of AF_XDP. What's more, current virtio
> > doesn't support being split at queue (pair) level. And it may still
> > suffer from the yes/no DMA API issue.
>
> I guess we should step even further back and ask Xuan what the use case
> is, because I'm not very sure. All we hear is "enable AF_XDP on virtio"
> but AF_XDP is barely used on real HW, so why?


Why just for real HW?

I want to enable AF_XDP on virtio-net. Then the user can send/recv packets by
AF_XDP bypass through the kernel. That has be used on large scale.

I donot know what is the problem of the virtio-net.
Why do you think that the virtio-net cannot work with AF_XDP?


>
> Bifurcating makes (used to make?) some sense in case of real HW when you
> had only one PCI function and had to subdivide it.

Sorry I do not get this.


> Virtio is either a SW
> construct or offloaded to very capable HW, so either way cost of
> creating an extra instance for DPDK or whatever else is very low.


The extra instance is virtio-net?

I think there is a gap. So let me give you a brief introduction of our case.

Firstly, we donot use dpdk. We use the AF_XDP, because of that the AF_XDP is
more simpler and easy to deploy for the nginx.

We use the AF_XDP to speedup the UDP of the quic. By the library, the APP just
needs some simple change.

On the AliYun, the net driver is virtio-net. So we want the virtio-net support
the AF_XDP.

I guess what you mean is that we can speed up through the cooperation of devices
and drivers, but our machines are public clouds, and we cannot change the
back-end devices of virtio under normal circumstances.

Here I do not know the different of the real hw and the virtio-net.


Thanks.
Jakub Kicinski Aug. 1, 2023, 2:36 a.m. UTC | #20
On Tue, 1 Aug 2023 10:03:44 +0800 Xuan Zhuo wrote:
> > Virtio is either a SW
> > construct or offloaded to very capable HW, so either way cost of
> > creating an extra instance for DPDK or whatever else is very low.  
> 
> The extra instance is virtio-net?
> 
> I think there is a gap. So let me give you a brief introduction of our case.
> 
> Firstly, we donot use dpdk. We use the AF_XDP, because of that the AF_XDP is
> more simpler and easy to deploy for the nginx.
> 
> We use the AF_XDP to speedup the UDP of the quic. By the library, the APP just
> needs some simple change.
> 
> On the AliYun, the net driver is virtio-net. So we want the virtio-net support
> the AF_XDP.
> 
> I guess what you mean is that we can speed up through the cooperation of devices
> and drivers, but our machines are public clouds, and we cannot change the
> back-end devices of virtio under normal circumstances.
> 
> Here I do not know the different of the real hw and the virtio-net.

You have this working and benchmarked or this is just and idea?

What about io_uring zero copy w/ pre-registered buffers.
You'll get csum offload, GSO, all the normal perf features.
Xuan Zhuo Aug. 1, 2023, 2:57 a.m. UTC | #21
On Mon, 31 Jul 2023 19:36:06 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 1 Aug 2023 10:03:44 +0800 Xuan Zhuo wrote:
> > > Virtio is either a SW
> > > construct or offloaded to very capable HW, so either way cost of
> > > creating an extra instance for DPDK or whatever else is very low.
> >
> > The extra instance is virtio-net?
> >
> > I think there is a gap. So let me give you a brief introduction of our case.
> >
> > Firstly, we donot use dpdk. We use the AF_XDP, because of that the AF_XDP is
> > more simpler and easy to deploy for the nginx.
> >
> > We use the AF_XDP to speedup the UDP of the quic. By the library, the APP just
> > needs some simple change.
> >
> > On the AliYun, the net driver is virtio-net. So we want the virtio-net support
> > the AF_XDP.
> >
> > I guess what you mean is that we can speed up through the cooperation of devices
> > and drivers, but our machines are public clouds, and we cannot change the
> > back-end devices of virtio under normal circumstances.
> >
> > Here I do not know the different of the real hw and the virtio-net.
>
> You have this working and benchmarked or this is just and idea?

This is not just an idea. I said that has been used on large scale.

This is the library for the APP to use the AF_XDP. We has open it.
https://gitee.com/anolis/libxudp

This is the Alibaba version of the nginx. That has been opened, that supported
to work with the libray to use AF_XDP.
http://tengine.taobao.org/

I supported this on our kernel release Anolis/Alinux.

The work was done about 2 years ago. You know, I pushed the first version to
enable AF_XDP on virtio-net about two years ago. I never thought the job would
be so difficult.

The nic (virtio-net) of AliYun can reach 24,000,000PPS.
So I think there is no different with the real HW on the performance.

With the AF_XDP, the UDP pps is seven times that of the kernel udp stack.

>
> What about io_uring zero copy w/ pre-registered buffers.
> You'll get csum offload, GSO, all the normal perf features.

We tried io-uring, but it was not suitable for our scenario.

Yes, now the AF_XDP does not support the csum offload and GSO.
This is indeed a small problem.

Thanks.
Jakub Kicinski Aug. 1, 2023, 3:45 p.m. UTC | #22
On Tue, 1 Aug 2023 10:57:30 +0800 Xuan Zhuo wrote:
> > You have this working and benchmarked or this is just and idea?  
> 
> This is not just an idea. I said that has been used on large scale.
> 
> This is the library for the APP to use the AF_XDP. We has open it.
> https://gitee.com/anolis/libxudp
> 
> This is the Alibaba version of the nginx. That has been opened, that supported
> to work with the libray to use AF_XDP.
> http://tengine.taobao.org/
> 
> I supported this on our kernel release Anolis/Alinux.

Interesting!

> The work was done about 2 years ago. You know, I pushed the first version to
> enable AF_XDP on virtio-net about two years ago. I never thought the job would
> be so difficult.

Me neither, but it is what it is.

> The nic (virtio-net) of AliYun can reach 24,000,000PPS.
> So I think there is no different with the real HW on the performance.
> 
> With the AF_XDP, the UDP pps is seven times that of the kernel udp stack.

UDP pps or QUIC pps? UDP with or without GSO?

Do you have measurements of how much it saves in real world workloads?
I'm asking mostly out of curiosity, not to question the use case.

> > What about io_uring zero copy w/ pre-registered buffers.
> > You'll get csum offload, GSO, all the normal perf features.  
> 
> We tried io-uring, but it was not suitable for our scenario.
> 
> Yes, now the AF_XDP does not support the csum offload and GSO.
> This is indeed a small problem.

Can you say more about io-uring suitability? It can do zero copy
and recently-ish Pavel optimized it quite a bit.
Michael S. Tsirkin Aug. 1, 2023, 4:17 p.m. UTC | #23
On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > >
> > > > > Yes, please.
> > > >
> > > >
> > > > I am not sure I got this fully.
> > > >
> > > > Are you mean this:
> > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > >
> > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > No care the device is non-dma device or dma device.
> > >
> > > yes
> > >
> > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > >
> > > We'll worry about AF_XDP when the patch is posted.
> >
> > YES.
> >
> > We discussed it. They voted 'no'.
> >
> > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> 
> 
> Hi guys, this topic is stuck again. How should I proceed with this work?
> 
> Let me briefly summarize:
> 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> the driver layer, we need to support these APIs. The current conclusion of
> AF_XDP is no.
> 
> 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> driver. This idea seems to be inconsistent with the framework design of DMA. The
> conclusion is no.
> 
> 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> uses DMA API. And this type of device is the future direction, so we only
> support DMA premapped for this type of virtio device. The problem with this
> solution is that virtqueue_dma_dev() only returns dev in some cases, because
> VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> This option is currently NO.
> 
> So I'm wondering what should I do, from a DMA point of view, is there any
> solution in case of using DMA API?
> 
> Thank you


I think it's ok at this point, Christoph just asked you
to add wrappers for map/unmap for use in virtio code.
Seems like a cosmetic change, shouldn't be hard.
Otherwise I haven't seen significant comments.


Christoph do I summarize what you are saying correctly?
Xuan Zhuo Aug. 2, 2023, 1:36 a.m. UTC | #24
On Tue, 1 Aug 2023 08:45:10 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
> On Tue, 1 Aug 2023 10:57:30 +0800 Xuan Zhuo wrote:
> > > You have this working and benchmarked or this is just and idea?
> >
> > This is not just an idea. I said that has been used on large scale.
> >
> > This is the library for the APP to use the AF_XDP. We has open it.
> > https://gitee.com/anolis/libxudp
> >
> > This is the Alibaba version of the nginx. That has been opened, that supported
> > to work with the libray to use AF_XDP.
> > http://tengine.taobao.org/
> >
> > I supported this on our kernel release Anolis/Alinux.
>
> Interesting!
>
> > The work was done about 2 years ago. You know, I pushed the first version to
> > enable AF_XDP on virtio-net about two years ago. I never thought the job would
> > be so difficult.
>
> Me neither, but it is what it is.
>
> > The nic (virtio-net) of AliYun can reach 24,000,000PPS.
> > So I think there is no different with the real HW on the performance.
> >
> > With the AF_XDP, the UDP pps is seven times that of the kernel udp stack.
>
> UDP pps or QUIC pps? UDP with or without GSO?

UDP PPS without GSO.

>
> Do you have measurements of how much it saves in real world workloads?
> I'm asking mostly out of curiosity, not to question the use case.

YES,the result is affected by the request size, we can reach 10-40%.
The smaller the request size, the lower the result.

>
> > > What about io_uring zero copy w/ pre-registered buffers.
> > > You'll get csum offload, GSO, all the normal perf features.
> >
> > We tried io-uring, but it was not suitable for our scenario.
> >
> > Yes, now the AF_XDP does not support the csum offload and GSO.
> > This is indeed a small problem.
>
> Can you say more about io-uring suitability? It can do zero copy
> and recently-ish Pavel optimized it quite a bit.

First, AF_XDP is also zero-copy. We also use XDP for a few things.

And this was all about two years ago, so we have to say something about io-uring
two years ago.

As far as I know, io-uring still use kernel udp stack, AF_XDP can
skip all kernel stack directly to driver.

So here, io-ring does not have too much advantage.

Thanks.
Xuan Zhuo Aug. 2, 2023, 1:49 a.m. UTC | #25
On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > >
> > > > > > Yes, please.
> > > > >
> > > > >
> > > > > I am not sure I got this fully.
> > > > >
> > > > > Are you mean this:
> > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > >
> > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > No care the device is non-dma device or dma device.
> > > >
> > > > yes
> > > >
> > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > >
> > > > We'll worry about AF_XDP when the patch is posted.
> > >
> > > YES.
> > >
> > > We discussed it. They voted 'no'.
> > >
> > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> >
> >
> > Hi guys, this topic is stuck again. How should I proceed with this work?
> >
> > Let me briefly summarize:
> > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > the driver layer, we need to support these APIs. The current conclusion of
> > AF_XDP is no.
> >
> > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > conclusion is no.
> >
> > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > uses DMA API. And this type of device is the future direction, so we only
> > support DMA premapped for this type of virtio device. The problem with this
> > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> > This option is currently NO.
> >
> > So I'm wondering what should I do, from a DMA point of view, is there any
> > solution in case of using DMA API?
> >
> > Thank you
>
>
> I think it's ok at this point, Christoph just asked you
> to add wrappers for map/unmap for use in virtio code.
> Seems like a cosmetic change, shouldn't be hard.

Yes, that is not hard, I has this code.

But, you mean that the wrappers is just used for the virtio driver code?
And we also offer the  API virtqueue_dma_dev() at the same time?
Then the driver will has two chooses to do DMA.

Is that so?


> Otherwise I haven't seen significant comments.
>
>
> Christoph do I summarize what you are saying correctly?
> --
> MST
>
Pavel Begunkov Aug. 2, 2023, 11:12 a.m. UTC | #26
On 8/2/23 02:36, Xuan Zhuo wrote:
> On Tue, 1 Aug 2023 08:45:10 -0700, Jakub Kicinski <kuba@kernel.org> wrote:
>> On Tue, 1 Aug 2023 10:57:30 +0800 Xuan Zhuo wrote:
>>>> You have this working and benchmarked or this is just and idea?
>>>
>>> This is not just an idea. I said that has been used on large scale.
>>>
>>> This is the library for the APP to use the AF_XDP. We has open it.
>>> https://gitee.com/anolis/libxudp
>>>
>>> This is the Alibaba version of the nginx. That has been opened, that supported
>>> to work with the libray to use AF_XDP.
>>> http://tengine.taobao.org/
>>>
>>> I supported this on our kernel release Anolis/Alinux.
>>
>> Interesting!
>>
>>> The work was done about 2 years ago. You know, I pushed the first version to
>>> enable AF_XDP on virtio-net about two years ago. I never thought the job would
>>> be so difficult.
>>
>> Me neither, but it is what it is.
>>
>>> The nic (virtio-net) of AliYun can reach 24,000,000PPS.
>>> So I think there is no different with the real HW on the performance.
>>>
>>> With the AF_XDP, the UDP pps is seven times that of the kernel udp stack.
>>
>> UDP pps or QUIC pps? UDP with or without GSO?
> 
> UDP PPS without GSO.
> 
>>
>> Do you have measurements of how much it saves in real world workloads?
>> I'm asking mostly out of curiosity, not to question the use case.
> 
> YES,the result is affected by the request size, we can reach 10-40%.
> The smaller the request size, the lower the result.
> 
>>
>>>> What about io_uring zero copy w/ pre-registered buffers.
>>>> You'll get csum offload, GSO, all the normal perf features.
>>>
>>> We tried io-uring, but it was not suitable for our scenario.
>>>
>>> Yes, now the AF_XDP does not support the csum offload and GSO.
>>> This is indeed a small problem.
>>
>> Can you say more about io-uring suitability? It can do zero copy
>> and recently-ish Pavel optimized it quite a bit.
> 
> First, AF_XDP is also zero-copy. We also use XDP for a few things.
> 
> And this was all about two years ago, so we have to say something about io-uring
> two years ago.
> 
> As far as I know, io-uring still use kernel udp stack, AF_XDP can
> skip all kernel stack directly to driver.
> 
> So here, io-ring does not have too much advantage.

Unfortunately I'd agree. Most of it is in the net stack. It can be
optimised to a certain extent (IMHO far more modest than 7x) but would
need extensive reworking, and I don't think I saw any appetite for that
Xuan Zhuo Aug. 7, 2023, 6:14 a.m. UTC | #27
On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > >
> > > > > > > Yes, please.
> > > > > >
> > > > > >
> > > > > > I am not sure I got this fully.
> > > > > >
> > > > > > Are you mean this:
> > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > >
> > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > No care the device is non-dma device or dma device.
> > > > >
> > > > > yes
> > > > >
> > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > >
> > > > > We'll worry about AF_XDP when the patch is posted.
> > > >
> > > > YES.
> > > >
> > > > We discussed it. They voted 'no'.
> > > >
> > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > >
> > >
> > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > >
> > > Let me briefly summarize:
> > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > the driver layer, we need to support these APIs. The current conclusion of
> > > AF_XDP is no.
> > >
> > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > conclusion is no.
> > >
> > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > uses DMA API. And this type of device is the future direction, so we only
> > > support DMA premapped for this type of virtio device. The problem with this
> > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases. Otherwise NULL is returned.
> > > This option is currently NO.
> > >
> > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > solution in case of using DMA API?
> > >
> > > Thank you
> >
> >
> > I think it's ok at this point, Christoph just asked you
> > to add wrappers for map/unmap for use in virtio code.
> > Seems like a cosmetic change, shouldn't be hard.
>
> Yes, that is not hard, I has this code.
>
> But, you mean that the wrappers is just used for the virtio driver code?
> And we also offer the  API virtqueue_dma_dev() at the same time?
> Then the driver will has two chooses to do DMA.
>
> Is that so?

Ping.

Thanks

>
>
> > Otherwise I haven't seen significant comments.
> >
> >
> > Christoph do I summarize what you are saying correctly?
> > --
> > MST
> >
>
Jason Wang Aug. 8, 2023, 2:26 a.m. UTC | #28
On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > >
> > > > > > > > Yes, please.
> > > > > > >
> > > > > > >
> > > > > > > I am not sure I got this fully.
> > > > > > >
> > > > > > > Are you mean this:
> > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > >
> > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > No care the device is non-dma device or dma device.
> > > > > >
> > > > > > yes
> > > > > >
> > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > >
> > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > >
> > > > > YES.
> > > > >
> > > > > We discussed it. They voted 'no'.
> > > > >
> > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > >
> > > >
> > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > >
> > > > Let me briefly summarize:
> > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > AF_XDP is no.
> > > >
> > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > conclusion is no.
> > > >
> > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > uses DMA API. And this type of device is the future direction, so we only
> > > > support DMA premapped for this type of virtio device. The problem with this
> > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.

Could you explain the issue a little bit more?

E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
virtqueue_dma_dev() only return dev in some cases?

Thanks

>Otherwise NULL is returned.
> > > > This option is currently NO.
> > > >
> > > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > > solution in case of using DMA API?
> > > >
> > > > Thank you
> > >
> > >
> > > I think it's ok at this point, Christoph just asked you
> > > to add wrappers for map/unmap for use in virtio code.
> > > Seems like a cosmetic change, shouldn't be hard.
> >
> > Yes, that is not hard, I has this code.
> >
> > But, you mean that the wrappers is just used for the virtio driver code?
> > And we also offer the  API virtqueue_dma_dev() at the same time?
> > Then the driver will has two chooses to do DMA.
> >
> > Is that so?
>
> Ping.
>
> Thanks
>
> >
> >
> > > Otherwise I haven't seen significant comments.
> > >
> > >
> > > Christoph do I summarize what you are saying correctly?
> > > --
> > > MST
> > >
> >
>
Xuan Zhuo Aug. 8, 2023, 2:47 a.m. UTC | #29
On Tue, 8 Aug 2023 10:26:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > > >
> > > > > > > > > Yes, please.
> > > > > > > >
> > > > > > > >
> > > > > > > > I am not sure I got this fully.
> > > > > > > >
> > > > > > > > Are you mean this:
> > > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > > >
> > > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > > No care the device is non-dma device or dma device.
> > > > > > >
> > > > > > > yes
> > > > > > >
> > > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > > >
> > > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > > >
> > > > > > YES.
> > > > > >
> > > > > > We discussed it. They voted 'no'.
> > > > > >
> > > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > > >
> > > > >
> > > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > > >
> > > > > Let me briefly summarize:
> > > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > > AF_XDP is no.
> > > > >
> > > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > > conclusion is no.
> > > > >
> > > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > > uses DMA API. And this type of device is the future direction, so we only
> > > > > support DMA premapped for this type of virtio device. The problem with this
> > > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.
>
> Could you explain the issue a little bit more?
>
> E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
> virtqueue_dma_dev() only return dev in some cases?

The behavior of virtqueue_dma_dev() is not related to AF_XDP.

The return value of virtqueue_dma_dev() is used for the DMA APIs. So it can
return dma dev when the virtio is with ACCESS_PLATFORM. If virtio is without
ACCESS_PLATFORM then it MUST return NULL.

In the virtio-net driver, if the virtqueue_dma_dev() returns dma dev,
we can enable AF_XDP. If not, we return error to AF_XDP.

Thanks




>
> Thanks
>
> >Otherwise NULL is returned.
> > > > > This option is currently NO.
> > > > >
> > > > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > > > solution in case of using DMA API?
> > > > >
> > > > > Thank you
> > > >
> > > >
> > > > I think it's ok at this point, Christoph just asked you
> > > > to add wrappers for map/unmap for use in virtio code.
> > > > Seems like a cosmetic change, shouldn't be hard.
> > >
> > > Yes, that is not hard, I has this code.
> > >
> > > But, you mean that the wrappers is just used for the virtio driver code?
> > > And we also offer the  API virtqueue_dma_dev() at the same time?
> > > Then the driver will has two chooses to do DMA.
> > >
> > > Is that so?
> >
> > Ping.
> >
> > Thanks
> >
> > >
> > >
> > > > Otherwise I haven't seen significant comments.
> > > >
> > > >
> > > > Christoph do I summarize what you are saying correctly?
> > > > --
> > > > MST
> > > >
> > >
> >
>
Jason Wang Aug. 8, 2023, 3:08 a.m. UTC | #30
On Tue, Aug 8, 2023 at 10:52 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Tue, 8 Aug 2023 10:26:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > > > >
> > > > > > > > > > Yes, please.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > I am not sure I got this fully.
> > > > > > > > >
> > > > > > > > > Are you mean this:
> > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > > > >
> > > > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > > > No care the device is non-dma device or dma device.
> > > > > > > >
> > > > > > > > yes
> > > > > > > >
> > > > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > > > >
> > > > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > > > >
> > > > > > > YES.
> > > > > > >
> > > > > > > We discussed it. They voted 'no'.
> > > > > > >
> > > > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > > > >
> > > > > >
> > > > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > > > >
> > > > > > Let me briefly summarize:
> > > > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > > > AF_XDP is no.
> > > > > >
> > > > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > > > conclusion is no.
> > > > > >
> > > > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > > > uses DMA API. And this type of device is the future direction, so we only
> > > > > > support DMA premapped for this type of virtio device. The problem with this
> > > > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.
> >
> > Could you explain the issue a little bit more?
> >
> > E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
> > virtqueue_dma_dev() only return dev in some cases?
>
> The behavior of virtqueue_dma_dev() is not related to AF_XDP.
>
> The return value of virtqueue_dma_dev() is used for the DMA APIs. So it can
> return dma dev when the virtio is with ACCESS_PLATFORM. If virtio is without
> ACCESS_PLATFORM then it MUST return NULL.
>
> In the virtio-net driver, if the virtqueue_dma_dev() returns dma dev,
> we can enable AF_XDP. If not, we return error to AF_XDP.

Yes, as discussed, just having wrappers in the virtio_ring and doing
the switch there. Then can virtio-net use them without worrying about
DMA details?

Thanks

>
> Thanks
>
>
>
>
> >
> > Thanks
> >
> > >Otherwise NULL is returned.
> > > > > > This option is currently NO.
> > > > > >
> > > > > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > > > > solution in case of using DMA API?
> > > > > >
> > > > > > Thank you
> > > > >
> > > > >
> > > > > I think it's ok at this point, Christoph just asked you
> > > > > to add wrappers for map/unmap for use in virtio code.
> > > > > Seems like a cosmetic change, shouldn't be hard.
> > > >
> > > > Yes, that is not hard, I has this code.
> > > >
> > > > But, you mean that the wrappers is just used for the virtio driver code?
> > > > And we also offer the  API virtqueue_dma_dev() at the same time?
> > > > Then the driver will has two chooses to do DMA.
> > > >
> > > > Is that so?
> > >
> > > Ping.
> > >
> > > Thanks
> > >
> > > >
> > > >
> > > > > Otherwise I haven't seen significant comments.
> > > > >
> > > > >
> > > > > Christoph do I summarize what you are saying correctly?
> > > > > --
> > > > > MST
> > > > >
> > > >
> > >
> >
>
Xuan Zhuo Aug. 8, 2023, 3:09 a.m. UTC | #31
On Tue, 8 Aug 2023 11:08:09 +0800, Jason Wang <jasowang@redhat.com> wrote:
> On Tue, Aug 8, 2023 at 10:52 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Tue, 8 Aug 2023 10:26:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > > > > >
> > > > > > > > > > > Yes, please.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I am not sure I got this fully.
> > > > > > > > > >
> > > > > > > > > > Are you mean this:
> > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > > > > >
> > > > > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > > > > No care the device is non-dma device or dma device.
> > > > > > > > >
> > > > > > > > > yes
> > > > > > > > >
> > > > > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > > > > >
> > > > > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > > > > >
> > > > > > > > YES.
> > > > > > > >
> > > > > > > > We discussed it. They voted 'no'.
> > > > > > > >
> > > > > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > > > > >
> > > > > > >
> > > > > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > > > > >
> > > > > > > Let me briefly summarize:
> > > > > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > > > > AF_XDP is no.
> > > > > > >
> > > > > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > > > > conclusion is no.
> > > > > > >
> > > > > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > > > > uses DMA API. And this type of device is the future direction, so we only
> > > > > > > support DMA premapped for this type of virtio device. The problem with this
> > > > > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.
> > >
> > > Could you explain the issue a little bit more?
> > >
> > > E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
> > > virtqueue_dma_dev() only return dev in some cases?
> >
> > The behavior of virtqueue_dma_dev() is not related to AF_XDP.
> >
> > The return value of virtqueue_dma_dev() is used for the DMA APIs. So it can
> > return dma dev when the virtio is with ACCESS_PLATFORM. If virtio is without
> > ACCESS_PLATFORM then it MUST return NULL.
> >
> > In the virtio-net driver, if the virtqueue_dma_dev() returns dma dev,
> > we can enable AF_XDP. If not, we return error to AF_XDP.
>
> Yes, as discussed, just having wrappers in the virtio_ring and doing
> the switch there. Then can virtio-net use them without worrying about
> DMA details?


Yes. In the virtio drivers, we can use the wrappers. That is ok.

But we also need to support virtqueue_dma_dev() for AF_XDP, because that the
AF_XDP will not use the wrappers.

So that is ok for you?

Thanks.

>
> Thanks
>
> >
> > Thanks
> >
> >
> >
> >
> > >
> > > Thanks
> > >
> > > >Otherwise NULL is returned.
> > > > > > > This option is currently NO.
> > > > > > >
> > > > > > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > > > > > solution in case of using DMA API?
> > > > > > >
> > > > > > > Thank you
> > > > > >
> > > > > >
> > > > > > I think it's ok at this point, Christoph just asked you
> > > > > > to add wrappers for map/unmap for use in virtio code.
> > > > > > Seems like a cosmetic change, shouldn't be hard.
> > > > >
> > > > > Yes, that is not hard, I has this code.
> > > > >
> > > > > But, you mean that the wrappers is just used for the virtio driver code?
> > > > > And we also offer the  API virtqueue_dma_dev() at the same time?
> > > > > Then the driver will has two chooses to do DMA.
> > > > >
> > > > > Is that so?
> > > >
> > > > Ping.
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > >
> > > > > > Otherwise I haven't seen significant comments.
> > > > > >
> > > > > >
> > > > > > Christoph do I summarize what you are saying correctly?
> > > > > > --
> > > > > > MST
> > > > > >
> > > > >
> > > >
> > >
> >
>
Jason Wang Aug. 8, 2023, 3:49 a.m. UTC | #32
On Tue, Aug 8, 2023 at 11:12 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Tue, 8 Aug 2023 11:08:09 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Tue, Aug 8, 2023 at 10:52 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Tue, 8 Aug 2023 10:26:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > >
> > > > > On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > > > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > > > > > >
> > > > > > > > > > > > Yes, please.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > I am not sure I got this fully.
> > > > > > > > > > >
> > > > > > > > > > > Are you mean this:
> > > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > > > > > >
> > > > > > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > > > > > No care the device is non-dma device or dma device.
> > > > > > > > > >
> > > > > > > > > > yes
> > > > > > > > > >
> > > > > > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > > > > > >
> > > > > > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > > > > > >
> > > > > > > > > YES.
> > > > > > > > >
> > > > > > > > > We discussed it. They voted 'no'.
> > > > > > > > >
> > > > > > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > > > > > >
> > > > > > > >
> > > > > > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > > > > > >
> > > > > > > > Let me briefly summarize:
> > > > > > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > > > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > > > > > AF_XDP is no.
> > > > > > > >
> > > > > > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > > > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > > > > > conclusion is no.
> > > > > > > >
> > > > > > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > > > > > uses DMA API. And this type of device is the future direction, so we only
> > > > > > > > support DMA premapped for this type of virtio device. The problem with this
> > > > > > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > > > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.
> > > >
> > > > Could you explain the issue a little bit more?
> > > >
> > > > E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
> > > > virtqueue_dma_dev() only return dev in some cases?
> > >
> > > The behavior of virtqueue_dma_dev() is not related to AF_XDP.
> > >
> > > The return value of virtqueue_dma_dev() is used for the DMA APIs. So it can
> > > return dma dev when the virtio is with ACCESS_PLATFORM. If virtio is without
> > > ACCESS_PLATFORM then it MUST return NULL.
> > >
> > > In the virtio-net driver, if the virtqueue_dma_dev() returns dma dev,
> > > we can enable AF_XDP. If not, we return error to AF_XDP.
> >
> > Yes, as discussed, just having wrappers in the virtio_ring and doing
> > the switch there. Then can virtio-net use them without worrying about
> > DMA details?
>
>
> Yes. In the virtio drivers, we can use the wrappers. That is ok.
>
> But we also need to support virtqueue_dma_dev() for AF_XDP, because that the
> AF_XDP will not use the wrappers.

You mean AF_XDP core or other? Could you give me an example?

Thanks

>
> So that is ok for you?
>
> Thanks.
>
> >
> > Thanks
> >
> > >
> > > Thanks
> > >
> > >
> > >
> > >
> > > >
> > > > Thanks
> > > >
> > > > >Otherwise NULL is returned.
> > > > > > > > This option is currently NO.
> > > > > > > >
> > > > > > > > So I'm wondering what should I do, from a DMA point of view, is there any
> > > > > > > > solution in case of using DMA API?
> > > > > > > >
> > > > > > > > Thank you
> > > > > > >
> > > > > > >
> > > > > > > I think it's ok at this point, Christoph just asked you
> > > > > > > to add wrappers for map/unmap for use in virtio code.
> > > > > > > Seems like a cosmetic change, shouldn't be hard.
> > > > > >
> > > > > > Yes, that is not hard, I has this code.
> > > > > >
> > > > > > But, you mean that the wrappers is just used for the virtio driver code?
> > > > > > And we also offer the  API virtqueue_dma_dev() at the same time?
> > > > > > Then the driver will has two chooses to do DMA.
> > > > > >
> > > > > > Is that so?
> > > > >
> > > > > Ping.
> > > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > >
> > > > > > > Otherwise I haven't seen significant comments.
> > > > > > >
> > > > > > >
> > > > > > > Christoph do I summarize what you are saying correctly?
> > > > > > > --
> > > > > > > MST
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Jason Wang Aug. 8, 2023, 3:59 a.m. UTC | #33
On Tue, Aug 8, 2023 at 11:57 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Tue, 8 Aug 2023 11:49:08 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > On Tue, Aug 8, 2023 at 11:12 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Tue, 8 Aug 2023 11:08:09 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > On Tue, Aug 8, 2023 at 10:52 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > >
> > > > > On Tue, 8 Aug 2023 10:26:04 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > > > On Mon, Aug 7, 2023 at 2:15 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > >
> > > > > > > On Wed, 2 Aug 2023 09:49:31 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > > > On Tue, 1 Aug 2023 12:17:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > > > On Fri, Jul 28, 2023 at 02:02:33PM +0800, Xuan Zhuo wrote:
> > > > > > > > > > On Tue, 25 Jul 2023 19:07:23 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > > > > > > > > On Tue, 25 Jul 2023 03:34:34 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > > > > > > > > On Tue, Jul 25, 2023 at 10:13:48AM +0800, Xuan Zhuo wrote:
> > > > > > > > > > > > > On Mon, 24 Jul 2023 09:43:42 -0700, Christoph Hellwig <hch@infradead.org> wrote:
> > > > > > > > > > > > > > On Thu, Jul 20, 2023 at 01:21:07PM -0400, Michael S. Tsirkin wrote:
> > > > > > > > > > > > > > > Well I think we can add wrappers like virtio_dma_sync and so on.
> > > > > > > > > > > > > > > There are NOP for non-dma so passing the dma device is harmless.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Yes, please.
> > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > I am not sure I got this fully.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Are you mean this:
> > > > > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-8-xuanzhuo@linux.alibaba.com/
> > > > > > > > > > > > > https://lore.kernel.org/all/20230214072704.126660-9-xuanzhuo@linux.alibaba.com/
> > > > > > > > > > > > >
> > > > > > > > > > > > > Then the driver must do dma operation(map and sync) by these virtio_dma_* APIs.
> > > > > > > > > > > > > No care the device is non-dma device or dma device.
> > > > > > > > > > > >
> > > > > > > > > > > > yes
> > > > > > > > > > > >
> > > > > > > > > > > > > Then the AF_XDP must use these virtio_dma_* APIs for virtio device.
> > > > > > > > > > > >
> > > > > > > > > > > > We'll worry about AF_XDP when the patch is posted.
> > > > > > > > > > >
> > > > > > > > > > > YES.
> > > > > > > > > > >
> > > > > > > > > > > We discussed it. They voted 'no'.
> > > > > > > > > > >
> > > > > > > > > > > http://lore.kernel.org/all/20230424082856.15c1e593@kernel.org
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Hi guys, this topic is stuck again. How should I proceed with this work?
> > > > > > > > > >
> > > > > > > > > > Let me briefly summarize:
> > > > > > > > > > 1. The problem with adding virtio_dma_{map, sync} api is that, for AF_XDP and
> > > > > > > > > > the driver layer, we need to support these APIs. The current conclusion of
> > > > > > > > > > AF_XDP is no.
> > > > > > > > > >
> > > > > > > > > > 2. Set dma_set_mask_and_coherent, then we can use DMA API uniformly inside
> > > > > > > > > > driver. This idea seems to be inconsistent with the framework design of DMA. The
> > > > > > > > > > conclusion is no.
> > > > > > > > > >
> > > > > > > > > > 3. We noticed that if the virtio device supports VIRTIO_F_ACCESS_PLATFORM, it
> > > > > > > > > > uses DMA API. And this type of device is the future direction, so we only
> > > > > > > > > > support DMA premapped for this type of virtio device. The problem with this
> > > > > > > > > > solution is that virtqueue_dma_dev() only returns dev in some cases, because
> > > > > > > > > > VIRTIO_F_ACCESS_PLATFORM is supported in such cases.
> > > > > >
> > > > > > Could you explain the issue a little bit more?
> > > > > >
> > > > > > E.g if we limit AF_XDP to ACESS_PLATFROM only, why does
> > > > > > virtqueue_dma_dev() only return dev in some cases?
> > > > >
> > > > > The behavior of virtqueue_dma_dev() is not related to AF_XDP.
> > > > >
> > > > > The return value of virtqueue_dma_dev() is used for the DMA APIs. So it can
> > > > > return dma dev when the virtio is with ACCESS_PLATFORM. If virtio is without
> > > > > ACCESS_PLATFORM then it MUST return NULL.
> > > > >
> > > > > In the virtio-net driver, if the virtqueue_dma_dev() returns dma dev,
> > > > > we can enable AF_XDP. If not, we return error to AF_XDP.
> > > >
> > > > Yes, as discussed, just having wrappers in the virtio_ring and doing
> > > > the switch there. Then can virtio-net use them without worrying about
> > > > DMA details?
> > >
> > >
> > > Yes. In the virtio drivers, we can use the wrappers. That is ok.
> > >
> > > But we also need to support virtqueue_dma_dev() for AF_XDP, because that the
> > > AF_XDP will not use the wrappers.
> >
> > You mean AF_XDP core or other? Could you give me an example?
>
>
> Yes. The AF_XDP core.
>
> Now the AF_XDP core will do the dma operation.  Because that the memory is
> allocated by the user from the user space.  So before putting the memory to the
> driver, the AF_XDP will do the dma mapping.
>
>
> int xp_dma_map(struct xsk_buff_pool *pool, struct device *dev,
>                unsigned long attrs, struct page **pages, u32 nr_pages)
> {

I think it's the driver who passes the device pointer here. Anything I missed?

Thanks
Xuan Zhuo Aug. 10, 2023, 1:56 a.m. UTC | #34
Ping!!

Could we push this to the next linux version?

Thanks.
Jason Wang Aug. 10, 2023, 6:37 a.m. UTC | #35
On Thu, Aug 10, 2023 at 9:59 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
>
> Ping!!
>
> Could we push this to the next linux version?

How about implementing the wrappers along with virtqueue_dma_dev() to
see if Christoph is happy?

Thanks

>
> Thanks.
>
Michael S. Tsirkin Aug. 10, 2023, 6:39 a.m. UTC | #36
On Thu, Aug 10, 2023 at 09:56:54AM +0800, Xuan Zhuo wrote:
> 
> Ping!!
> 
> Could we push this to the next linux version?
> 
> Thanks.

You sent v12, so not this one for sure.
v12 triggered kbuild warnings, you need to fix them and repost.
Note that I'm on vacation from monday, so if you want this
merged this needs to be addressed ASAP.
Michael S. Tsirkin Aug. 10, 2023, 6:40 a.m. UTC | #37
On Thu, Aug 10, 2023 at 02:37:20PM +0800, Jason Wang wrote:
> On Thu, Aug 10, 2023 at 9:59 AM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> >
> > Ping!!
> >
> > Could we push this to the next linux version?
> 
> How about implementing the wrappers along with virtqueue_dma_dev() to
> see if Christoph is happy?
> 
> Thanks

That, too.

> >
> > Thanks.
> >
Xuan Zhuo Aug. 10, 2023, 6:47 a.m. UTC | #38
On Thu, 10 Aug 2023 02:39:47 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Aug 10, 2023 at 09:56:54AM +0800, Xuan Zhuo wrote:
> >
> > Ping!!
> >
> > Could we push this to the next linux version?
> >
> > Thanks.
>
> You sent v12, so not this one for sure.
> v12 triggered kbuild warnings, you need to fix them and repost.
> Note that I'm on vacation from monday, so if you want this
> merged this needs to be addressed ASAP.

I will post a new version today. The driver will use the wrappers.

Thanks.


>
> --
> MST
>
diff mbox series

Patch

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index d471dee3f4f7..1fb2c6dca9ea 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2265,6 +2265,23 @@  int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
 }
 EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
 
+/**
+ * virtqueue_dma_dev - get the dma dev
+ * @_vq: the struct virtqueue we're talking about.
+ *
+ * Returns the dma dev. That can been used for dma api.
+ */
+struct device *virtqueue_dma_dev(struct virtqueue *_vq)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+
+	if (vq->use_dma_api)
+		return vring_dma_dev(vq);
+	else
+		return NULL;
+}
+EXPORT_SYMBOL_GPL(virtqueue_dma_dev);
+
 /**
  * virtqueue_kick_prepare - first half of split virtqueue_kick call.
  * @_vq: the struct virtqueue
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 2efd07b79ecf..35d175121cc6 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -61,6 +61,8 @@  int virtqueue_add_sgs(struct virtqueue *vq,
 		      void *data,
 		      gfp_t gfp);
 
+struct device *virtqueue_dma_dev(struct virtqueue *vq);
+
 bool virtqueue_kick(struct virtqueue *vq);
 
 bool virtqueue_kick_prepare(struct virtqueue *vq);