mbox series

[v3,00/17] virtio pci support VIRTIO_F_RING_RESET

Message ID 20220126073533.44994-1-xuanzhuo@linux.alibaba.com (mailing list archive)
Headers show
Series virtio pci support VIRTIO_F_RING_RESET | expand

Message

Xuan Zhuo Jan. 26, 2022, 7:35 a.m. UTC 
================================================================================
The virtio spec already supports the virtio queue reset function. This patch set
is to add this function to the kernel. The relevant virtio spec information is
here:

    https://github.com/oasis-tcs/virtio-spec/issues/124

Also regarding MMIO support for queue reset, I plan to support it after this
patch is passed.

#14-#17 is the disable/enable function of rx/tx pair implemented by virtio-net
using the new helper. This function is not currently referenced by other
functions. It is more to show the usage of the new helper, I not sure if they
are going to be merged together.

Please review. Thanks.

v3:
  1. keep vq, irq unreleased

Xuan Zhuo (17):
  virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
  virtio: queue_reset: add VIRTIO_F_RING_RESET
  virtio: queue_reset: struct virtio_config_ops add callbacks for
    queue_reset
  virtio: queue_reset: add helper
  vritio_ring: queue_reset: extract the release function of the vq ring
  virtio_ring: queue_reset: split: add __vring_init_virtqueue()
  virtio_ring: queue_reset: split: support enable reset queue
  virtio_ring: queue_reset: packed: support enable reset queue
  virtio_ring: queue_reset: add vring_reset_virtqueue()
  virtio_pci: queue_reset: update struct virtio_pci_common_cfg and
    option functions
  virtio_pci: queue_reset: release vq by vp_dev->vqs
  virtio_pci: queue_reset: setup_vq use vring_setup_virtqueue()
  virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
  virtio_net: virtnet_tx_timeout() fix style
  virtio_net: virtnet_tx_timeout() stop ref sq->vq
  virtio_net: split free_unused_bufs()
  virtio_net: support pair disable/enable

 drivers/net/virtio_net.c               | 220 ++++++++++++++++++++++---
 drivers/virtio/virtio_pci_common.c     |  62 ++++---
 drivers/virtio/virtio_pci_common.h     |  11 +-
 drivers/virtio/virtio_pci_legacy.c     |   5 +-
 drivers/virtio/virtio_pci_modern.c     | 120 +++++++++++++-
 drivers/virtio/virtio_pci_modern_dev.c |  28 ++++
 drivers/virtio/virtio_ring.c           | 144 +++++++++++-----
 include/linux/virtio.h                 |   1 +
 include/linux/virtio_config.h          |  75 ++++++++-
 include/linux/virtio_pci_modern.h      |   2 +
 include/linux/virtio_ring.h            |  42 +++--
 include/uapi/linux/virtio_config.h     |   7 +-
 include/uapi/linux/virtio_pci.h        |   2 +
 13 files changed, 618 insertions(+), 101 deletions(-)

--
2.31.0

Comments

Jason Wang Feb. 7, 2022, 3:39 a.m. UTC | #1 
On Wed, Jan 26, 2022 at 3:35 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> ================================================================================
> The virtio spec already supports the virtio queue reset function. This patch set
> is to add this function to the kernel. The relevant virtio spec information is
> here:
>
>     https://github.com/oasis-tcs/virtio-spec/issues/124
>
> Also regarding MMIO support for queue reset, I plan to support it after this
> patch is passed.
>
> #14-#17 is the disable/enable function of rx/tx pair implemented by virtio-net
> using the new helper.

One thing that came to mind is the steering. E.g if we disable an RX,
should we stop steering packets to that queue?

Thanks

