A number of issues have been detected that currently break virtio-user server mode. This series addresses such issues. Note that virtio-user server mode is broken by design and many of the problems that it currently has should be fixed by a bigger rework. -- v2->v3: Fix potential concurrency problem on get/set state Handle STATUS protocol feature flag Fix undefined behaviour if STATUS feature is not supported Ensure packed virtqueues are reset on reconnection v1->v2: Added patch 2 and 3 addressing additional issues Check errno to select vhost-user backend and log the detected backend type Adrian Moreno (6): net/virtio-user: fix backend selection if stat fails net/virtio-user: don't set/get_status until FEATURES_OK net/virtio-user: ignore result if STATUS is unsupported net/virtio-user: lock-protect status updates net/virtio-user: don't assume vhost status feature net/virtio-user: set status on socket reconnect drivers/net/virtio/virtio_user/vhost_user.c | 14 ++--- .../net/virtio/virtio_user/virtio_user_dev.c | 53 +++++++++++++------ .../net/virtio/virtio_user/virtio_user_dev.h | 5 +- drivers/net/virtio/virtio_user_ethdev.c | 40 ++++++++++---- 4 files changed, 78 insertions(+), 34 deletions(-) -- 2.26.2
If stat fails because the file does not exist, it means that the backend must be vhost-user in server mode. Also, log the detected backend type. Bugzilla ID: 559 Fixes: f908b22ea47a ("net/virtio: move backend type selection to ethdev") Cc: stable@dpdk.org Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Acked-by: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user/virtio_user_dev.c | 7 +++++++ drivers/net/virtio/virtio_user/virtio_user_dev.h | 1 + drivers/net/virtio/virtio_user_ethdev.c | 6 +++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 3681758ef..27814eadb 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -22,6 +22,13 @@ #define VIRTIO_USER_MEM_EVENT_CLB_NAME "virtio_user_mem_event_clb" +const char * const virtio_user_backend_strings[] = { + [VIRTIO_USER_BACKEND_UNKNOWN] = "VIRTIO_USER_BACKEND_UNKNOWN", + [VIRTIO_USER_BACKEND_VHOST_USER] = "VHOST_USER", + [VIRTIO_USER_BACKEND_VHOST_KERNEL] = "VHOST_NET", + [VIRTIO_USER_BACKEND_VHOST_VDPA] = "VHOST_VDPA", +}; + static int virtio_user_create_queue(struct virtio_user_dev *dev, uint32_t queue_sel) { diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h index 3e9d1a1eb..998986875 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h @@ -83,4 +83,5 @@ void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs); int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status); int virtio_user_update_status(struct virtio_user_dev *dev); +extern const char * const virtio_user_backend_strings[]; #endif diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 042665bc0..e870fb2ff 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -560,6 +560,9 @@ virtio_user_backend_type(const char *path) struct stat sb; if (stat(path, &sb) == -1) { + if (errno == ENOENT) + return VIRTIO_USER_BACKEND_VHOST_USER; + PMD_INIT_LOG(ERR, "Stat fails: %s (%s)\n", path, strerror(errno)); return VIRTIO_USER_BACKEND_UNKNOWN; @@ -697,7 +700,8 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev) path); goto end; } - + PMD_INIT_LOG(INFO, "Backend type detected: %s", + virtio_user_backend_strings[backend_type]); if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) { if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) { -- 2.26.2
According to the virtio spec, ACK and DRIVER status bits should be set before feature negotiation. However, until the protocol features are negotiated, the driver does not know if the device actually supports those vhost-user messages. Therefore, until FEATURES_OK is set, the GET/SET_STATUS messages should not be sent. Fixes: 57912824615f ("net/virtio-user: support vhost status setting") Cc: maxime.coquelin@redhat.com Cc: stable@dpdk.org Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user/vhost_user.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index ef290c357..450d77e92 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -278,8 +278,9 @@ vhost_user_sock(struct virtio_user_dev *dev, switch (req) { case VHOST_USER_GET_STATUS: - if (!(dev->protocol_features & - (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) + if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) || + (!(dev->protocol_features & + (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))) return 0; /* Fallthrough */ case VHOST_USER_GET_FEATURES: @@ -288,8 +289,9 @@ vhost_user_sock(struct virtio_user_dev *dev, break; case VHOST_USER_SET_STATUS: - if (!(dev->protocol_features & - (1ULL << VHOST_USER_PROTOCOL_F_STATUS))) + if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) || + (!(dev->protocol_features & + (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))) return 0; if (has_reply_ack) -- 2.26.2
GET/SET STATUS is an optional feature, so it may not be negotiated. In that case, the VIRTIO_GET_STATUS call will not update the status (given as a pointer argument). Failing to identify this case would lead to undefined behavior as the device status will be updated with the value of a stack-allocated variable. To fix this, return ENOTSUP if the feature is not supported and, in that case, don't update device status. Fixes: 44102e6298e7 ("net/virtio: check protocol feature in user backend") Cc: maxime.coquelin@redhat.com Cc stable@dpdk.org Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user/vhost_user.c | 4 +-- .../net/virtio/virtio_user/virtio_user_dev.c | 28 +++++++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index 450d77e92..b93e65c60 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -281,7 +281,7 @@ vhost_user_sock(struct virtio_user_dev *dev, if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) || (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))) - return 0; + return -ENOTSUP; /* Fallthrough */ case VHOST_USER_GET_FEATURES: case VHOST_USER_GET_PROTOCOL_FEATURES: @@ -292,7 +292,7 @@ vhost_user_sock(struct virtio_user_dev *dev, if (!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK) || (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))) - return 0; + return -ENOTSUP; if (has_reply_ack) msg.flags |= VHOST_USER_NEED_REPLY_MASK; diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 27814eadb..5a1e76006 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -818,15 +818,13 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status) ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &status); else - return 0; + ret = -ENOTSUP; - if (ret) { + if (ret && ret != -ENOTSUP) { PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret, strerror(errno)); - return -1; } - - return 0; + return ret; } int @@ -849,17 +847,12 @@ virtio_user_update_status(struct virtio_user_dev *dev) err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &status); } else { - return 0; - } - - if (err) { - PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err, - strerror(errno)); - return -1; + err = -ENOTSUP; } - dev->status = status; - PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n" + if (!err) { + dev->status = status; + PMD_INIT_LOG(DEBUG, "Updated Device Status(0x%08x):\n" "\t-RESET: %u\n" "\t-ACKNOWLEDGE: %u\n" "\t-DRIVER: %u\n" @@ -875,5 +868,10 @@ virtio_user_update_status(struct virtio_user_dev *dev) !!(dev->status & VIRTIO_CONFIG_STATUS_FEATURES_OK), !!(dev->status & VIRTIO_CONFIG_STATUS_DEV_NEED_RESET), !!(dev->status & VIRTIO_CONFIG_STATUS_FAILED)); - return 0; + } else if (err != -ENOTSUP) { + PMD_INIT_LOG(ERR, "VHOST_USER_GET_STATUS failed (%d): %s", err, + strerror(errno)); + } + + return err; } -- 2.26.2
In order to safely set and get the device status from different threads (e.g: interrupt handlers). Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 +++++++++++--- drivers/net/virtio/virtio_user/virtio_user_dev.h | 4 ++-- drivers/net/virtio/virtio_user_ethdev.c | 6 +++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 5a1e76006..36e5619df 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -806,11 +806,13 @@ virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx) } int -virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status) +virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status) { int ret; uint64_t arg = status; + pthread_mutex_lock(&dev->mutex); + dev->status = status; if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) ret = dev->ops->send_request(dev, VHOST_USER_SET_STATUS, &arg); @@ -824,22 +826,26 @@ virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status) PMD_INIT_LOG(ERR, "VHOST_USER_SET_STATUS failed (%d): %s", ret, strerror(errno)); } + + pthread_mutex_unlock(&dev->mutex); return ret; } int -virtio_user_update_status(struct virtio_user_dev *dev) +virtio_user_dev_update_status(struct virtio_user_dev *dev) { uint64_t ret; uint8_t status; int err; + pthread_mutex_lock(&dev->mutex); if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) { err = dev->ops->send_request(dev, VHOST_USER_GET_STATUS, &ret); if (!err && ret > UINT8_MAX) { PMD_INIT_LOG(ERR, "Invalid VHOST_USER_GET_STATUS " "response 0x%" PRIx64 "\n", ret); - return -1; + err = -1; + goto error; } status = ret; @@ -873,5 +879,7 @@ virtio_user_update_status(struct virtio_user_dev *dev) strerror(errno)); } +error: + pthread_mutex_unlock(&dev->mutex); return err; } diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h index 998986875..e053897d8 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h @@ -81,7 +81,7 @@ void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx); void virtio_user_handle_cq_packed(struct virtio_user_dev *dev, uint16_t queue_idx); uint8_t virtio_user_handle_mq(struct virtio_user_dev *dev, uint16_t q_pairs); -int virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status); -int virtio_user_update_status(struct virtio_user_dev *dev); +int virtio_user_dev_set_status(struct virtio_user_dev *dev, uint8_t status); +int virtio_user_dev_update_status(struct virtio_user_dev *dev); extern const char * const virtio_user_backend_strings[]; #endif diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index e870fb2ff..97ddc5651 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -278,8 +278,8 @@ virtio_user_set_status(struct virtio_hw *hw, uint8_t status) virtio_user_start_device(dev); else if (status == VIRTIO_CONFIG_STATUS_RESET) virtio_user_reset(hw); - dev->status = status; - virtio_user_send_status_update(dev, status); + + virtio_user_dev_set_status(dev, status); } static uint8_t @@ -287,7 +287,7 @@ virtio_user_get_status(struct virtio_hw *hw) { struct virtio_user_dev *dev = virtio_user_get_dev(hw); - virtio_user_update_status(dev); + virtio_user_dev_update_status(dev); return dev->status; } -- 2.26.2
There are some status reads and updates that need to happen before the protocol features are negotiated. Therefore, assuming the backend does support this feature can lead to failures. On server mode, do not assume the backend supports VHOST_USER_PROTOCOL_F_STATUS. Activate it back on reconnection and cleare it on disconnection. Fixes: 57912824615f ("net/virtio-user: support vhost status setting") Cc: maxime.coquelin@redhat.com Cc: stable@dpdk.org Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++ drivers/net/virtio/virtio_user_ethdev.c | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index 36e5619df..053f0267c 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -523,6 +523,12 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, * later. */ dev->device_features = VIRTIO_USER_SUPPORTED_FEATURES; + + /* We cannot assume VHOST_USER_PROTOCOL_F_STATUS is supported + * until it's negotiated + */ + dev->protocol_features &= + ~(1ULL << VHOST_USER_PROTOCOL_F_STATUS); } diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 97ddc5651..142bdc0bd 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -92,6 +92,9 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) &protocol_features)) return -1; + /* Offer VHOST_USER_PROTOCOL_F_STATUS */ + dev->protocol_features |= + (1ULL << VHOST_USER_PROTOCOL_F_STATUS); dev->protocol_features &= protocol_features; if (dev->ops->send_request(dev, @@ -168,6 +171,11 @@ virtio_user_delayed_handler(void *param) if (dev->vhostfd >= 0) { close(dev->vhostfd); dev->vhostfd = -1; + /* Until the featuers are negotiated again, don't assume + * the backend supports VHOST_USER_PROTOCOL_F_STATUS + */ + dev->protocol_features &= + ~(1ULL << VHOST_USER_PROTOCOL_F_STATUS); } eth_dev->intr_handle->fd = dev->listenfd; rte_intr_callback_register(eth_dev->intr_handle, -- 2.26.2
Newer vhost-user backends will rely on SET_STATUS to start the device so this required to support them. Fixes: 57912824615f ("net/virtio-user: support vhost status setting") Cc: maxime.coquelin@redhat.com Cc: stable@dpdk.org Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- drivers/net/virtio/virtio_user_ethdev.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 142bdc0bd..40345193e 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -67,8 +67,7 @@ virtio_user_reset_queues_packed(struct rte_eth_dev *dev) static int virtio_user_server_reconnect(struct virtio_user_dev *dev) { - int ret; - int connectfd; + int ret, connectfd, old_status; struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id]; struct virtio_hw *hw = eth_dev->data->dev_private; uint64_t protocol_features; @@ -78,6 +77,14 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) return -1; dev->vhostfd = connectfd; + old_status = vtpci_get_status(hw); + + vtpci_reset(hw); + + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_ACK); + + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER); + if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, &dev->device_features) < 0) { PMD_INIT_LOG(ERR, "get_features failed: %s", @@ -116,14 +123,17 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev) /* For packed ring, resetting queues is required in reconnection. */ if (vtpci_packed_queue(hw) && - (vtpci_get_status(hw) & VIRTIO_CONFIG_STATUS_DRIVER_OK)) { + (old_status & VIRTIO_CONFIG_STATUS_DRIVER_OK)) { PMD_INIT_LOG(NOTICE, "Packets on the fly will be dropped" " when packed ring reconnecting."); virtio_user_reset_queues_packed(eth_dev); } - ret = virtio_user_start_device(dev); - if (ret < 0) + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_FEATURES_OK); + + /* Start the device */ + vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_DRIVER_OK); + if (!dev->started) return -1; if (dev->queue_pairs > 1) { -- 2.26.2
Tested-by: JiangYuX <yux.jiang@intel.com>
Best Regards
Jiang yu
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Adrian Moreno
> Sent: Tuesday, October 27, 2020 12:39 AM
> To: dev@dpdk.org
> Cc: Wang, Yinan <yinan.wang@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; Xia, Chenbo <chenbo.xia@intel.com>; Wang,
> Zhihong <zhihong.wang@intel.com>; maxime.coquelin@redhat.com; Adrian
> Moreno <amorenoz@redhat.com>
> Subject: [dpdk-dev] [PATCH v3 0/6] net/virtio-user: fix server mode
>
> A number of issues have been detected that currently break virtio-user
> server mode.
> This series addresses such issues.
> Note that virtio-user server mode is broken by design and many of the
> problems that it currently has should be fixed by a bigger rework.
>
> --
> v2->v3:
> Fix potential concurrency problem on get/set state Handle STATUS protocol
> feature flag Fix undefined behaviour if STATUS feature is not supported
> Ensure packed virtqueues are reset on reconnection
>
>
> v1->v2:
> Added patch 2 and 3 addressing additional issues Check errno to select vhost-
> user backend and log the detected backend type
>
> Adrian Moreno (6):
> net/virtio-user: fix backend selection if stat fails
> net/virtio-user: don't set/get_status until FEATURES_OK
> net/virtio-user: ignore result if STATUS is unsupported
> net/virtio-user: lock-protect status updates
> net/virtio-user: don't assume vhost status feature
> net/virtio-user: set status on socket reconnect
>
> drivers/net/virtio/virtio_user/vhost_user.c | 14 ++---
> .../net/virtio/virtio_user/virtio_user_dev.c | 53 +++++++++++++------
> .../net/virtio/virtio_user/virtio_user_dev.h | 5 +-
> drivers/net/virtio/virtio_user_ethdev.c | 40 ++++++++++----
> 4 files changed, 78 insertions(+), 34 deletions(-)
>
> --
> 2.26.2
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> GET/SET STATUS is an optional feature, so it may not be negotiated. In
> that case, the VIRTIO_GET_STATUS call will not update the status (given
> as a pointer argument). Failing to identify this case would lead to
> undefined behavior as the device status will be updated with the value
> of a stack-allocated variable.
>
> To fix this, return ENOTSUP if the feature is not supported and, in that
> case, don't update device status.
>
> Fixes: 44102e6298e7 ("net/virtio: check protocol feature in user backend")
> Cc: maxime.coquelin@redhat.com
> Cc stable@dpdk.org
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> drivers/net/virtio/virtio_user/vhost_user.c | 4 +--
> .../net/virtio/virtio_user/virtio_user_dev.c | 28 +++++++++----------
> 2 files changed, 15 insertions(+), 17 deletions(-)
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> In order to safely set and get the device status from different
> threads (e.g: interrupt handlers).
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> drivers/net/virtio/virtio_user/virtio_user_dev.c | 14 +++++++++++---
> drivers/net/virtio/virtio_user/virtio_user_dev.h | 4 ++--
> drivers/net/virtio/virtio_user_ethdev.c | 6 +++---
> 3 files changed, 16 insertions(+), 8 deletions(-)
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> There are some status reads and updates that need to happen before the
> protocol features are negotiated. Therefore, assuming the backend does
> support this feature can lead to failures.
>
> On server mode, do not assume the backend supports
> VHOST_USER_PROTOCOL_F_STATUS. Activate it back on reconnection and
> cleare it on disconnection.
>
> Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> drivers/net/virtio/virtio_user/virtio_user_dev.c | 6 ++++++
> drivers/net/virtio/virtio_user_ethdev.c | 8 ++++++++
> 2 files changed, 14 insertions(+)
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> Newer vhost-user backends will rely on SET_STATUS to start the device
> so this required to support them.
>
> Fixes: 57912824615f ("net/virtio-user: support vhost status setting")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> drivers/net/virtio/virtio_user_ethdev.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Thanks,
Maxime
On 10/26/20 5:39 PM, Adrian Moreno wrote:
> A number of issues have been detected that currently break virtio-user
> server mode.
> This series addresses such issues.
> Note that virtio-user server mode is broken by design and many of the
> problems that it currently has should be fixed by a bigger rework.
>
> --
> v2->v3:
> Fix potential concurrency problem on get/set state
> Handle STATUS protocol feature flag
> Fix undefined behaviour if STATUS feature is not supported
> Ensure packed virtqueues are reset on reconnection
>
>
> v1->v2:
> Added patch 2 and 3 addressing additional issues
> Check errno to select vhost-user backend and log the detected backend
> type
>
> Adrian Moreno (6):
> net/virtio-user: fix backend selection if stat fails
> net/virtio-user: don't set/get_status until FEATURES_OK
> net/virtio-user: ignore result if STATUS is unsupported
> net/virtio-user: lock-protect status updates
> net/virtio-user: don't assume vhost status feature
> net/virtio-user: set status on socket reconnect
>
> drivers/net/virtio/virtio_user/vhost_user.c | 14 ++---
> .../net/virtio/virtio_user/virtio_user_dev.c | 53 +++++++++++++------
> .../net/virtio/virtio_user/virtio_user_dev.h | 5 +-
> drivers/net/virtio/virtio_user_ethdev.c | 40 ++++++++++----
> 4 files changed, 78 insertions(+), 34 deletions(-)
>
Applied to dpdk-next-virtio/main.
Thanks!
Maxime