[dpdk-dev] [PATCH v2 38/44] net/virtio: move protocol features to Vhost-user

Xia, Chenbo chenbo.xia at intel.com
Fri Jan 22 09:20:39 CET 2021


> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin at redhat.com>
> Sent: Wednesday, January 20, 2021 5:25 AM
> To: dev at dpdk.org; Xia, Chenbo <chenbo.xia at intel.com>; olivier.matz at 6wind.com;
> amorenoz at redhat.com; david.marchand at redhat.com
> Cc: Maxime Coquelin <maxime.coquelin at redhat.com>
> Subject: [PATCH v2 38/44] net/virtio: move protocol features to Vhost-user
> 
> Since only protocol features are specific to Vhost-user
> backend, this patch moves all related code to Vhost-user
> file.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost.h        | 20 +----
>  drivers/net/virtio/virtio_user/vhost_kernel.c |  9 ++
>  drivers/net/virtio/virtio_user/vhost_user.c   | 83 +++++++++++++++----
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   | 34 +++++++-
>  .../net/virtio/virtio_user/virtio_user_dev.c  | 47 ++---------
>  drivers/net/virtio/virtio_user_ethdev.c       | 17 ----
>  6 files changed, 113 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h
> b/drivers/net/virtio/virtio_user/vhost.h
> index 2aa6b2cb70..dfa2735a20 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -46,23 +46,6 @@ struct vhost_vring_addr {
>  	uint64_t log_guest_addr;
>  };
> 
> -#ifndef VHOST_USER_F_PROTOCOL_FEATURES
> -#define VHOST_USER_F_PROTOCOL_FEATURES 30
> -#endif
> -
> -/** Protocol features. */
> -#ifndef VHOST_USER_PROTOCOL_F_MQ
> -#define VHOST_USER_PROTOCOL_F_MQ 0
> -#endif
> -
> -#ifndef VHOST_USER_PROTOCOL_F_REPLY_ACK
> -#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3
> -#endif
> -
> -#ifndef VHOST_USER_PROTOCOL_F_STATUS
> -#define VHOST_USER_PROTOCOL_F_STATUS 16
> -#endif
> -
>  #ifndef VHOST_BACKEND_F_IOTLB_MSG_V2
>  #define VHOST_BACKEND_F_IOTLB_MSG_V2 1
>  #endif
> @@ -82,11 +65,10 @@ struct virtio_user_dev;
> 
>  struct virtio_user_backend_ops {
>  	int (*setup)(struct virtio_user_dev *dev);
> +	int (*get_backend_features)(uint64_t *features);
>  	int (*set_owner)(struct virtio_user_dev *dev);
>  	int (*get_features)(struct virtio_user_dev *dev, uint64_t *features);
>  	int (*set_features)(struct virtio_user_dev *dev, uint64_t features);
> -	int (*get_protocol_features)(struct virtio_user_dev *dev, uint64_t
> *features);
> -	int (*set_protocol_features)(struct virtio_user_dev *dev, uint64_t
> features);
>  	int (*set_memory_table)(struct virtio_user_dev *dev);
>  	int (*set_vring_num)(struct virtio_user_dev *dev, struct
> vhost_vring_state *state);
>  	int (*set_vring_base)(struct virtio_user_dev *dev, struct
> vhost_vring_state *state);
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c
> b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 68f0c0b83d..5477eb7015 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -445,8 +445,17 @@ vhost_kernel_enable_queue_pair(struct virtio_user_dev
> *dev,
>  	return 0;
>  }
> 
> +static int
> +vhost_kernel_get_backend_features(uint64_t *features)
> +{
> +	*features = 0;
> +
> +	return 0;
> +}
> +
>  struct virtio_user_backend_ops virtio_ops_kernel = {
>  	.setup = vhost_kernel_setup,
> +	.get_backend_features = vhost_kernel_get_backend_features,
>  	.set_owner = vhost_kernel_set_owner,
>  	.get_features = vhost_kernel_get_features,
>  	.set_features = vhost_kernel_set_features,
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c
> b/drivers/net/virtio/virtio_user/vhost_user.c
> index a48fadd8c9..986d8ac1ac 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -17,6 +17,29 @@
>  #include "vhost.h"
>  #include "virtio_user_dev.h"
> 
> +
> +#ifndef VHOST_USER_F_PROTOCOL_FEATURES
> +#define VHOST_USER_F_PROTOCOL_FEATURES 30
> +#endif
> +
> +/** Protocol features. */
> +#ifndef VHOST_USER_PROTOCOL_F_MQ
> +#define VHOST_USER_PROTOCOL_F_MQ 0
> +#endif
> +
> +#ifndef VHOST_USER_PROTOCOL_F_REPLY_ACK
> +#define VHOST_USER_PROTOCOL_F_REPLY_ACK 3
> +#endif
> +
> +#ifndef VHOST_USER_PROTOCOL_F_STATUS
> +#define VHOST_USER_PROTOCOL_F_STATUS 16
> +#endif
> +
> +#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES		\
> +	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
> +	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
> +	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
> +
>  /* The version of the protocol we support */
>  #define VHOST_USER_VERSION    0x1
> 
> @@ -205,11 +228,11 @@ vhost_user_set_owner(struct virtio_user_dev *dev)
>  }
> 
>  static int
> -vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
> +vhost_user_get_protocol_features(struct virtio_user_dev *dev, uint64_t
> *features)
>  {
>  	int ret;
>  	struct vhost_user_msg msg = {
> -		.request = VHOST_USER_GET_FEATURES,
> +		.request = VHOST_USER_GET_PROTOCOL_FEATURES,
>  		.flags = VHOST_USER_VERSION,
>  	};
> 
> @@ -221,7 +244,7 @@ vhost_user_get_features(struct virtio_user_dev *dev,
> uint64_t *features)
>  	if (ret < 0)
>  		goto err;
> 
> -	if (msg.request != VHOST_USER_GET_FEATURES) {
> +	if (msg.request != VHOST_USER_GET_PROTOCOL_FEATURES) {
>  		PMD_DRV_LOG(ERR, "Unexpected request type (%d)", msg.request);
>  		goto err;
>  	}
> @@ -235,27 +258,25 @@ vhost_user_get_features(struct virtio_user_dev *dev,
> uint64_t *features)
> 
>  	return 0;
>  err:
> -	PMD_DRV_LOG(ERR, "Failed to get backend features");
> +	PMD_DRV_LOG(ERR, "Failed to get backend protocol features");
> 
>  	return -1;
>  }
> 
>  static int
> -vhost_user_set_features(struct virtio_user_dev *dev, uint64_t features)
> +vhost_user_set_protocol_features(struct virtio_user_dev *dev, uint64_t
> features)
>  {
>  	int ret;
>  	struct vhost_user_msg msg = {
> -		.request = VHOST_USER_SET_FEATURES,
> +		.request = VHOST_USER_SET_PROTOCOL_FEATURES,
>  		.flags = VHOST_USER_VERSION,
>  		.size = sizeof(features),
>  		.payload.u64 = features,
>  	};
> 
> -	msg.payload.u64 |= dev->device_features & (1ULL <<
> VHOST_USER_F_PROTOCOL_FEATURES);
> -
>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
>  	if (ret < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to set features");
> +		PMD_DRV_LOG(ERR, "Failed to set protocol features");
>  		return -1;
>  	}
> 
> @@ -263,11 +284,11 @@ vhost_user_set_features(struct virtio_user_dev *dev,
> uint64_t features)
>  }
> 
>  static int
> -vhost_user_get_protocol_features(struct virtio_user_dev *dev, uint64_t
> *features)
> +vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
>  {
>  	int ret;
>  	struct vhost_user_msg msg = {
> -		.request = VHOST_USER_GET_PROTOCOL_FEATURES,
> +		.request = VHOST_USER_GET_FEATURES,
>  		.flags = VHOST_USER_VERSION,
>  	};
> 
> @@ -279,7 +300,7 @@ vhost_user_get_protocol_features(struct virtio_user_dev
> *dev, uint64_t *features
>  	if (ret < 0)
>  		goto err;
> 
> -	if (msg.request != VHOST_USER_GET_PROTOCOL_FEATURES) {
> +	if (msg.request != VHOST_USER_GET_FEATURES) {
>  		PMD_DRV_LOG(ERR, "Unexpected request type (%d)", msg.request);
>  		goto err;
>  	}
> @@ -291,27 +312,46 @@ vhost_user_get_protocol_features(struct virtio_user_dev
> *dev, uint64_t *features
> 
>  	*features = msg.payload.u64;
> 
> +	if (!(*features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
> +		return 0;
> +
> +	/* Negotiate protocol features */
> +	ret = vhost_user_get_protocol_features(dev, &dev->protocol_features);
> +	if (ret < 0)
> +		goto err;
> +
> +	dev->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
> +
> +	ret = vhost_user_set_protocol_features(dev, dev->protocol_features);
> +	if (ret < 0)
> +		goto err;
> +
> +	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
> +		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
> +
>  	return 0;
>  err:
> -	PMD_DRV_LOG(ERR, "Failed to get backend protocol features");
> +	PMD_DRV_LOG(ERR, "Failed to get backend features");
> 
>  	return -1;
>  }
> 
>  static int
> -vhost_user_set_protocol_features(struct virtio_user_dev *dev, uint64_t
> features)
> +vhost_user_set_features(struct virtio_user_dev *dev, uint64_t features)
>  {
>  	int ret;
>  	struct vhost_user_msg msg = {
> -		.request = VHOST_USER_SET_PROTOCOL_FEATURES,
> +		.request = VHOST_USER_SET_FEATURES,
>  		.flags = VHOST_USER_VERSION,
>  		.size = sizeof(features),
>  		.payload.u64 = features,
>  	};
> 
> +	msg.payload.u64 |= dev->device_features & (1ULL <<
> VHOST_USER_F_PROTOCOL_FEATURES);
> +
>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
>  	if (ret < 0) {
> -		PMD_DRV_LOG(ERR, "Failed to set protocol features");
> +		PMD_DRV_LOG(ERR, "Failed to set features");
>  		return -1;
>  	}
> 
> @@ -780,13 +820,20 @@ vhost_user_enable_queue_pair(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> +static int
> +vhost_user_get_backend_features(uint64_t *features)
> +{
> +	*features = 1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
> +
> +	return 0;
> +}
> +
>  struct virtio_user_backend_ops virtio_ops_user = {
>  	.setup = vhost_user_setup,
> +	.get_backend_features = vhost_user_get_backend_features,
>  	.set_owner = vhost_user_set_owner,
>  	.get_features = vhost_user_get_features,
>  	.set_features = vhost_user_set_features,
> -	.get_protocol_features = vhost_user_get_protocol_features,
> -	.set_protocol_features = vhost_user_set_protocol_features,
>  	.set_memory_table = vhost_user_set_memory_table,
>  	.set_vring_num = vhost_user_set_vring_num,
>  	.set_vring_base = vhost_user_set_vring_base,
> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> index 7fb1249569..8d354b298e 100644
> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -13,6 +13,10 @@
>  #include "vhost.h"
>  #include "virtio_user_dev.h"
> 
> +#define VHOST_VDPA_SUPPORTED_BACKEND_FEATURES		\
> +	(1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2	|	\
> +	1ULL << VHOST_BACKEND_F_IOTLB_BATCH)
> +
>  /* vhost kernel & vdpa ioctls */
>  #define VHOST_VIRTIO 0xAF
>  #define VHOST_GET_FEATURES _IOR(VHOST_VIRTIO, 0x00, __u64)
> @@ -88,13 +92,13 @@ vhost_vdpa_set_owner(struct virtio_user_dev *dev)
>  }
> 
>  static int
> -vhost_vdpa_get_backend_features(struct virtio_user_dev *dev, uint64_t
> *features)
> +vhost_vdpa_get_protocol_features(struct virtio_user_dev *dev, uint64_t
> *features)
>  {
>  	return vhost_vdpa_ioctl(dev->vhostfd, VHOST_GET_BACKEND_FEATURES,
> features);
>  }
> 
>  static int
> -vhost_vdpa_set_backend_features(struct virtio_user_dev *dev, uint64_t
> features)
> +vhost_vdpa_set_protocol_features(struct virtio_user_dev *dev, uint64_t
> features)
>  {
>  	return vhost_vdpa_ioctl(dev->vhostfd, VHOST_SET_BACKEND_FEATURES,
> &features);
>  }
> @@ -113,6 +117,21 @@ vhost_vdpa_get_features(struct virtio_user_dev *dev,
> uint64_t *features)
>  	/* Multiqueue not supported for now */
>  	*features &= ~(1ULL << VIRTIO_NET_F_MQ);
> 
> +	/* Negotiated vDPA backend features */
> +	ret = vhost_vdpa_get_protocol_features(dev, &dev->protocol_features);
> +	if (ret < 0) {
> +		PMD_DRV_LOG(ERR, "Failed to get backend features");
> +		return -1;
> +	}
> +
> +	dev->protocol_features &= VHOST_VDPA_SUPPORTED_BACKEND_FEATURES;
> +
> +	ret = vhost_vdpa_set_protocol_features(dev, dev->protocol_features);
> +	if (ret < 0) {
> +		PMD_DRV_LOG(ERR, "Failed to set backend features");
> +		return -1;
> +	}
> +
>  	return 0;
>  }
> 
> @@ -438,13 +457,20 @@ vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
>  	return 0;
>  }
> 
> +static int
> +vhost_vdpa_get_backend_features(uint64_t *features)
> +{
> +	*features = 0;
> +
> +	return 0;
> +}
> +
>  struct virtio_user_backend_ops virtio_ops_vdpa = {
>  	.setup = vhost_vdpa_setup,
> +	.get_backend_features = vhost_vdpa_get_backend_features,
>  	.set_owner = vhost_vdpa_set_owner,
>  	.get_features = vhost_vdpa_get_features,
>  	.set_features = vhost_vdpa_set_features,
> -	.get_protocol_features = vhost_vdpa_get_backend_features,
> -	.set_protocol_features = vhost_vdpa_set_backend_features,
>  	.set_memory_table = vhost_vdpa_set_memory_table,
>  	.set_vring_num = vhost_vdpa_set_vring_num,
>  	.set_vring_base = vhost_vdpa_set_vring_base,
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index c2a41fe3a0..2614bcb349 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -480,24 +480,15 @@ virtio_user_dev_setup(struct virtio_user_dev *dev)
>  	 1ULL << VIRTIO_NET_F_GUEST_TSO6	|	\
>  	 1ULL << VIRTIO_F_IN_ORDER		|	\
>  	 1ULL << VIRTIO_F_VERSION_1		|	\
> -	 1ULL << VIRTIO_F_RING_PACKED		|	\
> -	 1ULL << VHOST_USER_F_PROTOCOL_FEATURES)
> +	 1ULL << VIRTIO_F_RING_PACKED)
> 
> -#define VHOST_USER_SUPPORTED_PROTOCOL_FEATURES		\
> -	(1ULL << VHOST_USER_PROTOCOL_F_MQ |		\
> -	 1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK |	\
> -	 1ULL << VHOST_USER_PROTOCOL_F_STATUS)
> -
> -#define VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES		\
> -	(1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2	|	\
> -	1ULL << VHOST_BACKEND_F_IOTLB_BATCH)
>  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,
>  		     enum virtio_user_backend_type backend_type)
>  {
> -	uint64_t protocol_features = 0;
> +	uint64_t backend_features;
> 
>  	pthread_mutex_init(&dev->mutex, NULL);
>  	strlcpy(dev->path, path, PATH_MAX);
> @@ -508,14 +499,9 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  	dev->is_server = server;
>  	dev->mac_specified = 0;
>  	dev->frontend_features = 0;
> -	dev->unsupported_features = ~VIRTIO_USER_SUPPORTED_FEATURES;
> +	dev->unsupported_features = 0;
>  	dev->backend_type = backend_type;
> 
> -	if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_USER)
> -		dev->protocol_features = VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
> -	else if (dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA)
> -		dev->protocol_features = VHOST_VDPA_SUPPORTED_PROTOCOL_FEATURES;
> -
>  	parse_mac(dev, mac);
> 
>  	if (*ifname) {
> @@ -528,38 +514,21 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char
> *path, int queues,
>  		return -1;
>  	}
> 
> -	if (dev->backend_type != VIRTIO_USER_BACKEND_VHOST_USER)
> -		dev->unsupported_features |=
> -			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES);
> -
>  	if (dev->ops->set_owner(dev) < 0) {
>  		PMD_INIT_LOG(ERR, "(%s) Failed to set backend owner", dev->path);
>  		return -1;
>  	}
> 
> -	if (dev->ops->get_features(dev, &dev->device_features) < 0) {
> +	if (dev->ops->get_backend_features(&backend_features) < 0) {
>  		PMD_INIT_LOG(ERR, "(%s) Failed to get backend features", dev-
> >path);
>  		return -1;
>  	}
> 
> -	if ((dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) ||
> -			dev->backend_type == VIRTIO_USER_BACKEND_VHOST_VDPA) {
> -		if (dev->ops->get_protocol_features(dev, &protocol_features)) {
> -			PMD_INIT_LOG(ERR, "(%s) Failed to get backend protocol
> features",
> -					dev->path);
> -			return -1;
> -		}
> -
> -		dev->protocol_features &= protocol_features;
> -
> -		if (dev->ops->set_protocol_features(dev, dev->protocol_features))
> {
> -			PMD_INIT_LOG(ERR, "(%s) Failed to set backend protocol
> features",
> -					dev->path);
> -			return -1;
> -		}
> +	dev->unsupported_features = ~(VIRTIO_USER_SUPPORTED_FEATURES |
> backend_features);
> 
> -		if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
> -			dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
> +	if (dev->ops->get_features(dev, &dev->device_features) < 0) {
> +		PMD_INIT_LOG(ERR, "(%s) Failed to get device features", dev->path);
> +		return -1;
>  	}
> 
>  	if (!mrg_rxbuf)
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c
> b/drivers/net/virtio/virtio_user_ethdev.c
> index e822d93690..e2c606a838 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -70,7 +70,6 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev)
>  	int ret, connectfd, old_status;
>  	struct rte_eth_dev *eth_dev = &rte_eth_devices[dev->port_id];
>  	struct virtio_hw *hw = &dev->hw;
> -	uint64_t protocol_features;
> 
>  	connectfd = accept(dev->listenfd, NULL, NULL);
>  	if (connectfd < 0)
> @@ -91,22 +90,6 @@ virtio_user_server_reconnect(struct virtio_user_dev *dev)
>  		return -1;
>  	}
> 
> -	if (dev->device_features &
> -			(1ULL << VHOST_USER_F_PROTOCOL_FEATURES)) {
> -		if (dev->ops->get_protocol_features(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->set_protocol_features(dev, dev->protocol_features))
> -			return -1;
> -
> -		if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
> -			dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
> -	}
> -
>  	dev->device_features |= dev->frontend_features;
> 
>  	/* umask vhost-user unsupported features */
> --
> 2.29.2

Reviewed-by: Chenbo Xia <chenbo.xia at intel.com>


More information about the dev mailing list