netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	David Ahern <dsahern@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Itay Avraham <itayavr@nvidia.com>,
	Leon Romanovsky <leon@kernel.org>,
	linux-doc@vger.kernel.org, linux-rdma@vger.kernel.org,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Tariq Toukan <tariqt@nvidia.com>,
	Andy Gospodarek <andrew.gospodarek@broadcom.com>,
	Aron Silverton <aron.silverton@oracle.com>,
	Christoph Hellwig <hch@infradead.org>,
	Jiri Pirko <jiri@nvidia.com>, Leonid Bloch <lbloch@nvidia.com>,
	Leon Romanovsky <leonro@nvidia.com>,
	linux-cxl@vger.kernel.org, patches@lists.linux.dev
Subject: Re: [PATCH 0/8] Introduce fwctl subystem
Date: Thu, 6 Jun 2024 11:41:02 -0300	[thread overview]
Message-ID: <20240606144102.GB19897@nvidia.com> (raw)
In-Reply-To: <6661416ed4334_2d412294a1@dwillia2-mobl3.amr.corp.intel.com.notmuch>

On Wed, Jun 05, 2024 at 09:56:14PM -0700, Dan Williams wrote:

> > If people come and say we need X and the maintainer says no, they
> > don't just give up and stop doing X, the go and do X anyhow out of
> > tree. This has become especially true now that the center of business
> > activity in server-Linux is driven by the hyperscale crowd that don't
> > care much about upstream.
> 
> "...don't care much about upstream...". This could be a whole separate
> thread unto itself.

Heh, it is a topic, but perhaps not one for polite company :)

> > Linux maintainer's don't actually have the power to force the industry
> > to do things, though people do keep trying.. Maintainers can only
> > lead, and productive leading is not done with a NO.
> > 
> > You will start to see this pain in maybe 5-10 years if CXL starts to
> > be something deployed in an enterprise RedHat/Dell/etc sort of
> > environment. Then that missing X becomes a critical issue because it
> > turns out the hyperscale folks long since figured out it is really
> > important but didn't do anything to enable it upstream.
> 
> This matches other feedback I have heard recently. Yes, distros hate
> contending with every vendor's userspace toolkit, that was the
> original

I'm not sure that is 100% true. Sure nobody likes that you have to
type 'abc X' and 'def Y' to do a similar thing, but from a distro
perpective if abc and def are both open sourced and packaged in the
distro it is still a far better outcome than users doing OOT drivers
and binary-only tools.

eg one of the long standing main Mellanox tools that is being ported
to fwctl is open source and in all distros:

 https://rpmfind.net/linux/rpm2html/search.php?query=mstflint

Projects have already experimented building tooling on top of it to
make a more cross-vendor experience in some areas.

In my view it is wrong to think the kernel is the only place we can
make generic things or that allowing userspace to see the raw device
interface immediately means fragmentation and chaos. The industry is
more robust than that. Giving people working in userspace room to
invent their own solutions is actually helpful to driving some
commonality. There are already soft targets in the K8S that people
need to fit into, if the first few steps are with abc/def tools and
that brings us to an eventual true commonality, then great.

> distro feedback motivating CONFIG_CXL_MEM_RAW_COMMANDS to have a poison
> pill of WARN() on use. However, allowing more vendor commands is more
> preferable than contending with vendor out-of-tree drivers that likely
> help keep the enterprise-distro-kernel stable-ABI train rolling. In
> other words, legalize it in order to centrally regulate it.

I also liked Jakub's idea of putting a taint in for things that were
likely to have an impact on support and debug, I included that concept
in fwctl.

> > >   Effects Log". In that "trust Command Effects" scenario the kernel still
> > >   has no idea what the command is actually doing, but it can at least
> > >   assert that the device does not claim that the command changes the
> > >   contents of system-memory. Now, you might say, "the device can just
> > >   lie", but that betrays a conceit of the kernel restriction. A device
> > >   could lie that a Linux wrapped command when passed certain payloads does
> > >   not in turn proxy to a restricted command.
> > 
> > Yeah, we have to trust the device. If the device is hostile toward the
> > OS then there are already big problems. We need to allow for
> > unintentional defects in the devices, but we don't need to be
> > paranoid.
> > 
> > IMHO a command effects report, in conjunction with a robust OS centric
> > defintion is something we can trust in.
> 
> So this is where I want to start and see if we can bridge the trust gap.
> 
> I am warming to your assertion that there is a wide array of
> vendor-specific configuration and debug that are not an efficient use of
> upstream's time to wrap in a shared Linux ABI. I want to explore fwctl
> for CXL for that use case, I personally don't want to marshal a Linux
> command to each vendor's slightly different backend CXL toggles.

