From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; Sun, 22 Dec 2019 18:56:00 +0100 (CET) Received: by mail-pj1-f65.google.com with SMTP id d5so6448533pjz.5 for ; 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 To: ferruh.yigit@intel.com Cc: dev@dpdk.org, Stephen Hemminger , 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- 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 #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); @@ -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