iscsi: Handle unmapped I/O requests.

[jhb]

Summary

Don't assume that csio->data_ptr is pointer to a data buffer that can
be passed to icl_get_pdu_data and icl_append_data. For unmapped I/O
requests, csio->data_ptr is instead a pointer to a struct bio as
indicated by CAM_DATA_BIO. To support these requests, add
icl_pdu_append_bio and icl_pdu_get_bio methods which pass a pointer to
the bio and an offset and length relative to the bio's buffer.

Note that only backends supporting unmapped requests need to implement
these hooks.

Implement simple no-op hooks for the iser backend.

Sponsored by: Chelsio Communications

[jhb]

I tested this in conjunction with a patch to enable unmapped I/O requests on cxgbei. I added counters (in another debugging commit) to verify the new code paths were exercised. I tested this with both fio directly on an disk as well as mounting a UFS filesystem and running iozone. The latter test previously failed with errors when I tried unmapped I/O support in cxgbei without these changes in iscsi(4).

[jhb]

Hmm, I may revisit this to instead add new variants of the functions that take a bio. In particular, that would allow cxgbei to avoid the map/unmap overhead in the DDP case, and if the append cases could also make use of ICL_NOCOPY, this would allow cxgbei to use unmapped mbufs for the payload. I need to see though if ICL_NOCOPY is feasible or not. Presumably the csio's data buffer is "pinned" until the I/O request completes, but perhaps an "outbound" data write completes when the data is queued to be sent.

[jhb]

• Add new icl_conn methods to append/get PDU data from/to bios.

[mav]

It mostly looks good to me, except one comment inline. Theoretically CAM has other addressing methods aside of VADDR and BIO, but those are not really used now, so this is fine for me.

sys/dev/iser/icl_iser.c

120

I guess it is a copy-paste from below, but both cases are wrong, since bhs_opcode is not a bitmask. It should be something like: (request->ip_bhs->bhs_opcode & ~ISCSI_BHS_OPCODE_IMMEDIATE) == ISCSI_BHS_OPCODE_LOGIN_REQUEST ...

[jhb]

sys/dev/iser/icl_iser.c

120

Yes, I think there are other issues in this file. E.g., iser_conn_pdu_get_data() should probably just MPASS that ip_data_mbuf is NULL since I don't see any code that ever sets it to a non-zero value? I might try to fix those as separate commits before this one. I just don't have a way to test them easily. I guess I could try to figure out how to setup iSER as it should work over Chelsio NICs?

[mav]

sys/dev/iser/icl_iser.c

120

I guess Chelsio NIC should be able to do iSER over iWARP if you have respective target, but I've never tried that.

[jhb]

Hmm, apparently we (Chelsio) have never worked with iser, so I might just post a simple patch and let you review it. I suspect part of the code is the way it here for opcodes is that the author mixed up clearing the immediate bit from the opcode.

[jhb]

• Fix opcode check in iser