Personally I think this idea to marshal/unmarshal everything in the
kernel is often misguided. If it is truely obvious and actually shared
multi-vendor capability then by all means go and do it.

But if you are spending weeks/months fighting about uAPI because all
the vendors are so different, it isn't obvious what is "generic" then
you've probably already lost. The very worst outcome is a per-device
uAPI masquerading as an obfuscated "generic" uAPI that wasted ages of
peoples time to argue out.

> At the same time, I also agree with the contention that a "do anything
> you want and get away with it" tunnel invites shenanigans from folks
> that may not care about the long term health of the Linux kernel vs
> their short term interests.

IMHO this is disproven by history. The above mstflint I linked to is
as old as as mlx5 HW, it runs today over PCI config space and an OOT
driver. Where is real the damage to the long term health of Linux or
the ecosystem?

Like I said before I view there is a difference between DRM wanting a
Vulkan stack and doing some device specific
configuration/debugging. One has vastly more open source value than
the other.

> So my questions to try to understand the specific sticking points more
> are:
> 
> 1/ Can you think of a Command Effect that the device could enumerate to
> address the specific shenanigan's that netdev is worried about? 

Nothing comes to mind..

> In other words if every command a device enables has the stated
> effect of "Configuration Change after Reset" does that cut out a
> significant portion of the concern?

Related to configuration - one of Saeed's oringinal ideas was to
implement a devlink command to set the configurables in the flash in a
way that mlx5 could implement all of its options, ideally with
configurables discovered dynamically from the running device. This LPC
presentation was so agressively rejected by Jakub that Saeed abandoned
it. In the discussion it was clear Jakub is requesting to review and
possibly reject every configurable.

On this topic, unfortunately, I don't see any technical middle ground
between "netdev is the gatekeeper for all FLASH configurables" and
"devices can be fully configured regardless of their design".

> 2/ About the "what if the device lies?" question. We can't revert code
> that used to work, but we can definitely work with enterprise distros to
> turn off fwctl where there is concern it may lead or is leading to
> shenanigans. 

Security is the one place where Linus has tolerated userspace
regressions. In this specific case I documented (or at least that was
the intent) there would be regression consequences to breaking the
security rules. Commands can be retroactively restricted to higher CAP
levels and rejected from lockdown if the device attracts a CVE.

IMHO the ecosystem is strongly motived to do security seriously these
days, I am not so worried.

> So, document what each subsystem's stance towards fwctl is,
> like maybe a distro only wants fwctl to front publicly documented vendor
> commands, or maybe private vendor commands ok, but only with a
> constrained set of Command Effects (I potentially see CXL here). 

I wouldn't say subsystem here, but techonology. I think it is
reasonable that a CXL fwctl driver have some kconfig tunables like you
already have. This idea works alot better if the underlying thing is
already standards based.

Linux subsystem isn't a meaningful concept for a multi-function device
like mlx5 and others.

Thanks,
Jason

  parent reply	other threads:[~2024-06-06 14:41 UTC|newest]

