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 C0D02A034F; Fri, 26 Feb 2021 18:43:53 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 68DE4407FF; Fri, 26 Feb 2021 18:43:53 +0100 (CET) Received: from mail-io1-f53.google.com (mail-io1-f53.google.com [209.85.166.53]) by mails.dpdk.org (Postfix) with ESMTP id 9AA3040692 for ; Fri, 26 Feb 2021 18:43:51 +0100 (CET) Received: by mail-io1-f53.google.com with SMTP id u20so10458098iot.9 for ; Fri, 26 Feb 2021 09:43:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=EXGXnoNtjF7KlNSRlwV6UoxKKl/8osxgjjaiERsWOD4=; b=ocE4hcAG+APjRJbORNEBOnQNJs0wK27aXUtoVU03EhghwY1Mg2L8TWWCZIv6A5yioP Gaf8wjWhlgadT0SBqVXtuZt69k5ptHjLX6zfMNY7cRg5wfxXcAVVar9EjgTMOAY3hR1Z 54ILoS9yloQWPAwHROeETjERhn4x19d8r+YRlY4gqB0ZyMp/+Tv8L6oq/dPAQeRco2/5 SrrwEBRV9dr9mttavm1HhCLJ3SyIzniW3LN7WBMh0DdPknk/nJ5tyD5aeXqWGdrGDLoU rlgCsjH9LfIiXk4+imhlDboPIdZsfYDkod7IUz/uftw/5fufSRTO0UaF+t9XSNIiQYpp vi+w== 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=EXGXnoNtjF7KlNSRlwV6UoxKKl/8osxgjjaiERsWOD4=; b=gXAZJ4hLvsEWjAPWhKmrcoulOayCmFeYq6jm80DIQ4VjJbMfJgAj7zYMl/7+Zect9H KXsM0dcHQlOA7jjy7DLlRRWPVEF/ge/Dg2mgaOOAM+y4NUchDg2EvrOpF/Uzqm2YLV/z n0yHj6ijT2154TkCjXypkHC2TGLDEgLPJzE41ZVLUrPwLp/ikHpRNvesmBiB6sFVczr+ ShwKEuA4I0pano1u3O5vHl4plf9McbmVCDRCY823zVJiu03xS6FkIf1RkploeJbC5mLH VD02L7+UybfBy5RTl5haVgHYGn0WNJfY54OUK9B7Z1GEfwhyC5fM6vJ8YAHbQxK0yE6y 31dw== X-Gm-Message-State: AOAM530le55iRA+wygdKgMp+FcA8uU45V2DQTrR5jjQXwa7+VqqGGMEQ TWmqOhEdouFd7c4rvWwwtLWSlUWHhttzk71YtCU= X-Google-Smtp-Source: ABdhPJw8YA/PKg0dC3KjAN8yZyP4Jw7T/2+JawduJtEuYGNRIdt62ov6tEYU5nyinHD/CLcuyYCAzRUmHT4w3i7tO0k= X-Received: by 2002:a5e:8d12:: with SMTP id m18mr3734183ioj.47.1614361431001; Fri, 26 Feb 2021 09:43:51 -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: <20210226074817.7e9e0a71@hermes.local> From: Elad Nachman Date: Fri, 26 Feb 2021 19:43:39 +0200 Message-ID: To: Stephen Hemminger Cc: Igor Ryzhov , Ferruh Yigit , dev Content-Type: text/plain; charset="UTF-8" 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" 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.