mbox series

[v4,00/44] net/virtio: Virtio PMD rework

Message ID 20210126101639.250481-1-maxime.coquelin@redhat.com (mailing list archive)
Headers
Series net/virtio: Virtio PMD rework |

Message

Maxime Coquelin Jan. 26, 2021, 10:15 a.m. UTC
  This V3 fixes comments from Chenbo on patch 44 and
implements the ABI exception in patch 2.

This series significantly rework Virtio PMD to improve
the Virtio-user PMD and its backends integration.

First part of the series removes the dependency of
Virtio-user ethdev on Virtio PCI, by creating generic
files, adding per-bus meta data, ...

Main (if not single) functionnal change of this first
part is to remove the hack for Virtio-user to work in
IOVA as PA mode, this hack being very fragile.

Second part of the series reworks Virtio-user internal,
by reworking the requests handling so that vDPA and Kernel
backends no more hack into being Vhost-user backend. It
implies implementing new ops for all the request types.
Also, all the backend specific actions are moved from the
virtio_user_dev.c and virtio_user_ethdev.c to their
backend files.

Only functionnal change in this second part is making the
Vhost-user server mode blocking at init time, as long as
a client is not connected. The goal of this change is to
make the Vhost-user support much more robust, as without
blocking, the driver has to assume features that are going
to be supported by the client, which is very fragile and
error prone. As a side-effect, it also simplifies the
logic nin several place of the virtio-user PMD.

Main changes in v4:
- Add ABI exception (David)
- Close FDs only up to max_queue_pairs
- virtio_user_dev_uninit_notify() to return void

Main changes in v3:
- Rename .intr_event to .intr_detect
- Rework last patch, properly clean allocated resources
  on failure.
- Rebase on top of latest net-next/main
- Minor typo fixes in comments and log improvements

Main changes in v2:
===================
- Introduce vdev driver flag for drivers to require IOVA VA mode
- Rebase on top of -rc1 changes
- Fix regressions introduced in V1 (vhost-kernel broken, vhost-user reconnect...)
- Various minor issues & typos fixed
- Fix status feature issue introduced in v20.11, only reproduceable now that server
  mode is made blocking
- Improve failure handling in Virtio-user
- Improve logging

Testing coverage (All passed)
=============================
- Virtio-pci PMD
 * Virtio PMD in guest with Vhost-user backend in host
 * Virtio PMD in guest with Vhost-kernel backend in host
- Virtio-user PMD with Vhost-user backend
 * Vhost-user PMD server <-> Virtio-user client PMD IO loopback
 * Vhost-user PMD client <-> Virtio-user server PMD IO loopback
 * Vhost-user PMD client <-> Virtio-user server PMD reconnect
- Virtio-user PMD with Vhost-kernel backend
 * iperf test case
 * Txonly testpmd
- Virtio-user PMD with Vhost-vDPA backend
 * vdpa-sim (IO loopback)
 * CX-6 DX Kernel vDPA (Tx only)

