On Tue, Dec 8, 2009 at 12:35 AM, FUJITA Tomonori <fujita.tomonori@xxxxxxxxxxxxx> wrote: > On Sun, 6 Dec 2009 11:07:03 +0100 > Bart Van Assche <bart.vanassche@xxxxxxxxx> wrote: > >> While reverse engineering the libsrp source code I came across the >> following statement in the function called srp_transfer_data(): >> >> offset = cmd->add_cdb_len * 4; >> >> This offset is computed such that the code below it can skip the >> ADDITIONAL CDB section in SRP_CMD requests. According to the header >> file include/scsi/srp.h, the field add_cdb_len corresponds to the >> one-byte field at byte offset 31 in the SRP_CMD request. And according >> to the SRP spec (draft r16a), the layout of the field at offset 31 is >> as follows: >> * Bits 0 and 1 are reserved. >> * Bits 2 to 7 represent the ADDITIONAL CDB LENGTH field, symbolically >> represented as n. >> The SRP spec also makes clear that the ADDITIONAL CDB section takes 4*n bytes. >> >> If my interpretation of the srp.h header file and the SRP spec is >> correct, the above statement should be replaced by: >> >> offset = (cmd->add_cdb_len >> 2) * 4; >> >> or, equivalently: >> >> offset = cmd->add_cdb_len & ~3; > > Oops, good catch. > > Either is fine by me. The reserved bits shall be set to zero in SCSI > though. OK, I will send a patch. >> Not ignoring the lower two bits of the add_cdb_len field would be >> dangerous because at least the ibmvscsi driver uses these bits as >> flags. > > Hmm, where does the ibmvscsi driver use these bits? I might have confused the fields ibmvfc_cmd::add_cdb_len and srp_cmd::add_cdb_len. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
