From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id CA5E9A0352;
	Sun, 22 Dec 2019 18:56:03 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id DBF3D37B4;
	Sun, 22 Dec 2019 18:56:01 +0100 (CET)
Received: from mail-pj1-f65.google.com (mail-pj1-f65.google.com
 [209.85.216.65]) by dpdk.org (Postfix) with ESMTP id 0050A25B3
 for <dev@dpdk.org>; Sun, 22 Dec 2019 18:56:00 +0100 (CET)
Received: by mail-pj1-f65.google.com with SMTP id d5so6448533pjz.5
 for <dev@dpdk.org>; Sun, 22 Dec 2019 09:56:00 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=networkplumber-org.20150623.gappssmtp.com; s=20150623;
 h=from:to:cc:subject:date:message-id:mime-version
 :content-transfer-encoding;
 bh=COgfimnoZh66GafJ7QBZ3Cy+7KvkAZojdLcxKBVUGDI=;
 b=T5xdyc59Yih0/ZZTHlZf1Gm1P+Ky+WihUUW5LCPaR3ekfU2o9k7J2Id8JdNY2hvMPV
 88oM71JWGjGHbE+7vUhTMmEKHexgY5dEyXR5h1fuwIULwXbduu0QVPg0K4Wb323fJBSs
 BKIMGdDoFwqA32LL82h1vQ2+8wGNrGVV/jLodIgMpd8N8teQTmr7JbhwT6fjrIfbYei8
 +nq8IfXguKH9WVPgMBF4qv/YRc01Rh2ebPTcrpV4gK14vX0gVGPdqbb/Jh5b7O9INVqh
 zfGtJ6UwcM5UtO0zO2ShEGIjgpIABz1u3Suc5YD7Zq1b4ztV2QhuVZAH4oyOYxctW5H4
 udWA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version
 :content-transfer-encoding;
 bh=COgfimnoZh66GafJ7QBZ3Cy+7KvkAZojdLcxKBVUGDI=;
 b=i/layZr5JfD7UR2I8AEdIqhNM1f6LEhxMES5mnQil7ywwKdpvobineZLNzWlvxuw+r
 mfs48ywab3vsYPr2V1gRbjBYn01THhE2IunX/3i1LttBRN3eqGVIcZPGVPXb8SqLaaEt
 ZgTiNDazqYUYKe0EdA7BqYbT0DZsnzI2n8pyM+smYu3UD8Pf12bHw+XGz8m72yc3/eCT
 SrZdOhamiM32I/oh/gpxjf0C3QBVGWRTs+NFVPGgNTdIx+mf080Jw4GRb4qoWRtPZ+8b
 AX7kTLWLWRbQ/12ifSSs18QMmt8OK+jjGjyew9ERYj/wGZ4lxYoEA4cteZNCZzsFqRPs
 oxkQ==
X-Gm-Message-State: APjAAAXk+p6ac8u7XVdn4belUG4TytobZbqcl0IlilNwYYoXWsVy8Xta
 M7PxmW/jQZR/x1cnYEZLvdIPnA==
X-Google-Smtp-Source: APXvYqwadR6NnF4MiBqVUKnu1aFJueqc0bg84Ihf7FbGq9vjru7XbRikP+N32kK3b3MoojvXbuS3Gw==
X-Received: by 2002:a17:90a:c214:: with SMTP id
 e20mr29072775pjt.98.1577037359878; 
 Sun, 22 Dec 2019 09:55:59 -0800 (PST)
Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127])
 by smtp.gmail.com with ESMTPSA id r30sm18340800pfl.162.2019.12.22.09.55.58
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Sun, 22 Dec 2019 09:55:58 -0800 (PST)
From: Stephen Hemminger <stephen@networkplumber.org>
To: ferruh.yigit@intel.com
Cc: dev@dpdk.org, Stephen Hemminger <stephen@networkplumber.org>,
 stable@dpdk.org
Date: Sun, 22 Dec 2019 09:55:51 -0800
Message-Id: <20191222175551.17684-1-stephen@networkplumber.org>
X-Mailer: git-send-email 2.20.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [dpdk-dev] [PATCH] kni: fix kernel deadlock when using mlx devices
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <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>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

This fixes a deadlock when using KNI with bifurcated drivers.
Bringing kni device up always times out when using Mellanox
devices.

The kernel KNI driver sends message to userspace to complete
the request. For the case of bifurcated driver, this may involve
an additional request to kernel to change state. This request
would deadlock because KNI was holding the RTNL mutex.

This was a bad design which goes back to the original code.
A workaround is for KNI driver to drop RTNL while waiting.
To prevent the device from disappearing while the operation
is in progress, it needs to hold reference to network device
while waiting.

As an added benefit, an useless error check can also be removed.

Fixes: 3fc5ca2f6352 ("kni: initial import")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 kernel/linux/kni/kni_net.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/kernel/linux/kni/kni_net.c b/kernel/linux/kni/kni_net.c
index 1ba9b1b99f66..b7337c1410b8 100644
--- a/kernel/linux/kni/kni_net.c
+++ b/kernel/linux/kni/kni_net.c
@@ -17,6 +17,7 @@
 #include <linux/skbuff.h>
 #include <linux/kthread.h>
 #include <linux/delay.h>
+#include <linux/rtnetlink.h>
 
 #include <rte_kni_common.h>
 #include <kni_fifo.h>
@@ -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);
 
@@ -125,8 +124,17 @@ kni_net_process_request(struct kni_dev *kni, struct rte_kni_request *req)
 		goto fail;
 	}
 
+	/* Since we need to wait and RTNL mutex is held
+	 * drop the mutex and hold refernce to keep device
+	 */
+	dev_hold(dev);
+	rtnl_unlock();
+
 	ret_val = wait_event_interruptible_timeout(kni->wq,
 			kni_fifo_count(kni->resp_q), 3 * HZ);
+	rtnl_lock();
+	dev_put(dev);
+
 	if (signal_pending(current) || ret_val <= 0) {
 		ret = -ETIME;
 		goto fail;
@@ -155,7 +163,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 (dflt_carrier == 1)
@@ -168,7 +175,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 +185,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 +194,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;
 }
@@ -638,14 +644,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;
 
@@ -656,7 +661,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));
 
@@ -678,7 +682,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);
 }
 
 /*
@@ -737,7 +741,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));
@@ -749,8 +752,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);
 }
-- 
2.20.1