netdev - Re: [Intel-wired-lan] [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support multiple consumers

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: <20250317115728.GT1322339@unreal>
Date: Mon, 17 Mar 2025 13:57:28 +0200
From: Leon Romanovsky <leon@...nel.org>
To: "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
Cc: "Ertman, David M" <david.m.ertman@...el.com>,
	Jakub Kicinski <kuba@...nel.org>,
	"Nikolova, Tatyana E" <tatyana.e.nikolova@...el.com>,
	"jgg@...dia.com" <jgg@...dia.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [iwl-next v4 1/1] iidc/ice/irdma: Update IDC
 to support multiple consumers

On Fri, Mar 14, 2025 at 06:18:00PM -0700, Samudrala, Sridhar wrote:
> 
> 
> On 3/14/2025 11:12 AM, Leon Romanovsky wrote:
> > On Thu, Mar 13, 2025 at 04:38:39PM -0700, Samudrala, Sridhar wrote:
> > > 
> > > 
> > > On 3/2/2025 12:26 AM, Leon Romanovsky wrote:
> > > > On Wed, Feb 26, 2025 at 11:01:52PM +0000, Ertman, David M wrote:
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Leon Romanovsky <leon@...nel.org>
> > > > > > Sent: Wednesday, February 26, 2025 10:50 AM
> > > > > > To: Ertman, David M <david.m.ertman@...el.com>
> > > > > > Cc: Nikolova, Tatyana E <tatyana.e.nikolova@...el.com>; jgg@...dia.com;
> > > > > > intel-wired-lan@...ts.osuosl.org; linux-rdma@...r.kernel.org;
> > > > > > netdev@...r.kernel.org
> > > > > > Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support multiple
> > > > > > consumers
> > > > > > 
> > > > > > On Wed, Feb 26, 2025 at 05:36:44PM +0000, Ertman, David M wrote:
> > > > > > > > -----Original Message-----
> > > > > > > > From: Leon Romanovsky <leon@...nel.org>
> > > > > > > > Sent: Monday, February 24, 2025 11:56 PM
> > > > > > > > To: Nikolova, Tatyana E <tatyana.e.nikolova@...el.com>
> > > > > > > > Cc: jgg@...dia.com; intel-wired-lan@...ts.osuosl.org; linux-
> > > > > > > > rdma@...r.kernel.org; netdev@...r.kernel.org; Ertman, David M
> > > > > > > > <david.m.ertman@...el.com>
> > > > > > > > Subject: Re: [iwl-next v4 1/1] iidc/ice/irdma: Update IDC to support
> > > > > > multiple
> > > > > > > > consumers
> > > > > > > > 
> > > > > > > > On Mon, Feb 24, 2025 at 11:04:28PM -0600, Tatyana Nikolova wrote:
> > > > > > > > > From: Dave Ertman <david.m.ertman@...el.com>
> > > > > > > > > 
> > > > > > > > > To support RDMA for E2000 product, the idpf driver will use the IDC
> > > > > > > > > interface with the irdma auxiliary driver, thus becoming a second
> > > > > > > > > consumer of it. This requires the IDC be updated to support multiple
> > > > > > > > > consumers. The use of exported symbols no longer makes sense
> > > > > > because it
> > > > > > > > > will require all core drivers (ice/idpf) that can interface with irdma
> > > > > > > > > auxiliary driver to be loaded even if hardware is not present for those
> > > > > > > > > drivers.
> > > > > > > > 
> > > > > > > > In auxiliary bus world, the code drivers (ice/idpf) need to created
> > > > > > > > auxiliary devices only if specific device present. That auxiliary device
> > > > > > > > will trigger the load of specific module (irdma in our case).
> > > > > > > > 
> > > > > > > > EXPORT_SYMBOL won't trigger load of irdma driver, but the opposite is
> > > > > > > > true, load of irdma will trigger load of ice/idpf drivers (depends on
> > > > > > > > their exported symbol).
> > > > > > > > 
> > > > > > > > > 
> > > > > > > > > To address this, implement an ops struct that will be universal set of
> > > > > > > > > naked function pointers that will be populated by each core driver for
> > > > > > > > > the irdma auxiliary driver to call.
> > > > > > > > 
> > > > > > > > No, we worked very hard to make proper HW discovery and driver
> > > > > > autoload,
> > > > > > > > let's not return back. For now, it is no-go.
> > > > > > > 
> > > > > > > Hi Leon,
> > > > > > > 
> > > > > > > I am a little confused about what the problem here is.  The main issue I pull
> > > > > > > from your response is: Removing exported symbols will stop ice/idpf from
> > > > > > > autoloading when irdma loads.  Is this correct or did I miss your point?
> > > > > > 
> > > > > > It is one of the main points.
> > > > > > 
> > > > > > > 
> > > > > > > But, if there is an ice or idpf supported device present in the system, the
> > > > > > > appropriate driver will have already been loaded anyway (and gone
> > > > > > through its
> > > > > > > probe flow to create auxiliary devices).  If it is not loaded, then the system
> > > > > > owner
> > > > > > > has either unloaded it manually or blacklisted it.  This would not cause an
> > > > > > issue
> > > > > > > anyway, since irdma and ice/idpf can load in any order.
> > > > > > 
> > > > > > There are two assumptions above, which both not true.
> > > > > > 1. Users never issue "modprobe irdma" command alone and always will call
> > > > > > to whole chain "modprobe ice ..." before.
> > > > > > 2. You open-code module subsystem properly with reference counters,
> > > > > > ownership and locks to protect from function pointers to be set/clear
> > > > > > dynamically.
> > > > > 
> > > > > Ah, I see your reasoning now.  Our goal was to make the two modules independent,
> > > > > with no prescribed load order mandated, and utilize the auxiliary bus and device subsystem
> > > > > to handle load order and unload of one or the other module.  The auxiliary device only has
> > > > > the lifespan of the core PCI driver, so if the core driver unloads, then the auxiliary device gets
> > > > > destroyed, and the associated link based off it will be gone.  We wanted to be able to unload
> > > > > and reload either of the modules (core or irdma) and have the interaction be able to restart with a
> > > > > new probe.  All our inter-driver function calls are protected by device lock on the auxiliary
> > > > > device for the duration of the call.
> > > > 
> > > > Yes, you are trying to return to pre-aux era.
> > > 
> > > 
> > > The main motivation to go with callbacks to the parent driver instead of
> > > using exported symbols is to allow loading only the parent driver required
> > > for a particular deployment. irdma is a common rdma auxilary driver that
> > > supports multiple parent pci drivers(ice, idpf, i40e, iavf). If we use
> > > exported symbols, all these modules will get loaded even on a system with
> > > only idpf device.
> > 
> > It is not how kernel works. IRDMA doesn't call to all exported symbols
> > of all these modules. It will call to only one exported symbol and that
> > module will be loaded.
> 
> If we are using plain exported symbols from all the parent pci drivers and
> make direct calls from irdma, i would expect that all the drivers need to
> load based on module dependencies.

Are you doing it already?

Thanks