Maxime Coquelin (44):
  bus/vdev: add helper to get vdev from ethdev
  bus/vdev: add driver IOVA VA mode requirement
  net/virtio: fix getting old status on reconnect
  net/virtio: introduce Virtio bus type
  net/virtio: refactor virtio-user device
  net/virtio: introduce PCI device metadata
  net/virtio: move PCI device init in dedicated file
  net/virtio: move PCI specific dev init to PCI ethdev init
  net/virtio: move MSIX detection to PCI ethdev
  net/virtio: force IOVA as VA mode for Virtio-user
  net/virtio: store PCI type in Virtio device metadata
  net/virtio: add callback for device closing
  net/virtio: validate features at bus level
  net/virtio: remove bus type enum
  net/virtio: move PCI-specific fields to PCI device
  net/virtio: pack virtio HW struct
  net/virtio: move legacy IO to Virtio PCI
  net/virtio: introduce generic virtio header
  net/virtio: move features definition to generic header
  net/virtio: move virtqueue defines in generic header
  net/virtio: move config definitions to generic header
  net/virtio: make interrupt handling more generic
  net/virtio: move vring alignment to generic header
  net/virtio: remove last PCI refs in non-PCI code
  net/virtio: make Vhost-user request sender consistent
  net/virtio: add Virtio-user ops to set owner
  net/virtio: add Virtio-user features ops
  net/virtio: add Virtio-user protocol features ops
  net/virtio: add Virtio-user memory tables ops
  net/virtio: add Virtio-user vring setting ops
  net/virtio: add Virtio-user vring file ops
  net/virtio: add Virtio-user vring address ops
  net/virtio: add Virtio-user status ops
  net/virtio: remove useless request ops
  net/virtio: improve Virtio-user errors handling
  net/virtio: move Vhost-user requests to Vhost-user backend
  net/virtio: make server mode blocking
  net/virtio: move protocol features to Vhost-user
  net/virtio: introduce backend data
  net/virtio: move Vhost-user specifics to its backend
  net/virtio: move Vhost-kernel data to its backend
  net/virtio: move Vhost-vDPA data to its backend
  net/virtio: improve Vhost-user error logging
  net/virtio: handle Virtio-user setup failure properly

 devtools/libabigail.abignore                  |   2 +
 drivers/bus/vdev/rte_bus_vdev.h               |   6 +
 drivers/bus/vdev/vdev.c                       |  29 +
 drivers/net/virtio/meson.build                |   6 +-
 drivers/net/virtio/virtio.c                   |  71 ++
 drivers/net/virtio/virtio.h                   | 246 +++++
 drivers/net/virtio/virtio_ethdev.c            | 457 +++------
 drivers/net/virtio/virtio_ethdev.h            |   6 +-
 drivers/net/virtio/virtio_pci.c               | 448 +++++----
 drivers/net/virtio/virtio_pci.h               | 286 +-----
 drivers/net/virtio/virtio_pci_ethdev.c        | 226 +++++
 drivers/net/virtio/virtio_ring.h              |   2 +-
 drivers/net/virtio/virtio_rxtx.c              |  90 +-
 drivers/net/virtio/virtio_rxtx_packed.h       |  10 +-
 drivers/net/virtio/virtio_rxtx_packed_avx.h   |  10 +-
 drivers/net/virtio/virtio_rxtx_packed_neon.h  |  10 +-
 drivers/net/virtio/virtio_rxtx_simple.h       |   3 +-
 drivers/net/virtio/virtio_user/vhost.h        |  79 +-
 drivers/net/virtio/virtio_user/vhost_kernel.c | 461 ++++++---
 .../net/virtio/virtio_user/vhost_kernel_tap.c |  25 +-
 .../net/virtio/virtio_user/vhost_kernel_tap.h |   1 +
 drivers/net/virtio/virtio_user/vhost_user.c   | 898 ++++++++++++++----
 drivers/net/virtio/virtio_user/vhost_vdpa.c   | 323 +++++--
 .../net/virtio/virtio_user/virtio_user_dev.c  | 573 ++++++-----
 .../net/virtio/virtio_user/virtio_user_dev.h  |  21 +-
 drivers/net/virtio/virtio_user_ethdev.c       | 301 +-----
 drivers/net/virtio/virtqueue.c                |   6 +-
 drivers/net/virtio/virtqueue.h                |  45 +-
 28 files changed, 2742 insertions(+), 1899 deletions(-)
 create mode 100644 drivers/net/virtio/virtio.c
 create mode 100644 drivers/net/virtio/virtio.h
 create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c
  

Comments

