iSER Connection via LIO not working

Commit Message

Sagi Grimberg June 20, 2017, 7:23 a.m. UTC

> Good job tracking this down.   Thanks!
> 
> static void
> iser_calc_scsi_params(struct iser_conn *iser_conn,
>                        unsigned int max_sectors)
> {
>          struct iser_device *device = iser_conn->ib_conn.device;
>          unsigned short sg_tablesize, sup_sg_tablesize;
> 
>          sg_tablesize = DIV_ROUND_UP(max_sectors * 512, SIZE_4K);
>          sup_sg_tablesize = min_t(unsigned, ISCSI_ISER_MAX_SG_TABLESIZE,
>                                   device->ib_device->attrs.max_fast_reg_page_list_len);
> 
>          iser_conn->scsi_sg_tablesize = min(sg_tablesize, sup_sg_tablesize);
> }
> 
> The bug is that device->ib_device->attrs.max_fast_reg_page_list_len should only be considered valid IFF IB_DEVICE_MEM_MGT_EXTENSIONS is set in
> device->ib_device->device_cap_flags.  The assignment you suggest should be an else on an if test of the device_cap_flags bit.

Hi Guys,

Good job tracking it down!

I think you are the first one to test iSER on top of qib.

Mike, What other variable do we need to look at to determine the
maximum registered size per fmr? I was under the impression that
max_fast_reg_page_list_len is not directly related to
IB_DEVICE_MEM_MGT_EXTENSIONS as fmrs also accepts page lists.

Anyway,
Can we please please please add proper support for wr based memory
registrations in qib? I have a patch piped for removing FMRs from iser
altogether, now that this came up I obviously can't send it...

Looking at the code, looks like qib inherits rdmavt memory registration
with the new api, the only missing piece afaict is the remote invalidate
and qib can turn on IB_DEVICE_MEM_MGT_EXTENSIONS.

Can something like the (untested) below work:
--
                         goto nack_op_err;
