Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/104620/?format=api
{ "id": 104620, "url": "http://patchwork.dpdk.org/api/patches/104620/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/patch/20211123164618.3585878-1-ferruh.yigit@intel.com/", "project": { "id": 1, "url": "http://patchwork.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<20211123164618.3585878-1-ferruh.yigit@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/20211123164618.3585878-1-ferruh.yigit@intel.com", "date": "2021-11-23T16:46:17", "name": "[v2] kni: restrict bifurcated device support", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": true, "hash": "3f52a069bafd150f69cb157c8b3236469b7f9440", "submitter": { "id": 324, "url": "http://patchwork.dpdk.org/api/people/324/?format=api", "name": "Ferruh Yigit", "email": "ferruh.yigit@intel.com" }, "delegate": { "id": 1, "url": "http://patchwork.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patchwork.dpdk.org/project/dpdk/patch/20211123164618.3585878-1-ferruh.yigit@intel.com/mbox/", "series": [ { "id": 20717, "url": "http://patchwork.dpdk.org/api/series/20717/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/list/?series=20717", "date": "2021-11-23T16:46:17", "name": "[v2] kni: restrict bifurcated device support", "version": 2, "mbox": "http://patchwork.dpdk.org/series/20717/mbox/" } ], "comments": "http://patchwork.dpdk.org/api/patches/104620/comments/", "check": "success", "checks": "http://patchwork.dpdk.org/api/patches/104620/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@inbox.dpdk.org", "Delivered-To": "patchwork@inbox.dpdk.org", "Received": [ "from mails.dpdk.org (mails.dpdk.org [217.70.189.124])\n\tby inbox.dpdk.org (Postfix) with ESMTP id 0A004A0C4C;\n\tTue, 23 Nov 2021 17:46:25 +0100 (CET)", "from [217.70.189.124] (localhost [127.0.0.1])\n\tby mails.dpdk.org (Postfix) with ESMTP id E25D640040;\n\tTue, 23 Nov 2021 17:46:24 +0100 (CET)", "from mga07.intel.com (mga07.intel.com [134.134.136.100])\n by mails.dpdk.org (Postfix) with ESMTP id 2E5C04003C;\n Tue, 23 Nov 2021 17:46:23 +0100 (CET)", "from orsmga006.jf.intel.com ([10.7.209.51])\n by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n 23 Nov 2021 08:46:22 -0800", "from silpixa00399752.ir.intel.com (HELO\n silpixa00399752.ger.corp.intel.com) ([10.237.222.27])\n by orsmga006.jf.intel.com with ESMTP; 23 Nov 2021 08:46:20 -0800" ], "X-IronPort-AV": [ "E=McAfee;i=\"6200,9189,10177\"; a=\"298469689\"", "E=Sophos;i=\"5.87,258,1631602800\"; d=\"scan'208\";a=\"298469689\"", "E=Sophos;i=\"5.87,258,1631602800\"; d=\"scan'208\";a=\"457121666\"" ], "X-ExtLoop1": "1", "From": "Ferruh Yigit <ferruh.yigit@intel.com>", "To": "Thomas Monjalon <thomas@monjalon.net>,\n David Marchand <david.marchand@redhat.com>, Elad Nachman <eladv6@gmail.com>", "Cc": "Ferruh Yigit <ferruh.yigit@intel.com>, dev@dpdk.org, stable@dpdk.org,\n Igor Ryzhov <iryzhov@nfware.com>, Eric Christian <erclists@gmail.com>,\n Stephen Hemminger <stephen@networkplumber.org>,\n Sahithi Singam <sahithi.singam@oracle.com>", "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 <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n <mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n <mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org" }, "content": "To enable bifurcated device support, rtnl_lock is released before calling\nuserspace callbacks and asynchronous requests are enabled.\n\nBut these changes caused more issues, like bug #809, #816. To reduce the\nscope of the problems, the bifurcated device support related changes are\nonly enabled when it is requested explicitly with new 'enable_bifurcated'\nmodule parameter.\nAnd bifurcated device support is disabled by default.\n\nSo the bifurcated device related problems are isolated and they can be\nfixed without impacting all use cases.\n\nBugzilla ID: 816\nFixes: 631217c76135 (\"kni: fix kernel deadlock with bifurcated device\")\nCc: stable@dpdk.org\n\nSigned-off-by: Ferruh Yigit <ferruh.yigit@intel.com>\nAcked-by: Igor Ryzhov <iryzhov@nfware.com>\n---\nCc: Igor Ryzhov <iryzhov@nfware.com>\nCc: Elad Nachman <eladv6@gmail.com>\nCc: Eric Christian <erclists@gmail.com>\nCc: Stephen Hemminger <stephen@networkplumber.org>\nCc: Sahithi Singam <sahithi.singam@oracle.com>\n\nv2:\n* Updated release note\n* Verified the fix with two KNI interface setting MAC address in endless\n loop, and confirmed deadlock is not observed (which was observed\n without this patch).\n---\n .../prog_guide/kernel_nic_interface.rst | 28 +++++++++++++\n doc/guides/rel_notes/release_21_11.rst | 6 +++\n kernel/linux/kni/kni_dev.h | 3 ++\n kernel/linux/kni/kni_misc.c | 36 ++++++++++++++++\n kernel/linux/kni/kni_net.c | 42 +++++++++++--------\n 5 files changed, 98 insertions(+), 17 deletions(-)", "diff": "diff --git a/doc/guides/prog_guide/kernel_nic_interface.rst b/doc/guides/prog_guide/kernel_nic_interface.rst\nindex 1ce03ec1a374..771c7d7fdac4 100644\n--- a/doc/guides/prog_guide/kernel_nic_interface.rst\n+++ b/doc/guides/prog_guide/kernel_nic_interface.rst\n@@ -56,6 +56,12 @@ can be specified when the module is loaded to control its behavior:\n off Interfaces will be created with carrier state set to off.\n on Interfaces will be created with carrier state set to on.\n (charp)\n+ parm: enable_bifurcated: Enable request processing support for\n+ bifurcated drivers, which means releasing rtnl_lock before calling\n+ userspace callback and supporting async requests (default=off):\n+ on Enable request processing support for bifurcated drivers.\n+ (charp)\n+\n \n Loading the ``rte_kni`` kernel module without any optional parameters is\n the typical way a DPDK application gets packets into and out of the kernel\n@@ -174,6 +180,28 @@ To set the default carrier state to *off*:\n If the ``carrier`` parameter is not specified, the default carrier state\n of KNI interfaces will be set to *off*.\n \n+.. _kni_bifurcated_device_support:\n+\n+Bifurcated Device Support\n+~~~~~~~~~~~~~~~~~~~~~~~~~\n+\n+User callbacks are executed while kernel module holds the ``rtnl`` lock, this\n+causes a deadlock when callbacks run control commands on another Linux kernel\n+network interface.\n+\n+Bifurcated devices has kernel network driver part and to prevent deadlock for\n+them ``enable_bifurcated`` is used.\n+\n+To enable bifurcated device support:\n+\n+.. code-block:: console\n+\n+ # insmod <build_dir>/kernel/linux/kni/rte_kni.ko enable_bifurcated=on\n+\n+Enabling bifurcated device support releases ``rtnl`` lock before calling\n+callback and locks it back after callback. Also enables asynchronous request to\n+support callbacks that requires rtnl lock to work (interface down).\n+\n KNI Creation and Deletion\n -------------------------\n \ndiff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst\nindex 4d8c59472af1..fa2ce760d806 100644\n--- a/doc/guides/rel_notes/release_21_11.rst\n+++ b/doc/guides/rel_notes/release_21_11.rst\n@@ -75,6 +75,12 @@ New Features\n operations.\n * Added multi-process support.\n \n+* **Updated default KNI behavior on net devices control callbacks.**\n+\n+ Updated KNI net devices control callbacks to run with ``rtnl`` kernel lock\n+ held by default. A newly added ``enable_bifurcated`` KNI kernel module\n+ parameter can be used to run callbacks with ``rtnl`` lock released.\n+\n * **Added HiSilicon DMA driver.**\n \n The HiSilicon DMA driver provides device drivers for the Kunpeng's DMA devices.\ndiff --git a/kernel/linux/kni/kni_dev.h b/kernel/linux/kni/kni_dev.h\nindex c15da311ba25..e8633486eeb8 100644\n--- a/kernel/linux/kni/kni_dev.h\n+++ b/kernel/linux/kni/kni_dev.h\n@@ -34,6 +34,9 @@\n /* Default carrier state for created KNI network interfaces */\n extern uint32_t kni_dflt_carrier;\n \n+/* Request processing support for bifurcated drivers. */\n+extern uint32_t bifurcated_support;\n+\n /**\n * A structure describing the private information for a kni device.\n */\ndiff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c\nindex f4944e1ddf33..f10dcd069d00 100644\n--- a/kernel/linux/kni/kni_misc.c\n+++ b/kernel/linux/kni/kni_misc.c\n@@ -41,6 +41,10 @@ static uint32_t multiple_kthread_on;\n static char *carrier;\n uint32_t kni_dflt_carrier;\n \n+/* Request processing support for bifurcated drivers. */\n+static char *enable_bifurcated;\n+uint32_t bifurcated_support;\n+\n #define KNI_DEV_IN_USE_BIT_NUM 0 /* Bit number for device in use */\n \n static int kni_net_id;\n@@ -565,6 +569,22 @@ kni_parse_carrier_state(void)\n \treturn 0;\n }\n \n+static int __init\n+kni_parse_bifurcated_support(void)\n+{\n+\tif (!enable_bifurcated) {\n+\t\tbifurcated_support = 0;\n+\t\treturn 0;\n+\t}\n+\n+\tif (strcmp(enable_bifurcated, \"on\") == 0)\n+\t\tbifurcated_support = 1;\n+\telse\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n static int __init\n kni_init(void)\n {\n@@ -590,6 +610,13 @@ kni_init(void)\n \telse\n \t\tpr_debug(\"Default carrier state set to on.\\n\");\n \n+\tif (kni_parse_bifurcated_support() < 0) {\n+\t\tpr_err(\"Invalid parameter for bifurcated support\\n\");\n+\t\treturn -EINVAL;\n+\t}\n+\tif (bifurcated_support == 1)\n+\t\tpr_debug(\"bifurcated support is enabled.\\n\");\n+\n #ifdef HAVE_SIMPLIFIED_PERNET_OPERATIONS\n \trc = register_pernet_subsys(&kni_net_ops);\n #else\n@@ -656,3 +683,12 @@ MODULE_PARM_DESC(carrier,\n \"\\t\\ton Interfaces will be created with carrier state set to on.\\n\"\n \"\\t\\t\"\n );\n+\n+module_param(enable_bifurcated, charp, 0644);\n+MODULE_PARM_DESC(enable_bifurcated,\n+\"Enable request processing support for bifurcated drivers, \"\n+\"which means releasing rtnl_lock before calling userspace callback and \"\n+\"supporting async requests (default=off):\\n\"\n+\"\\t\\ton Enable request processing support for bifurcated drivers.\\n\"\n+\"\\t\\t\"\n+);\ndiff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c\nindex 611719b5ee27..29e5b9e21f9e 100644\n--- a/kernel/linux/kni/kni_net.c\n+++ b/kernel/linux/kni/kni_net.c\n@@ -113,12 +113,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)\n \n \tASSERT_RTNL();\n \n-\t/* If we need to wait and RTNL mutex is held\n-\t * drop the mutex and hold reference to keep device\n-\t */\n-\tif (req->async == 0) {\n-\t\tdev_hold(dev);\n-\t\trtnl_unlock();\n+\tif (bifurcated_support) {\n+\t\t/* If we need to wait and RTNL mutex is held\n+\t\t * drop the mutex and hold reference to keep device\n+\t\t */\n+\t\tif (req->async == 0) {\n+\t\t\tdev_hold(dev);\n+\t\t\trtnl_unlock();\n+\t\t}\n \t}\n \n \tmutex_lock(&kni->sync_lock);\n@@ -132,12 +134,14 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)\n \t\tgoto fail;\n \t}\n \n-\t/* No result available since request is handled\n-\t * asynchronously. set response to success.\n-\t */\n-\tif (req->async != 0) {\n-\t\treq->result = 0;\n-\t\tgoto async;\n+\tif (bifurcated_support) {\n+\t\t/* No result available since request is handled\n+\t\t * asynchronously. set response to success.\n+\t\t */\n+\t\tif (req->async != 0) {\n+\t\t\treq->result = 0;\n+\t\t\tgoto async;\n+\t\t}\n \t}\n \n \tret_val = wait_event_interruptible_timeout(kni->wq,\n@@ -160,9 +164,11 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req)\n \n fail:\n \tmutex_unlock(&kni->sync_lock);\n-\tif (req->async == 0) {\n-\t\trtnl_lock();\n-\t\tdev_put(dev);\n+\tif (bifurcated_support) {\n+\t\tif (req->async == 0) {\n+\t\t\trtnl_lock();\n+\t\t\tdev_put(dev);\n+\t\t}\n \t}\n \treturn ret;\n }\n@@ -207,8 +213,10 @@ kni_net_release(struct net_device *dev)\n \t/* Setting if_up to 0 means down */\n \treq.if_up = 0;\n \n-\t/* request async because of the deadlock problem */\n-\treq.async = 1;\n+\tif (bifurcated_support) {\n+\t\t/* request async because of the deadlock problem */\n+\t\treq.async = 1;\n+\t}\n \n \tret = kni_net_process_request(dev, &req);\n \n", "prefixes": [ "v2" ] }