On Mon, 2018-04-30 at 09:34 -0500, Steve Wise wrote: > Hey Bart, I see this warning when srpt adds a 4 port cxgb4 rnic: > > [ 1247.223340] WARNING: CPU: 2 PID: 7639 at > drivers/infiniband/ulp/srpt/ib_srpt.c:3026 srpt_add_one+0x3fa/0x410 [ib_srpt] > > from srpt_add_one(): > ... > WARN_ON(sdev->device->phys_port_cnt > ARRAY_SIZE(sdev->port)); > ... > > It looks like the srpt code assumes only two port cards: > > struct srpt_device { > struct ib_device *device; > struct ib_pd *pd; > u32 lkey; > struct ib_srq *srq; > struct ib_cm_id *cm_id; > int srq_size; > struct mutex sdev_mutex; > bool use_srq; > struct srpt_recv_ioctx **ioctx_ring; > struct srpt_port port[2]; <<-------- HERE > struct ib_event_handler event_handler; > struct list_head list; > }; > > That assumption is certainly invalid since Chelsio has 4 port adapters. > But this also begs the question if ib_srp and ib_srpt should actually > try and use iWARP devices? Isn't it an IB-only protocol? Hello Steve, Can you submit a patch that increases the size of that array or do you perhaps prefer that I do that myself? I think that code was written before quad port adapters appeared on the market. Regarding your question about the SRP protocol: the only aspects of SRP that are specific to InfiniBand are the target port discovery mechanism (because it uses MADs) and also the format of the login data (which is too large to fit into the RDMA/CM login data buffer). Other aspects of the SRP protocol do not depend on the IB protocol. As you may have noticed RDMA/CM support for the SRP initiator was added in kernel v4.16 and RDMA/CM support for the SRP target driver was added in kernel v4.17-rc1. I have not yet tried to test this code with the iWARP transport. But it is easy to test this code against RoCE as follows: * Clone https://github.com/bvanassche/srp-test * Run the following command: srp-test/run_tests -c This is what I know about how the SRP drivers compare to the iSER and NVMeOF drivers: - The SRP initiator driver has scsi-mq support since considerable time. - Both the SRP initiator and target drivers have an excellent reputation with regard to stability. - There have been plans to add scsi-mq support to the iSER initiator driver but these plans have been shelved. See e.g. "[LSF/MM TOPIC] iSCSI MQ adoption via MCS discussion" (https://www.mail-archive.com/open-iscsi@xxxxxxxxxxxxxxxx/msg08723.html). - Maintaining the ib_isert code base is challenging because of the complexity of the iSCSI target driver code that driver is based on. - Although I trust that over time the NVMeOF initiator and target drivers will achieve the same level of stability as the SRP initiator and target drivers, I don't think that this has already been achieved. Bart. ��.n��������+%������w��{.n�����{���fk��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f