> This function is not currently referenced by other
> functions. It is more to show the usage of the new helper, I not sure if they
> are going to be merged together.
>
> Please review. Thanks.
>
> v3:
>   1. keep vq, irq unreleased
>
> Xuan Zhuo (17):
>   virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
>   virtio: queue_reset: add VIRTIO_F_RING_RESET
>   virtio: queue_reset: struct virtio_config_ops add callbacks for
>     queue_reset
>   virtio: queue_reset: add helper
>   vritio_ring: queue_reset: extract the release function of the vq ring
>   virtio_ring: queue_reset: split: add __vring_init_virtqueue()
>   virtio_ring: queue_reset: split: support enable reset queue
>   virtio_ring: queue_reset: packed: support enable reset queue
>   virtio_ring: queue_reset: add vring_reset_virtqueue()
>   virtio_pci: queue_reset: update struct virtio_pci_common_cfg and
>     option functions
>   virtio_pci: queue_reset: release vq by vp_dev->vqs
>   virtio_pci: queue_reset: setup_vq use vring_setup_virtqueue()
>   virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
>   virtio_net: virtnet_tx_timeout() fix style
>   virtio_net: virtnet_tx_timeout() stop ref sq->vq
>   virtio_net: split free_unused_bufs()
>   virtio_net: support pair disable/enable
>
>  drivers/net/virtio_net.c               | 220 ++++++++++++++++++++++---
>  drivers/virtio/virtio_pci_common.c     |  62 ++++---
>  drivers/virtio/virtio_pci_common.h     |  11 +-
>  drivers/virtio/virtio_pci_legacy.c     |   5 +-
>  drivers/virtio/virtio_pci_modern.c     | 120 +++++++++++++-
>  drivers/virtio/virtio_pci_modern_dev.c |  28 ++++
>  drivers/virtio/virtio_ring.c           | 144 +++++++++++-----
>  include/linux/virtio.h                 |   1 +
>  include/linux/virtio_config.h          |  75 ++++++++-
>  include/linux/virtio_pci_modern.h      |   2 +
>  include/linux/virtio_ring.h            |  42 +++--
>  include/uapi/linux/virtio_config.h     |   7 +-
>  include/uapi/linux/virtio_pci.h        |   2 +
>  13 files changed, 618 insertions(+), 101 deletions(-)
>
> --
> 2.31.0
>
Xuan Zhuo Feb. 8, 2022, 7:51 a.m. UTC | #2 
On Tue, 08 Feb 2022 11:14:45 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> On Tue, 8 Feb 2022 10:59:48 +0800, Jason Wang <jasowang@redhat.com> wrote:
> >
> > 在 2022/2/7 下午2:02, Xuan Zhuo 写道:
> > > On Mon, 7 Feb 2022 11:39:36 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > >> On Wed, Jan 26, 2022 at 3:35 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >>> ================================================================================
> > >>> The virtio spec already supports the virtio queue reset function. This patch set
> > >>> is to add this function to the kernel. The relevant virtio spec information is
> > >>> here:
> > >>>
> > >>>      https://github.com/oasis-tcs/virtio-spec/issues/124
> > >>>
> > >>> Also regarding MMIO support for queue reset, I plan to support it after this
> > >>> patch is passed.
> > >>>
> > >>> #14-#17 is the disable/enable function of rx/tx pair implemented by virtio-net
> > >>> using the new helper.
> > >> One thing that came to mind is the steering. E.g if we disable an RX,
> > >> should we stop steering packets to that queue?

Regarding this spec, if there are multiple queues disabled at the same time, it
will be a troublesome problem for the backend to select the queue, so I want to
directly define that only one queue is allowed to reset at the same time, do you
think this is appropriate?

In terms of the implementation of backend queue reselection, it would be more
convenient to implement if we drop packets directly. Do you think we must
implement this reselection function?

Thanks.

