From patchwork Mon Mar 29 14:36:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ferruh Yigit X-Patchwork-Id: 90017 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id ABE50A0547; Mon, 29 Mar 2021 16:37:02 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 33948406FF; Mon, 29 Mar 2021 16:37:02 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 144C54069D for ; Mon, 29 Mar 2021 16:36:59 +0200 (CEST) IronPort-SDR: SO9HhiNHsiT4cAfSuoylwjLA8U1wTq5M5qVdaE+ZZwkg1gabTejuwEvSfiObXLBAVAVU4rtBMC +g2lySt6CdJg== X-IronPort-AV: E=McAfee;i="6000,8403,9938"; a="191589134" X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="191589134" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Mar 2021 07:36:58 -0700 IronPort-SDR: DZzcXyHKbQekT/lp/PO9Tyt/WLfsViNoNhMo+pheSGBA41PELQ2pOU6+GX4mRRS/umeXcm9lNn mMZVhOpI9Y+g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="526979647" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga004.jf.intel.com with ESMTP; 29 Mar 2021 07:36:57 -0700 From: Ferruh Yigit To: dev@dpdk.org Cc: Elad Nachman , Stephen Hemminger Date: Mon, 29 Mar 2021 15:36:53 +0100 Message-Id: <20210329143655.521750-1-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20201126144613.4986-1-eladv6@gmail.com> References: <20201126144613.4986-1-eladv6@gmail.com> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v5 1/3] kni: refactor user request processing 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" From: Elad Nachman Refactor the parameter kni_net_process_request() gets, this is preparation for addressing a user request processing deadlock problem. Signed-off-by: Stephen Hemminger Signed-off-by: Elad Nachman --- kernel/linux/kni/kni_net.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 4b752083da28..b830054c7491 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -102,17 +103,15 @@ get_data_kva(struct kni_dev *kni, void *pkt_kva) * It can be called to process the request. */ static int -kni_net_process_request(struct kni_dev *kni, struct rte_kni_request *req) +kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) { + struct kni_dev *kni = netdev_priv(dev); int ret = -1; void *resp_va; uint32_t num; int ret_val; - if (!kni || !req) { - pr_err("No kni instance or request\n"); - return -EINVAL; - } + ASSERT_RTNL(); mutex_lock(&kni->sync_lock); @@ -155,7 +154,6 @@ kni_net_open(struct net_device *dev) { int ret; struct rte_kni_request req; - struct kni_dev *kni = netdev_priv(dev); netif_start_queue(dev); if (kni_dflt_carrier == 1) @@ -168,7 +166,7 @@ kni_net_open(struct net_device *dev) /* Setting if_up to non-zero means up */ req.if_up = 1; - ret = kni_net_process_request(kni, &req); + ret = kni_net_process_request(dev, &req); return (ret == 0) ? req.result : ret; } @@ -178,7 +176,6 @@ kni_net_release(struct net_device *dev) { int ret; struct rte_kni_request req; - struct kni_dev *kni = netdev_priv(dev); netif_stop_queue(dev); /* can't transmit any more */ netif_carrier_off(dev); @@ -188,7 +185,7 @@ kni_net_release(struct net_device *dev) /* Setting if_up to 0 means down */ req.if_up = 0; - ret = kni_net_process_request(kni, &req); + ret = kni_net_process_request(dev, &req); return (ret == 0) ? req.result : ret; } @@ -643,14 +640,13 @@ kni_net_change_mtu(struct net_device *dev, int new_mtu) { int ret; struct rte_kni_request req; - struct kni_dev *kni = netdev_priv(dev); pr_debug("kni_net_change_mtu new mtu %d to be set\n", new_mtu); memset(&req, 0, sizeof(req)); req.req_id = RTE_KNI_REQ_CHANGE_MTU; req.new_mtu = new_mtu; - ret = kni_net_process_request(kni, &req); + ret = kni_net_process_request(dev, &req); if (ret == 0 && req.result == 0) dev->mtu = new_mtu; @@ -661,7 +657,6 @@ static void kni_net_change_rx_flags(struct net_device *netdev, int flags) { struct rte_kni_request req; - struct kni_dev *kni = netdev_priv(netdev); memset(&req, 0, sizeof(req)); @@ -683,7 +678,7 @@ kni_net_change_rx_flags(struct net_device *netdev, int flags) req.promiscusity = 0; } - kni_net_process_request(kni, &req); + kni_net_process_request(netdev, &req); } /* @@ -742,7 +737,6 @@ kni_net_set_mac(struct net_device *netdev, void *p) { int ret; struct rte_kni_request req; - struct kni_dev *kni; struct sockaddr *addr = p; memset(&req, 0, sizeof(req)); @@ -754,8 +748,7 @@ kni_net_set_mac(struct net_device *netdev, void *p) memcpy(req.mac_addr, addr->sa_data, netdev->addr_len); memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len); - kni = netdev_priv(netdev); - ret = kni_net_process_request(kni, &req); + ret = kni_net_process_request(netdev, &req); return (ret == 0 ? req.result : ret); } From patchwork Mon Mar 29 14:36:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ferruh Yigit X-Patchwork-Id: 90018 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id D427FA0547; Mon, 29 Mar 2021 16:37:07 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 39E17140D9C; Mon, 29 Mar 2021 16:37:04 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 32446406B4 for ; Mon, 29 Mar 2021 16:37:02 +0200 (CEST) IronPort-SDR: ls/qndb8yLgXvXChv3Yln023KqD4QNNoaeF7dMBgv5ZHnTyx1D4S1sWx2cEX3fh5j02Yh4xqNC yty2Olgk5w0w== X-IronPort-AV: E=McAfee;i="6000,8403,9938"; a="191589140" X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="191589140" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Mar 2021 07:37:01 -0700 IronPort-SDR: VdVi4C45ivJSK5LTOj+iDH3X55F8oPzbONX6LRB5hleWh+T/BxV/F5R/FsjRKscWYVZsyWlOsx r30D9wdWM+4g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="526979703" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga004.jf.intel.com with ESMTP; 29 Mar 2021 07:37:01 -0700 From: Ferruh Yigit To: dev@dpdk.org Cc: Elad Nachman Date: Mon, 29 Mar 2021 15:36:54 +0100 Message-Id: <20210329143655.521750-2-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210329143655.521750-1-ferruh.yigit@intel.com> References: <20201126144613.4986-1-eladv6@gmail.com> <20210329143655.521750-1-ferruh.yigit@intel.com> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v5 2/3] kni: support async user request 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" Adding async userspace requests which don't wait for the userspace response and always return success. This is preparation to address a regression in KNI. Signed-off-by: Elad Nachman Signed-off-by: Ferruh Yigit --- kernel/linux/kni/kni_net.c | 9 +++++++++ lib/librte_kni/rte_kni.c | 7 +++++-- lib/librte_kni/rte_kni_common.h | 1 + 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index b830054c7491..6cf99da0dc92 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -124,6 +124,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; + } + ret_val = wait_event_interruptible_timeout(kni->wq, kni_fifo_count(kni->resp_q), 3 * HZ); if (signal_pending(current) || ret_val <= 0) { @@ -139,6 +147,7 @@ kni_net_process_request(struct net_device *dev, struct rte_kni_request *req) } memcpy(req, kni->sync_kva, sizeof(struct rte_kni_request)); +async: ret = 0; fail: diff --git a/lib/librte_kni/rte_kni.c b/lib/librte_kni/rte_kni.c index 837d0217d2d1..9dae6a8d7c0c 100644 --- a/lib/librte_kni/rte_kni.c +++ b/lib/librte_kni/rte_kni.c @@ -591,8 +591,11 @@ rte_kni_handle_request(struct rte_kni *kni) break; } - /* Construct response mbuf and put it back to resp_q */ - ret = kni_fifo_put(kni->resp_q, (void **)&req, 1); + /* if needed, construct response buffer and put it back to resp_q */ + if (!req->async) + ret = kni_fifo_put(kni->resp_q, (void **)&req, 1); + else + ret = 1; if (ret != 1) { RTE_LOG(ERR, KNI, "Fail to put the muf back to resp_q\n"); return -1; /* It is an error of can't putting the mbuf back */ diff --git a/lib/librte_kni/rte_kni_common.h b/lib/librte_kni/rte_kni_common.h index ffb3182731a0..b547ea550171 100644 --- a/lib/librte_kni/rte_kni_common.h +++ b/lib/librte_kni/rte_kni_common.h @@ -48,6 +48,7 @@ struct rte_kni_request { uint8_t promiscusity;/**< 1: promisc mode enable, 0: disable */ uint8_t allmulti; /**< 1: all-multicast mode enable, 0: disable */ }; + int32_t async : 1; /**< 1: request is asynchronous */ int32_t result; /**< Result for processing request */ } __attribute__((__packed__)); From patchwork Mon Mar 29 14:36:55 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ferruh Yigit X-Patchwork-Id: 90019 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: 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]) by inbox.dpdk.org (Postfix) with ESMTP id 2FC79A0547; Mon, 29 Mar 2021 16:37:15 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 93794140DB4; Mon, 29 Mar 2021 16:37:08 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mails.dpdk.org (Postfix) with ESMTP id 67BC3140DA5; Mon, 29 Mar 2021 16:37:06 +0200 (CEST) IronPort-SDR: TbYrxaDisUDWJ800QBon06oXVwK/MgE6wnuvxBYwNxN6Du+iWv5i/3imdOs7E8ndrmGmUezr09 FpNohSEjBUtA== X-IronPort-AV: E=McAfee;i="6000,8403,9938"; a="191589153" X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="191589153" Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Mar 2021 07:37:05 -0700 IronPort-SDR: erv4UKTQHhOH9GFq/NFM9glhzxG0ykD4VSurSTrRcwh8meyjeB79NEAF7sJh9//04xCnqjba1A Dik3zAkvTgmg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,288,1610438400"; d="scan'208";a="526979731" Received: from silpixa00399752.ir.intel.com (HELO silpixa00399752.ger.corp.intel.com) ([10.237.222.27]) by orsmga004.jf.intel.com with ESMTP; 29 Mar 2021 07:37:04 -0700 From: Ferruh Yigit To: dev@dpdk.org Cc: stable@dpdk.org, Elad Nachman , Stephen Hemminger , Igor Ryzhov , Dan Gora Date: Mon, 29 Mar 2021 15:36:55 +0100 Message-Id: <20210329143655.521750-3-ferruh.yigit@intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210329143655.521750-1-ferruh.yigit@intel.com> References: <20201126144613.4986-1-eladv6@gmail.com> <20210329143655.521750-1-ferruh.yigit@intel.com> MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v5 3/3] kni: fix kernel deadlock when using mlx devices 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" KNI runs userspace callback with rtnl lock held, this is not working fine with some devices that needs to interact with kernel interface in the callback, like Mellanox devices. The solution is releasing the rtnl lock before calling the userspace callback. But it requires two consideration: 1. The rtnl lock needs to released before 'kni->sync_lock', otherwise it causes deadlock with multiple KNI devices, please check below the A. for the details of the deadlock condition. 2. When rtnl lock is released for interface down event, it cause a regression and deadlock, so can't release the rtnl lock for interface down event, please check below B. for the details. As a solution, interface down event is handled asynchronously and for all other events rtnl lock is released before processing the callback. A. KNI sync lock is being locked while rtnl is held. If two threads are calling kni_net_process_request() , then the first one will take the sync lock, release rtnl lock then sleep. The second thread will try to lock sync lock while holding rtnl. The first thread will wake, and try to lock rtnl, resulting in a deadlock. The remedy is to release rtnl before locking the KNI sync lock. Since in between nothing is accessing Linux network-wise, no rtnl locking is needed. B. There is a race condition in __dev_close_many() processing the close_list while the application terminates. It looks like if two KNI interfaces are terminating, and one releases the rtnl lock, the other takes it, updating the close_list in an unstable state, causing the close_list to become a circular linked list, hence list_for_each_entry() will endlessly loop inside __dev_close_many() . To summarize: request != interface down : unlock rtnl, send request to user-space, wait for response, send the response error code to caller in user-space. request == interface down: send request to user-space, return immediately with error code of 0 (success) to user-space. Fixes: 3fc5ca2f6352 ("kni: initial import") Cc: stable@dpdk.org Signed-off-by: Elad Nachman --- Cc: Stephen Hemminger Cc: Igor Ryzhov Cc: Dan Gora # kernel/linux/kni/kni_net.c.rej --- kernel/linux/kni/kni_net.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c index 6cf99da0dc92..f259327954b2 100644 --- a/kernel/linux/kni/kni_net.c +++ b/kernel/linux/kni/kni_net.c @@ -113,6 +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(); + } + mutex_lock(&kni->sync_lock); /* Construct data */ @@ -152,6 +160,10 @@ 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); + } return ret; } @@ -194,6 +206,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; + ret = kni_net_process_request(dev, &req); return (ret == 0) ? req.result : ret;