On Tue, Jan 26, 2021 at 12:39:53AM +0000, Saleem, Shiraz wrote: > > Subject: Re: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and > > implement private channel OPs > > > > On Sun, Jan 24, 2021 at 03:45:51PM +0200, Leon Romanovsky wrote: > > > On Fri, Jan 22, 2021 at 05:48:12PM -0600, Shiraz Saleem wrote: > > > > From: Mustafa Ismail <mustafa.ismail@xxxxxxxxx> > > > > > > > > Register irdma as an auxiliary driver which can attach to auxiliary > > > > RDMA devices from Intel PCI netdev drivers i40e and ice. Implement > > > > the private channel ops, add basic devlink support in the driver and > > > > register net notifiers. > > > > > > Devlink part in "the RDMA client" is interesting thing. > > > > > > The idea behind auxiliary bus was that PCI logic will stay at one > > > place and devlink considered as the tool to manage that. > > > > Yes, this doesn't seem right, I don't think these auxiliary bus objects should have > > devlink instances, or at least someone from devlink land should approve of the > > idea. > > > > In our model, we have one auxdev (for RDMA) per PCI device function > owned by netdev driver and one devlink instance per auxdev. Plus > there is an Intel netdev driver for each HW generation. Moving the > devlink logic to the PCI netdev driver would mean duplicating the > same set of RDMA params in each Intel netdev driver. Additionally, > plumbing RDMA specific params in the netdev driver sort of seems > misplaced to me. That's because it is not supposed to be "the netdev driver" but the shared physical PCI function driver, and devlink is a shared part of the PCI function. > devlink is on a per 'struct device' object right? Should we be > limiting ourselves in its usage to only the PCI driver and PCI dev? > And not other devices like auxdev? The devlink should not be created on the aux device, devlink should be created against PCI functions. It is important to follow establish convention here, otherwise it is a user mess to know what to do Jason
