From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 31B9FC4338F for ; Thu, 5 Aug 2021 18:02:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1207C60E96 for ; Thu, 5 Aug 2021 18:02:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240416AbhHESCn (ORCPT ); Thu, 5 Aug 2021 14:02:43 -0400 Received: from mail.kernel.org ([198.145.29.99]:43126 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233016AbhHESCm (ORCPT ); Thu, 5 Aug 2021 14:02:42 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 5236760E96; Thu, 5 Aug 2021 18:02:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628186548; bh=AZj4JmN/N5elWfE7nTxhmcna7JfDIXrkufu9lK3jRyU=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=itsgFtLBc+BylJgoU+IFkfBJesGvVcb9Ck5GtU0nRIMTVk12igT0qaBXCa6m9A6Yg RGMIEUpXV9QDnMDwsJThbXCZ0gGoLkmkSTrCTPtBfvkO+UDZDORc/euNE7b8MCvE+l GeIp/AAkdIfgwj+/EQhZQH6Zm3EelI6++CpHRFdrV1/XL7dDcyR3M1Ldc7iXbRbPmt dIv/P0V0bCHcwRn8PK05tls7QJwB+5MsRA4WDYwo7UkMbSeo4O0qWp+T6EIK4foGsz ksu8+HoLfJw7NuqD+FrSCbAlgw3Yn9tlfjJ5tGUCipjkQxBVgXTchuARtfKSlcinQj Jc3Uoz+jbS2zA== Date: Thu, 5 Aug 2021 21:02:23 +0300 From: Leon Romanovsky To: Jakub Kicinski Cc: "David S . Miller" , linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: Re: [PATCH net-next v1] netdevsim: Forbid devlink reload when adding or deleting ports Message-ID: References: <53cd1a28dd34ced9fb4c39885c6e13523e97d62c.1628161323.git.leonro@nvidia.com> <20210805061547.3e0869ad@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <20210805072342.17faf851@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <20210805082756.0b4e61d7@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, Aug 05, 2021 at 08:35:59PM +0300, Leon Romanovsky wrote: > On Thu, Aug 05, 2021 at 08:27:56AM -0700, Jakub Kicinski wrote: > > On Thu, 5 Aug 2021 17:33:35 +0300 Leon Romanovsky wrote: > > > On Thu, Aug 05, 2021 at 07:23:42AM -0700, Jakub Kicinski wrote: > > > > > This is what devlink_reload_disable() returns, so I kept same error. > > > > > It is not important at all. > > > > > > > > > > What about the following change on top of this patch? > > > > > > > > LGTM, the only question is whether we should leave in_reload true > > > > if nsim_dev->fail_reload is set. > > > > > > I don't think so, it will block add/delete ports. > > > > As it should, given add/delete ports takes the port_list_lock which is > > destroyed by down but not (due to the forced failure) re-initialized by > > up. > > > > If we want to handle adding ports while down we can just bump port > > count and return, although I don't think there's a practical need > > to support that. > > Sorry, but for me netdevsim looks like complete dumpster. It was > intended for fast prototyping, but ended to be huge pile of debugfs > entries and selftest to execute random flows. > > Do you want me to move in_reload = false line to be after if (nsim_dev->fail_reload) > check? BTW, the current implementation where in_reload before if, actually preserves same behaviour as was with devlink_reload_enable() implementation. Thanks > > Thanks