From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 046F2A034F; Sat, 9 Oct 2021 01:58:41 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8AC7B40143; Sat, 9 Oct 2021 01:58:41 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mails.dpdk.org (Postfix) with ESMTP id CDE4840140; Sat, 9 Oct 2021 01:58:39 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10131"; a="207420938" X-IronPort-AV: E=Sophos;i="5.85,358,1624345200"; d="scan'208";a="207420938" Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Oct 2021 16:58:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.85,358,1624345200"; d="scan'208";a="440805534" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga003.jf.intel.com with ESMTP; 08 Oct 2021 16:58:32 -0700 From: Ferruh Yigit To: Elad Nachman Cc: Ferruh Yigit , dev@dpdk.org, stable@dpdk.org, Igor Ryzhov , Eric Christian , Stephen Hemminger Date: Sat, 9 Oct 2021 00:58:30 +0100 Message-Id: <20211008235830.127167-1-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [dpdk-dev] [PATCH] kni: restrict bifurcated device support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" To enable bifurcated device support, rtnl_lock is released before calling userspace callbacks and asynchronous requests are enabled. But these changes caused more issues, like bug #809, #816. To reduce the scope of the problems, the bifurcated device support related changes are only enabled when it is requested explicitly with new 'enable_bifurcated' module parameter. And bifurcated device support is disabled by default. So the bifurcated device related problems are isolated and they can be fixed without impacting all use cases. Bugzilla ID: 816 Fixes: 631217c76135 ("kni: fix kernel deadlock with bifurcated device") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit --- Cc: Igor Ryzhov Cc: Elad Nachman Cc: Eric Christian Cc: Stephen Hemminger --- .../prog_guide/kernel_nic_interface.rst | 28 +++++++++++++ kernel/linux/kni/kni_dev.h | 3 ++ kernel/linux/kni/kni_misc.c | 36 ++++++++++++++++ kernel/linux/kni/kni_net.c | 42 +++++++++++-------- 4 files changed, 92 insertions(+), 17 deletions(-) diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst index 1ce03ec1a374..771c7d7fdac4 100644 --- a/doc/guides/prog_guide/kernel_nic_interface.rst +++ b/doc/guides/prog_guide/kernel_nic_interface.rst @@ -56,6 +56,12 @@ can be specified when the module is loaded to control its behavior: off Interfaces will be created with carrier state set to off. on Interfaces will be created with carrier state set to on. (charp) + parm: enable_bifurcated: Enable request processing support for + bifurcated drivers, which means releasing rtnl_lock before calling + userspace callback and supporting async requests (default=off): + on Enable request processing support for bifurcated drivers. + (charp) + Loading the ``rte_kni`` kernel module without any optional parameters is the typical way a DPDK application gets packets into and out of the kernel @@ -174,6 +180,28 @@ To set the default carrier state to *off*: If the ``carrier`` parameter is not specified, the default carrier state of KNI interfaces will be set to *off*. +.. _kni_bifurcated_device_support: + +Bifurcated Device Support +~~~~~~~~~~~~~~~~~~~~~~~~~ + +User callbacks are executed while kernel module holds the ``rtnl`` lock, this +causes a deadlock when callbacks run control commands on another Linux kernel +network interface. + +Bifurcated devices has kernel network driver part and to prevent deadlock for +them ``enable_bifurcated`` is used. + +To enable bifurcated device support: + +.. code-block:: console + + # insmod /kernel/linux/kni/rte_kni.ko enable_bifurcated=on + +Enabling bifurcated device support releases ``rtnl`` lock before calling +callback and locks it back after callback. Also enables asynchronous request to +support callbacks that requires rtnl lock to work (interface down). + KNI Creation and Deletion ------------------------- diff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h index c15da311ba25..e8633486eeb8 100644 --- a/kernel/linux/kni/kni_dev.h +++ b/kernel/linux/kni/kni_dev.h @@ -34,6 +34,9 @@ /* Default carrier state for created KNI network interfaces */ extern uint32_t kni_dflt_carrier; +/* Request processing support for bifurcated drivers. */ +extern uint32_t bifurcated_support; + /** * A structure describing the private information for a kni device. */ diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c index 2b464c438113..aae977c187a9 100644 --- a/kernel/linux/kni/kni_misc.c +++ b/kernel/linux/kni/kni_misc.c @@ -41,6 +41,10 @@ static uint32_t multiple_kthread_on; static char *carrier; uint32_t kni_dflt_carrier; +/* Request processing support for bifurcated drivers. */ +static char *enable_bifurcated; +uint32_t bifurcated_support; + #define KNI_DEV_IN_USE_BIT_NUM 0 /* Bit number for device in use */ static int kni_net_id; @@ -568,6 +572,22 @@ kni_parse_carrier_state(void) return 0; } +static int __init +kni_parse_bifurcated_support(void) +{ + if (!enable_bifurcated) { + bifurcated_support = 0; + return 0; + } + + if (strcmp(enable_bifurcated, "on") == 0) + bifurcated_support = 1; + else + return -1; + + return 0; +} + static int __init kni_init(void) { @@ -593,6 +613,13 @@ kni_init(void) else pr_debug("Default carrier state set to on.\n"); + if (kni_parse_bifurcated_support() < 0) { + pr_err("Invalid parameter for bifurcated support\n"); + return -EINVAL; + } + if (bifurcated_support == 1) + pr_debug("bifurcated support is enabled.\n"); + #ifdef HAVE_SIMPLIFIED_PERNET_OPERATIONS rc = register_pernet_subsys(&kni_net_ops); #else @@ -659,3 +686,12 @@ MODULE_PARM_DESC(carrier, "\t\ton Interfaces will be created with carrier state set to on.\n" "\t\t" ); + +module_param(enable_bifurcated, charp, 0644); +MODULE_PARM_DESC(enable_bifurcated, +"Enable request processing support for bifurcated drivers, " +"which means releasing rtnl_lock before calling userspace callback and " +"supporting async requests (default=off):\n" +"\t\ton Enable request processing support for bifurcated drivers.\n" +"\t\t" +); diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 611719b5ee27..29e5b9e21f9e 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -113,12 +113,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) ASSERT_RTNL(); - /* If we need to wait and RTNL mutex is held - * drop the mutex and hold reference to keep device - */ - if (req->async == 0) { - dev_hold(dev); - rtnl_unlock(); + if (bifurcated_support) { + /* If we need to wait and RTNL mutex is held + * drop the mutex and hold reference to keep device + */ + if (req->async == 0) { + dev_hold(dev); + rtnl_unlock(); + } } mutex_lock(&kni->sync_lock); @@ -132,12 +134,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) goto fail; } - /* No result available since request is handled - * asynchronously. set response to success. - */ - if (req->async != 0) { - req->result = 0; - goto async; + if (bifurcated_support) { + /* No result available since request is handled + * asynchronously. set response to success. + */ + if (req->async != 0) { + req->result = 0; + goto async; + } } ret_val = wait_event_interruptible_timeout(kni->wq, @@ -160,9 +164,11 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) fail: mutex_unlock(&kni->sync_lock); - if (req->async == 0) { - rtnl_lock(); - dev_put(dev); + if (bifurcated_support) { + if (req->async == 0) { + rtnl_lock(); + dev_put(dev); + } } return ret; } @@ -207,8 +213,10 @@ kni_net_release(struct net_device *dev) /* Setting if_up to 0 means down */ req.if_up = 0; - /* request async because of the deadlock problem */ - req.async = 1; + if (bifurcated_support) { + /* request async because of the deadlock problem */ + req.async = 1; + } ret = kni_net_process_request(dev, &req); -- 2.31.1