<div dir="ltr">Acked-by: Igor Ryzhov <<a href="mailto:iryzhov@nfware.com">iryzhov@nfware.com</a>></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Oct 9, 2021 at 2:58 AM Ferruh Yigit <<a href="mailto:ferruh.yigit@intel.com">ferruh.yigit@intel.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">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: <a href="mailto:stable@dpdk.org" target="_blank">stable@dpdk.org</a> Signed-off-by: Ferruh Yigit <<a href="mailto:ferruh.yigit@intel.com" target="_blank">ferruh.yigit@intel.com</a>> --- Cc: Igor Ryzhov <<a href="mailto:iryzhov@nfware.com" target="_blank">iryzhov@nfware.com</a>> Cc: Elad Nachman <<a href="mailto:eladv6@gmail.com" target="_blank">eladv6@gmail.com</a>> Cc: Eric Christian <<a href="mailto:erclists@gmail.com" target="_blank">erclists@gmail.com</a>> Cc: Stephen Hemminger <<a href="mailto:stephen@networkplumber.org" target="_blank">stephen@networkplumber.org</a>> --- .../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 </blockquote></div>