Maxime Coquelin Jan. 27, 2021, 11:59 a.m. UTC | #1
On 1/26/21 11:15 AM, Maxime Coquelin wrote:
> This V3 fixes comments from Chenbo on patch 44 and
> implements the ABI exception in patch 2.
> 
> This series significantly rework Virtio PMD to improve
> the Virtio-user PMD and its backends integration.
> 
> First part of the series removes the dependency of
> Virtio-user ethdev on Virtio PCI, by creating generic
> files, adding per-bus meta data, ...
> 
> Main (if not single) functionnal change of this first
> part is to remove the hack for Virtio-user to work in
> IOVA as PA mode, this hack being very fragile.
> 
> Second part of the series reworks Virtio-user internal,
> by reworking the requests handling so that vDPA and Kernel
> backends no more hack into being Vhost-user backend. It
> implies implementing new ops for all the request types.
> Also, all the backend specific actions are moved from the
> virtio_user_dev.c and virtio_user_ethdev.c to their
> backend files.
> 
> Only functionnal change in this second part is making the
> Vhost-user server mode blocking at init time, as long as
> a client is not connected. The goal of this change is to
> make the Vhost-user support much more robust, as without
> blocking, the driver has to assume features that are going
> to be supported by the client, which is very fragile and
> error prone. As a side-effect, it also simplifies the
> logic nin several place of the virtio-user PMD.
> 
> Main changes in v4:
> - Add ABI exception (David)
> - Close FDs only up to max_queue_pairs
> - virtio_user_dev_uninit_notify() to return void
> 
> Main changes in v3:
> - Rename .intr_event to .intr_detect
> - Rework last patch, properly clean allocated resources
>   on failure.
> - Rebase on top of latest net-next/main
> - Minor typo fixes in comments and log improvements
> 
> Main changes in v2:
> ===================
> - Introduce vdev driver flag for drivers to require IOVA VA mode
> - Rebase on top of -rc1 changes
> - Fix regressions introduced in V1 (vhost-kernel broken, vhost-user reconnect...)
> - Various minor issues & typos fixed
> - Fix status feature issue introduced in v20.11, only reproduceable now that server
>   mode is made blocking
> - Improve failure handling in Virtio-user
> - Improve logging
> 
> Testing coverage (All passed)
> =============================
> - Virtio-pci PMD
>  * Virtio PMD in guest with Vhost-user backend in host
>  * Virtio PMD in guest with Vhost-kernel backend in host
> - Virtio-user PMD with Vhost-user backend
>  * Vhost-user PMD server <-> Virtio-user client PMD IO loopback
>  * Vhost-user PMD client <-> Virtio-user server PMD IO loopback
>  * Vhost-user PMD client <-> Virtio-user server PMD reconnect
> - Virtio-user PMD with Vhost-kernel backend
>  * iperf test case
>  * Txonly testpmd
> - Virtio-user PMD with Vhost-vDPA backend
>  * vdpa-sim (IO loopback)
>  * CX-6 DX Kernel vDPA (Tx only)
> 
> Maxime Coquelin (44):
>   bus/vdev: add helper to get vdev from ethdev
>   bus/vdev: add driver IOVA VA mode requirement
>   net/virtio: fix getting old status on reconnect
>   net/virtio: introduce Virtio bus type
>   net/virtio: refactor virtio-user device
>   net/virtio: introduce PCI device metadata
>   net/virtio: move PCI device init in dedicated file
>   net/virtio: move PCI specific dev init to PCI ethdev init
>   net/virtio: move MSIX detection to PCI ethdev
>   net/virtio: force IOVA as VA mode for Virtio-user
>   net/virtio: store PCI type in Virtio device metadata
>   net/virtio: add callback for device closing
>   net/virtio: validate features at bus level
>   net/virtio: remove bus type enum
>   net/virtio: move PCI-specific fields to PCI device
>   net/virtio: pack virtio HW struct
>   net/virtio: move legacy IO to Virtio PCI
>   net/virtio: introduce generic virtio header
>   net/virtio: move features definition to generic header
>   net/virtio: move virtqueue defines in generic header
>   net/virtio: move config definitions to generic header
>   net/virtio: make interrupt handling more generic
>   net/virtio: move vring alignment to generic header
>   net/virtio: remove last PCI refs in non-PCI code
>   net/virtio: make Vhost-user request sender consistent
>   net/virtio: add Virtio-user ops to set owner
>   net/virtio: add Virtio-user features ops
>   net/virtio: add Virtio-user protocol features ops
>   net/virtio: add Virtio-user memory tables ops
>   net/virtio: add Virtio-user vring setting ops
>   net/virtio: add Virtio-user vring file ops
>   net/virtio: add Virtio-user vring address ops
>   net/virtio: add Virtio-user status ops
>   net/virtio: remove useless request ops
>   net/virtio: improve Virtio-user errors handling
>   net/virtio: move Vhost-user requests to Vhost-user backend
>   net/virtio: make server mode blocking
>   net/virtio: move protocol features to Vhost-user
>   net/virtio: introduce backend data
>   net/virtio: move Vhost-user specifics to its backend
>   net/virtio: move Vhost-kernel data to its backend
>   net/virtio: move Vhost-vDPA data to its backend
>   net/virtio: improve Vhost-user error logging
>   net/virtio: handle Virtio-user setup failure properly
> 
>  devtools/libabigail.abignore                  |   2 +
>  drivers/bus/vdev/rte_bus_vdev.h               |   6 +
>  drivers/bus/vdev/vdev.c                       |  29 +
>  drivers/net/virtio/meson.build                |   6 +-
>  drivers/net/virtio/virtio.c                   |  71 ++
>  drivers/net/virtio/virtio.h                   | 246 +++++
>  drivers/net/virtio/virtio_ethdev.c            | 457 +++------
>  drivers/net/virtio/virtio_ethdev.h            |   6 +-
>  drivers/net/virtio/virtio_pci.c               | 448 +++++----
>  drivers/net/virtio/virtio_pci.h               | 286 +-----
>  drivers/net/virtio/virtio_pci_ethdev.c        | 226 +++++
>  drivers/net/virtio/virtio_ring.h              |   2 +-
>  drivers/net/virtio/virtio_rxtx.c              |  90 +-
>  drivers/net/virtio/virtio_rxtx_packed.h       |  10 +-
>  drivers/net/virtio/virtio_rxtx_packed_avx.h   |  10 +-
>  drivers/net/virtio/virtio_rxtx_packed_neon.h  |  10 +-
>  drivers/net/virtio/virtio_rxtx_simple.h       |   3 +-
>  drivers/net/virtio/virtio_user/vhost.h        |  79 +-
>  drivers/net/virtio/virtio_user/vhost_kernel.c | 461 ++++++---
>  .../net/virtio/virtio_user/vhost_kernel_tap.c |  25 +-
>  .../net/virtio/virtio_user/vhost_kernel_tap.h |   1 +
>  drivers/net/virtio/virtio_user/vhost_user.c   | 898 ++++++++++++++----
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 323 +++++--
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 573 ++++++-----
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  21 +-
>  drivers/net/virtio/virtio_user_ethdev.c       | 301 +-----
>  drivers/net/virtio/virtqueue.c                |   6 +-
>  drivers/net/virtio/virtqueue.h                |  45 +-
>  28 files changed, 2742 insertions(+), 1899 deletions(-)
>  create mode 100644 drivers/net/virtio/virtio.c
>  create mode 100644 drivers/net/virtio/virtio.h
>  create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c
> 

Applied to dpdk-next-virtio/main.

Thanks,
Maxime
  
Wang, Yinan Feb. 1, 2021, 8:44 a.m. UTC | #2
Hi Maxime.

We found three virtio issues and bad commit is this patch set. Could you help to take a look?

https://bugs.dpdk.org/show_bug.cgi?id=631
Issue1: vdev_primary_secondary/Virtio_primary_and_secondary_process: start dpdk-symmetric_mp occur core dumped in vm

Issue2: start testpmd occur core dumped in vm when use more than 1 virtio-pmd.

https://bugs.dpdk.org/show_bug.cgi?id=630

