From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail01.arraynetworks.com.cn (mail.arraynetworks.com.cn [124.42.99.121]) by dpdk.org (Postfix) with ESMTP id E55A81094 for ; Tue, 28 Mar 2017 05:02:59 +0200 (CEST) Received: from liuwfthinkpad (10.6.0.244) by mail01.arraynetworks.com.cn (10.3.0.251) with Microsoft SMTP Server id 14.3.123.3; Tue, 28 Mar 2017 10:59:39 +0800 From: Wenfeng Liu To: "'Tan, Jianfeng'" , , CC: References: <1489246618-16898-1-git-send-email-liuwf@arraynetworks.com.cn> In-Reply-To: Date: Tue, 28 Mar 2017 11:02:56 +0800 Message-ID: <004401d2a76f$cd209a00$6761ce00$@com.cn> MIME-Version: 1.0 Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Thread-Index: AdKnZ0nfntTeI78BRjmlENcfWLpmvgABijWQ Content-Language: zh-cn X-Originating-IP: [10.6.0.244] Subject: [dpdk-dev] =?gb2312?b?tPC4tDogIFtQQVRDSF0gbmV0L3ZpcnRpby11c2Vy?= =?gb2312?b?OiBzdXBwb3J0IGNoYW5naW5nIHRhcCBpbnRlcmZhY2UgbmFtZQ==?= X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Mar 2017 03:03:01 -0000 Hi Jianfeng, At 2017-03-28 10:05:11, "Tan, Jianfeng" 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 >> --- >> 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 >