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 0A004A0C4C; Tue, 23 Nov 2021 17:46:25 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E25D640040; Tue, 23 Nov 2021 17:46:24 +0100 (CET) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by mails.dpdk.org (Postfix) with ESMTP id 2E5C04003C; Tue, 23 Nov 2021 17:46:23 +0100 (CET) X-IronPort-AV: E=McAfee;i="6200,9189,10177"; a="298469689" X-IronPort-AV: E=Sophos;i="5.87,258,1631602800"; d="scan'208";a="298469689" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Nov 2021 08:46:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.87,258,1631602800"; d="scan'208";a="457121666" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga006.jf.intel.com with ESMTP; 23 Nov 2021 08:46:20 -0800 From: Ferruh Yigit To: Thomas Monjalon , David Marchand , Elad Nachman Cc: Ferruh Yigit , dev@dpdk.org, stable@dpdk.org, Igor Ryzhov , Eric Christian , Stephen Hemminger , Sahithi Singam Subject: [PATCH v2] kni: restrict bifurcated device support Date: Tue, 23 Nov 2021 16:46:17 +0000 Message-Id: <20211123164618.3585878-1-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211008235830.127167-1-ferruh.yigit@intel.com> References: <20211008235830.127167-1-ferruh.yigit@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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 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 Acked-by: Igor Ryzhov --- Cc: Igor Ryzhov Cc: Elad Nachman Cc: Eric Christian Cc: Stephen Hemminger Cc: Sahithi Singam v2: * Updated release note * Verified the fix with two KNI interface setting MAC address in endless loop, and confirmed deadlock is not observed (which was observed without this patch). --- .../prog_guide/kernel_nic_interface.rst | 28 +++++++++++++ doc/guides/rel_notes/release_21_11.rst | 6 +++ kernel/linux/kni/kni_dev.h | 3 ++ kernel/linux/kni/kni_misc.c | 36 ++++++++++++++++ kernel/linux/kni/kni_net.c | 42 +++++++++++-------- 5 files changed, 98 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/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst index 4d8c59472af1..fa2ce760d806 100644 --- a/doc/guides/rel_notes/release_21_11.rst +++ b/doc/guides/rel_notes/release_21_11.rst @@ -75,6 +75,12 @@ New Features operations. * Added multi-process support. +* **Updated default KNI behavior on net devices control callbacks.** + + Updated KNI net devices control callbacks to run with ``rtnl`` kernel lock + held by default. A newly added ``enable_bifurcated`` KNI kernel module + parameter can be used to run callbacks with ``rtnl`` lock released. + * **Added HiSilicon DMA driver.** The HiSilicon DMA driver provides device drivers for the Kunpeng's DMA devices. 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 f4944e1ddf33..f10dcd069d00 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; @@ -565,6 +569,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) { @@ -590,6 +610,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 @@ -656,3 +683,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