issue3: hotplug_mp/attach_detach_virtio_user: The host display is abnormal after dpdk-hotplug_mp exits


BR,
Yinan
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: 2021?1?26? 18:16
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
> olivier.matz@6wind.com; amorenoz@redhat.com;
> david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH v4 00/44] net/virtio: Virtio PMD rework
> 
> This V3 fixes comments from Chenbo on patch 44 and
> implements the ABI exception in patch 2.
> 
> This series significantly rework Virtio PMD to improve
> the Virtio-user PMD and its backends integration.
> 
> First part of the series removes the dependency of
> Virtio-user ethdev on Virtio PCI, by creating generic
> files, adding per-bus meta data, ...
> 
> Main (if not single) functionnal change of this first
> part is to remove the hack for Virtio-user to work in
> IOVA as PA mode, this hack being very fragile.
> 
> Second part of the series reworks Virtio-user internal,
> by reworking the requests handling so that vDPA and Kernel
> backends no more hack into being Vhost-user backend. It
> implies implementing new ops for all the request types.
> Also, all the backend specific actions are moved from the
> virtio_user_dev.c and virtio_user_ethdev.c to their
> backend files.
> 
> Only functionnal change in this second part is making the
> Vhost-user server mode blocking at init time, as long as
> a client is not connected. The goal of this change is to
> make the Vhost-user support much more robust, as without
> blocking, the driver has to assume features that are going
> to be supported by the client, which is very fragile and
> error prone. As a side-effect, it also simplifies the
> logic nin several place of the virtio-user PMD.
> 
> Main changes in v4:
> - Add ABI exception (David)
> - Close FDs only up to max_queue_pairs
> - virtio_user_dev_uninit_notify() to return void
> 
> Main changes in v3:
> - Rename .intr_event to .intr_detect
> - Rework last patch, properly clean allocated resources
>   on failure.
> - Rebase on top of latest net-next/main
> - Minor typo fixes in comments and log improvements
> 
> Main changes in v2:
> ===================
> - Introduce vdev driver flag for drivers to require IOVA VA mode
> - Rebase on top of -rc1 changes
> - Fix regressions introduced in V1 (vhost-kernel broken, vhost-user
> reconnect...)
> - Various minor issues & typos fixed
> - Fix status feature issue introduced in v20.11, only reproduceable now that
> server
>   mode is made blocking
> - Improve failure handling in Virtio-user
> - Improve logging
> 
> Testing coverage (All passed)
> =============================
> - Virtio-pci PMD
>  * Virtio PMD in guest with Vhost-user backend in host
>  * Virtio PMD in guest with Vhost-kernel backend in host
> - Virtio-user PMD with Vhost-user backend
>  * Vhost-user PMD server <-> Virtio-user client PMD IO loopback
>  * Vhost-user PMD client <-> Virtio-user server PMD IO loopback
>  * Vhost-user PMD client <-> Virtio-user server PMD reconnect
> - Virtio-user PMD with Vhost-kernel backend
>  * iperf test case
>  * Txonly testpmd
> - Virtio-user PMD with Vhost-vDPA backend
>  * vdpa-sim (IO loopback)
>  * CX-6 DX Kernel vDPA (Tx only)
> 
> Maxime Coquelin (44):
>   bus/vdev: add helper to get vdev from ethdev
>   bus/vdev: add driver IOVA VA mode requirement
>   net/virtio: fix getting old status on reconnect
>   net/virtio: introduce Virtio bus type
>   net/virtio: refactor virtio-user device
>   net/virtio: introduce PCI device metadata
>   net/virtio: move PCI device init in dedicated file
>   net/virtio: move PCI specific dev init to PCI ethdev init
>   net/virtio: move MSIX detection to PCI ethdev
>   net/virtio: force IOVA as VA mode for Virtio-user
>   net/virtio: store PCI type in Virtio device metadata
>   net/virtio: add callback for device closing
>   net/virtio: validate features at bus level
>   net/virtio: remove bus type enum
>   net/virtio: move PCI-specific fields to PCI device
>   net/virtio: pack virtio HW struct
>   net/virtio: move legacy IO to Virtio PCI
>   net/virtio: introduce generic virtio header
>   net/virtio: move features definition to generic header
>   net/virtio: move virtqueue defines in generic header
>   net/virtio: move config definitions to generic header
>   net/virtio: make interrupt handling more generic
>   net/virtio: move vring alignment to generic header
>   net/virtio: remove last PCI refs in non-PCI code
>   net/virtio: make Vhost-user request sender consistent
>   net/virtio: add Virtio-user ops to set owner
>   net/virtio: add Virtio-user features ops
>   net/virtio: add Virtio-user protocol features ops
>   net/virtio: add Virtio-user memory tables ops
>   net/virtio: add Virtio-user vring setting ops
>   net/virtio: add Virtio-user vring file ops
>   net/virtio: add Virtio-user vring address ops
>   net/virtio: add Virtio-user status ops
>   net/virtio: remove useless request ops
>   net/virtio: improve Virtio-user errors handling
>   net/virtio: move Vhost-user requests to Vhost-user backend
>   net/virtio: make server mode blocking
>   net/virtio: move protocol features to Vhost-user
>   net/virtio: introduce backend data
>   net/virtio: move Vhost-user specifics to its backend
>   net/virtio: move Vhost-kernel data to its backend
>   net/virtio: move Vhost-vDPA data to its backend
>   net/virtio: improve Vhost-user error logging
>   net/virtio: handle Virtio-user setup failure properly
> 
>  devtools/libabigail.abignore                  |   2 +
>  drivers/bus/vdev/rte_bus_vdev.h               |   6 +
>  drivers/bus/vdev/vdev.c                       |  29 +
>  drivers/net/virtio/meson.build                |   6 +-
>  drivers/net/virtio/virtio.c                   |  71 ++
>  drivers/net/virtio/virtio.h                   | 246 +++++
>  drivers/net/virtio/virtio_ethdev.c            | 457 +++------
>  drivers/net/virtio/virtio_ethdev.h            |   6 +-
>  drivers/net/virtio/virtio_pci.c               | 448 +++++----
>  drivers/net/virtio/virtio_pci.h               | 286 +-----
>  drivers/net/virtio/virtio_pci_ethdev.c        | 226 +++++
>  drivers/net/virtio/virtio_ring.h              |   2 +-
>  drivers/net/virtio/virtio_rxtx.c              |  90 +-
>  drivers/net/virtio/virtio_rxtx_packed.h       |  10 +-
>  drivers/net/virtio/virtio_rxtx_packed_avx.h   |  10 +-
>  drivers/net/virtio/virtio_rxtx_packed_neon.h  |  10 +-
>  drivers/net/virtio/virtio_rxtx_simple.h       |   3 +-
>  drivers/net/virtio/virtio_user/vhost.h        |  79 +-
>  drivers/net/virtio/virtio_user/vhost_kernel.c | 461 ++++++---
>  .../net/virtio/virtio_user/vhost_kernel_tap.c |  25 +-
>  .../net/virtio/virtio_user/vhost_kernel_tap.h |   1 +
>  drivers/net/virtio/virtio_user/vhost_user.c   | 898 ++++++++++++++----
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 323 +++++--
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 573 ++++++-----
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  21 +-
>  drivers/net/virtio/virtio_user_ethdev.c       | 301 +-----
>  drivers/net/virtio/virtqueue.c                |   6 +-
>  drivers/net/virtio/virtqueue.h                |  45 +-
>  28 files changed, 2742 insertions(+), 1899 deletions(-)
>  create mode 100644 drivers/net/virtio/virtio.c
>  create mode 100644 drivers/net/virtio/virtio.h
>  create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c
> 
> --
> 2.29.2
  
