* [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name
@ 2017-03-11 15:36 Wenfeng Liu
2017-03-14 7:02 ` Yuanhan Liu
2017-03-28 2:05 ` Tan, Jianfeng
0 siblings, 2 replies; 5+ messages in thread
From: Wenfeng Liu @ 2017-03-11 15:36 UTC (permalink / raw)
To: yuanhan.liu, maxime.coquelin; +Cc: dev
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@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;
-
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) {
PMD_INIT_LOG(ERR, "multi-q requires ctrl-q");
goto end;
}
@@ -426,7 +441,7 @@
hw = eth_dev->data->dev_private;
if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq,
- queue_size, mac_addr) < 0) {
+ queue_size, mac_addr, &ifname) < 0) {
PMD_INIT_LOG(ERR, "virtio_user_dev_init fails");
virtio_user_eth_dev_free(eth_dev);
goto end;
@@ -447,6 +462,8 @@
free(path);
if (mac_addr)
free(mac_addr);
+ if (ifname)
+ free(ifname);
return ret;
}
@@ -492,4 +509,5 @@
"mac=<mac addr> "
"cq=<int> "
"queue_size=<int> "
- "queues=<int>");
+ "queues=<int> "
+ "iface=<string>");
--
1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name
2017-03-11 15:36 [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name Wenfeng Liu
@ 2017-03-14 7:02 ` Yuanhan Liu
2017-03-28 2:05 ` Tan, Jianfeng
1 sibling, 0 replies; 5+ messages in thread
From: Yuanhan Liu @ 2017-03-14 7:02 UTC (permalink / raw)
To: Tan, Jianfeng, Wenfeng Liu; +Cc: maxime.coquelin, dev
On Sat, Mar 11, 2017 at 03:36:58PM +0000, 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@arraynetworks.com.cn>
Jianfeng, any comments?
--yliu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name
2017-03-11 15:36 [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name Wenfeng Liu
2017-03-14 7:02 ` Yuanhan Liu
@ 2017-03-28 2:05 ` Tan, Jianfeng
2017-03-28 3:02 ` [dpdk-dev] 答复: " Wenfeng Liu
1 sibling, 1 reply; 5+ messages in thread
From: Tan, Jianfeng @ 2017-03-28 2:05 UTC (permalink / raw)
To: Wenfeng Liu, yuanhan.liu, maxime.coquelin; +Cc: dev
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@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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-dev] 答复: [PATCH] net/virtio-user: support changing tap interface name
2017-03-28 2:05 ` Tan, Jianfeng
@ 2017-03-28 3:02 ` Wenfeng Liu
2017-03-28 3:40 ` Tan, Jianfeng
0 siblings, 1 reply; 5+ messages in thread
From: Wenfeng Liu @ 2017-03-28 3:02 UTC (permalink / raw)
To: 'Tan, Jianfeng', yuanhan.liu, maxime.coquelin; +Cc: dev
Hi Jianfeng,
At 2017-03-28 10:05:11, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:
>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@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().
Yes, I put the free in virtio_user_dev_uninit(), which will be called by
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);
Here is the free in virtio_user_dev_uninit().
>> }
>>
>> 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.
Because cq was initialized to VIRTIO_USER_DEF_CQ_EN in this function, I
thought this comparison was intend to tell whether cq had been given a
different value, so I used
VIRTIO_USER_DEF_CQ_EN instead the magic number 0. Did I understand this
correctly? If not, I am happy to change it back.
>
>Thanks,
>Jianfeng
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] 答复: [PATCH] net/virtio-user: support changing tap interface name
2017-03-28 3:02 ` [dpdk-dev] 答复: " Wenfeng Liu
@ 2017-03-28 3:40 ` Tan, Jianfeng
0 siblings, 0 replies; 5+ messages in thread
From: Tan, Jianfeng @ 2017-03-28 3:40 UTC (permalink / raw)
To: Wenfeng Liu, yuanhan.liu, maxime.coquelin; +Cc: dev
On 3/28/2017 11:02 AM, Wenfeng Liu wrote:
> Hi Jianfeng,
>
> At 2017-03-28 10:05:11, "Tan, Jianfeng" <jianfeng.tan@intel.com> wrote:
>> 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@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().
> Yes, I put the free in virtio_user_dev_uninit(), which will be called by
> 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);
> Here is the free in virtio_user_dev_uninit().
Oops, I missed that.
>
>>> }
>>>
>>> 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.
> Because cq was initialized to VIRTIO_USER_DEF_CQ_EN in this function, I
> thought this comparison was intend to tell whether cq had been given a
> different value, so I used
> VIRTIO_USER_DEF_CQ_EN instead the magic number 0. Did I understand this
> correctly? If not, I am happy to change it back.
No, our purpose is not to check if cq is assigned a different value. We
are checking if cq is enabled or not as mq requires cq.
Besides that, it looks good to me.
Reviewed-by: Jianfeng Tan <jianfeng.tan@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-28 3:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-11 15:36 [dpdk-dev] [PATCH] net/virtio-user: support changing tap interface name Wenfeng Liu
2017-03-14 7:02 ` Yuanhan Liu
2017-03-28 2:05 ` Tan, Jianfeng
2017-03-28 3:02 ` [dpdk-dev] 答复: " Wenfeng Liu
2017-03-28 3:40 ` Tan, Jianfeng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).