From: "Nikolova, Tatyana E" <tatyana.e.nikolova@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: "dledford@redhat.com" <dledford@redhat.com>,
"leon@kernel.org" <leon@kernel.org>,
"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: RE: [PATCH v2 rdma-core] irdma: Add ice and irdma to kernel-boot rules
Date: Wed, 2 Nov 2022 16:40:20 +0000 [thread overview]
Message-ID: <DM6PR11MB4692B502C54F459A2EF9E79CCB399@DM6PR11MB4692.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20211014233644.GA2744544@nvidia.com>
Hi Jason,
I know it has been a while since we discussed this. Based on your feedback, we are proposing another solution for the irdma kernel-boot rules. Could you please review it?
> > udevadm info --attribute-walk /sys/class/infiniband/rocep47s0f0
> >
> > looking at device
> '/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/infiniband/rocep47s0f0':
>
> This looks like the problem. For any of this to work the infiniband
> device needs to be parented to the aux device, not the PCI device.
>
> mlx5 did not due this for backwards compat reasons, but this is a new
> driver so it could do it properly.
>
> Then you can use SUBSYSTEMS and so forth as I first suggested.
Here is a patch for irdma driver to set aux_dev as the ibdev parent:
diff --git a/drivers/infiniband/hw/irdma/i40iw_if.c b/drivers/infiniband/hw/irdma/i40iw_if.c
index 4053ead32416..e08fbfe148ea 100644
--- a/drivers/infiniband/hw/irdma/i40iw_if.c
+++ b/drivers/infiniband/hw/irdma/i40iw_if.c
@@ -90,6 +90,7 @@ static void i40iw_fill_device_info(struct irdma_device *iwdev, struct i40e_info
iwdev->rcv_wnd = IRDMA_CM_DEFAULT_RCV_WND_SCALED;
iwdev->rcv_wscale = IRDMA_CM_DEFAULT_RCV_WND_SCALE;
iwdev->netdev = cdev_info->netdev;
+ iwdev->aux_dev = cdev_info->aux_dev;
iwdev->vsi_num = 0;
}
diff --git a/drivers/infiniband/hw/irdma/main.c b/drivers/infiniband/hw/irdma/main.c
index 514453777e07..835a087a3329 100644
--- a/drivers/infiniband/hw/irdma/main.c
+++ b/drivers/infiniband/hw/irdma/main.c
@@ -279,6 +279,7 @@ static int irdma_probe(struct auxiliary_device *aux_dev, const struct auxiliary_
}
irdma_fill_device_info(iwdev, pf, vsi);
+ iwdev->aux_dev = aux_dev;
rf = iwdev->rf;
err = irdma_ctrl_init_hw(rf);
diff --git a/drivers/infiniband/hw/irdma/main.h b/drivers/infiniband/hw/irdma/main.h
index 65e966ad3453..f2f86b882cef 100644
--- a/drivers/infiniband/hw/irdma/main.h
+++ b/drivers/infiniband/hw/irdma/main.h
@@ -329,6 +329,7 @@ struct irdma_device {
struct ib_device ibdev;
struct irdma_pci_f *rf;
struct net_device *netdev;
+ struct auxiliary_device *aux_dev;
struct workqueue_struct *cleanup_wq;
struct irdma_sc_vsi vsi;
struct irdma_cm_core cm_core;
diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index a22afbb25bc5..f1304b6b58b3 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -4549,7 +4549,6 @@ static int irdma_init_iw_device(struct irdma_device *iwdev)
*/
static int irdma_init_rdma_device(struct irdma_device *iwdev) {
- struct pci_dev *pcidev = iwdev->rf->pcidev;
int ret;
if (iwdev->roce_mode) {
@@ -4561,7 +4560,7 @@ static int irdma_init_rdma_device(struct irdma_device *iwdev)
}
iwdev->ibdev.phys_port_cnt = 1;
iwdev->ibdev.num_comp_vectors = iwdev->rf->ceqs_count;
- iwdev->ibdev.dev.parent = &pcidev->dev;
+ iwdev->ibdev.dev.parent = &iwdev->aux_dev->dev;
ib_set_device_ops(&iwdev->ibdev, &irdma_dev_ops);
return 0;
Here is the udev output after this change:
looking at device
'/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/ice.roce.0/infiniband/rocep47s0f0':
KERNEL=="rocep47s0f0"
SUBSYSTEM=="infiniband"
DRIVER==""
ATTR{fw_ver}=="1.48"
ATTR{node_desc}==""
ATTR{node_guid}=="6a05:caff:fec1:c790"
ATTR{node_type}=="1: CA"
ATTR{sys_image_guid}=="6a05:caff:fec1:c790"
looking at parent device
'/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0/ice.roce.0':
KERNELS=="ice.roce.0"
SUBSYSTEMS=="auxiliary"
DRIVERS=="irdma"
looking at parent device
'/devices/pci0000:2e/0000:2e:00.0/0000:2f:00.0':
KERNELS=="0000:2f:00.0"
SUBSYSTEMS=="pci"
DRIVERS=="ice"
Having the aux_dev as a parent of the irdma ibdev allows for udev rules to set ID_RDMA_IWARP or ID_RDMA_ROCE environment vars based on the auxiliary device name with the following patch:
diff --git a/kernel-boot/rdma-description.rules b/kernel-boot/rdma-description.rules
index 48a7ced..0b844bd 100644
--- a/kernel-boot/rdma-description.rules
+++ b/kernel-boot/rdma-description.rules
@@ -29,6 +29,8 @@ DRIVERS=="i40e", ENV{ID_RDMA_IWARP}="1"
DRIVERS=="be2net", ENV{ID_RDMA_ROCE}="1"
DRIVERS=="bnxt_en", ENV{ID_RDMA_ROCE}="1"
DRIVERS=="hns", ENV{ID_RDMA_ROCE}="1"
+DRIVERS=="irdma", KERNELS=="*iw*", ENV{ID_RDMA_IWARP}="1"
+DRIVERS=="irdma", KERNELS=="*roce*", ENV{ID_RDMA_ROCE}="1"
DRIVERS=="mlx4_core", ENV{ID_RDMA_ROCE}="1"
DRIVERS=="mlx5_core", ENV{ID_RDMA_ROCE}="1"
DRIVERS=="qede", ENV{ID_RDMA_ROCE}="1"
In addition to this patch, changes to rdma-core rdma_rename logic are necessary to handle ib devices with aux_dev parent and rename them by_onboard or by_pci the same way as it did before introducing the aux_dev parent.
Please review rdma-core PR at https://github.com/linux-rdma/rdma-core/pull/1248
Thank you,
Tatyana
next prev parent reply other threads:[~2022-11-02 16:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-23 15:48 [PATCH v2 rdma-core] irdma: Add ice and irdma to kernel-boot rules Tatyana Nikolova
2021-08-23 16:11 ` Jason Gunthorpe
2021-09-02 15:29 ` Nikolova, Tatyana E
2021-09-02 15:40 ` Jason Gunthorpe
2021-09-20 19:41 ` Nikolova, Tatyana E
2021-09-20 23:23 ` Jason Gunthorpe
2021-10-14 20:11 ` Nikolova, Tatyana E
2021-10-14 23:36 ` Jason Gunthorpe
2022-11-02 16:40 ` Nikolova, Tatyana E [this message]
2022-11-09 13:45 ` Jason Gunthorpe
2023-01-13 23:57 ` Saleem, Shiraz
2023-01-14 0:01 ` Jason Gunthorpe
2023-01-17 20:27 ` Saleem, Shiraz
2023-01-20 18:02 ` Jason Gunthorpe
2021-09-02 16:03 ` Leon Romanovsky
2021-09-02 16:13 ` Nikolova, Tatyana E
2021-09-02 23:23 ` Leon Romanovsky
2021-10-10 9:42 ` Leon Romanovsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB4692B502C54F459A2EF9E79CCB399@DM6PR11MB4692.namprd11.prod.outlook.com \
--to=tatyana.e.nikolova@intel.com \
--cc=dledford@redhat.com \
--cc=jgg@nvidia.com \
--cc=leon@kernel.org \
--cc=linux-rdma@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).