DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] kni: restrict bifurcated device support
@ 2021-10-08 23:58 Ferruh Yigit
  2021-10-09  2:03 ` Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ferruh Yigit @ 2021-10-08 23:58 UTC (permalink / raw)
  To: Elad Nachman
  Cc: Ferruh Yigit, dev, stable, Igor Ryzhov, Eric Christian,
	Stephen Hemminger

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 <ferruh.yigit@intel.com>
---
Cc: Igor Ryzhov <iryzhov@nfware.com>
Cc: Elad Nachman <eladv6@gmail.com>
Cc: Eric Christian <erclists@gmail.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
 .../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 <build_dir>/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


^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-11-24 13:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-08 23:58 [dpdk-dev] [PATCH] kni: restrict bifurcated device support Ferruh Yigit
2021-10-09  2:03 ` Stephen Hemminger
2021-11-23  9:54   ` Ferruh Yigit
2021-11-23 16:22     ` Stephen Hemminger
2021-11-23 16:51       ` Ferruh Yigit
2021-11-23 19:10         ` Stephen Hemminger
2021-11-17 16:42 ` Igor Ryzhov
2021-11-23 16:46 ` [PATCH v2] " Ferruh Yigit
2021-11-24 13:51   ` Thomas Monjalon

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).