Maxime Coquelin Feb. 1, 2021, 8:49 a.m. UTC | #3
Hi Yinan,


On 2/1/21 9:44 AM, Wang, Yinan wrote:
> Hi Maxime.
> 
> We found three virtio issues and bad commit is this patch set. Could you help to take a look?

Thanks for the testing and reports.
I'm already working on Bz630, will keep you updated.

> https://bugs.dpdk.org/show_bug.cgi?id=631
> Issue1: vdev_primary_secondary/Virtio_primary_and_secondary_process: start dpdk-symmetric_mp occur core dumped in vm
> 
> Issue2: start testpmd occur core dumped in vm when use more than 1 virtio-pmd.
> 
> https://bugs.dpdk.org/show_bug.cgi?id=630
> 
> issue3: hotplug_mp/attach_detach_virtio_user: The host display is abnormal after dpdk-hotplug_mp exits
> 

Thanks,
Maxime

> BR,
> Yinan
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>> Sent: 2021?1?26? 18:16
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>;
>> olivier.matz@6wind.com; amorenoz@redhat.com;
>> david.marchand@redhat.com
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [dpdk-dev] [PATCH v4 00/44] net/virtio: Virtio PMD rework
>>
>> This V3 fixes comments from Chenbo on patch 44 and
>> implements the ABI exception in patch 2.
>>
>> This series significantly rework Virtio PMD to improve
>> the Virtio-user PMD and its backends integration.
>>
>> First part of the series removes the dependency of
>> Virtio-user ethdev on Virtio PCI, by creating generic
>> files, adding per-bus meta data, ...
>>
>> Main (if not single) functionnal change of this first
>> part is to remove the hack for Virtio-user to work in
>> IOVA as PA mode, this hack being very fragile.
>>
>> Second part of the series reworks Virtio-user internal,
>> by reworking the requests handling so that vDPA and Kernel
>> backends no more hack into being Vhost-user backend. It
>> implies implementing new ops for all the request types.
>> Also, all the backend specific actions are moved from the
>> virtio_user_dev.c and virtio_user_ethdev.c to their
>> backend files.
>>
>> Only functionnal change in this second part is making the
>> Vhost-user server mode blocking at init time, as long as
>> a client is not connected. The goal of this change is to
>> make the Vhost-user support much more robust, as without
>> blocking, the driver has to assume features that are going
>> to be supported by the client, which is very fragile and
>> error prone. As a side-effect, it also simplifies the
>> logic nin several place of the virtio-user PMD.
>>
>> Main changes in v4:
>> - Add ABI exception (David)
>> - Close FDs only up to max_queue_pairs
>> - virtio_user_dev_uninit_notify() to return void
>>
>> Main changes in v3:
>> - Rename .intr_event to .intr_detect
>> - Rework last patch, properly clean allocated resources
>>   on failure.
>> - Rebase on top of latest net-next/main
>> - Minor typo fixes in comments and log improvements
>>
>> Main changes in v2:
>> ===================
>> - Introduce vdev driver flag for drivers to require IOVA VA mode
>> - Rebase on top of -rc1 changes
>> - Fix regressions introduced in V1 (vhost-kernel broken, vhost-user
>> reconnect...)
>> - Various minor issues & typos fixed
>> - Fix status feature issue introduced in v20.11, only reproduceable now that
>> server
>>   mode is made blocking
>> - Improve failure handling in Virtio-user
>> - Improve logging
>>
>> Testing coverage (All passed)
>> =============================
>> - Virtio-pci PMD
>>  * Virtio PMD in guest with Vhost-user backend in host
>>  * Virtio PMD in guest with Vhost-kernel backend in host
>> - Virtio-user PMD with Vhost-user backend
>>  * Vhost-user PMD server <-> Virtio-user client PMD IO loopback
>>  * Vhost-user PMD client <-> Virtio-user server PMD IO loopback
>>  * Vhost-user PMD client <-> Virtio-user server PMD reconnect
>> - Virtio-user PMD with Vhost-kernel backend
>>  * iperf test case
>>  * Txonly testpmd
>> - Virtio-user PMD with Vhost-vDPA backend
>>  * vdpa-sim (IO loopback)
>>  * CX-6 DX Kernel vDPA (Tx only)
>>
>> Maxime Coquelin (44):
>>   bus/vdev: add helper to get vdev from ethdev
>>   bus/vdev: add driver IOVA VA mode requirement
>>   net/virtio: fix getting old status on reconnect
>>   net/virtio: introduce Virtio bus type
>>   net/virtio: refactor virtio-user device
>>   net/virtio: introduce PCI device metadata
>>   net/virtio: move PCI device init in dedicated file
>>   net/virtio: move PCI specific dev init to PCI ethdev init
>>   net/virtio: move MSIX detection to PCI ethdev
>>   net/virtio: force IOVA as VA mode for Virtio-user
>>   net/virtio: store PCI type in Virtio device metadata
>>   net/virtio: add callback for device closing
>>   net/virtio: validate features at bus level
>>   net/virtio: remove bus type enum
>>   net/virtio: move PCI-specific fields to PCI device
>>   net/virtio: pack virtio HW struct
>>   net/virtio: move legacy IO to Virtio PCI
>>   net/virtio: introduce generic virtio header
>>   net/virtio: move features definition to generic header
>>   net/virtio: move virtqueue defines in generic header
>>   net/virtio: move config definitions to generic header
>>   net/virtio: make interrupt handling more generic
>>   net/virtio: move vring alignment to generic header
>>   net/virtio: remove last PCI refs in non-PCI code
>>   net/virtio: make Vhost-user request sender consistent
>>   net/virtio: add Virtio-user ops to set owner
>>   net/virtio: add Virtio-user features ops
>>   net/virtio: add Virtio-user protocol features ops
>>   net/virtio: add Virtio-user memory tables ops
>>   net/virtio: add Virtio-user vring setting ops
>>   net/virtio: add Virtio-user vring file ops
>>   net/virtio: add Virtio-user vring address ops
>>   net/virtio: add Virtio-user status ops
>>   net/virtio: remove useless request ops
>>   net/virtio: improve Virtio-user errors handling
>>   net/virtio: move Vhost-user requests to Vhost-user backend
>>   net/virtio: make server mode blocking
>>   net/virtio: move protocol features to Vhost-user
>>   net/virtio: introduce backend data
>>   net/virtio: move Vhost-user specifics to its backend
>>   net/virtio: move Vhost-kernel data to its backend
>>   net/virtio: move Vhost-vDPA data to its backend
>>   net/virtio: improve Vhost-user error logging
>>   net/virtio: handle Virtio-user setup failure properly
>>
>>  devtools/libabigail.abignore                  |   2 +
>>  drivers/bus/vdev/rte_bus_vdev.h               |   6 +
>>  drivers/bus/vdev/vdev.c                       |  29 +
>>  drivers/net/virtio/meson.build                |   6 +-
>>  drivers/net/virtio/virtio.c                   |  71 ++
>>  drivers/net/virtio/virtio.h                   | 246 +++++
>>  drivers/net/virtio/virtio_ethdev.c            | 457 +++------
>>  drivers/net/virtio/virtio_ethdev.h            |   6 +-
>>  drivers/net/virtio/virtio_pci.c               | 448 +++++----
>>  drivers/net/virtio/virtio_pci.h               | 286 +-----
>>  drivers/net/virtio/virtio_pci_ethdev.c        | 226 +++++
>>  drivers/net/virtio/virtio_ring.h              |   2 +-
>>  drivers/net/virtio/virtio_rxtx.c              |  90 +-
>>  drivers/net/virtio/virtio_rxtx_packed.h       |  10 +-
>>  drivers/net/virtio/virtio_rxtx_packed_avx.h   |  10 +-
>>  drivers/net/virtio/virtio_rxtx_packed_neon.h  |  10 +-
>>  drivers/net/virtio/virtio_rxtx_simple.h       |   3 +-
>>  drivers/net/virtio/virtio_user/vhost.h        |  79 +-
>>  drivers/net/virtio/virtio_user/vhost_kernel.c | 461 ++++++---
>>  .../net/virtio/virtio_user/vhost_kernel_tap.c |  25 +-
>>  .../net/virtio/virtio_user/vhost_kernel_tap.h |   1 +
>>  drivers/net/virtio/virtio_user/vhost_user.c   | 898 ++++++++++++++----
>>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 323 +++++--
>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 573 ++++++-----
>>  .../net/virtio/virtio_user/virtio_user_dev.h  |  21 +-
>>  drivers/net/virtio/virtio_user_ethdev.c       | 301 +-----
>>  drivers/net/virtio/virtqueue.c                |   6 +-
>>  drivers/net/virtio/virtqueue.h                |  45 +-
>>  28 files changed, 2742 insertions(+), 1899 deletions(-)
>>  create mode 100644 drivers/net/virtio/virtio.c
>>  create mode 100644 drivers/net/virtio/virtio.h
>>  create mode 100644 drivers/net/virtio/virtio_pci_ethdev.c
>>
>> --
>> 2.29.2
>
  
