From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9555DA055D; Mon, 1 Mar 2021 09:10:16 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5548940684; Mon, 1 Mar 2021 09:10:14 +0100 (CET) Received: from mail-ej1-f50.google.com (mail-ej1-f50.google.com [209.85.218.50]) by mails.dpdk.org (Postfix) with ESMTP id 462AF4014E for ; Mon, 1 Mar 2021 09:10:13 +0100 (CET) Received: by mail-ej1-f50.google.com with SMTP id hs11so26526104ejc.1 for ; Mon, 01 Mar 2021 00:10:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nfware-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=sqXHEuidi9HlrOoxFRxsvae9d8lFSrpqG5fqUjzNu98=; b=VtBe4L0jVtJDPXVCE3y2I6B5l6FEu0RLDBlHCgn+Ciqmnvd0H8cww5heIlG9oC/PmV 6wxu8U4/rHXdB2ws+rHaCjjt0+PLsbfL/GeA3pC8K9v3eCHRrxOxxgJ4DVZxXj4bz2Cs nlsBQeWAWTCtvBZ4Vt17faiJzpT1d6OEmCOhGLhlrY1N4/DAt47v+nzK4dp70nm2My/v MVmZ+Fx7TB+UP6GXV9OtugHek1Kqcg6dGgHeVlxpOFn7rkKebiVY8eoL4ao93yfRhf5f kEpNXfNLt+brlOLcs/MdWNAB3IbrrBw9uVx/sZmR2R8QZoxy8F+mHBkXOiSOEWvhFMUL Gmcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=sqXHEuidi9HlrOoxFRxsvae9d8lFSrpqG5fqUjzNu98=; b=Pli36SrsWvi0QQEgxwsekK/REHjDPI7KEwsSy8VdnaUwqC0ffsF8HupquSJ5lkgtdt YjmJSNN6JawOROWLMkCCZWAO1ssHf2t8UI7MXC5WosZsFQU/tD3ndJwrqSv71mIZxnMy Cg8a/Nuk8Tx9sCaP8gv/ftie56NLBbrKUM9074wOPQfYC5N2YzI2n40ELWVDI1blLiCg YFhCzWdXtU3CQFiYzBRzrlHOhKsVdabGnI7W+iKJcBz5tK174FWtOioih6M+qXgIKX5J II+gwyQriHchkdd/kEOqL8KIsvxSKJ8gnu1nJUgtwshqVi7y8Cd3hhEVTCr1uHnY+1l2 e0gA== X-Gm-Message-State: AOAM533gFNvyoZmXjre7HIiSNFXInjUH4dl3O6zL+HmpxCkleqTiddPt 95F8lxbN4FC02PV2k+cTMc+I/4AXkkD1B1CYLfsVPw== X-Google-Smtp-Source: ABdhPJxWAacKCVnvKc/2bm5iowI3d/ph8x13xTsPodGSjj3Tnrzy4rEU7nxh1ho5Qi9TDI2q9dNLcGGB3zKdSxRpm7A= X-Received: by 2002:a17:906:38d2:: with SMTP id r18mr7568740ejd.104.1614586212871; Mon, 01 Mar 2021 00:10:12 -0800 (PST) MIME-Version: 1.0 References: <20201126144613.4986-1-eladv6@gmail.com> <20210225143239.14220-1-eladv6@gmail.com> <20210225143239.14220-2-eladv6@gmail.com> <20210226074817.7e9e0a71@hermes.local> In-Reply-To: From: Igor Ryzhov Date: Mon, 1 Mar 2021 11:10:01 +0300 Message-ID: To: Elad Nachman Cc: Stephen Hemminger , Ferruh Yigit , dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [dpdk-dev] [PATCH 2/2] kni: fix rtnl deadlocks and race conditions v4 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" Stephen, No, I don't have a better proposal, but I think it is not correct to change the behavior of KNI (making link down without a real response). Even though we know that communicating with userspace under rtnl_lock is a bad idea, it works as it is for many years already. Elad, I agree with you that KNI should be removed from the main tree if it is not possible to fix this __dev_close_many issue. There were discussions about this multiple times already, but no one is working on this AFAIK. Last time the discussion was a month ago: https://www.mail-archive.com/dev@dpdk.org/msg196033.html Igor On Fri, Feb 26, 2021 at 8:43 PM Elad Nachman wrote: > The way the kernel handles its locks and lists for the dev close many > path, there is no way you can go around this with rtnl unlocked : > " > > There is a race condition in __dev_close_many() processing the > close_list while the application terminates. > It looks like if two vEth devices 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() . > > " > And I don't expect David Miller will bend the kernel networking for DPDK > or KNI. > > But - Stephen - if you can personally convince David to accept a > kernel patch which will separate the close_list locking mechanism to a > separate (RCU?) lock, then I can introduce first a patch to the kernel > which will add a lock for the close_list, this way rtnl can be > unlocked for the if down case. > > After that kernel patch, your original patch + relocation of the sync > mutex locking will do the job . > > Otherwise, rtnl has to be kept locked all of the way for the if down > case in order to prevent corruption causing a circular linked list out > of the close_list, causing a hang in the kernel. > > Currently, the rtnl lock is the only thing keeping the close_list from > corruption. > > If you doubt rtnl cannot be unlocked for dev close path, you can > consult David for his opinion, as I think it is critical to understand > what the kernel can or cannot do, or expects to be done before we can > unlock its locks as we wish inside rte_kni.ko . > > Otherwise, if we are still in disagreement on how to patch this set of > problems, I think the responsible way around it is to completely > remove kni from the main dpdk tree and move it to dpdk-kmods > repository. > > I know BSD style open-source does not carry legal responsibility from > the developers, but I think when a bunch of developers know a piece of > code is highly buggy, they should not leave it for countless new users > to bounce their head desperately against, if they cannot agree on a > correct way to solve the bunch of problems, of which I think we all > agree exist (we just do not agree on the proper solution or patch)... > > That's my two cents, > > Elad. > > On Fri, Feb 26, 2021 at 5:49 PM Stephen Hemminger > wrote: > > > > On Fri, 26 Feb 2021 00:01:01 +0300 > > Igor Ryzhov wrote: > > > > > Hi Elad, > > > > > > Thanks for the patch, but this is still NACK from me. > > > > > > The only real advantage of KNI over other exceptional-path techniques > > > like virtio-user is the ability to configure DPDK-managed interfaces > > > directly > > > from the kernel using well-known utils like iproute2. A very important > part > > > of this is getting responses from the DPDK app and knowing the actual > > > result of command execution. > > > If you're making async requests to the application and you don't know > > > the result, then what's the point of using KNI at all? > > > > > > Igor > > > > Do you have a better proposal that keeps the request result but does not > > call userspace with lock held. > > > > PS: I also have strong dislike of KNI, as designed it would have been > rejected > > by Linux kernel developers. A better solution would be userspace > version of > > something like devlink devices. But doing control operations by proxy is > > a locking nightmare. >