Re: [PATCH V3 2/5] misc: mlx5ctl: Add mlx5ctl misc driver

Greg Kroah-Hartman
2023-11-29T09:08:20Z

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.

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 :)

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