Ilya Maximets Feb. 1, 2021, 1 p.m. UTC | #4
On 1/26/21 11:15 AM, Maxime Coquelin wrote:
> 
> Only functionnal change in this second part is making the
> Vhost-user server mode blocking at init time, as long as
> a client is not connected. The goal of this change is to
> make the Vhost-user support much more robust, as without
> blocking, the driver has to assume features that are going
> to be supported by the client, which is very fragile and
> error prone. As a side-effect, it also simplifies the
> logic nin several place of the virtio-user PMD.

Hi, Maxime.

I didn't actually look at the code, but I have a question.
Does above text mean that with this change OVS will hang inside
driver_register() or similar function until client is connected
to dpdkvhostuser (server mode) port?

If so, I think, we will not be able to support server mode
in OVS anymore and will have to actually remove it.  It's
deprecated for a long time now, but I think it might still be in
use by some people, especially for virtio-user usecase.

Best regards, Ilya Maximets.
  
Ilya Maximets Feb. 1, 2021, 1:03 p.m. UTC | #5
CC: ovs-dev

On 2/1/21 2:00 PM, Ilya Maximets wrote:
> On 1/26/21 11:15 AM, Maxime Coquelin wrote:
>>
>> Only functionnal change in this second part is making the
>> Vhost-user server mode blocking at init time, as long as
>> a client is not connected. The goal of this change is to
>> make the Vhost-user support much more robust, as without
>> blocking, the driver has to assume features that are going
>> to be supported by the client, which is very fragile and
>> error prone. As a side-effect, it also simplifies the
>> logic nin several place of the virtio-user PMD.
> 
> Hi, Maxime.
> 
> I didn't actually look at the code, but I have a question.
> Does above text mean that with this change OVS will hang inside
> driver_register() or similar function until client is connected
> to dpdkvhostuser (server mode) port?
> 
> If so, I think, we will not be able to support server mode
> in OVS anymore and will have to actually remove it.  It's
> deprecated for a long time now, but I think it might still be in
> use by some people, especially for virtio-user usecase.
> 
> Best regards, Ilya Maximets.
>
  
