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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 54974C433F5 for ; Wed, 29 Sep 2021 15:31:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3A7966140F for ; Wed, 29 Sep 2021 15:31:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345208AbhI2Pdh (ORCPT ); Wed, 29 Sep 2021 11:33:37 -0400 Received: from mail.kernel.org ([198.145.29.99]:40962 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345178AbhI2Pdg (ORCPT ); Wed, 29 Sep 2021 11:33:36 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 87BE661159; Wed, 29 Sep 2021 15:31:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1632929515; bh=HXwoS1HjJA1S0yfWFpWczWoEmqeqJ8/hI4qGlsQfiLE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=a8sMw03Rp6PGGokI/cGFo4lnq8A92MECDtnNRH0qAYiC9K1Vj5DaLvJfztY1rEwx2 J3FjihL9ii52e8Zfc1LoVAXfQPLIFtkcWyusSyw6hrLPxYhHTnB49JgfL17mN5pY50 8YPYUUzVPu6Gkd6TA90y1EU7fcrWlEMBwJGhL7hOX7n7IYjBvJbjvmnzc7myNa6TOJ xOL0M+HYncEtqnaVZr7i4ot3tZbMpp7lbMxXHKZaRL6OYsWT1I2t1Q0jXuKcshj3+i EhRojJsY7u7SHu6Llx2j3/ql9Ac0a4DTztxk62AfY9CPj7ePahwroyGlhgI7Xfabug yDOSXw/s46Y1Q== Date: Wed, 29 Sep 2021 18:31:51 +0300 From: Leon Romanovsky To: Jakub Kicinski Cc: "David S . Miller" , Alexandre Belloni , Andrew Lunn , Ariel Elior , Bin Luo , Claudiu Manoil , Coiby Xu , Derek Chickles , drivers@pensando.io, Eric Dumazet , Felix Manlunas , Florian Fainelli , Geetha sowjanya , Greg Kroah-Hartman , GR-everest-linux-l2@marvell.com, GR-Linux-NIC-Dev@marvell.com, hariprasad , Ido Schimmel , intel-wired-lan@lists.osuosl.org, Ioana Ciornei , Jerin Jacob , Jesse Brandeburg , Jiri Pirko , Jonathan Lemon , Linu Cherian , linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-staging@lists.linux.dev, Manish Chopra , Michael Chan , Moshe Shemesh , netdev@vger.kernel.org, oss-drivers@corigine.com, Richard Cochran , Saeed Mahameed , Salil Mehta , Satanand Burla , Shannon Nelson , Shay Drory , Simon Horman , Subbaraya Sundeep , Sunil Goutham , Taras Chornyi , Tariq Toukan , Tony Nguyen , UNGLinuxDriver@microchip.com, Vadym Kochan , Vivien Didelot , Vladimir Oltean , Yisen Zhuang Subject: Re: [PATCH net-next v1 0/5] Devlink reload and missed notifications fix Message-ID: References: <20210929064004.3172946e@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> <20210929073940.5d7ed022@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210929073940.5d7ed022@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org On Wed, Sep 29, 2021 at 07:39:40AM -0700, Jakub Kicinski wrote: > On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote: > > On Wed, Sep 29, 2021 at 06:40:04AM -0700, Jakub Kicinski wrote: > > > On Wed, 29 Sep 2021 15:00:41 +0300 Leon Romanovsky wrote: > > > > This series starts from the fixing the bug introduced by implementing > > > > devlink delayed notifications logic, where I missed some of the > > > > notifications functions. > > > > > > > > The rest series provides a way to dynamically set devlink ops that is > > > > needed for mlx5 multiport device and starts cleanup by removing > > > > not-needed logic. > > > > > > > > In the next series, we will delete various publish API, drop general > > > > lock, annotate the code and rework logic around devlink->lock. > > > > > > > > All this is possible because driver initialization is separated from the > > > > user input now. > > > > > > Swapping ops is a nasty hack in my book. > > > > > > And all that to avoid having two op structures in one driver. > > > Or to avoid having counters which are always 0? > > > > We don't need to advertise counters for feature that is not supported. > > In multiport mlx5 devices, the reload functionality is not supported, so > > this change at least make that device to behave like all other netdev > > devices that don't support devlink reload. > > > > The ops structure is set very early to make sure that internal devlink > > routines will be able access driver back during initialization (btw very > > questionable design choice) > > Indeed, is this fixable? Or now that devlink_register() was moved to > the end of probe netdev can call ops before instance is registered? > > > and at that stage the driver doesn't know > > yet which device type it is going to drive. > > > > So the answer is: > > 1. Can't have two structures. > > I still don't understand why. To be clear - swapping full op structures > is probably acceptable if it's a pure upgrade (existing pointers match). > Poking new ops into a structure (in alphabetical order if I understand > your reply to Greg, not destructor-before-contructor) is what I deem > questionable. It is sorted simply for readability and not for any other technical reason. Regarding new ops, this is how we are setting callbacks in RDMA based on actual device support. It works like a charm. > > > 2. Same behaviour across all netdev devices. > > Unclear what this is referring to. If your device doesn't support devlink reload, it won't print any reload counters at all. It is not the case for the multiport mlx5 device. It doesn't support, but still present these counters. Thanks