netdev - Re: [PATCH v4 07/10] fwctl/mlx5: Support for communicating with mlx5 fw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250213191853.GH3885104@nvidia.com>
Date: Thu, 13 Feb 2025 15:18:53 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: Andy Gospodarek <andrew.gospodarek@...adcom.com>,
	Aron Silverton <aron.silverton@...cle.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	Dave Jiang <dave.jiang@...el.com>, David Ahern <dsahern@...nel.org>,
	Andy Gospodarek <gospo@...adcom.com>,
	Christoph Hellwig <hch@...radead.org>,
	Itay Avraham <itayavr@...dia.com>, Jiri Pirko <jiri@...dia.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Jakub Kicinski <kuba@...nel.org>, Leonid Bloch <lbloch@...dia.com>,
	Leon Romanovsky <leonro@...dia.com>, linux-cxl@...r.kernel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	Saeed Mahameed <saeedm@...dia.com>,
	"Nelson, Shannon" <shannon.nelson@....com>
Subject: Re: [PATCH v4 07/10] fwctl/mlx5: Support for communicating with mlx5
 fw

On Thu, Feb 13, 2025 at 02:19:38PM +0100, Przemek Kitszel wrote:
> On 2/7/25 01:13, Jason Gunthorpe wrote:
> > From: Saeed Mahameed <saeedm@...dia.com>
> 
> In part this is a general feedback for the subsystem too.
> 
> > +FWCTL MLX5 DRIVER
> 
> I don't like this design.
> That way each and every real driver would need to make another one to
> just use fwctl.

It is not mandatory, drivers could call fwctl_register() directly from
a pci probe function. That is just very undesirable for a secondary
subsystem like fwctl for reasons Leon explained. We want loose
coupling controled by modules and driver binding, not strong coupling
where if you load, say, mlx5_core, you get a million other modules
automatically pulled in as well. Users should have control over this.

> Or the intention is to have this little driver replaced by OOT one,
> but keep the real (say networking) driver as-is from intree?

The design of the FW interface would have to be really off to motivate
an OOT one. IMHO you are more likely to see an intree fwctl driver and
maybe an OOT netdev or something.
 
> > +++ b/drivers/fwctl/mlx5/main.c
> > @@ -0,0 +1,340 @@
> > +// SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
> > +/*
> > + * Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES
> 
> -2025

Time flies, thanks

> 
> > + */
> > +#include <linux/fwctl.h>
> > +#include <linux/auxiliary_bus.h>
> > +#include <linux/mlx5/device.h>
> > +#include <linux/mlx5/driver.h>
> 
> this breaks abstraction (at least your headers are in nice place, but
> this is rather uncommon, typical solution is to have them backed inside
> the driver directory) - the two drivers will be tightly coupled

It is part of the auxdev methodology. These headers pre-exist for all
the other mlx5 family aux devices to use.

> >   enum fwctl_device_type {
> >   	FWCTL_DEVICE_TYPE_ERROR = 0,
> > +	FWCTL_DEVICE_TYPE_MLX5 = 1,
> 
> is that for fwctl info to be able to properly report what device user
> has asked ioctl on?

Yes.

> Would be great to embed 32byte long cstring of
> DRIVER_NAME, to don't need each and every device to come to you and
> ask for inclusion, 

I think we want people to have to ask though, don't we? We don't want
to make it easy to write OOT drivers.

> that would also resolve problem of conflicting IDs (my-driver-id
> prior-to and after upstreaming)

Yes it would, but I suggest people get their driver posted before they
start shipping it :)

Jonathan had suggested using a uuid IIRC for the same reason.

Jason