Thread overview: 73+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-03 15:53 [PATCH 0/8] Introduce fwctl subystem Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 1/8] fwctl: Add basic structure for a class subsystem with a cdev Jason Gunthorpe
2024-06-04  9:32   ` Leon Romanovsky
2024-06-04 15:50     ` Jason Gunthorpe
2024-06-04 17:05       ` Jonathan Cameron
2024-06-04 18:52         ` Jason Gunthorpe
2024-06-05 11:08           ` Jonathan Cameron
2024-06-04 16:42   ` Randy Dunlap
2024-06-04 16:44     ` Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 2/8] fwctl: Basic ioctl dispatch for the character device Jason Gunthorpe
2024-06-04 12:16   ` Zhu Yanjun
2024-06-04 12:22     ` Leon Romanovsky
2024-06-04 16:50       ` Jonathan Cameron
2024-06-04 16:58         ` Jason Gunthorpe
2024-06-05 11:07           ` Jonathan Cameron
2024-06-05 18:27             ` Jason Gunthorpe
2024-06-06 13:34               ` Jonathan Cameron
2024-06-06 15:37                 ` Randy Dunlap
2024-06-05 15:42   ` Przemek Kitszel
2024-06-05 15:49     ` Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 3/8] fwctl: FWCTL_INFO to return basic information about the device Jason Gunthorpe
2024-06-13 23:32   ` Dave Jiang
2024-06-13 23:40     ` Jason Gunthorpe
2024-06-14 16:37       ` Dave Jiang
2024-06-03 15:53 ` [PATCH 4/8] taint: Add TAINT_FWCTL Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 5/8] fwctl: FWCTL_RPC to execute a Remote Procedure Call to device firmware Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 6/8] fwctl: Add documentation Jason Gunthorpe
2024-06-05  2:31   ` Randy Dunlap
2024-06-05 16:03     ` Jason Gunthorpe
2024-06-05 20:14       ` Randy Dunlap
2024-06-03 15:53 ` [PATCH 7/8] fwctl/mlx5: Support for communicating with mlx5 fw Jason Gunthorpe
2024-06-03 15:53 ` [PATCH 8/8] mlx5: Create an auxiliary device for fwctl_mlx5 Jason Gunthorpe
2024-06-03 18:42 ` [PATCH 0/8] Introduce fwctl subystem Jakub Kicinski
2024-06-04  3:01   ` David Ahern
2024-06-04 14:04     ` Jakub Kicinski
2024-06-04 21:28       ` Saeed Mahameed
2024-06-04 22:32         ` Jakub Kicinski
2024-06-05 14:50           ` Jason Gunthorpe
2024-06-05 15:41             ` Jakub Kicinski
2024-06-04 23:56       ` Dan Williams
2024-06-05  3:05         ` Jakub Kicinski
2024-06-05 11:19         ` Jonathan Cameron
2024-06-05 13:59         ` Jason Gunthorpe
2024-06-06  2:35           ` David Ahern
2024-06-06 14:18             ` Jakub Kicinski
2024-06-06 14:48               ` Jason Gunthorpe
2024-06-06 15:05                 ` Jakub Kicinski
2024-06-06 17:47                   ` David Ahern
2024-06-07  6:48                     ` Jiri Pirko
2024-06-07 14:50                       ` David Ahern
2024-06-07 15:14                         ` Jason Gunthorpe
2024-06-07 15:50                           ` Jiri Pirko
2024-06-07 17:24                             ` Jason Gunthorpe
2024-06-07  7:34               ` Jiri Pirko
2024-06-07 12:49                 ` Andrew Lunn
2024-06-07 13:34                   ` Jiri Pirko
2024-06-08  1:43                     ` Jakub Kicinski
2024-06-06  4:56           ` Dan Williams
2024-06-06  8:50             ` Leon Romanovsky
2024-06-06 22:11               ` Dan Williams
2024-06-07  0:02                 ` Jason Gunthorpe
2024-06-07 13:12                 ` Leon Romanovsky
2024-06-06 14:41             ` Jason Gunthorpe [this message]
2024-06-06 14:58               ` Jakub Kicinski
2024-06-06 17:24               ` Dan Williams
2024-06-07  0:25                 ` Jason Gunthorpe
2024-06-07 10:47                   ` Przemek Kitszel
2024-06-11 15:36           ` Daniel Vetter
2024-06-11 16:17             ` Jason Gunthorpe
2024-06-11 16:54               ` Daniel Vetter
2024-06-06  1:58       ` David Ahern
2024-06-05  3:11 ` Jakub Kicinski
2024-06-05 12:06   ` Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240606144102.GB19897@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=andrew.gospodarek@broadcom.com \
    --cc=aron.silverton@oracle.com \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=dsahern@kernel.org \
    --cc=hch@infradead.org \
    --cc=itayavr@nvidia.com \
    --cc=jiri@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=lbloch@nvidia.com \
    --cc=leon@kernel.org \
    --cc=leonro@nvidia.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=patches@lists.linux.dev \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).