From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f67.google.com (mail-oi0-f67.google.com [209.85.218.67]) by dpdk.org (Postfix) with ESMTP id 188C81B51D for ; Fri, 29 Jun 2018 03:55:51 +0200 (CEST) Received: by mail-oi0-f67.google.com with SMTP id c2-v6so7060217oic.1 for ; Thu, 28 Jun 2018 18:55:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:in-reply-to:references; bh=ytOC7mp7wKfFaY5qMuyGH7qZSDHYGCEQGPP3dfwcCDQ=; b=huHb1NHLLSvLUP+i+6vNVhqHnSGBjajqrIgMqdIwj1MGclIHL4QYRACZQ7IrWgZcXu w8TXrNkGWiTncavIZu0tSCWVIqr95Kodi6Ww3DAhBdrR32SgB5dOwJi/XrvXQ0MkQLMS LsLm3y2wcOeVQ7c7BclPV26yNo9BaPNXbXxJg6ZFz8MJCyyGoSYg6PP9SgAzDq1397yc LiHbFTNAhMeV8BEQ18liBb7XpBTZ+vuhl1VZSfawNORJbNt2pi8pKq8pHa5g5cf9BFyl efMqXdM5TIMEVbaMgE7osoNNKVAH2Hr4vFhPlrHYSx8/k/9EkjK70eUMCfNvMr99Gvqw e4vw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=ytOC7mp7wKfFaY5qMuyGH7qZSDHYGCEQGPP3dfwcCDQ=; b=MQN6sMcG3wQbaxB4LjTu9lCG6gsIjzqVh5YtTjyEdUZ4ZCSMqGpWIel0fXklntBMrz 9IP9pW9m8AyUu0OVGrYH8dRbmw0p+YCUJUKTm92huOijZR6yXp3W3G7CPZUQh9K4Cg4E BIfBjuFVut3hyPdbqLuTlup6x2ATxK/QBeps9BO2c2I3vu/aw0e6/RIl/nr9VLstYWVO otpPNpKW6p3weXxmjLfRtjzTAXQW3m2cH5k/jgYdz5tGmPVpb1fIXQvfwKCX/TAHyLsX OYTRWQ6WL2zEfqENW85jbjYtPj81qLHtTOdEVUcEVo+/C0B5bPU4+ID0s5oQ96zSXHeX KSrg== X-Gm-Message-State: APt69E20I3PCUA7lqLB8NsdC9OniOrG8V9gfMlXHa+hQRGbCacjWGWEt 7j4wRVcPYsjBPDCjobLrvYY= X-Google-Smtp-Source: AAOMgpeLI717oN0XODjdUaVW/L2ceaz0Bi0SSTHZ9AV1Ui8CIbBiSSeTHr3DWvHys7ta0Wc94e2Dhg== X-Received: by 2002:aca:686:: with SMTP id 128-v6mr7364284oig.31.1530237350263; Thu, 28 Jun 2018 18:55:50 -0700 (PDT) Received: from linux.adax.com (172-11-198-60.lightspeed.sntcca.sbcglobal.net. [172.11.198.60]) by smtp.gmail.com with ESMTPSA id n3-v6sm3941288otk.38.2018.06.28.18.55.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 28 Jun 2018 18:55:49 -0700 (PDT) Sender: Dan Gora From: Dan Gora To: ferruh.yigit@intel.com Cc: dev@dpdk.org, Dan Gora Date: Thu, 28 Jun 2018 18:55:00 -0700 Message-Id: <20180629015508.26599-3-dg@adax.com> X-Mailer: git-send-email 2.18.0.rc1.1.g6f333ff2f In-Reply-To: <20180629015508.26599-1-dg@adax.com> References: <20180628224513.18391-1-dg@adax.com> <20180629015508.26599-1-dg@adax.com> Subject: [dpdk-dev] [PATCH v2 02/10] kni: separate releasing netdev from freeing KNI interface 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: Fri, 29 Jun 2018 01:55:51 -0000 Currently the rte_kni kernel driver suffers from a problem where when the interface is released, it generates a callback to the DPDK application to change the interface state to Down. However, after the DPDK application handles the callback and generates a response back to the kernel, the rte_kni driver cannot wake the thread which is asleep waiting for the response, because it is holding the kni_link_lock semaphore and it has already removed the 'struct kni_dev' from the list of interfaces to poll for responses. This means that if the KNI interface is in the Up state when rte_kni_release() is called, it will always sleep for three seconds until kni_net_release gives up waiting for a response from the DPDK application. To fix this, we must separate the step to release the kernel network interface from the steps to remove the KNI interface from the list of interfaces to poll. When the kernel network interface is removed with unregister_netdev(), if the interface is up, it will generate a callback to mark the interface down, which calls kni_net_release(). kni_net_release() will block waiting for the DPDK application to call rte_kni_handle_request() to handle the callback, but it also needs the thread in the KNI driver (either the per-dev thread for multi-thread or the per-driver thread) to call kni_net_poll_resp() in order to wake the thread sleeping in kni_net_release (actually kni_net_process_request()). So now, KNI interfaces should be removed as such: 1) The user calls rte_kni_release(). This only unregisters the netdev in the kernel, but touches nothing else. This allows all the threads to run which are necessary to handle the callback into the DPDK application to mark the interface down. 2) The user stops the thread running rte_kni_handle_request(). After rte_kni_release() has been called, there will be no more callbacks for that interface so it is not necessary. It cannot be running at the same time that rte_kni_free() frees all of the FIFOs and DPDK memory for that KNI interface. 3) The user calls rte_kni_free(). This performs the RTE_KNI_IOCTL_FREE ioctl which calls kni_ioctl_free(). This function removes the struct kni_dev from the list of interfaces to poll (and kills the per-dev kthread, if configured for multi-thread), then frees the memory in the FIFOs. Signed-off-by: Dan Gora --- kernel/linux/kni/kni_misc.c | 69 +++++++++++++++---- .../eal/include/exec-env/rte_kni_common.h | 1 + 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index fa69f8e63..d889ffc4b 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -192,8 +192,6 @@ kni_dev_remove(struct kni_dev *dev) free_netdev(dev->net_dev); } - kni_net_release_fifo_phy(dev); - return 0; } @@ -224,6 +222,7 @@ kni_release(struct inode *inode, struct file *file) } kni_dev_remove(dev); + kni_net_release_fifo_phy(dev); list_del(&dev->list); } up_write(&knet->kni_list_lock); @@ -263,7 +262,6 @@ kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind) kni->pthread = kthread_create(kni_thread_multiple, (void *)kni, "kni_%s", kni->name); if (IS_ERR(kni->pthread)) { - kni_dev_remove(kni); return -ECANCELED; } @@ -278,7 +276,6 @@ kni_run_thread(struct kni_net *knet, struct kni_dev *kni, uint8_t force_bind) (void *)knet, "kni_single"); if (IS_ERR(knet->kni_kthread)) { mutex_unlock(&knet->kni_kthread_lock); - kni_dev_remove(kni); return -ECANCELED; } @@ -460,15 +457,17 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num, if (ret) { pr_err("error %i registering device \"%s\"\n", ret, dev_info.name); - kni->net_dev = NULL; - kni_dev_remove(kni); + kni_net_release_fifo_phy(kni); free_netdev(net_dev); return -ENODEV; } ret = kni_run_thread(knet, kni, dev_info.force_bind); - if (ret != 0) + if (ret != 0) { + kni_dev_remove(kni); + kni_net_release_fifo_phy(kni); return ret; + } down_write(&knet->kni_list_lock); list_add(&kni->list, &knet->kni_list_head); @@ -495,6 +494,46 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, return -EIO; } + /* Release the network device according to its name */ + if (strlen(dev_info.name) == 0) + return ret; + + down_read(&knet->kni_list_lock); + list_for_each_entry_safe(dev, n, &knet->kni_list_head, list) { + if (strncmp(dev->name, dev_info.name, RTE_KNI_NAMESIZE) != 0) + continue; + + kni_dev_remove(dev); + up_read(&knet->kni_list_lock); + pr_info("Successfully released kni interface: %s\n", + dev_info.name); + return 0; + } + up_read(&knet->kni_list_lock); + pr_info("Error: failed to find kni interface: %s\n", + dev_info.name); + + return ret; +} + +static int +kni_ioctl_free(struct net *net, uint32_t ioctl_num, + unsigned long ioctl_param) +{ + struct kni_net *knet = net_generic(net, kni_net_id); + int ret = -EINVAL; + struct kni_dev *dev, *n; + struct rte_kni_device_info dev_info; + + if (_IOC_SIZE(ioctl_num) > sizeof(dev_info)) + return -EINVAL; + + ret = copy_from_user(&dev_info, (void *)ioctl_param, sizeof(dev_info)); + if (ret) { + pr_err("copy_from_user in kni_ioctl_release"); + return -EIO; + } + /* Release the network device according to its name */ if (strlen(dev_info.name) == 0) return ret; @@ -509,14 +548,17 @@ kni_ioctl_release(struct net *net, uint32_t ioctl_num, dev->pthread = NULL; } - kni_dev_remove(dev); + kni_net_release_fifo_phy(dev); list_del(&dev->list); - ret = 0; - break; + up_write(&knet->kni_list_lock); + pr_info("Successfully freed kni interface: %s\n", + dev_info.name); + return 0; } up_write(&knet->kni_list_lock); - pr_info("%s release kni named %s\n", - (ret == 0 ? "Successfully" : "Unsuccessfully"), dev_info.name); + + pr_info("Error: failed to find kni interface: %s\n", + dev_info.name); return ret; } @@ -542,6 +584,9 @@ kni_ioctl(struct inode *inode, uint32_t ioctl_num, unsigned long ioctl_param) case _IOC_NR(RTE_KNI_IOCTL_RELEASE): ret = kni_ioctl_release(net, ioctl_num, ioctl_param); break; + case _IOC_NR(RTE_KNI_IOCTL_FREE): + ret = kni_ioctl_free(net, ioctl_num, ioctl_param); + break; default: pr_debug("IOCTL default\n"); break; diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h index cfa9448bd..318a3f939 100644 --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h @@ -129,5 +129,6 @@ struct rte_kni_device_info { #define RTE_KNI_IOCTL_TEST _IOWR(0, 1, int) #define RTE_KNI_IOCTL_CREATE _IOWR(0, 2, struct rte_kni_device_info) #define RTE_KNI_IOCTL_RELEASE _IOWR(0, 3, struct rte_kni_device_info) +#define RTE_KNI_IOCTL_FREE _IOWR(0, 4, struct rte_kni_device_info) #endif /* _RTE_KNI_COMMON_H_ */ -- 2.18.0.rc1.1.g6f333ff2f