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:

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.
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.

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.

Are you sure you need to implement this?

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