Message-ID: <oxtcvxwbj2hzv4lxnxubo3hoxn7diyzhm2oj3tsw2toxbc3og4@ddglhm6r3oa5>
Date:   Mon, 4 Dec 2023 15:37:56 -0600
From:   Aron Silverton <aron.silverton@...cle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Saeed Mahameed <saeed@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>,
        Jason Gunthorpe <jgg@...dia.com>,
        David Ahern <dsahern@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Leon Romanovsky <leonro@...dia.com>,
        Jiri Pirko <jiri@...dia.com>, Leonid Bloch <lbloch@...dia.com>,
        Itay Avraham <itayavr@...dia.com>,
        linux-kernel@...r.kernel.org, Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH V3 2/5] misc: mlx5ctl: Add mlx5ctl misc driver

Hi,

On Wed, Nov 29, 2023 at 09:08:03AM +0000, Greg Kroah-Hartman wrote:
> On Tue, Nov 28, 2023 at 12:10:00PM -0800, Saeed Mahameed wrote:
> > On 28 Nov 10:33, Jakub Kicinski wrote:
> > > On Tue, 28 Nov 2023 13:52:24 -0400 Jason Gunthorpe wrote:
> > > > > The question at LPC was about making devlink params completely
> > > > > transparent to the kernel. Basically added directly from FW.
> > > > > That what I was not happy about.
> > > > 
> > > > It is creating a back-porting nightmare for all the enterprise
> > > > distributions.
> > > 
> > > We don't care about enterprise distros, Jason, or stable kernel APIs.
> > > 
> > 
> > Oh, I missed this one, so you don't care about users?
> 
> Ok, time out please.  This isn't going anywhere fast except to make
> everyone else mad.
> 
> To Jakub's point, no, we don't care about enterprise distros, they are a
> consumer of our releases and while some of them pay the salaries of our
> developers, they really don't have much influence over our development
> rules as they are just so far behind in releases that their releases
> look nothing like what we do in places (i.e. Linux "like" just like many
> Android systems.)
> 
> If the enterprise distros pop in here and give us their opinions of the
> patchset, I would GREATLY appreciate it, as having more people review
> code at this point in time would be most helpful instead of having to
> hear about how the interfaces are broken 2 years from now.

We will be happy to test and review v4 of this series.

Fully interactive debugging has become essential to getting to the root
cause of complex issues that arise between the types of devices being
discussed and their interactions with various software layers. Turning
knobs and dumping registers just isn't sufficient, and I wish we'd had
this capability long ago.

Our customers have already benefited from the interactive debugging
capability that these patches provide, but the full potential won't be
realized until this is upstream.

> 
> And I think that's what is driving your work now, the "enterprise"
> distros finally picked up the "lock down the kernel from random PCI
> device access in userspace" which caused you to have to drop your
> userspace implementation to create a real kernel driver, correct?
> 
> And as for stable kernel apis, you all know that's just not a thing, and
> has nothing to do with users EXCEPT it benefits users because it keeps
> kernel code smaller and faster overall, that's well documented and users
> appreciate it.
> 
> > Users often pay to distros for support, and distros always turn to vendors
> > for debug situations, in fact one of the high stakeholders for this is an
> > enterprise distro..
> 
> Hey, I was right, an enterprise distro wants this driver, great, can you
> get them to review it as well?  I'm tired of being the only one to
> review patches like this and could use the help if they are going to
> rely on this (why do they pass that work to me?).  They should be the
> ones helping us catch basic things like the reference count issues I
> pointed out, as they can test the code, I can't :)

Guilty as charged? But, I'm sure we are not the only ones. I'm sorry that
we were not able to spot this and provide a review earlier, but we are
happy to do so for v4.

> 
> But, let's step back a bit further.
> 
> It seems the network device normal interface for this is using devlink.
> And drivers are allowed to have driver-specific devlink interfaces, as
> you know, your driver has lots of them today.  Why not just add more?
> What's wrong with 600+ more interfaces being added as that way they
> would be well documented and fit in with the existing infrastructure
> that you have today.
> 
> Is the issue that the firmware doesn't guarantee that these interfaces
> will be documented or stable or even known at this point in time?  If
> so, how are your going to have a good userspace tool for this?  What am
> I missing that requires a totally-new-and-custom user/kernel api from
> what all other network drivers are using today?
> 
> thanks,
> 
> greg k-h

Aron

Powered by blists - more mailing lists