> > > Yes, we should reselect a queue.
> > >
> > > Thanks.
> >
> >
> > Maybe a spec patch for that?
>
> Yes, I also realized this. Although virtio-net's disable/enable is implemented
> based on queue reset, virtio-net still has to define its own flag and define
> some more detailed implementations.
>
> I'll think about it and post a spec patch.
>
> Thanks.
>
> >
> > Thanks
> >
> >
> > >
> > >> Thanks
> > >>
> > >>> This function is not currently referenced by other
> > >>> functions. It is more to show the usage of the new helper, I not sure if they
> > >>> are going to be merged together.
> > >>>
> > >>> Please review. Thanks.
> > >>>
> > >>> v3:
> > >>>    1. keep vq, irq unreleased
> > >>>
> > >>> Xuan Zhuo (17):
> > >>>    virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
> > >>>    virtio: queue_reset: add VIRTIO_F_RING_RESET
> > >>>    virtio: queue_reset: struct virtio_config_ops add callbacks for
> > >>>      queue_reset
> > >>>    virtio: queue_reset: add helper
> > >>>    vritio_ring: queue_reset: extract the release function of the vq ring
> > >>>    virtio_ring: queue_reset: split: add __vring_init_virtqueue()
> > >>>    virtio_ring: queue_reset: split: support enable reset queue
> > >>>    virtio_ring: queue_reset: packed: support enable reset queue
> > >>>    virtio_ring: queue_reset: add vring_reset_virtqueue()
> > >>>    virtio_pci: queue_reset: update struct virtio_pci_common_cfg and
> > >>>      option functions
> > >>>    virtio_pci: queue_reset: release vq by vp_dev->vqs
> > >>>    virtio_pci: queue_reset: setup_vq use vring_setup_virtqueue()
> > >>>    virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
> > >>>    virtio_net: virtnet_tx_timeout() fix style
> > >>>    virtio_net: virtnet_tx_timeout() stop ref sq->vq
> > >>>    virtio_net: split free_unused_bufs()
> > >>>    virtio_net: support pair disable/enable
> > >>>
> > >>>   drivers/net/virtio_net.c               | 220 ++++++++++++++++++++++---
> > >>>   drivers/virtio/virtio_pci_common.c     |  62 ++++---
> > >>>   drivers/virtio/virtio_pci_common.h     |  11 +-
> > >>>   drivers/virtio/virtio_pci_legacy.c     |   5 +-
> > >>>   drivers/virtio/virtio_pci_modern.c     | 120 +++++++++++++-
> > >>>   drivers/virtio/virtio_pci_modern_dev.c |  28 ++++
> > >>>   drivers/virtio/virtio_ring.c           | 144 +++++++++++-----
> > >>>   include/linux/virtio.h                 |   1 +
> > >>>   include/linux/virtio_config.h          |  75 ++++++++-
> > >>>   include/linux/virtio_pci_modern.h      |   2 +
> > >>>   include/linux/virtio_ring.h            |  42 +++--
> > >>>   include/uapi/linux/virtio_config.h     |   7 +-
> > >>>   include/uapi/linux/virtio_pci.h        |   2 +
> > >>>   13 files changed, 618 insertions(+), 101 deletions(-)
> > >>>
> > >>> --
> > >>> 2.31.0
> > >>>
> >
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Jason Wang Feb. 9, 2022, 5:39 a.m. UTC | #3 
On Tue, Feb 8, 2022 at 3:56 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Tue, 08 Feb 2022 11:14:45 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > On Tue, 8 Feb 2022 10:59:48 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > 在 2022/2/7 下午2:02, Xuan Zhuo 写道:
> > > > On Mon, 7 Feb 2022 11:39:36 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > >> On Wed, Jan 26, 2022 at 3:35 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >>> ================================================================================
> > > >>> The virtio spec already supports the virtio queue reset function. This patch set
> > > >>> is to add this function to the kernel. The relevant virtio spec information is
> > > >>> here:
> > > >>>
> > > >>>      https://github.com/oasis-tcs/virtio-spec/issues/124
> > > >>>
> > > >>> Also regarding MMIO support for queue reset, I plan to support it after this
> > > >>> patch is passed.
> > > >>>
> > > >>> #14-#17 is the disable/enable function of rx/tx pair implemented by virtio-net
> > > >>> using the new helper.
> > > >> One thing that came to mind is the steering. E.g if we disable an RX,
> > > >> should we stop steering packets to that queue?
>
> Regarding this spec, if there are multiple queues disabled at the same time, it
> will be a troublesome problem for the backend to select the queue,

I don't understand this, for such a kind of backend or device it can
simply claim not to support this feature.

