[v3,3/8] net/virtio: move backend type selection to ethdev

Message ID 20200929161404.124580-4-maxime.coquelin@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series virtio-user: introduce vhost-vdpa backend |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Sept. 29, 2020, 4:13 p.m. UTC
  From: Adrian Moreno <amorenoz@redhat.com>

This is a preparation patch with no functional change.

Use an enum instead of a boolean for the backend type.
Move the detection logic to the ethdev layer (where it is needed for the
first time).
The virtio_user_dev stores the backend type in the virtio_user_dev
struct so the type is only determined once

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
 .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
 drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
 3 files changed, 50 insertions(+), 26 deletions(-)
  

Comments

Chenbo Xia Sept. 30, 2020, 5:49 a.m. UTC | #1
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Wednesday, September 30, 2020 12:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH v3 3/8] net/virtio: move backend type selection to ethdev
> 
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> This is a preparation patch with no functional change.
> 
> Use an enum instead of a boolean for the backend type.
> Move the detection logic to the ethdev layer (where it is needed for the
> first time).
> The virtio_user_dev stores the backend type in the virtio_user_dev
> struct so the type is only determined once
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>  3 files changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 2a0c861085..b79a9f84aa 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> -int
> -is_vhost_user_by_type(const char *path)
> -{
> -	struct stat sb;
> -
> -	if (stat(path, &sb) == -1)
> -		return 0;
> -
> -	return S_ISSOCK(sb.st_mode);
> -}
> -
>  int
>  virtio_user_start_device(struct virtio_user_dev *dev)
>  {
> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	rte_mcfg_mem_read_lock();
>  	pthread_mutex_lock(&dev->mutex);
> 
> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> +			dev->vhostfd < 0)
>  		goto error;
> 
>  	/* Step 0: tell vhost to create queues */
> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  	dev->tapfds = NULL;
> 
>  	if (dev->is_server) {
> -		if (access(dev->path, F_OK) == 0 &&
> -		    !is_vhost_user_by_type(dev->path)) {
> -			PMD_DRV_LOG(ERR, "Server mode doesn't support vhost-
> kernel!");
> +		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
> +			PMD_DRV_LOG(ERR, "Server mode only supports vhost-
> user!");
>  			return -1;
>  		}
>  		dev->ops = &virtio_ops_user;
>  	} else {
> -		if (is_vhost_user_by_type(dev->path)) {
> +		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
>  			dev->ops = &virtio_ops_user;
> -		} else {
> +		} else if (dev->backend_type ==
> +					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			dev->ops = &virtio_ops_kernel;
> 
>  			dev->vhostfds = malloc(dev->max_queue_pairs *
> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		     int cq, int queue_size, const char *mac, char **ifname,
> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> +		     enum virtio_user_backend_type backend_type)
>  {
>  	uint64_t protocol_features = 0;
> 
> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->frontend_features = 0;
>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>  	dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> +	dev->backend_type = backend_type;
> +
>  	parse_mac(dev, mac);
> 
>  	if (*ifname) {
> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->unsupported_features |=
>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> 
> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	}
> 
>  	/* The backend will not report this feature, we add it explicitly */
> -	if (is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> 
>  	/*
> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  	uint64_t arg = status;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	int err;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1UL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 9377d5ba66..575bf430c0 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -10,6 +10,12 @@
>  #include "../virtio_pci.h"
>  #include "../virtio_ring.h"
> 
> +enum virtio_user_backend_type {
> +	VIRTIO_USER_BACKEND_UNKNOWN,
> +	VIRTIO_USER_BACKEND_VHOST_USER,
> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> +};
> +
>  struct virtio_user_queue {
>  	uint16_t used_idx;
>  	bool avail_wrap_counter;
> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>  };
> 
>  struct virtio_user_dev {
> +	enum virtio_user_backend_type backend_type;
>  	/* for vhost_user backend */
>  	int		vhostfd;
>  	int		listenfd;   /* listening fd */
> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>  	bool		started;
>  };
> 
> -int is_vhost_user_by_type(const char *path);
>  int virtio_user_start_device(struct virtio_user_dev *dev);
>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int
> queues,
>  			 int cq, int queue_size, const char *mac, char **ifname,
>  			 int server, int mrg_rxbuf, int in_order,
> -			 int packed_vq);
> +			 int packed_vq,
> +			 enum virtio_user_backend_type backend_type);
>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>  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,
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 60d17af888..38b49bad5f 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -6,6 +6,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <sys/stat.h>
>  #include <sys/socket.h>
> 
>  #include <rte_malloc.h>
> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>  	return -errno;
>  }
> 
> +static enum virtio_user_backend_type
> +virtio_user_backend_type(const char *path)
> +{
> +	struct stat sb;
> +
> +	if (stat(path, &sb) == -1)
> +		return VIRTIO_USER_BACKEND_UNKNOWN;
> +
> +	return S_ISSOCK(sb.st_mode) ?
> +		VIRTIO_USER_BACKEND_VHOST_USER :
> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +}
> +
>  static struct rte_eth_dev *
>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>  {
> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	struct rte_kvargs *kvlist = NULL;
>  	struct rte_eth_dev *eth_dev;
>  	struct virtio_hw *hw;
> +	enum virtio_user_backend_type backend_type =
> VIRTIO_USER_BACKEND_UNKNOWN;
>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  		goto end;
>  	}
> 
> +	backend_type = virtio_user_backend_type(path);
> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> +		PMD_INIT_LOG(ERR,
> +			     "unable to determine backend type for path %s",
> +			path);
> +		goto end;
> +	}
> +
> +
>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
> -		if (is_vhost_user_by_type(path)) {
> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			PMD_INIT_LOG(ERR,
>  				"arg %s applies only to vhost-kernel backend",
>  				VIRTIO_USER_ARG_INTERFACE_NAME);
> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	hw = eth_dev->data->dev_private;
>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>  			 queue_size, mac_addr, &ifname, server_mode,
> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>  		virtio_user_eth_dev_free(eth_dev);
>  		goto end;
> --
> 2.26.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  
Wang, Yinan Oct. 16, 2020, 5:42 a.m. UTC | #2
Hi Adrian,

This patch introduce a regression issue that vhost-user can't launch with server mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
This issue blocked amount of regression cases, could you help to take a look?
Thanks in advance!


BR,
Yinan


> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> Sent: 2020?9?30? 0:14
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> <patrick.fu@intel.com>; amorenoz@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> ethdev
> 
> From: Adrian Moreno <amorenoz@redhat.com>
> 
> This is a preparation patch with no functional change.
> 
> Use an enum instead of a boolean for the backend type.
> Move the detection logic to the ethdev layer (where it is needed for the
> first time).
> The virtio_user_dev stores the backend type in the virtio_user_dev
> struct so the type is only determined once
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>  3 files changed, 50 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 2a0c861085..b79a9f84aa 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> -int
> -is_vhost_user_by_type(const char *path)
> -{
> -	struct stat sb;
> -
> -	if (stat(path, &sb) == -1)
> -		return 0;
> -
> -	return S_ISSOCK(sb.st_mode);
> -}
> -
>  int
>  virtio_user_start_device(struct virtio_user_dev *dev)
>  {
> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>  	rte_mcfg_mem_read_lock();
>  	pthread_mutex_lock(&dev->mutex);
> 
> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> +			dev->vhostfd < 0)
>  		goto error;
> 
>  	/* Step 0: tell vhost to create queues */
> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  	dev->tapfds = NULL;
> 
>  	if (dev->is_server) {
> -		if (access(dev->path, F_OK) == 0 &&
> -		    !is_vhost_user_by_type(dev->path)) {
> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> vhost-kernel!");
> +		if (dev->backend_type !=
> VIRTIO_USER_BACKEND_VHOST_USER) {
> +			PMD_DRV_LOG(ERR, "Server mode only supports
> vhost-user!");
>  			return -1;
>  		}
>  		dev->ops = &virtio_ops_user;
>  	} else {
> -		if (is_vhost_user_by_type(dev->path)) {
> +		if (dev->backend_type ==
> VIRTIO_USER_BACKEND_VHOST_USER) {
>  			dev->ops = &virtio_ops_user;
> -		} else {
> +		} else if (dev->backend_type ==
> +
> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			dev->ops = &virtio_ops_kernel;
> 
>  			dev->vhostfds = malloc(dev->max_queue_pairs *
> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  int
>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  		     int cq, int queue_size, const char *mac, char **ifname,
> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> +		     enum virtio_user_backend_type backend_type)
>  {
>  	uint64_t protocol_features = 0;
> 
> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->frontend_features = 0;
>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>  	dev->protocol_features =
> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> +	dev->backend_type = backend_type;
> +
>  	parse_mac(dev, mac);
> 
>  	if (*ifname) {
> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->unsupported_features |=
>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> 
> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	}
> 
>  	/* The backend will not report this feature, we add it explicitly */
> -	if (is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> 
>  	/*
> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> *dev, uint8_t status)
>  	uint64_t arg = status;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1ULL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>  	int err;
> 
>  	/* Vhost-user only for now */
> -	if (!is_vhost_user_by_type(dev->path))
> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>  		return 0;
> 
>  	if (!(dev->protocol_features & (1UL <<
> VHOST_USER_PROTOCOL_F_STATUS)))
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 9377d5ba66..575bf430c0 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -10,6 +10,12 @@
>  #include "../virtio_pci.h"
>  #include "../virtio_ring.h"
> 
> +enum virtio_user_backend_type {
> +	VIRTIO_USER_BACKEND_UNKNOWN,
> +	VIRTIO_USER_BACKEND_VHOST_USER,
> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> +};
> +
>  struct virtio_user_queue {
>  	uint16_t used_idx;
>  	bool avail_wrap_counter;
> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>  };
> 
>  struct virtio_user_dev {
> +	enum virtio_user_backend_type backend_type;
>  	/* for vhost_user backend */
>  	int		vhostfd;
>  	int		listenfd;   /* listening fd */
> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>  	bool		started;
>  };
> 
> -int is_vhost_user_by_type(const char *path);
>  int virtio_user_start_device(struct virtio_user_dev *dev);
>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>  			 int cq, int queue_size, const char *mac, char **ifname,
>  			 int server, int mrg_rxbuf, int in_order,
> -			 int packed_vq);
> +			 int packed_vq,
> +			 enum virtio_user_backend_type backend_type);
>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>  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,
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index 60d17af888..38b49bad5f 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -6,6 +6,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <sys/stat.h>
>  #include <sys/socket.h>
> 
>  #include <rte_malloc.h>
> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>  	return -errno;
>  }
> 
> +static enum virtio_user_backend_type
> +virtio_user_backend_type(const char *path)
> +{
> +	struct stat sb;
> +
> +	if (stat(path, &sb) == -1)
> +		return VIRTIO_USER_BACKEND_UNKNOWN;
> +
> +	return S_ISSOCK(sb.st_mode) ?
> +		VIRTIO_USER_BACKEND_VHOST_USER :
> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> +}
> +
>  static struct rte_eth_dev *
>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>  {
> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	struct rte_kvargs *kvlist = NULL;
>  	struct rte_eth_dev *eth_dev;
>  	struct virtio_hw *hw;
> +	enum virtio_user_backend_type backend_type =
> VIRTIO_USER_BACKEND_UNKNOWN;
>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  		goto end;
>  	}
> 
> +	backend_type = virtio_user_backend_type(path);
> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> +		PMD_INIT_LOG(ERR,
> +			     "unable to determine backend type for path %s",
> +			path);
> +		goto end;
> +	}
> +
> +
>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> {
> -		if (is_vhost_user_by_type(path)) {
> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>  			PMD_INIT_LOG(ERR,
>  				"arg %s applies only to vhost-kernel backend",
>  				VIRTIO_USER_ARG_INTERFACE_NAME);
> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>  	hw = eth_dev->data->dev_private;
>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>  			 queue_size, mac_addr, &ifname, server_mode,
> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>  		virtio_user_eth_dev_free(eth_dev);
>  		goto end;
> --
> 2.26.2
  
Wang, Yinan Oct. 16, 2020, 5:58 a.m. UTC | #3
Correct my typo issue that vhost-user can't launch with client mode.

BR,
Yinan

> -----Original Message-----
> From: Wang, Yinan
> Sent: 2020?10?16? 13:42
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
> amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
> to ethdev
> 
> Hi Adrian,
> 
> This patch introduce a regression issue that vhost-user can't launch with server
> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
> This issue blocked amount of regression cases, could you help to take a look?
> Thanks in advance!
> 
> 
> BR,
> Yinan
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> > Sent: 2020?9?30? 0:14
> > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> > <patrick.fu@intel.com>; amorenoz@redhat.com
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> > ethdev
> >
> > From: Adrian Moreno <amorenoz@redhat.com>
> >
> > This is a preparation patch with no functional change.
> >
> > Use an enum instead of a boolean for the backend type.
> > Move the detection logic to the ethdev layer (where it is needed for the
> > first time).
> > The virtio_user_dev stores the backend type in the virtio_user_dev
> > struct so the type is only determined once
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> > ---
> >  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
> >  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
> >  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
> >  3 files changed, 50 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 2a0c861085..b79a9f84aa 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
> >  	return 0;
> >  }
> >
> > -int
> > -is_vhost_user_by_type(const char *path)
> > -{
> > -	struct stat sb;
> > -
> > -	if (stat(path, &sb) == -1)
> > -		return 0;
> > -
> > -	return S_ISSOCK(sb.st_mode);
> > -}
> > -
> >  int
> >  virtio_user_start_device(struct virtio_user_dev *dev)
> >  {
> > @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
> >  	rte_mcfg_mem_read_lock();
> >  	pthread_mutex_lock(&dev->mutex);
> >
> > -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> > +			dev->vhostfd < 0)
> >  		goto error;
> >
> >  	/* Step 0: tell vhost to create queues */
> > @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  	dev->tapfds = NULL;
> >
> >  	if (dev->is_server) {
> > -		if (access(dev->path, F_OK) == 0 &&
> > -		    !is_vhost_user_by_type(dev->path)) {
> > -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> > vhost-kernel!");
> > +		if (dev->backend_type !=
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> > +			PMD_DRV_LOG(ERR, "Server mode only supports
> > vhost-user!");
> >  			return -1;
> >  		}
> >  		dev->ops = &virtio_ops_user;
> >  	} else {
> > -		if (is_vhost_user_by_type(dev->path)) {
> > +		if (dev->backend_type ==
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> >  			dev->ops = &virtio_ops_user;
> > -		} else {
> > +		} else if (dev->backend_type ==
> > +
> > 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			dev->ops = &virtio_ops_kernel;
> >
> >  			dev->vhostfds = malloc(dev->max_queue_pairs *
> > @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  int
> >  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		     int cq, int queue_size, const char *mac, char **ifname,
> > -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> > +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> > +		     enum virtio_user_backend_type backend_type)
> >  {
> >  	uint64_t protocol_features = 0;
> >
> > @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	dev->frontend_features = 0;
> >  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
> >  	dev->protocol_features =
> > VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> > +	dev->backend_type = backend_type;
> > +
> >  	parse_mac(dev, mac);
> >
> >  	if (*ifname) {
> > @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  		return -1;
> >  	}
> >
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->unsupported_features |=
> >  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> >
> > @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	}
> >
> >  	/* The backend will not report this feature, we add it explicitly */
> > -	if (is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> >
> >  	/*
> > @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> > *dev, uint8_t status)
> >  	uint64_t arg = status;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
> >  	int err;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1UL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > index 9377d5ba66..575bf430c0 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > @@ -10,6 +10,12 @@
> >  #include "../virtio_pci.h"
> >  #include "../virtio_ring.h"
> >
> > +enum virtio_user_backend_type {
> > +	VIRTIO_USER_BACKEND_UNKNOWN,
> > +	VIRTIO_USER_BACKEND_VHOST_USER,
> > +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> > +};
> > +
> >  struct virtio_user_queue {
> >  	uint16_t used_idx;
> >  	bool avail_wrap_counter;
> > @@ -17,6 +23,7 @@ struct virtio_user_queue {
> >  };
> >
> >  struct virtio_user_dev {
> > +	enum virtio_user_backend_type backend_type;
> >  	/* for vhost_user backend */
> >  	int		vhostfd;
> >  	int		listenfd;   /* listening fd */
> > @@ -60,13 +67,13 @@ struct virtio_user_dev {
> >  	bool		started;
> >  };
> >
> > -int is_vhost_user_by_type(const char *path);
> >  int virtio_user_start_device(struct virtio_user_dev *dev);
> >  int virtio_user_stop_device(struct virtio_user_dev *dev);
> >  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  			 int cq, int queue_size, const char *mac, char **ifname,
> >  			 int server, int mrg_rxbuf, int in_order,
> > -			 int packed_vq);
> > +			 int packed_vq,
> > +			 enum virtio_user_backend_type backend_type);
> >  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
> >  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,
> > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > index 60d17af888..38b49bad5f 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -6,6 +6,7 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > +#include <sys/stat.h>
> >  #include <sys/socket.h>
> >
> >  #include <rte_malloc.h>
> > @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
> >  	return -errno;
> >  }
> >
> > +static enum virtio_user_backend_type
> > +virtio_user_backend_type(const char *path)
> > +{
> > +	struct stat sb;
> > +
> > +	if (stat(path, &sb) == -1)
> > +		return VIRTIO_USER_BACKEND_UNKNOWN;
> > +
> > +	return S_ISSOCK(sb.st_mode) ?
> > +		VIRTIO_USER_BACKEND_VHOST_USER :
> > +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> > +}
> > +
> >  static struct rte_eth_dev *
> >  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
> >  {
> > @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	struct rte_kvargs *kvlist = NULL;
> >  	struct rte_eth_dev *eth_dev;
> >  	struct virtio_hw *hw;
> > +	enum virtio_user_backend_type backend_type =
> > VIRTIO_USER_BACKEND_UNKNOWN;
> >  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
> >  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
> >  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> > @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  		goto end;
> >  	}
> >
> > +	backend_type = virtio_user_backend_type(path);
> > +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> > +		PMD_INIT_LOG(ERR,
> > +			     "unable to determine backend type for path %s",
> > +			path);
> > +		goto end;
> > +	}
> > +
> > +
> >  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> > {
> > -		if (is_vhost_user_by_type(path)) {
> > +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			PMD_INIT_LOG(ERR,
> >  				"arg %s applies only to vhost-kernel backend",
> >  				VIRTIO_USER_ARG_INTERFACE_NAME);
> > @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	hw = eth_dev->data->dev_private;
> >  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> >  			 queue_size, mac_addr, &ifname, server_mode,
> > -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> > +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
> >  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
> >  		virtio_user_eth_dev_free(eth_dev);
> >  		goto end;
> > --
> > 2.26.2
  
Wang, Yinan Oct. 20, 2020, 1:52 a.m. UTC | #4
Hi Adrian/Maxime,

Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.
Thanks in advance!

BR,
Yinan

> -----Original Message-----
> From: Wang, Yinan
> Sent: 2020?10?16? 13:42
> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
> amorenoz@redhat.com
> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
> to ethdev
> 
> Hi Adrian,
> 
> This patch introduce a regression issue that vhost-user can't launch with server
> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
> This issue blocked amount of regression cases, could you help to take a look?
> Thanks in advance!
> 
> 
> BR,
> Yinan
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
> > Sent: 2020?9?30? 0:14
> > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
> > <patrick.fu@intel.com>; amorenoz@redhat.com
> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
> > ethdev
> >
> > From: Adrian Moreno <amorenoz@redhat.com>
> >
> > This is a preparation patch with no functional change.
> >
> > Use an enum instead of a boolean for the backend type.
> > Move the detection logic to the ethdev layer (where it is needed for the
> > first time).
> > The virtio_user_dev stores the backend type in the virtio_user_dev
> > struct so the type is only determined once
> >
> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> > ---
> >  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
> >  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
> >  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
> >  3 files changed, 50 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > index 2a0c861085..b79a9f84aa 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> > @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
> >  	return 0;
> >  }
> >
> > -int
> > -is_vhost_user_by_type(const char *path)
> > -{
> > -	struct stat sb;
> > -
> > -	if (stat(path, &sb) == -1)
> > -		return 0;
> > -
> > -	return S_ISSOCK(sb.st_mode);
> > -}
> > -
> >  int
> >  virtio_user_start_device(struct virtio_user_dev *dev)
> >  {
> > @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
> >  	rte_mcfg_mem_read_lock();
> >  	pthread_mutex_lock(&dev->mutex);
> >
> > -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
> > +			dev->vhostfd < 0)
> >  		goto error;
> >
> >  	/* Step 0: tell vhost to create queues */
> > @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  	dev->tapfds = NULL;
> >
> >  	if (dev->is_server) {
> > -		if (access(dev->path, F_OK) == 0 &&
> > -		    !is_vhost_user_by_type(dev->path)) {
> > -			PMD_DRV_LOG(ERR, "Server mode doesn't support
> > vhost-kernel!");
> > +		if (dev->backend_type !=
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> > +			PMD_DRV_LOG(ERR, "Server mode only supports
> > vhost-user!");
> >  			return -1;
> >  		}
> >  		dev->ops = &virtio_ops_user;
> >  	} else {
> > -		if (is_vhost_user_by_type(dev->path)) {
> > +		if (dev->backend_type ==
> > VIRTIO_USER_BACKEND_VHOST_USER) {
> >  			dev->ops = &virtio_ops_user;
> > -		} else {
> > +		} else if (dev->backend_type ==
> > +
> > 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			dev->ops = &virtio_ops_kernel;
> >
> >  			dev->vhostfds = malloc(dev->max_queue_pairs *
> > @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
> >  int
> >  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  		     int cq, int queue_size, const char *mac, char **ifname,
> > -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
> > +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
> > +		     enum virtio_user_backend_type backend_type)
> >  {
> >  	uint64_t protocol_features = 0;
> >
> > @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	dev->frontend_features = 0;
> >  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
> >  	dev->protocol_features =
> > VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
> > +	dev->backend_type = backend_type;
> > +
> >  	parse_mac(dev, mac);
> >
> >  	if (*ifname) {
> > @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  		return -1;
> >  	}
> >
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->unsupported_features |=
> >  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> >
> > @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> > *path, int queues,
> >  	}
> >
> >  	/* The backend will not report this feature, we add it explicitly */
> > -	if (is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> >  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
> >
> >  	/*
> > @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
> > *dev, uint8_t status)
> >  	uint64_t arg = status;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
> >  	int err;
> >
> >  	/* Vhost-user only for now */
> > -	if (!is_vhost_user_by_type(dev->path))
> > +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> >  		return 0;
> >
> >  	if (!(dev->protocol_features & (1UL <<
> > VHOST_USER_PROTOCOL_F_STATUS)))
> > diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > index 9377d5ba66..575bf430c0 100644
> > --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> > @@ -10,6 +10,12 @@
> >  #include "../virtio_pci.h"
> >  #include "../virtio_ring.h"
> >
> > +enum virtio_user_backend_type {
> > +	VIRTIO_USER_BACKEND_UNKNOWN,
> > +	VIRTIO_USER_BACKEND_VHOST_USER,
> > +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
> > +};
> > +
> >  struct virtio_user_queue {
> >  	uint16_t used_idx;
> >  	bool avail_wrap_counter;
> > @@ -17,6 +23,7 @@ struct virtio_user_queue {
> >  };
> >
> >  struct virtio_user_dev {
> > +	enum virtio_user_backend_type backend_type;
> >  	/* for vhost_user backend */
> >  	int		vhostfd;
> >  	int		listenfd;   /* listening fd */
> > @@ -60,13 +67,13 @@ struct virtio_user_dev {
> >  	bool		started;
> >  };
> >
> > -int is_vhost_user_by_type(const char *path);
> >  int virtio_user_start_device(struct virtio_user_dev *dev);
> >  int virtio_user_stop_device(struct virtio_user_dev *dev);
> >  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
> >  			 int cq, int queue_size, const char *mac, char **ifname,
> >  			 int server, int mrg_rxbuf, int in_order,
> > -			 int packed_vq);
> > +			 int packed_vq,
> > +			 enum virtio_user_backend_type backend_type);
> >  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
> >  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,
> > diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> > b/drivers/net/virtio/virtio_user_ethdev.c
> > index 60d17af888..38b49bad5f 100644
> > --- a/drivers/net/virtio/virtio_user_ethdev.c
> > +++ b/drivers/net/virtio/virtio_user_ethdev.c
> > @@ -6,6 +6,7 @@
> >  #include <sys/types.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > +#include <sys/stat.h>
> >  #include <sys/socket.h>
> >
> >  #include <rte_malloc.h>
> > @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
> >  	return -errno;
> >  }
> >
> > +static enum virtio_user_backend_type
> > +virtio_user_backend_type(const char *path)
> > +{
> > +	struct stat sb;
> > +
> > +	if (stat(path, &sb) == -1)
> > +		return VIRTIO_USER_BACKEND_UNKNOWN;
> > +
> > +	return S_ISSOCK(sb.st_mode) ?
> > +		VIRTIO_USER_BACKEND_VHOST_USER :
> > +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
> > +}
> > +
> >  static struct rte_eth_dev *
> >  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
> >  {
> > @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	struct rte_kvargs *kvlist = NULL;
> >  	struct rte_eth_dev *eth_dev;
> >  	struct virtio_hw *hw;
> > +	enum virtio_user_backend_type backend_type =
> > VIRTIO_USER_BACKEND_UNKNOWN;
> >  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
> >  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
> >  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
> > @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  		goto end;
> >  	}
> >
> > +	backend_type = virtio_user_backend_type(path);
> > +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
> > +		PMD_INIT_LOG(ERR,
> > +			     "unable to determine backend type for path %s",
> > +			path);
> > +		goto end;
> > +	}
> > +
> > +
> >  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
> > {
> > -		if (is_vhost_user_by_type(path)) {
> > +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
> >  			PMD_INIT_LOG(ERR,
> >  				"arg %s applies only to vhost-kernel backend",
> >  				VIRTIO_USER_ARG_INTERFACE_NAME);
> > @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
> >  	hw = eth_dev->data->dev_private;
> >  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
> >  			 queue_size, mac_addr, &ifname, server_mode,
> > -			 mrg_rxbuf, in_order, packed_vq) < 0) {
> > +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
> >  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
> >  		virtio_user_eth_dev_free(eth_dev);
> >  		goto end;
> > --
> > 2.26.2
  
Maxime Coquelin Oct. 20, 2020, 7:11 a.m. UTC | #5
Hi Yinan,

On 10/20/20 3:52 AM, Wang, Yinan wrote:
> Hi Adrian/Maxime,
> 
> Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.

We are working on it. Adrian managed to reproduce it, and fixed one
part of the issue. We''l continue the debugging today.

BTW, you are mentionning automated regression tests, any chances these
test land in the Intel functionnal CI at some point? I would be great to
catch these issues before the series are merged.

Thanks!
Maxime

> Thanks in advance!
> 
> BR,
> Yinan
> 
>> -----Original Message-----
>> From: Wang, Yinan
>> Sent: 2020?10?16? 13:42
>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
>> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
>> amorenoz@redhat.com
>> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
>> to ethdev
>>
>> Hi Adrian,
>>
>> This patch introduce a regression issue that vhost-user can't launch with server
>> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
>> This issue blocked amount of regression cases, could you help to take a look?
>> Thanks in advance!
>>
>>
>> BR,
>> Yinan
>>
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>>> Sent: 2020?9?30? 0:14
>>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
>>> <patrick.fu@intel.com>; amorenoz@redhat.com
>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
>>> ethdev
>>>
>>> From: Adrian Moreno <amorenoz@redhat.com>
>>>
>>> This is a preparation patch with no functional change.
>>>
>>> Use an enum instead of a boolean for the backend type.
>>> Move the detection logic to the ethdev layer (where it is needed for the
>>> first time).
>>> The virtio_user_dev stores the backend type in the virtio_user_dev
>>> struct so the type is only determined once
>>>
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>> ---
>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>>>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>>>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>>>  3 files changed, 50 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> index 2a0c861085..b79a9f84aa 100644
>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>>>  	return 0;
>>>  }
>>>
>>> -int
>>> -is_vhost_user_by_type(const char *path)
>>> -{
>>> -	struct stat sb;
>>> -
>>> -	if (stat(path, &sb) == -1)
>>> -		return 0;
>>> -
>>> -	return S_ISSOCK(sb.st_mode);
>>> -}
>>> -
>>>  int
>>>  virtio_user_start_device(struct virtio_user_dev *dev)
>>>  {
>>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>>>  	rte_mcfg_mem_read_lock();
>>>  	pthread_mutex_lock(&dev->mutex);
>>>
>>> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
>>> +			dev->vhostfd < 0)
>>>  		goto error;
>>>
>>>  	/* Step 0: tell vhost to create queues */
>>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>  	dev->tapfds = NULL;
>>>
>>>  	if (dev->is_server) {
>>> -		if (access(dev->path, F_OK) == 0 &&
>>> -		    !is_vhost_user_by_type(dev->path)) {
>>> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
>>> vhost-kernel!");
>>> +		if (dev->backend_type !=
>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>> +			PMD_DRV_LOG(ERR, "Server mode only supports
>>> vhost-user!");
>>>  			return -1;
>>>  		}
>>>  		dev->ops = &virtio_ops_user;
>>>  	} else {
>>> -		if (is_vhost_user_by_type(dev->path)) {
>>> +		if (dev->backend_type ==
>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>  			dev->ops = &virtio_ops_user;
>>> -		} else {
>>> +		} else if (dev->backend_type ==
>>> +
>>> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>  			dev->ops = &virtio_ops_kernel;
>>>
>>>  			dev->vhostfds = malloc(dev->max_queue_pairs *
>>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>  int
>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>  		     int cq, int queue_size, const char *mac, char **ifname,
>>> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
>>> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
>>> +		     enum virtio_user_backend_type backend_type)
>>>  {
>>>  	uint64_t protocol_features = 0;
>>>
>>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  	dev->frontend_features = 0;
>>>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>>>  	dev->protocol_features =
>>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
>>> +	dev->backend_type = backend_type;
>>> +
>>>  	parse_mac(dev, mac);
>>>
>>>  	if (*ifname) {
>>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  		return -1;
>>>  	}
>>>
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		dev->unsupported_features |=
>>>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>>>
>>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>> *path, int queues,
>>>  	}
>>>
>>>  	/* The backend will not report this feature, we add it explicitly */
>>> -	if (is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
>>>
>>>  	/*
>>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
>>> *dev, uint8_t status)
>>>  	uint64_t arg = status;
>>>
>>>  	/* Vhost-user only for now */
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		return 0;
>>>
>>>  	if (!(dev->protocol_features & (1ULL <<
>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>>>  	int err;
>>>
>>>  	/* Vhost-user only for now */
>>> -	if (!is_vhost_user_by_type(dev->path))
>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>  		return 0;
>>>
>>>  	if (!(dev->protocol_features & (1UL <<
>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> index 9377d5ba66..575bf430c0 100644
>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>> @@ -10,6 +10,12 @@
>>>  #include "../virtio_pci.h"
>>>  #include "../virtio_ring.h"
>>>
>>> +enum virtio_user_backend_type {
>>> +	VIRTIO_USER_BACKEND_UNKNOWN,
>>> +	VIRTIO_USER_BACKEND_VHOST_USER,
>>> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
>>> +};
>>> +
>>>  struct virtio_user_queue {
>>>  	uint16_t used_idx;
>>>  	bool avail_wrap_counter;
>>> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>>>  };
>>>
>>>  struct virtio_user_dev {
>>> +	enum virtio_user_backend_type backend_type;
>>>  	/* for vhost_user backend */
>>>  	int		vhostfd;
>>>  	int		listenfd;   /* listening fd */
>>> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>>>  	bool		started;
>>>  };
>>>
>>> -int is_vhost_user_by_type(const char *path);
>>>  int virtio_user_start_device(struct virtio_user_dev *dev);
>>>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>>>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>  			 int cq, int queue_size, const char *mac, char **ifname,
>>>  			 int server, int mrg_rxbuf, int in_order,
>>> -			 int packed_vq);
>>> +			 int packed_vq,
>>> +			 enum virtio_user_backend_type backend_type);
>>>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>>>  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,
>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>> index 60d17af888..38b49bad5f 100644
>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>> @@ -6,6 +6,7 @@
>>>  #include <sys/types.h>
>>>  #include <unistd.h>
>>>  #include <fcntl.h>
>>> +#include <sys/stat.h>
>>>  #include <sys/socket.h>
>>>
>>>  #include <rte_malloc.h>
>>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>>>  	return -errno;
>>>  }
>>>
>>> +static enum virtio_user_backend_type
>>> +virtio_user_backend_type(const char *path)
>>> +{
>>> +	struct stat sb;
>>> +
>>> +	if (stat(path, &sb) == -1)
>>> +		return VIRTIO_USER_BACKEND_UNKNOWN;
>>> +
>>> +	return S_ISSOCK(sb.st_mode) ?
>>> +		VIRTIO_USER_BACKEND_VHOST_USER :
>>> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
>>> +}
>>> +
>>>  static struct rte_eth_dev *
>>>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>>>  {
>>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  	struct rte_kvargs *kvlist = NULL;
>>>  	struct rte_eth_dev *eth_dev;
>>>  	struct virtio_hw *hw;
>>> +	enum virtio_user_backend_type backend_type =
>>> VIRTIO_USER_BACKEND_UNKNOWN;
>>>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>>>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>>>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
>>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  		goto end;
>>>  	}
>>>
>>> +	backend_type = virtio_user_backend_type(path);
>>> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
>>> +		PMD_INIT_LOG(ERR,
>>> +			     "unable to determine backend type for path %s",
>>> +			path);
>>> +		goto end;
>>> +	}
>>> +
>>> +
>>>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
>>> {
>>> -		if (is_vhost_user_by_type(path)) {
>>> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>  			PMD_INIT_LOG(ERR,
>>>  				"arg %s applies only to vhost-kernel backend",
>>>  				VIRTIO_USER_ARG_INTERFACE_NAME);
>>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>  	hw = eth_dev->data->dev_private;
>>>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>>>  			 queue_size, mac_addr, &ifname, server_mode,
>>> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
>>> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>>>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>>>  		virtio_user_eth_dev_free(eth_dev);
>>>  		goto end;
>>> --
>>> 2.26.2
>
  
Adrian Moreno Oct. 20, 2020, 7:20 a.m. UTC | #6
Hi Yinan,

Still testing, but submitting for early review:
https://patches.dpdk.org/patch/81431/

Thanks,

On 10/20/20 9:11 AM, Maxime Coquelin wrote:
> Hi Yinan,
> 
> On 10/20/20 3:52 AM, Wang, Yinan wrote:
>> Hi Adrian/Maxime,
>>
>> Could you help to take a look at this issue? Most of automated regression cases will be blocked due to this issue in 20.11 RC1 test.
> 
> We are working on it. Adrian managed to reproduce it, and fixed one
> part of the issue. We''l continue the debugging today.
> 

> BTW, you are mentionning automated regression tests, any chances these
> test land in the Intel functionnal CI at some point? I would be great to
> catch these issues before the series are merged.
> 
> Thanks!
> Maxime
> 
>> Thanks in advance!
>>
>> BR,
>> Yinan
>>
>>> -----Original Message-----
>>> From: Wang, Yinan
>>> Sent: 2020?10?16? 13:42
>>> To: Maxime Coquelin <maxime.coquelin@redhat.com>; dev@dpdk.org; Xia,
>>> Chenbo <Chenbo.Xia@intel.com>; Fu, Patrick <patrick.fu@intel.com>;
>>> amorenoz@redhat.com
>>> Subject: RE: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection
>>> to ethdev
>>>
>>> Hi Adrian,
>>>
>>> This patch introduce a regression issue that vhost-user can't launch with server
>>> mode, details in https://bugs.dpdk.org/show_bug.cgi?id=559.
>>> This issue blocked amount of regression cases, could you help to take a look?
>>> Thanks in advance!
>>>
>>>
>>> BR,
>>> Yinan
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Maxime Coquelin
>>>> Sent: 2020?9?30? 0:14
>>>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Fu, Patrick
>>>> <patrick.fu@intel.com>; amorenoz@redhat.com
>>>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Subject: [dpdk-dev] [PATCH v3 3/8] net/virtio: move backend type selection to
>>>> ethdev
>>>>
>>>> From: Adrian Moreno <amorenoz@redhat.com>
>>>>
>>>> This is a preparation patch with no functional change.
>>>>
>>>> Use an enum instead of a boolean for the backend type.
>>>> Move the detection logic to the ethdev layer (where it is needed for the
>>>> first time).
>>>> The virtio_user_dev stores the backend type in the virtio_user_dev
>>>> struct so the type is only determined once
>>>>
>>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>>>> ---
>>>>  .../net/virtio/virtio_user/virtio_user_dev.c  | 37 ++++++++-----------
>>>>  .../net/virtio/virtio_user/virtio_user_dev.h  | 11 +++++-
>>>>  drivers/net/virtio/virtio_user_ethdev.c       | 28 +++++++++++++-
>>>>  3 files changed, 50 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> index 2a0c861085..b79a9f84aa 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>>>> @@ -111,17 +111,6 @@ virtio_user_queue_setup(struct virtio_user_dev *dev,
>>>>  	return 0;
>>>>  }
>>>>
>>>> -int
>>>> -is_vhost_user_by_type(const char *path)
>>>> -{
>>>> -	struct stat sb;
>>>> -
>>>> -	if (stat(path, &sb) == -1)
>>>> -		return 0;
>>>> -
>>>> -	return S_ISSOCK(sb.st_mode);
>>>> -}
>>>> -
>>>>  int
>>>>  virtio_user_start_device(struct virtio_user_dev *dev)
>>>>  {
>>>> @@ -144,7 +133,8 @@ virtio_user_start_device(struct virtio_user_dev *dev)
>>>>  	rte_mcfg_mem_read_lock();
>>>>  	pthread_mutex_lock(&dev->mutex);
>>>>
>>>> -	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
>>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
>>>> +			dev->vhostfd < 0)
>>>>  		goto error;
>>>>
>>>>  	/* Step 0: tell vhost to create queues */
>>>> @@ -360,16 +350,16 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>>  	dev->tapfds = NULL;
>>>>
>>>>  	if (dev->is_server) {
>>>> -		if (access(dev->path, F_OK) == 0 &&
>>>> -		    !is_vhost_user_by_type(dev->path)) {
>>>> -			PMD_DRV_LOG(ERR, "Server mode doesn't support
>>>> vhost-kernel!");
>>>> +		if (dev->backend_type !=
>>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>> +			PMD_DRV_LOG(ERR, "Server mode only supports
>>>> vhost-user!");
>>>>  			return -1;
>>>>  		}
>>>>  		dev->ops = &virtio_ops_user;
>>>>  	} else {
>>>> -		if (is_vhost_user_by_type(dev->path)) {
>>>> +		if (dev->backend_type ==
>>>> VIRTIO_USER_BACKEND_VHOST_USER) {
>>>>  			dev->ops = &virtio_ops_user;
>>>> -		} else {
>>>> +		} else if (dev->backend_type ==
>>>> +
>>>> 	VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>>  			dev->ops = &virtio_ops_kernel;
>>>>
>>>>  			dev->vhostfds = malloc(dev->max_queue_pairs *
>>>> @@ -430,7 +420,8 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>>>>  int
>>>>  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>>  		     int cq, int queue_size, const char *mac, char **ifname,
>>>> -		     int server, int mrg_rxbuf, int in_order, int packed_vq)
>>>> +		     int server, int mrg_rxbuf, int in_order, int packed_vq,
>>>> +		     enum virtio_user_backend_type backend_type)
>>>>  {
>>>>  	uint64_t protocol_features = 0;
>>>>
>>>> @@ -445,6 +436,8 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  	dev->frontend_features = 0;
>>>>  	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
>>>>  	dev->protocol_features =
>>>> VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
>>>> +	dev->backend_type = backend_type;
>>>> +
>>>>  	parse_mac(dev, mac);
>>>>
>>>>  	if (*ifname) {
>>>> @@ -457,7 +450,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  		return -1;
>>>>  	}
>>>>
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		dev->unsupported_features |=
>>>>  			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
>>>>
>>>> @@ -539,7 +532,7 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
>>>> *path, int queues,
>>>>  	}
>>>>
>>>>  	/* The backend will not report this feature, we add it explicitly */
>>>> -	if (is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
>>>>
>>>>  	/*
>>>> @@ -792,7 +785,7 @@ virtio_user_send_status_update(struct virtio_user_dev
>>>> *dev, uint8_t status)
>>>>  	uint64_t arg = status;
>>>>
>>>>  	/* Vhost-user only for now */
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		return 0;
>>>>
>>>>  	if (!(dev->protocol_features & (1ULL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> @@ -815,7 +808,7 @@ virtio_user_update_status(struct virtio_user_dev *dev)
>>>>  	int err;
>>>>
>>>>  	/* Vhost-user only for now */
>>>> -	if (!is_vhost_user_by_type(dev->path))
>>>> +	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
>>>>  		return 0;
>>>>
>>>>  	if (!(dev->protocol_features & (1UL <<
>>>> VHOST_USER_PROTOCOL_F_STATUS)))
>>>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> index 9377d5ba66..575bf430c0 100644
>>>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>>>> @@ -10,6 +10,12 @@
>>>>  #include "../virtio_pci.h"
>>>>  #include "../virtio_ring.h"
>>>>
>>>> +enum virtio_user_backend_type {
>>>> +	VIRTIO_USER_BACKEND_UNKNOWN,
>>>> +	VIRTIO_USER_BACKEND_VHOST_USER,
>>>> +	VIRTIO_USER_BACKEND_VHOST_KERNEL,
>>>> +};
>>>> +
>>>>  struct virtio_user_queue {
>>>>  	uint16_t used_idx;
>>>>  	bool avail_wrap_counter;
>>>> @@ -17,6 +23,7 @@ struct virtio_user_queue {
>>>>  };
>>>>
>>>>  struct virtio_user_dev {
>>>> +	enum virtio_user_backend_type backend_type;
>>>>  	/* for vhost_user backend */
>>>>  	int		vhostfd;
>>>>  	int		listenfd;   /* listening fd */
>>>> @@ -60,13 +67,13 @@ struct virtio_user_dev {
>>>>  	bool		started;
>>>>  };
>>>>
>>>> -int is_vhost_user_by_type(const char *path);
>>>>  int virtio_user_start_device(struct virtio_user_dev *dev);
>>>>  int virtio_user_stop_device(struct virtio_user_dev *dev);
>>>>  int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
>>>>  			 int cq, int queue_size, const char *mac, char **ifname,
>>>>  			 int server, int mrg_rxbuf, int in_order,
>>>> -			 int packed_vq);
>>>> +			 int packed_vq,
>>>> +			 enum virtio_user_backend_type backend_type);
>>>>  void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>>>>  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,
>>>> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
>>>> b/drivers/net/virtio/virtio_user_ethdev.c
>>>> index 60d17af888..38b49bad5f 100644
>>>> --- a/drivers/net/virtio/virtio_user_ethdev.c
>>>> +++ b/drivers/net/virtio/virtio_user_ethdev.c
>>>> @@ -6,6 +6,7 @@
>>>>  #include <sys/types.h>
>>>>  #include <unistd.h>
>>>>  #include <fcntl.h>
>>>> +#include <sys/stat.h>
>>>>  #include <sys/socket.h>
>>>>
>>>>  #include <rte_malloc.h>
>>>> @@ -518,6 +519,19 @@ get_integer_arg(const char *key __rte_unused,
>>>>  	return -errno;
>>>>  }
>>>>
>>>> +static enum virtio_user_backend_type
>>>> +virtio_user_backend_type(const char *path)
>>>> +{
>>>> +	struct stat sb;
>>>> +
>>>> +	if (stat(path, &sb) == -1)
>>>> +		return VIRTIO_USER_BACKEND_UNKNOWN;
>>>> +
>>>> +	return S_ISSOCK(sb.st_mode) ?
>>>> +		VIRTIO_USER_BACKEND_VHOST_USER :
>>>> +		VIRTIO_USER_BACKEND_VHOST_KERNEL;
>>>> +}
>>>> +
>>>>  static struct rte_eth_dev *
>>>>  virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
>>>>  {
>>>> @@ -579,6 +593,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  	struct rte_kvargs *kvlist = NULL;
>>>>  	struct rte_eth_dev *eth_dev;
>>>>  	struct virtio_hw *hw;
>>>> +	enum virtio_user_backend_type backend_type =
>>>> VIRTIO_USER_BACKEND_UNKNOWN;
>>>>  	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
>>>>  	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>>>>  	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
>>>> @@ -631,8 +646,17 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  		goto end;
>>>>  	}
>>>>
>>>> +	backend_type = virtio_user_backend_type(path);
>>>> +	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
>>>> +		PMD_INIT_LOG(ERR,
>>>> +			     "unable to determine backend type for path %s",
>>>> +			path);
>>>> +		goto end;
>>>> +	}
>>>> +
>>>> +
>>>>  	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1)
>>>> {
>>>> -		if (is_vhost_user_by_type(path)) {
>>>> +		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
>>>>  			PMD_INIT_LOG(ERR,
>>>>  				"arg %s applies only to vhost-kernel backend",
>>>>  				VIRTIO_USER_ARG_INTERFACE_NAME);
>>>> @@ -751,7 +775,7 @@ virtio_user_pmd_probe(struct rte_vdev_device *dev)
>>>>  	hw = eth_dev->data->dev_private;
>>>>  	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
>>>>  			 queue_size, mac_addr, &ifname, server_mode,
>>>> -			 mrg_rxbuf, in_order, packed_vq) < 0) {
>>>> +			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
>>>>  		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
>>>>  		virtio_user_eth_dev_free(eth_dev);
>>>>  		goto end;
>>>> --
>>>> 2.26.2
>>
>
  

Patch

diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 2a0c861085..b79a9f84aa 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -111,17 +111,6 @@  virtio_user_queue_setup(struct virtio_user_dev *dev,
 	return 0;
 }
 
-int
-is_vhost_user_by_type(const char *path)
-{
-	struct stat sb;
-
-	if (stat(path, &sb) == -1)
-		return 0;
-
-	return S_ISSOCK(sb.st_mode);
-}
-
 int
 virtio_user_start_device(struct virtio_user_dev *dev)
 {
@@ -144,7 +133,8 @@  virtio_user_start_device(struct virtio_user_dev *dev)
 	rte_mcfg_mem_read_lock();
 	pthread_mutex_lock(&dev->mutex);
 
-	if (is_vhost_user_by_type(dev->path) && dev->vhostfd < 0)
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER &&
+			dev->vhostfd < 0)
 		goto error;
 
 	/* Step 0: tell vhost to create queues */
@@ -360,16 +350,16 @@  virtio_user_dev_setup(struct virtio_user_dev *dev)
 	dev->tapfds = NULL;
 
 	if (dev->is_server) {
-		if (access(dev->path, F_OK) == 0 &&
-		    !is_vhost_user_by_type(dev->path)) {
-			PMD_DRV_LOG(ERR, "Server mode doesn't support vhost-kernel!");
+		if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER) {
+			PMD_DRV_LOG(ERR, "Server mode only supports vhost-user!");
 			return -1;
 		}
 		dev->ops = &virtio_ops_user;
 	} else {
-		if (is_vhost_user_by_type(dev->path)) {
+		if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER) {
 			dev->ops = &virtio_ops_user;
-		} else {
+		} else if (dev->backend_type ==
+					VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			dev->ops = &virtio_ops_kernel;
 
 			dev->vhostfds = malloc(dev->max_queue_pairs *
@@ -430,7 +420,8 @@  virtio_user_dev_setup(struct virtio_user_dev *dev)
 int
 virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		     int cq, int queue_size, const char *mac, char **ifname,
-		     int server, int mrg_rxbuf, int in_order, int packed_vq)
+		     int server, int mrg_rxbuf, int in_order, int packed_vq,
+		     enum virtio_user_backend_type backend_type)
 {
 	uint64_t protocol_features = 0;
 
@@ -445,6 +436,8 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	dev->frontend_features = 0;
 	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
 	dev->protocol_features = VIRTIO_USER_SUPPORTED_PROTOCOL_FEATURES;
+	dev->backend_type = backend_type;
+
 	parse_mac(dev, mac);
 
 	if (*ifname) {
@@ -457,7 +450,7 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 		return -1;
 	}
 
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->unsupported_features |=
 			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
 
@@ -539,7 +532,7 @@  virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 	}
 
 	/* The backend will not report this feature, we add it explicitly */
-	if (is_vhost_user_by_type(dev->path))
+	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
 		dev->frontend_features |= (1ull << VIRTIO_NET_F_STATUS);
 
 	/*
@@ -792,7 +785,7 @@  virtio_user_send_status_update(struct virtio_user_dev *dev, uint8_t status)
 	uint64_t arg = status;
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
@@ -815,7 +808,7 @@  virtio_user_update_status(struct virtio_user_dev *dev)
 	int err;
 
 	/* Vhost-user only for now */
-	if (!is_vhost_user_by_type(dev->path))
+	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
 		return 0;
 
 	if (!(dev->protocol_features & (1UL << VHOST_USER_PROTOCOL_F_STATUS)))
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 9377d5ba66..575bf430c0 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -10,6 +10,12 @@ 
 #include "../virtio_pci.h"
 #include "../virtio_ring.h"
 
+enum virtio_user_backend_type {
+	VIRTIO_USER_BACKEND_UNKNOWN,
+	VIRTIO_USER_BACKEND_VHOST_USER,
+	VIRTIO_USER_BACKEND_VHOST_KERNEL,
+};
+
 struct virtio_user_queue {
 	uint16_t used_idx;
 	bool avail_wrap_counter;
@@ -17,6 +23,7 @@  struct virtio_user_queue {
 };
 
 struct virtio_user_dev {
+	enum virtio_user_backend_type backend_type;
 	/* for vhost_user backend */
 	int		vhostfd;
 	int		listenfd;   /* listening fd */
@@ -60,13 +67,13 @@  struct virtio_user_dev {
 	bool		started;
 };
 
-int is_vhost_user_by_type(const char *path);
 int virtio_user_start_device(struct virtio_user_dev *dev);
 int virtio_user_stop_device(struct virtio_user_dev *dev);
 int virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues,
 			 int cq, int queue_size, const char *mac, char **ifname,
 			 int server, int mrg_rxbuf, int in_order,
-			 int packed_vq);
+			 int packed_vq,
+			 enum virtio_user_backend_type backend_type);
 void virtio_user_dev_uninit(struct virtio_user_dev *dev);
 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,
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 60d17af888..38b49bad5f 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -6,6 +6,7 @@ 
 #include <sys/types.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <sys/stat.h>
 #include <sys/socket.h>
 
 #include <rte_malloc.h>
@@ -518,6 +519,19 @@  get_integer_arg(const char *key __rte_unused,
 	return -errno;
 }
 
+static enum virtio_user_backend_type
+virtio_user_backend_type(const char *path)
+{
+	struct stat sb;
+
+	if (stat(path, &sb) == -1)
+		return VIRTIO_USER_BACKEND_UNKNOWN;
+
+	return S_ISSOCK(sb.st_mode) ?
+		VIRTIO_USER_BACKEND_VHOST_USER :
+		VIRTIO_USER_BACKEND_VHOST_KERNEL;
+}
+
 static struct rte_eth_dev *
 virtio_user_eth_dev_alloc(struct rte_vdev_device *vdev)
 {
@@ -579,6 +593,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	struct rte_kvargs *kvlist = NULL;
 	struct rte_eth_dev *eth_dev;
 	struct virtio_hw *hw;
+	enum virtio_user_backend_type backend_type = VIRTIO_USER_BACKEND_UNKNOWN;
 	uint64_t queues = VIRTIO_USER_DEF_Q_NUM;
 	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
 	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
@@ -631,8 +646,17 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 		goto end;
 	}
 
+	backend_type = virtio_user_backend_type(path);
+	if (backend_type == VIRTIO_USER_BACKEND_UNKNOWN) {
+		PMD_INIT_LOG(ERR,
+			     "unable to determine backend type for path %s",
+			path);
+		goto end;
+	}
+
+
 	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
-		if (is_vhost_user_by_type(path)) {
+		if (backend_type != VIRTIO_USER_BACKEND_VHOST_KERNEL) {
 			PMD_INIT_LOG(ERR,
 				"arg %s applies only to vhost-kernel backend",
 				VIRTIO_USER_ARG_INTERFACE_NAME);
@@ -751,7 +775,7 @@  virtio_user_pmd_probe(struct rte_vdev_device *dev)
 	hw = eth_dev->data->dev_private;
 	if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
 			 queue_size, mac_addr, &ifname, server_mode,
-			 mrg_rxbuf, in_order, packed_vq) < 0) {
+			 mrg_rxbuf, in_order, packed_vq, backend_type) < 0) {
 		PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
 		virtio_user_eth_dev_free(eth_dev);
 		goto end;