[dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name

[Tan, Jianfeng]

Hi Wenfeng,

Thank you for implementing this.

On 3/11/2017 11:36 PM, Wenfeng Liu wrote:
> This patch adds a new option 'iface' to change the interface name of
> tap device with vhost-kernel as backend.
>
> Signed-off-by: Wenfeng Liu <liuwf at arraynetworks.com.cn>
> ---
>   drivers/net/virtio/virtio_user/virtio_user_dev.c | 12 ++++++++----
>   drivers/net/virtio/virtio_user/virtio_user_dev.h |  2 +-
>   drivers/net/virtio/virtio_user_ethdev.c          | 24 +++++++++++++++++++++---
>   3 files changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> index 21ed00d..e7fd65f 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c
> @@ -193,9 +193,6 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
>   	for (i = 0; i < dev->max_queue_pairs; ++i)
>   		dev->ops->enable_qp(dev, i, 0);
>   
> -	free(dev->ifname);
> -	dev->ifname = NULL;

If we do not free it here, we need to free it at virtio_user_pmd_remove().

> -
>   	return 0;
>   }
>   
> @@ -268,7 +265,7 @@ 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)
> +		     int cq, int queue_size, const char *mac, char **ifname)
>   {
>   	snprintf(dev->path, PATH_MAX, "%s", path);
>   	dev->max_queue_pairs = queues;
> @@ -277,6 +274,11 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
>   	dev->mac_specified = 0;
>   	parse_mac(dev, mac);
>   
> +	if (*ifname) {
> +		dev->ifname = *ifname;
> +		*ifname = NULL;
> +	}
> +
>   	if (virtio_user_dev_setup(dev) < 0) {
>   		PMD_INIT_LOG(ERR, "backend set up fails");
>   		return -1;
> @@ -327,6 +329,8 @@ int virtio_user_stop_device(struct virtio_user_dev *dev)
>   		free(dev->vhostfds);
>   		free(dev->tapfds);
>   	}
> +
> +	free(dev->ifname);
>   }
>   
>   static 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 0d39f40..6ecb91e 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -69,7 +69,7 @@ struct virtio_user_dev {
>   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);
> +			 int cq, int queue_size, const char *mac, char **ifname);
>   void virtio_user_dev_uninit(struct virtio_user_dev *dev);
>   void virtio_user_handle_cq(struct virtio_user_dev *dev, uint16_t queue_idx);
>   #endif
> diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
> index 0b226ac..16d1526 100644
> --- a/drivers/net/virtio/virtio_user_ethdev.c
> +++ b/drivers/net/virtio/virtio_user_ethdev.c
> @@ -243,6 +243,8 @@
>   	VIRTIO_USER_ARG_PATH,
>   #define VIRTIO_USER_ARG_QUEUE_SIZE     "queue_size"
>   	VIRTIO_USER_ARG_QUEUE_SIZE,
> +#define VIRTIO_USER_ARG_INTERFACE_NAME "iface"
> +	VIRTIO_USER_ARG_INTERFACE_NAME,
>   	NULL
>   };
>   
> @@ -259,6 +261,9 @@
>   
>   	*(char **)extra_args = strdup(value);
>   
> +	if (!*(char **)extra_args)
> +		return -ENOMEM;
> +
>   	return 0;
>   }
>   
> @@ -347,6 +352,7 @@
>   	uint64_t cq = VIRTIO_USER_DEF_CQ_EN;
>   	uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ;
>   	char *path = NULL;
> +	char *ifname = NULL;
>   	char *mac_addr = NULL;
>   	int ret = -1;
>   
> @@ -375,6 +381,15 @@
>   		goto end;
>   	}
>   
> +	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME) == 1) {
> +		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_INTERFACE_NAME,
> +				       &get_string_arg, &ifname) < 0) {
> +			PMD_INIT_LOG(ERR, "error to parse %s",
> +				     VIRTIO_USER_ARG_INTERFACE_NAME);
> +			goto end;
> +		}
> +	}
> +
>   	if (rte_kvargs_count(kvlist, VIRTIO_USER_ARG_MAC) == 1) {
>   		if (rte_kvargs_process(kvlist, VIRTIO_USER_ARG_MAC,
>   				       &get_string_arg, &mac_addr) < 0) {
> @@ -413,7 +428,7 @@
>   		cq = 1;
>   	}
>   
> -	if (queues > 1 && cq == 0) {
> +	if (queues > 1 && cq == VIRTIO_USER_DEF_CQ_EN) {

Any specific reason for above change? What if we set 
VIRTIO_USER_DEF_CQ_EN=1 in future? I think zero is more clear.

Thanks,
Jianfeng