> so I want to
> directly define that only one queue is allowed to reset at the same time, do you
> think this is appropriate?

This sounds very complicated. E.g how can we define an API that can
reset more than one queues? (currently PCI only support MMIO access).

>
> In terms of the implementation of backend queue reselection, it would be more
> convenient to implement if we drop packets directly. Do you think we must
> implement this reselection function?

Rethink of this. It should be fine and much simpler.

Thanks

>
> Thanks.
>
> > > > Yes, we should reselect a queue.
> > > >
> > > > Thanks.
> > >
> > >
> > > Maybe a spec patch for that?
> >
> > Yes, I also realized this. Although virtio-net's disable/enable is implemented
> > based on queue reset, virtio-net still has to define its own flag and define
> > some more detailed implementations.
> >
> > I'll think about it and post a spec patch.
> >
> > Thanks.
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > >> Thanks
> > > >>
> > > >>> This function is not currently referenced by other
> > > >>> functions. It is more to show the usage of the new helper, I not sure if they
> > > >>> are going to be merged together.
> > > >>>
> > > >>> Please review. Thanks.
> > > >>>
> > > >>> v3:
> > > >>>    1. keep vq, irq unreleased
> > > >>>
> > > >>> Xuan Zhuo (17):
> > > >>>    virtio_pci: struct virtio_pci_common_cfg add queue_notify_data
> > > >>>    virtio: queue_reset: add VIRTIO_F_RING_RESET
> > > >>>    virtio: queue_reset: struct virtio_config_ops add callbacks for
> > > >>>      queue_reset
> > > >>>    virtio: queue_reset: add helper
> > > >>>    vritio_ring: queue_reset: extract the release function of the vq ring
> > > >>>    virtio_ring: queue_reset: split: add __vring_init_virtqueue()
> > > >>>    virtio_ring: queue_reset: split: support enable reset queue
> > > >>>    virtio_ring: queue_reset: packed: support enable reset queue
> > > >>>    virtio_ring: queue_reset: add vring_reset_virtqueue()
> > > >>>    virtio_pci: queue_reset: update struct virtio_pci_common_cfg and
> > > >>>      option functions
> > > >>>    virtio_pci: queue_reset: release vq by vp_dev->vqs
> > > >>>    virtio_pci: queue_reset: setup_vq use vring_setup_virtqueue()
> > > >>>    virtio_pci: queue_reset: support VIRTIO_F_RING_RESET
> > > >>>    virtio_net: virtnet_tx_timeout() fix style
> > > >>>    virtio_net: virtnet_tx_timeout() stop ref sq->vq
> > > >>>    virtio_net: split free_unused_bufs()
> > > >>>    virtio_net: support pair disable/enable
> > > >>>
> > > >>>   drivers/net/virtio_net.c               | 220 ++++++++++++++++++++++---
> > > >>>   drivers/virtio/virtio_pci_common.c     |  62 ++++---
> > > >>>   drivers/virtio/virtio_pci_common.h     |  11 +-
> > > >>>   drivers/virtio/virtio_pci_legacy.c     |   5 +-
> > > >>>   drivers/virtio/virtio_pci_modern.c     | 120 +++++++++++++-
> > > >>>   drivers/virtio/virtio_pci_modern_dev.c |  28 ++++
> > > >>>   drivers/virtio/virtio_ring.c           | 144 +++++++++++-----
> > > >>>   include/linux/virtio.h                 |   1 +
> > > >>>   include/linux/virtio_config.h          |  75 ++++++++-
> > > >>>   include/linux/virtio_pci_modern.h      |   2 +
> > > >>>   include/linux/virtio_ring.h            |  42 +++--
> > > >>>   include/uapi/linux/virtio_config.h     |   7 +-
> > > >>>   include/uapi/linux/virtio_pci.h        |   2 +
> > > >>>   13 files changed, 618 insertions(+), 101 deletions(-)
> > > >>>
> > > >>> --
> > > >>> 2.31.0
> > > >>>
> > >
> > _______________________________________________
> > Virtualization mailing list
> > Virtualization@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>