On 1/25/2021 4:39 PM, 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. > I agree that plumbing these parameters at the PCI side in the devlink of the parent device is weird. They don't seem to be parameters that the parent driver cares about. Maybe there is another mechanism that makes more sense? To me it is a bit like if we were plumbing netdev specific paramters into devlink instead of trying to expose them through netdevice specific interfaces like iproute2 or ethtool.
