diff mbox series

[37/40] net/virtio: introduce backend data

Message ID 20201220211405.313012-38-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio: Virtio PMD rework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Dec. 20, 2020, 9:14 p.m. UTC 
  The goal of this patch is to introduce backend-specific
data in order to better isolate what is backend-specific
from what is generic to Virtio-user.

For now, only Vhost-user protocol features are moved to
Vhost-user backend data.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_user/vhost.h        |  1 +
 drivers/net/virtio/virtio_user/vhost_kernel.c |  7 +++
 drivers/net/virtio/virtio_user/vhost_user.c   | 46 +++++++++++++++----
 drivers/net/virtio/virtio_user/vhost_vdpa.c   |  8 ++++
 .../net/virtio/virtio_user/virtio_user_dev.c  |  2 +
 .../net/virtio/virtio_user/virtio_user_dev.h  |  3 +-
 6 files changed, 58 insertions(+), 9 deletions(-)

Comments

David Marchand Jan. 5, 2021, 9:26 p.m. UTC | #1 
On Sun, Dec 20, 2020 at 10:16 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 2fd00afa84..023fddcd69 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -357,6 +357,12 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
>         return 0;
>  }
>
> +static int
> +vhost_kernel_destroy(struct virtio_user_dev *dev)
> +{

Missing a __rte_unused.


> +       return 0;
> +}
> +
>  static int
>  vhost_kernel_set_backend(int vhostfd, int tapfd)
>  {
> @@ -455,6 +461,7 @@ vhost_kernel_get_backend_features(uint64_t *features)
>
>  struct virtio_user_backend_ops virtio_ops_kernel = {
>         .setup = vhost_kernel_setup,
> +       .destroy = vhost_kernel_destroy,
>         .get_backend_features = vhost_kernel_get_backend_features,
>         .set_owner = vhost_kernel_set_owner,
>         .get_features = vhost_kernel_get_features,


> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> index c826f333e0..b29426d767 100644
> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -287,6 +287,13 @@ vhost_vdpa_setup(struct virtio_user_dev *dev)
>         return 0;
>  }
>
> +static int
> +vhost_vdpa_destroy(struct virtio_user_dev *dev __rte_unused)
> +{
> +       return;
> +       return 0;

Rebase damage ? :)

> +}
> +
Adrian Moreno Jan. 13, 2021, 5:18 p.m. UTC | #2 
On 12/20/20 10:14 PM, Maxime Coquelin wrote:
> The goal of this patch is to introduce backend-specific
> data in order to better isolate what is backend-specific
> from what is generic to Virtio-user.
> 
> For now, only Vhost-user protocol features are moved to
> Vhost-user backend data.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_user/vhost.h        |  1 +
>  drivers/net/virtio/virtio_user/vhost_kernel.c |  7 +++
>  drivers/net/virtio/virtio_user/vhost_user.c   | 46 +++++++++++++++----
>  drivers/net/virtio/virtio_user/vhost_vdpa.c   |  8 ++++
>  .../net/virtio/virtio_user/virtio_user_dev.c  |  2 +
>  .../net/virtio/virtio_user/virtio_user_dev.h  |  3 +-
>  6 files changed, 58 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
> index b488ac78a1..ee5598226d 100644
> --- a/drivers/net/virtio/virtio_user/vhost.h
> +++ b/drivers/net/virtio/virtio_user/vhost.h
> @@ -57,6 +57,7 @@ struct virtio_user_dev;
>  
>  struct virtio_user_backend_ops {
>  	int (*setup)(struct virtio_user_dev *dev);
> +	int (*destroy)(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);
> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
> index 2fd00afa84..023fddcd69 100644
> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
> @@ -357,6 +357,12 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +vhost_kernel_destroy(struct virtio_user_dev *dev)
> +{
> +	return 0;
> +}
> +
>  static int
>  vhost_kernel_set_backend(int vhostfd, int tapfd)
>  {
> @@ -455,6 +461,7 @@ vhost_kernel_get_backend_features(uint64_t *features)
>  
>  struct virtio_user_backend_ops virtio_ops_kernel = {
>  	.setup = vhost_kernel_setup,
> +	.destroy = vhost_kernel_destroy,
>  	.get_backend_features = vhost_kernel_get_backend_features,
>  	.set_owner = vhost_kernel_set_owner,
>  	.get_features = vhost_kernel_get_features,
> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
> index f67c40e047..e96b1d8b9c 100644
> --- a/drivers/net/virtio/virtio_user/vhost_user.c
> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
> @@ -17,6 +17,9 @@
>  #include "vhost.h"
>  #include "virtio_user_dev.h"
>  
> +struct vhost_user_data {
> +	uint64_t protocol_features;
> +};
>  
>  #ifndef VHOST_USER_F_PROTOCOL_FEATURES
>  #define VHOST_USER_F_PROTOCOL_FEATURES 30
> @@ -274,6 +277,7 @@ static int
>  vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
>  {
>  	int ret;
> +	struct vhost_user_data *data = dev->backend_data;
>  	struct vhost_user_msg msg = {
>  		.request = VHOST_USER_GET_FEATURES,
>  		.flags = VHOST_USER_VERSION,
> @@ -303,17 +307,17 @@ vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
>  		return 0;
>  
>  	/* Negotiate protocol features */
> -	ret = vhost_user_get_protocol_features(dev, &dev->protocol_features);
> +	ret = vhost_user_get_protocol_features(dev, &data->protocol_features);
>  	if (ret < 0)
>  		goto err;
>  
> -	dev->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
> +	data->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
>  
> -	ret = vhost_user_set_protocol_features(dev, dev->protocol_features);
> +	ret = vhost_user_set_protocol_features(dev, data->protocol_features);
>  	if (ret < 0)
>  		goto err;
>  
> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
>  		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
>  
>  	return 0;
> @@ -429,12 +433,13 @@ vhost_user_set_memory_table(struct virtio_user_dev *dev)
>  	struct walk_arg wa;
>  	int fds[VHOST_MEMORY_MAX_NREGIONS];
>  	int ret, fd_num;
> +	struct vhost_user_data *data = dev->backend_data;
>  	struct vhost_user_msg msg = {
>  		.request = VHOST_USER_SET_MEM_TABLE,
>  		.flags = VHOST_USER_VERSION,
>  	};
>  
> -	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
> +	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>  		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>  
>  	wa.region_nr = 0;
> @@ -613,6 +618,7 @@ static int
>  vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
>  {
>  	int ret;
> +	struct vhost_user_data *data = dev->backend_data;
>  	struct vhost_user_msg msg = {
>  		.request = VHOST_USER_GET_STATUS,
>  		.flags = VHOST_USER_VERSION,
> @@ -629,7 +635,7 @@ vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
>  	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
>  		return -ENOTSUP;
>  
> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>  		return -ENOTSUP;
>  
>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
> @@ -666,6 +672,7 @@ static int
>  vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>  {
>  	int ret;
> +	struct vhost_user_data *data = dev->backend_data;
>  	struct vhost_user_msg msg = {
>  		.request = VHOST_USER_SET_STATUS,
>  		.flags = VHOST_USER_VERSION,
> @@ -673,7 +680,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>  		.payload.u64 = status,
>  	};
>  
> -	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
> +	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>  		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>  
>  	/*
> @@ -687,7 +694,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>  	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
>  		return -ENOTSUP;
>  
> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>  		return -ENOTSUP;
>  
>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
> @@ -747,6 +754,17 @@ vhost_user_setup(struct virtio_user_dev *dev)
>  	int fd;
>  	int flag;
>  	struct sockaddr_un un;
> +	struct vhost_user_data *data;
> +
> +	data = malloc(sizeof(*data));
> +	if (!data) {
> +		PMD_DRV_LOG(ERR, "(%s) Failed to allocate Vhost-user data\n", dev->path);
> +		return -1;
> +	}
> +
> +	memset(data, 0, sizeof(*data));
> +
> +	dev->backend_data = data;
>  
>  	fd = socket(AF_UNIX, SOCK_STREAM, 0);
>  	if (fd < 0) {

There are a number of "return -1;" statements below which, if executed, would
now leak.
Also, there are two in virtio_user_dev.c:virtio_user_dev_setup()

> @@ -781,6 +799,17 @@ vhost_user_setup(struct virtio_user_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +vhost_user_destroy(struct virtio_user_dev *dev)
> +{
> +	if (dev->backend_data) {
> +		free(dev->backend_data);
> +		dev->backend_data = NULL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int
>  vhost_user_enable_queue_pair(struct virtio_user_dev *dev,
>  			     uint16_t pair_idx,
> @@ -815,6 +844,7 @@ vhost_user_get_backend_features(uint64_t *features)
>  
>  struct virtio_user_backend_ops virtio_ops_user = {
>  	.setup = vhost_user_setup,
> +	.destroy = vhost_user_destroy,
>  	.get_backend_features = vhost_user_get_backend_features,
>  	.set_owner = vhost_user_set_owner,
>  	.get_features = vhost_user_get_features,
> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> index c826f333e0..b29426d767 100644
> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
> @@ -287,6 +287,13 @@ vhost_vdpa_setup(struct virtio_user_dev *dev)
>  	return 0;
>  }
>  
> +static int
> +vhost_vdpa_destroy(struct virtio_user_dev *dev __rte_unused)
> +{
> +	return;
> +	return 0;
> +}
> +
>  static int
>  vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
>  			       uint16_t pair_idx,
> @@ -322,6 +329,7 @@ vhost_vdpa_get_backend_features(uint64_t *features)
>  
>  struct virtio_user_backend_ops virtio_ops_vdpa = {
>  	.setup = vhost_vdpa_setup,
> +	.destroy = vhost_vdpa_destroy,
>  	.get_backend_features = vhost_vdpa_get_backend_features,
>  	.set_owner = vhost_vdpa_set_owner,
>  	.get_features = vhost_vdpa_get_features,
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 974de133bc..8d19a0addd 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -620,6 +620,8 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
>  
>  	if (dev->is_server)
>  		unlink(dev->path);
> +
> +	dev->ops->destroy(dev);
>  }
>  
>  uint8_t
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index b2da2944e9..ec73d5de11 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -30,7 +30,6 @@ struct virtio_user_dev {
>  	int		vhostfd;
>  	int		listenfd;   /* listening fd */
>  	bool		is_server;  /* server or client mode */
> -	uint64_t	protocol_features; /* negotiated protocol features*/
>  
>  	/* for vhost_kernel backend */
>  	char		*ifname;
> @@ -66,6 +65,8 @@ struct virtio_user_dev {
>  	struct virtio_user_backend_ops *ops;
>  	pthread_mutex_t	mutex;
>  	bool		started;
> +
> +	void *backend_data;
>  };
>  
>  int virtio_user_dev_set_features(struct virtio_user_dev *dev);
>
Maxime Coquelin Jan. 15, 2021, 4:49 p.m. UTC | #3 
On 1/13/21 6:18 PM, Adrian Moreno wrote:
> 
> 
> On 12/20/20 10:14 PM, Maxime Coquelin wrote:
>> The goal of this patch is to introduce backend-specific
>> data in order to better isolate what is backend-specific
>> from what is generic to Virtio-user.
>>
>> For now, only Vhost-user protocol features are moved to
>> Vhost-user backend data.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/net/virtio/virtio_user/vhost.h        |  1 +
>>  drivers/net/virtio/virtio_user/vhost_kernel.c |  7 +++
>>  drivers/net/virtio/virtio_user/vhost_user.c   | 46 +++++++++++++++----
>>  drivers/net/virtio/virtio_user/vhost_vdpa.c   |  8 ++++
>>  .../net/virtio/virtio_user/virtio_user_dev.c  |  2 +
>>  .../net/virtio/virtio_user/virtio_user_dev.h  |  3 +-
>>  6 files changed, 58 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
>> index b488ac78a1..ee5598226d 100644
>> --- a/drivers/net/virtio/virtio_user/vhost.h
>> +++ b/drivers/net/virtio/virtio_user/vhost.h
>> @@ -57,6 +57,7 @@ struct virtio_user_dev;
>>  
>>  struct virtio_user_backend_ops {
>>  	int (*setup)(struct virtio_user_dev *dev);
>> +	int (*destroy)(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);
>> diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
>> index 2fd00afa84..023fddcd69 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_kernel.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
>> @@ -357,6 +357,12 @@ vhost_kernel_setup(struct virtio_user_dev *dev)
>>  	return 0;
>>  }
>>  
>> +static int
>> +vhost_kernel_destroy(struct virtio_user_dev *dev)
>> +{
>> +	return 0;
>> +}
>> +
>>  static int
>>  vhost_kernel_set_backend(int vhostfd, int tapfd)
>>  {
>> @@ -455,6 +461,7 @@ vhost_kernel_get_backend_features(uint64_t *features)
>>  
>>  struct virtio_user_backend_ops virtio_ops_kernel = {
>>  	.setup = vhost_kernel_setup,
>> +	.destroy = vhost_kernel_destroy,
>>  	.get_backend_features = vhost_kernel_get_backend_features,
>>  	.set_owner = vhost_kernel_set_owner,
>>  	.get_features = vhost_kernel_get_features,
>> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
>> index f67c40e047..e96b1d8b9c 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_user.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_user.c
>> @@ -17,6 +17,9 @@
>>  #include "vhost.h"
>>  #include "virtio_user_dev.h"
>>  
>> +struct vhost_user_data {
>> +	uint64_t protocol_features;
>> +};
>>  
>>  #ifndef VHOST_USER_F_PROTOCOL_FEATURES
>>  #define VHOST_USER_F_PROTOCOL_FEATURES 30
>> @@ -274,6 +277,7 @@ static int
>>  vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
>>  {
>>  	int ret;
>> +	struct vhost_user_data *data = dev->backend_data;
>>  	struct vhost_user_msg msg = {
>>  		.request = VHOST_USER_GET_FEATURES,
>>  		.flags = VHOST_USER_VERSION,
>> @@ -303,17 +307,17 @@ vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
>>  		return 0;
>>  
>>  	/* Negotiate protocol features */
>> -	ret = vhost_user_get_protocol_features(dev, &dev->protocol_features);
>> +	ret = vhost_user_get_protocol_features(dev, &data->protocol_features);
>>  	if (ret < 0)
>>  		goto err;
>>  
>> -	dev->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
>> +	data->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
>>  
>> -	ret = vhost_user_set_protocol_features(dev, dev->protocol_features);
>> +	ret = vhost_user_set_protocol_features(dev, data->protocol_features);
>>  	if (ret < 0)
>>  		goto err;
>>  
>> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
>> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
>>  		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
>>  
>>  	return 0;
>> @@ -429,12 +433,13 @@ vhost_user_set_memory_table(struct virtio_user_dev *dev)
>>  	struct walk_arg wa;
>>  	int fds[VHOST_MEMORY_MAX_NREGIONS];
>>  	int ret, fd_num;
>> +	struct vhost_user_data *data = dev->backend_data;
>>  	struct vhost_user_msg msg = {
>>  		.request = VHOST_USER_SET_MEM_TABLE,
>>  		.flags = VHOST_USER_VERSION,
>>  	};
>>  
>> -	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>> +	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>>  		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>>  
>>  	wa.region_nr = 0;
>> @@ -613,6 +618,7 @@ static int
>>  vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
>>  {
>>  	int ret;
>> +	struct vhost_user_data *data = dev->backend_data;
>>  	struct vhost_user_msg msg = {
>>  		.request = VHOST_USER_GET_STATUS,
>>  		.flags = VHOST_USER_VERSION,
>> @@ -629,7 +635,7 @@ vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
>>  	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
>>  		return -ENOTSUP;
>>  
>> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>>  		return -ENOTSUP;
>>  
>>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
>> @@ -666,6 +672,7 @@ static int
>>  vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>>  {
>>  	int ret;
>> +	struct vhost_user_data *data = dev->backend_data;
>>  	struct vhost_user_msg msg = {
>>  		.request = VHOST_USER_SET_STATUS,
>>  		.flags = VHOST_USER_VERSION,
>> @@ -673,7 +680,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>>  		.payload.u64 = status,
>>  	};
>>  
>> -	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>> +	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
>>  		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
>>  
>>  	/*
>> @@ -687,7 +694,7 @@ vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
>>  	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
>>  		return -ENOTSUP;
>>  
>> -	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>> +	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
>>  		return -ENOTSUP;
>>  
>>  	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
>> @@ -747,6 +754,17 @@ vhost_user_setup(struct virtio_user_dev *dev)
>>  	int fd;
>>  	int flag;
>>  	struct sockaddr_un un;
>> +	struct vhost_user_data *data;
>> +
>> +	data = malloc(sizeof(*data));
>> +	if (!data) {
>> +		PMD_DRV_LOG(ERR, "(%s) Failed to allocate Vhost-user data\n", dev->path);
>> +		return -1;
>> +	}
>> +
>> +	memset(data, 0, sizeof(*data));
>> +
>> +	dev->backend_data = data;
>>  
>>  	fd = socket(AF_UNIX, SOCK_STREAM, 0);
>>  	if (fd < 0) {
> 
> There are a number of "return -1;" statements below which, if executed, would
> now leak.

Fixed.

> Also, there are two in virtio_user_dev.c:virtio_user_dev_setup()

Good catch, I'm adding a new patch to handle failure properly.
Also, I'll fix destroy callbacks not to crash if called twice or called
after setup callback failure.

>> @@ -781,6 +799,17 @@ vhost_user_setup(struct virtio_user_dev *dev)
>>  	return 0;
>>  }
>>  
>> +static int
>> +vhost_user_destroy(struct virtio_user_dev *dev)
>> +{
>> +	if (dev->backend_data) {
>> +		free(dev->backend_data);
>> +		dev->backend_data = NULL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int
>>  vhost_user_enable_queue_pair(struct virtio_user_dev *dev,
>>  			     uint16_t pair_idx,
>> @@ -815,6 +844,7 @@ vhost_user_get_backend_features(uint64_t *features)
>>  
>>  struct virtio_user_backend_ops virtio_ops_user = {
>>  	.setup = vhost_user_setup,
>> +	.destroy = vhost_user_destroy,
>>  	.get_backend_features = vhost_user_get_backend_features,
>>  	.set_owner = vhost_user_set_owner,
>>  	.get_features = vhost_user_get_features,
>> diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
>> index c826f333e0..b29426d767 100644
>> --- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
>> +++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
>> @@ -287,6 +287,13 @@ vhost_vdpa_setup(struct virtio_user_dev *dev)
>>  	return 0;
>>  }
>>  
>> +static int
>> +vhost_vdpa_destroy(struct virtio_user_dev *dev __rte_unused)
>> +{
>> +	return;
>> +	return 0;
>> +}
>> +
>>  static int
>>  vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
>>  			       uint16_t pair_idx,
>> @@ -322,6 +329,7 @@ vhost_vdpa_get_backend_features(uint64_t *features)
>>  
>>  struct virtio_user_backend_ops virtio_ops_vdpa = {
>>  	.setup = vhost_vdpa_setup,
>> +	.destroy = vhost_vdpa_destroy,
>>  	.get_backend_features = vhost_vdpa_get_backend_features,
>>  	.set_owner = vhost_vdpa_set_owner,
>>  	.get_features = vhost_vdpa_get_features,
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> index 974de133bc..8d19a0addd 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
>> @@ -620,6 +620,8 @@ virtio_user_dev_uninit(struct virtio_user_dev *dev)
>>  
>>  	if (dev->is_server)
>>  		unlink(dev->path);
>> +
>> +	dev->ops->destroy(dev);
>>  }
>>  
>>  uint8_t
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> index b2da2944e9..ec73d5de11 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> @@ -30,7 +30,6 @@ struct virtio_user_dev {
>>  	int		vhostfd;
>>  	int		listenfd;   /* listening fd */
>>  	bool		is_server;  /* server or client mode */
>> -	uint64_t	protocol_features; /* negotiated protocol features*/
>>  
>>  	/* for vhost_kernel backend */
>>  	char		*ifname;
>> @@ -66,6 +65,8 @@ struct virtio_user_dev {
>>  	struct virtio_user_backend_ops *ops;
>>  	pthread_mutex_t	mutex;
>>  	bool		started;
>> +
>> +	void *backend_data;
>>  };
>>  
>>  int virtio_user_dev_set_features(struct virtio_user_dev *dev);
>>
> 
> 
> 
>
diff mbox series

Patch

diff --git a/drivers/net/virtio/virtio_user/vhost.h b/drivers/net/virtio/virtio_user/vhost.h
index b488ac78a1..ee5598226d 100644
--- a/drivers/net/virtio/virtio_user/vhost.h
+++ b/drivers/net/virtio/virtio_user/vhost.h
@@ -57,6 +57,7 @@  struct virtio_user_dev;
 
 struct virtio_user_backend_ops {
 	int (*setup)(struct virtio_user_dev *dev);
+	int (*destroy)(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);
diff --git a/drivers/net/virtio/virtio_user/vhost_kernel.c b/drivers/net/virtio/virtio_user/vhost_kernel.c
index 2fd00afa84..023fddcd69 100644
--- a/drivers/net/virtio/virtio_user/vhost_kernel.c
+++ b/drivers/net/virtio/virtio_user/vhost_kernel.c
@@ -357,6 +357,12 @@  vhost_kernel_setup(struct virtio_user_dev *dev)
 	return 0;
 }
 
+static int
+vhost_kernel_destroy(struct virtio_user_dev *dev)
+{
+	return 0;
+}
+
 static int
 vhost_kernel_set_backend(int vhostfd, int tapfd)
 {
@@ -455,6 +461,7 @@  vhost_kernel_get_backend_features(uint64_t *features)
 
 struct virtio_user_backend_ops virtio_ops_kernel = {
 	.setup = vhost_kernel_setup,
+	.destroy = vhost_kernel_destroy,
 	.get_backend_features = vhost_kernel_get_backend_features,
 	.set_owner = vhost_kernel_set_owner,
 	.get_features = vhost_kernel_get_features,
diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c
index f67c40e047..e96b1d8b9c 100644
--- a/drivers/net/virtio/virtio_user/vhost_user.c
+++ b/drivers/net/virtio/virtio_user/vhost_user.c
@@ -17,6 +17,9 @@ 
 #include "vhost.h"
 #include "virtio_user_dev.h"
 
+struct vhost_user_data {
+	uint64_t protocol_features;
+};
 
 #ifndef VHOST_USER_F_PROTOCOL_FEATURES
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
@@ -274,6 +277,7 @@  static int
 vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
 {
 	int ret;
+	struct vhost_user_data *data = dev->backend_data;
 	struct vhost_user_msg msg = {
 		.request = VHOST_USER_GET_FEATURES,
 		.flags = VHOST_USER_VERSION,
@@ -303,17 +307,17 @@  vhost_user_get_features(struct virtio_user_dev *dev, uint64_t *features)
 		return 0;
 
 	/* Negotiate protocol features */
-	ret = vhost_user_get_protocol_features(dev, &dev->protocol_features);
+	ret = vhost_user_get_protocol_features(dev, &data->protocol_features);
 	if (ret < 0)
 		goto err;
 
-	dev->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
+	data->protocol_features &= VHOST_USER_SUPPORTED_PROTOCOL_FEATURES;
 
-	ret = vhost_user_set_protocol_features(dev, dev->protocol_features);
+	ret = vhost_user_set_protocol_features(dev, data->protocol_features);
 	if (ret < 0)
 		goto err;
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
+	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_MQ)))
 		dev->unsupported_features |= (1ull << VIRTIO_NET_F_MQ);
 
 	return 0;
@@ -429,12 +433,13 @@  vhost_user_set_memory_table(struct virtio_user_dev *dev)
 	struct walk_arg wa;
 	int fds[VHOST_MEMORY_MAX_NREGIONS];
 	int ret, fd_num;
+	struct vhost_user_data *data = dev->backend_data;
 	struct vhost_user_msg msg = {
 		.request = VHOST_USER_SET_MEM_TABLE,
 		.flags = VHOST_USER_VERSION,
 	};
 
-	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
+	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
 		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
 
 	wa.region_nr = 0;
@@ -613,6 +618,7 @@  static int
 vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
 {
 	int ret;
+	struct vhost_user_data *data = dev->backend_data;
 	struct vhost_user_msg msg = {
 		.request = VHOST_USER_GET_STATUS,
 		.flags = VHOST_USER_VERSION,
@@ -629,7 +635,7 @@  vhost_user_get_status(struct virtio_user_dev *dev, uint8_t *status)
 	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
 		return -ENOTSUP;
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
 		return -ENOTSUP;
 
 	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
@@ -666,6 +672,7 @@  static int
 vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
 {
 	int ret;
+	struct vhost_user_data *data = dev->backend_data;
 	struct vhost_user_msg msg = {
 		.request = VHOST_USER_SET_STATUS,
 		.flags = VHOST_USER_VERSION,
@@ -673,7 +680,7 @@  vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
 		.payload.u64 = status,
 	};
 
-	if (dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
+	if (data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_REPLY_ACK))
 		msg.flags |= VHOST_USER_NEED_REPLY_MASK;
 
 	/*
@@ -687,7 +694,7 @@  vhost_user_set_status(struct virtio_user_dev *dev, uint8_t status)
 	if (!(dev->device_features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES)))
 		return -ENOTSUP;
 
-	if (!(dev->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
+	if (!(data->protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_STATUS)))
 		return -ENOTSUP;
 
 	ret = vhost_user_write(dev->vhostfd, &msg, NULL, 0);
@@ -747,6 +754,17 @@  vhost_user_setup(struct virtio_user_dev *dev)
 	int fd;
 	int flag;
 	struct sockaddr_un un;
+	struct vhost_user_data *data;
+
+	data = malloc(sizeof(*data));
+	if (!data) {
+		PMD_DRV_LOG(ERR, "(%s) Failed to allocate Vhost-user data\n", dev->path);
+		return -1;
+	}
+
+	memset(data, 0, sizeof(*data));
+
+	dev->backend_data = data;
 
 	fd = socket(AF_UNIX, SOCK_STREAM, 0);
 	if (fd < 0) {
@@ -781,6 +799,17 @@  vhost_user_setup(struct virtio_user_dev *dev)
 	return 0;
 }
 
+static int
+vhost_user_destroy(struct virtio_user_dev *dev)
+{
+	if (dev->backend_data) {
+		free(dev->backend_data);
+		dev->backend_data = NULL;
+	}
+
+	return 0;
+}
+
 static int
 vhost_user_enable_queue_pair(struct virtio_user_dev *dev,
 			     uint16_t pair_idx,
@@ -815,6 +844,7 @@  vhost_user_get_backend_features(uint64_t *features)
 
 struct virtio_user_backend_ops virtio_ops_user = {
 	.setup = vhost_user_setup,
+	.destroy = vhost_user_destroy,
 	.get_backend_features = vhost_user_get_backend_features,
 	.set_owner = vhost_user_set_owner,
 	.get_features = vhost_user_get_features,
diff --git a/drivers/net/virtio/virtio_user/vhost_vdpa.c b/drivers/net/virtio/virtio_user/vhost_vdpa.c
index c826f333e0..b29426d767 100644
--- a/drivers/net/virtio/virtio_user/vhost_vdpa.c
+++ b/drivers/net/virtio/virtio_user/vhost_vdpa.c
@@ -287,6 +287,13 @@  vhost_vdpa_setup(struct virtio_user_dev *dev)
 	return 0;
 }
 
+static int
+vhost_vdpa_destroy(struct virtio_user_dev *dev __rte_unused)
+{
+	return;
+	return 0;
+}
+
 static int
 vhost_vdpa_enable_queue_pair(struct virtio_user_dev *dev,
 			       uint16_t pair_idx,
@@ -322,6 +329,7 @@  vhost_vdpa_get_backend_features(uint64_t *features)
 
 struct virtio_user_backend_ops virtio_ops_vdpa = {
 	.setup = vhost_vdpa_setup,
+	.destroy = vhost_vdpa_destroy,
 	.get_backend_features = vhost_vdpa_get_backend_features,
 	.set_owner = vhost_vdpa_set_owner,
 	.get_features = vhost_vdpa_get_features,
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
index 974de133bc..8d19a0addd 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
@@ -620,6 +620,8 @@  virtio_user_dev_uninit(struct virtio_user_dev *dev)
 
 	if (dev->is_server)
 		unlink(dev->path);
+
+	dev->ops->destroy(dev);
 }
 
 uint8_t
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index b2da2944e9..ec73d5de11 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -30,7 +30,6 @@  struct virtio_user_dev {
 	int		vhostfd;
 	int		listenfd;   /* listening fd */
 	bool		is_server;  /* server or client mode */
-	uint64_t	protocol_features; /* negotiated protocol features*/
 
 	/* for vhost_kernel backend */
 	char		*ifname;
@@ -66,6 +65,8 @@  struct virtio_user_dev {
 	struct virtio_user_backend_ops *ops;
 	pthread_mutex_t	mutex;
 	bool		started;
+
+	void *backend_data;
 };
 
 int virtio_user_dev_set_features(struct virtio_user_dev *dev);