On Wed, Mar 24, 2021 at 11:46:42PM +0000, Saleem, Shiraz wrote:

quoted

Subject: Re: [PATCH v2 08/23] RDMA/irdma: Register auxiliary driver and
implement private channel OPs

On Wed, Mar 24, 2021 at 04:17:20PM +0200, Leon Romanovsky wrote:

quoted

On Wed, Mar 24, 2021 at 11:00:46AM -0300, Jason Gunthorpe wrote:

quoted

On Wed, Mar 24, 2021 at 03:47:34PM +0200, Leon Romanovsky wrote:

quoted

On Tue, Mar 23, 2021 at 06:59:52PM -0500, Shiraz Saleem wrote:

quoted

From: Mustafa Ismail <redacted>

Register auxiliary drivers which can attach to auxiliary RDMA
devices from Intel PCI netdev drivers i40e and ice. Implement
the private channel ops, and register net notifiers.

Signed-off-by: Mustafa Ismail <redacted>
Signed-off-by: Shiraz Saleem <redacted>
drivers/infiniband/hw/irdma/i40iw_if.c | 229 +++++++++++++
 drivers/infiniband/hw/irdma/main.c     | 382 ++++++++++++++++++++++
 drivers/infiniband/hw/irdma/main.h     | 565

+++++++++++++++++++++++++++++++++

quoted

quoted

quoted

quoted

 3 files changed, 1176 insertions(+)  create mode 100644
drivers/infiniband/hw/irdma/i40iw_if.c
 create mode 100644 drivers/infiniband/hw/irdma/main.c
 create mode 100644 drivers/infiniband/hw/irdma/main.h

<...>

quoted

+/* client interface functions */ static const struct
+i40e_client_ops i40e_ops = {
+	.open = i40iw_open,
+	.close = i40iw_close,
+	.l2_param_change = i40iw_l2param_change };
+
+static struct i40e_client i40iw_client = {
+	.ops = &i40e_ops,
+	.type = I40E_CLIENT_IWARP,
+};
+
+static int i40iw_probe(struct auxiliary_device *aux_dev, const
+struct auxiliary_device_id *id) {
+	struct i40e_auxiliary_device *i40e_adev = container_of(aux_dev,
+							       struct

i40e_auxiliary_device,

quoted

quoted

quoted

quoted

+							       aux_dev);
+	struct i40e_info *cdev_info = i40e_adev->ldev;
+
+	strncpy(i40iw_client.name, "irdma", I40E_CLIENT_STR_LENGTH);
+	cdev_info->client = &i40iw_client;
+	cdev_info->aux_dev = aux_dev;
+
+	return cdev_info->ops->client_device_register(cdev_info);

Why do we need all this indirection? I see it as leftover from
previous version where you mixed auxdev with your peer registration logic.

I think I said the new stuff has to be done sanely, but the i40iw
stuff is old and already like this.

They declared this specific "ops" a couple of lines above and all the
functions are static. At least for the new code, in the irdma, this "ops"
thing is not needed.

It is the code in the 'core' i40iw driver that requries this, AFAICT

 Yes.

It is worth to fix.

Thanks