Maxime Coquelin Feb. 1, 2021, 1:16 p.m. UTC | #6
Hi Ilya,

On 2/1/21 2:03 PM, Ilya Maximets wrote:
> CC: ovs-dev
> 
> On 2/1/21 2:00 PM, Ilya Maximets wrote:
>> On 1/26/21 11:15 AM, Maxime Coquelin wrote:
>>>
>>> Only functionnal change in this second part is making the
>>> Vhost-user server mode blocking at init time, as long as
>>> a client is not connected. The goal of this change is to
>>> make the Vhost-user support much more robust, as without
>>> blocking, the driver has to assume features that are going
>>> to be supported by the client, which is very fragile and
>>> error prone. As a side-effect, it also simplifies the
>>> logic nin several place of the virtio-user PMD.
>>
>> Hi, Maxime.
>>
>> I didn't actually look at the code, but I have a question.
>> Does above text mean that with this change OVS will hang inside
>> driver_register() or similar function until client is connected
>> to dpdkvhostuser (server mode) port?
>>
>> If so, I think, we will not be able to support server mode
>> in OVS anymore and will have to actually remove it.  It's
>> deprecated for a long time now, but I think it might still be in
>> use by some people, especially for virtio-user usecase.

No, there is no impact for OVS. My explanation was maybe a bit
confusion, sorry about that.

I meant that Virtio-user PMD, acting as Vhost-user master in server
mode, will be blocking waiting for client (OVS dpdkvhostuserclient in
your case) connection. This makes the Virtio-user PMD in server mode to
behave the same as QEMU Vhost-user port in server mode.

Only noticeable effect on OVS side should be more reliability, as
without this change, Virtio-user PMD could assume OVS Vhost-user backend
would support features before the connection happens.

>> Best regards, Ilya Maximets.
>>
> 

Regards,
Maxime
  
