RE: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and implement private channel OPs

Subject: Re: [PATCH 07/22] RDMA/irdma: Register an auxiliary driver and
implement private channel OPs

On Mon, Jan 25, 2021 at 05:01:40PM -0800, Jacob Keller wrote:

quoted

On 1/25/2021 4:39 PM, Saleem, Shiraz wrote:

quoted

quoted

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:

quoted

On Fri, Jan 22, 2021 at 05:48:12PM -0600, Shiraz Saleem wrote:

quoted

From: Mustafa Ismail <redacted>

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.

quoted

quoted

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.

quoted

quoted

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.

It does, the PCI driver is not supposed to spawn any aux devices for RDMA at all
if RDMA is disabled.

For an iWarp driver I would consider ENABLE_ROCE to really be a general
ENABLE_RDMA.

Well the driver supports iWARP and RoCE for E810 device.
Are you saying that this generic enable_roce devlink param really
is an enable 'rdma' traffic or not param?

Are you sure you need to implement this?

What we are after is some mechanism for user to switch the protocols iWARP vs RoCE
[default the device comes up as an iWARP dev]. The protocol info is really needed early-on
in the RDMA driver.probe(). i.e. when the rdma admin queue is created.

The same goes with the other param resource_limits_selector. It's a profile selector that a user
can chose to different # of max QP, CQs, MRs etc.

In any event, you just can't put the generic ENABLE_ROCE flag anyplace but the
PCI device for devlink, it breaks the expected user API established by mlx5

Jason