@@ -1871,6 +1872,13 @@ void qib_rc_rcv(struct qib_ctxtdata *rcd, struct 
ib_header *hdr,
                         goto no_immediate_data;
                 /* FALLTHROUGH for SEND_ONLY_WITH_IMMEDIATE */
         case OP(SEND_LAST_WITH_IMMEDIATE):
+       case OP(SEND_LAST_WITH_INVALIDATE):
+               if (opcode == OP(SEND_LAST_WITH_INVALIDATE)) {
+                       wc.ex.invalidate_rkey = 
be32_to_cpu(ohdr->u.imm_data);
+                       rvt_invalidate_rkey(qp, wc.ex.invalidate_rkey);
+                       wc.wc_flags = IB_WC_WITH_INVALIDATE;
+                       goto send_last;
+               }
  send_last_imm:
                 wc.ex.imm_data = ohdr->u.imm_data;
                 hdrsize += 4;
--
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marciniszyn, Mike June 20, 2017, 2:10 p.m. UTC | #1

PiANCj4gPiBHb29kIGpvYiB0cmFja2luZyB0aGlzIGRvd24uICAgVGhhbmtzIQ0KPiA+DQo+ID4g
c3RhdGljIHZvaWQNCj4gPiBpc2VyX2NhbGNfc2NzaV9wYXJhbXMoc3RydWN0IGlzZXJfY29ubiAq
aXNlcl9jb25uLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgdW5zaWduZWQgaW50IG1heF9z
ZWN0b3JzKQ0KPiA+IHsNCj4gPiAgICAgICAgICBzdHJ1Y3QgaXNlcl9kZXZpY2UgKmRldmljZSA9
IGlzZXJfY29ubi0+aWJfY29ubi5kZXZpY2U7DQo+ID4gICAgICAgICAgdW5zaWduZWQgc2hvcnQg
c2dfdGFibGVzaXplLCBzdXBfc2dfdGFibGVzaXplOw0KPiA+DQo+ID4gICAgICAgICAgc2dfdGFi
bGVzaXplID0gRElWX1JPVU5EX1VQKG1heF9zZWN0b3JzICogNTEyLCBTSVpFXzRLKTsNCj4gPiAg
ICAgICAgICBzdXBfc2dfdGFibGVzaXplID0gbWluX3QodW5zaWduZWQsIElTQ1NJX0lTRVJfTUFY
X1NHX1RBQkxFU0laRSwNCj4gPiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgZGV2
aWNlLT5pYl9kZXZpY2UtPmF0dHJzLm1heF9mYXN0X3JlZ19wYWdlX2xpc3RfbGVuKTsNCj4gPg0K
PiA+ICAgICAgICAgIGlzZXJfY29ubi0+c2NzaV9zZ190YWJsZXNpemUgPSBtaW4oc2dfdGFibGVz
aXplLCBzdXBfc2dfdGFibGVzaXplKTsNCj4gPiB9DQo+ID4NCj4gPiBUaGUgYnVnIGlzIHRoYXQg
ZGV2aWNlLT5pYl9kZXZpY2UtPmF0dHJzLm1heF9mYXN0X3JlZ19wYWdlX2xpc3RfbGVuDQo+IHNo
b3VsZCBvbmx5IGJlIGNvbnNpZGVyZWQgdmFsaWQgSUZGIElCX0RFVklDRV9NRU1fTUdUX0VYVEVO
U0lPTlMgaXMNCj4gc2V0IGluDQo+ID4gZGV2aWNlLT5pYl9kZXZpY2UtPmRldmljZV9jYXBfZmxh
Z3MuICBUaGUgYXNzaWdubWVudCB5b3Ugc3VnZ2VzdCBzaG91bGQNCj4gYmUgYW4gZWxzZSBvbiBh
biBpZiB0ZXN0IG9mIHRoZSBkZXZpY2VfY2FwX2ZsYWdzIGJpdC4NCj4gDQo+IEhpIEd1eXMsDQo+
IA0KPiBHb29kIGpvYiB0cmFja2luZyBpdCBkb3duIQ0KPiANCj4gSSB0aGluayB5b3UgYXJlIHRo
ZSBmaXJzdCBvbmUgdG8gdGVzdCBpU0VSIG9uIHRvcCBvZiBxaWIuDQo+IA0KPiBNaWtlLCBXaGF0
IG90aGVyIHZhcmlhYmxlIGRvIHdlIG5lZWQgdG8gbG9vayBhdCB0byBkZXRlcm1pbmUgdGhlDQo+
IG1heGltdW0gcmVnaXN0ZXJlZCBzaXplIHBlciBmbXI/IEkgd2FzIHVuZGVyIHRoZSBpbXByZXNz
aW9uIHRoYXQNCj4gbWF4X2Zhc3RfcmVnX3BhZ2VfbGlzdF9sZW4gaXMgbm90IGRpcmVjdGx5IHJl
bGF0ZWQgdG8NCj4gSUJfREVWSUNFX01FTV9NR1RfRVhURU5TSU9OUyBhcyBmbXJzIGFsc28gYWNj
ZXB0cyBwYWdlIGxpc3RzLg0KPiANCg0KSSdtIGdldHRpbmcgcmVhZHkgdG8gc2VuZCBhIHBhdGNo
Lg0KDQpGb3IgZm1ycywgdGhlcmUgaXMgbm8gc3VjaCBpbmRpY2F0aW9uIGF2YWlsYWJsZSBvdGhl
ciB0aGFuIGhhdmluZyB0aGUgVUxQcyB1c2UgdHJpYWwgZm1yIGNyZWF0aW9uIGNhbGxzLg0KDQpG
b3IgcWliLCB0aGUgb25seSBsaW1pdCBpcyBtZW1vcnkgdG8gY3JlYXRlIHRoZSBsaXN0IGVtYmVk
ZGVkIGluIHRoZSBNUi4gICBGb3IgaGZpMSwgdGhlIGxpbWl0IGlzIFVJTlRfTUFYLg0KDQpJIHN1
Z2dlc3QgdGhhdCB3ZSBqdXN0IGhhbmRsZSB0aGUgY2FzZSBvZiBOT1Qgc2VlaW5nIHRoZSBiaXQg
YnkgYXNzdW1pbmcgSVNDU0lfSVNFUl9NQVhfU0dfVEFCTEVfU0laRS4NCg0KQW4gYWx0ZXJuYXRp
dmUgaXMgZm9yIG5vbi1leHRlbnNpb24gZHJpdmVycyB0byBhc3NpZ24gdGhpcyBmaWVsZCBhcyB0
aGV5IHNlZSBmaXQuICAgU29tZSBvdGhlciBkcml2ZXJzIHNlZW0gdG8gaGF2ZSBsaW1pdHMgdGhh
dCB3aWxsIGZhaWwgY3JlYXRpb24uDQoNCm1heF9mYXN0X3JlZ19wYWdlX2xpc3RfbGVuIGlzIHBv
b3JseSBuYW1lZCB0aG91Z2guDQoNCkl0IHNlZW1zIHNpbXBsZXIganVzdCB0byBmaXggaXNlci4u
Lg0KIA0KPiBBbnl3YXksDQo+IENhbiB3ZSBwbGVhc2UgcGxlYXNlIHBsZWFzZSBhZGQgcHJvcGVy
IHN1cHBvcnQgZm9yIHdyIGJhc2VkIG1lbW9yeQ0KPiByZWdpc3RyYXRpb25zIGluIHFpYj8gSSBo
YXZlIGEgcGF0Y2ggcGlwZWQgZm9yIHJlbW92aW5nIEZNUnMgZnJvbSBpc2VyDQo+IGFsdG9nZXRo
ZXIsIG5vdyB0aGF0IHRoaXMgY2FtZSB1cCBJIG9idmlvdXNseSBjYW4ndCBzZW5kIGl0Li4uDQo+
IA0KDQpUaGlzIGNhbm5vdCBiZSBkb25lIHdpdGhvdXQgZHJvcHBpbmcgc3VwcG9ydCBmb3Igb2xk
ZXIgaGFyZHdhcmUuDQoNCnFpYiB2YXJpYW50cyAoYW5kIHBlcmhhcHMgb3RoZXIgaGFyZHdhcmUp
IGNhbm5vdCBkZWFsIHdpdGggdGhlIHJlY2VpdmluZyB0aGUgc2VuZCB3aXRoIGludmFsaWRhdGUg
cGFja2V0cy4NCg0KPiBMb29raW5nIGF0IHRoZSBjb2RlLCBsb29rcyBsaWtlIHFpYiBpbmhlcml0
cyByZG1hdnQgbWVtb3J5IHJlZ2lzdHJhdGlvbg0KPiB3aXRoIHRoZSBuZXcgYXBpLCB0aGUgb25s
eSBtaXNzaW5nIHBpZWNlIGFmYWljdCBpcyB0aGUgcmVtb3RlIGludmFsaWRhdGUNCj4gYW5kIHFp
YiBjYW4gdHVybiBvbiBJQl9ERVZJQ0VfTUVNX01HVF9FWFRFTlNJT05TLg0KDQpUaGUgY29kZSBp
biB0aGUgcGF0Y2ggeW91IHN1Z2dlc3RlZCB3aWxsIG5ldmVyIGdldCBleGVjdXRlZCBiZWNhdXNl
IG9mIHRoZSBhYm92ZSBoYXJkd2FyZSBsaW1pdGF0aW9uLiAgIFRoZSBoYXJkd2FyZSB3aWxsIHF1
ZXVlIGFuIGVycm9yIGFuZCBkcm9wIGFueSBkYXRhLg0KDQpyZG1hdnQgcmVhY3RzIHRvIGNvbnRy
b2wgdGFibGVzIHRvIGhhbmRsZSB0aGUgc3Vib3JkaW5hdGUgZHJpdmVyIGV4dGVuc2lvbiBzdXBw
b3J0IGNvcnJlY3RseS4NCg0KTWlrZQ0KDQoNCg0KDQoNCg0K
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/infiniband/hw/qib/qib_rc.c 
b/drivers/infiniband/hw/qib/qib_rc.c
index 4ddbcac5eabe..2bdfad6ecc11 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -1861,6 +1861,7 @@  void qib_rc_rcv(struct qib_ctxtdata *rcd, struct 
ib_header *hdr,

         case OP(SEND_ONLY):
         case OP(SEND_ONLY_WITH_IMMEDIATE):
+       case OP(SEND_ONLY_WITH_INVALIDATE):
                 ret = qib_get_rwqe(qp, 0);
                 if (ret < 0)