Ilya Maximets Feb. 1, 2021, 1:42 p.m. UTC | #7
On 2/1/21 2:16 PM, Maxime Coquelin wrote:
> Hi Ilya,
> 
> On 2/1/21 2:03 PM, Ilya Maximets wrote:
>> CC: ovs-dev
>>
>> On 2/1/21 2:00 PM, Ilya Maximets wrote:
>>> On 1/26/21 11:15 AM, Maxime Coquelin wrote:
>>>>
>>>> Only functionnal change in this second part is making the
>>>> Vhost-user server mode blocking at init time, as long as
>>>> a client is not connected. The goal of this change is to
>>>> make the Vhost-user support much more robust, as without
>>>> blocking, the driver has to assume features that are going
>>>> to be supported by the client, which is very fragile and
>>>> error prone. As a side-effect, it also simplifies the
>>>> logic nin several place of the virtio-user PMD.
>>>
>>> Hi, Maxime.
>>>
>>> I didn't actually look at the code, but I have a question.
>>> Does above text mean that with this change OVS will hang inside
>>> driver_register() or similar function until client is connected
>>> to dpdkvhostuser (server mode) port?
>>>
>>> If so, I think, we will not be able to support server mode
>>> in OVS anymore and will have to actually remove it.  It's
>>> deprecated for a long time now, but I think it might still be in
>>> use by some people, especially for virtio-user usecase.
> 
> No, there is no impact for OVS. My explanation was maybe a bit
> confusion, sorry about that.
> 
> I meant that Virtio-user PMD, acting as Vhost-user master in server
> mode, will be blocking waiting for client (OVS dpdkvhostuserclient in
> your case) connection. This makes the Virtio-user PMD in server mode to
> behave the same as QEMU Vhost-user port in server mode.

Oh.  OK.  So the only affected party is 'net_virtio_user' PMD and vhost
library is not affected, right?

And in this case the following command will block OVS:

  ovs-vsctl add-port br0 vu -- set Interface vu type=dpdk \
      options:dpdk-devargs="net_virtio_user,path=vu.sock,server=1"

Is that correct?

(I'm not sure if this works right now, though.  I didn't test it.)


> 
> Only noticeable effect on OVS side should be more reliability, as
> without this change, Virtio-user PMD could assume OVS Vhost-user backend
> would support features before the connection happens.
> 
>>> Best regards, Ilya Maximets.
>>>
>>
> 
> Regards,
> Maxime
>
  
Maxime Coquelin Feb. 1, 2021, 1:51 p.m. UTC | #8
On 2/1/21 2:42 PM, Ilya Maximets wrote:
> On 2/1/21 2:16 PM, Maxime Coquelin wrote:
>> Hi Ilya,
>>
>> On 2/1/21 2:03 PM, Ilya Maximets wrote:
>>> CC: ovs-dev
>>>
>>> On 2/1/21 2:00 PM, Ilya Maximets wrote:
>>>> On 1/26/21 11:15 AM, Maxime Coquelin wrote:
>>>>>
>>>>> Only functionnal change in this second part is making the
>>>>> Vhost-user server mode blocking at init time, as long as
>>>>> a client is not connected. The goal of this change is to
>>>>> make the Vhost-user support much more robust, as without
>>>>> blocking, the driver has to assume features that are going
>>>>> to be supported by the client, which is very fragile and
>>>>> error prone. As a side-effect, it also simplifies the
>>>>> logic nin several place of the virtio-user PMD.
>>>>
>>>> Hi, Maxime.
>>>>
>>>> I didn't actually look at the code, but I have a question.
>>>> Does above text mean that with this change OVS will hang inside
>>>> driver_register() or similar function until client is connected
>>>> to dpdkvhostuser (server mode) port?
>>>>
>>>> If so, I think, we will not be able to support server mode
>>>> in OVS anymore and will have to actually remove it.  It's
>>>> deprecated for a long time now, but I think it might still be in
>>>> use by some people, especially for virtio-user usecase.
>>
>> No, there is no impact for OVS. My explanation was maybe a bit
>> confusion, sorry about that.
>>
>> I meant that Virtio-user PMD, acting as Vhost-user master in server
>> mode, will be blocking waiting for client (OVS dpdkvhostuserclient in
>> your case) connection. This makes the Virtio-user PMD in server mode to
>> behave the same as QEMU Vhost-user port in server mode.
> 
> Oh.  OK.  So the only affected party is 'net_virtio_user' PMD and vhost
> library is not affected, right?

Right!

> And in this case the following command will block OVS:
> 
>   ovs-vsctl add-port br0 vu -- set Interface vu type=dpdk \
>       options:dpdk-devargs="net_virtio_user,path=vu.sock,server=1"
> 
> Is that correct?
> 
> (I'm not sure if this works right now, though.  I didn't test it.)

This is correct, even though I don't know a use-case for it (chained
OVS-DPDK?).

Thanks,
Maxime
> 
>>
>> Only noticeable effect on OVS side should be more reliability, as
>> without this change, Virtio-user PMD could assume OVS Vhost-user backend
>> would support features before the connection happens.
>>
>>>> Best regards, Ilya Maximets.
>>>>
>>>
>>
>> Regards,
>> Maxime
>>
>