From: Wenfeng Liu <liuwf@arraynetworks.com.cn>
To: "'Tan, Jianfeng'" <jianfeng.tan@intel.com>,
<yuanhan.liu@linux.intel.com>, <maxime.coquelin@redhat.com>
Cc: <dev@dpdk.org>
Subject: [dpdk-dev] 答复: [PATCH] net/virtio-user: support changing tap interface name
Date: Tue, 28 Mar 2017 11:02:56 +0800 [thread overview]
Message-ID: <004401d2a76f$cd209a00$6761ce00$@com.cn> (raw)
In-Reply-To: <a1b02dbf-b1d3-9fc7-74b5-05fe4168cd7b@intel.com>
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
>
next prev parent reply other threads:[~2017-03-28 3:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-11 15:36 [dpdk-dev] " Wenfeng Liu
2017-03-14 7:02 ` Yuanhan Liu
2017-03-28 2:05 ` Tan, Jianfeng
2017-03-28 3:02 ` Wenfeng Liu [this message]
2017-03-28 3:40 ` [dpdk-dev] 答复: " Tan, Jianfeng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='004401d2a76f$cd209a00$6761ce00$@com.cn' \
--to=liuwf@arraynetworks.com.cn \
--cc=dev@dpdk.org \
--cc=jianfeng.tan@intel.com \
--cc=maxime.coquelin@redhat.com \
--cc=yuanhan.liu@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).