On Thu, 2014-03-06 at 12:51 +0200, Sagi Grimberg wrote: > On 3/5/2014 11:38 PM, Nicholas A. Bellinger wrote: > > On Wed, 2014-03-05 at 15:38 +0200, Sagi Grimberg wrote: > >> On 2/19/2014 5:50 PM, Sagi Grimberg wrote: <SNIP> > >> It seems that loopback functionality was broken in this one (but it was > >> somewhat wrong to start with). > >> The thing is, the startup READs are not passed with protection buffers > >> and asks the LLD to insert/strip DIF (SCSI_PROT_READ_STRIP) > >> and a couple of READs later DIX is enabled and starts passing protection > >> information (SCSI_PROT_READ_PASS). > >> Since loopback initiator is not aware of this, startup READs will fail > >> in this commit. Same for disabling write_generate/read_verify. > >> > >> I think that the loopback initiator should conform to the protection > >> operation in scsi_cmnd. > >> Meaning it should INSERT/STRIP/PASS according to what was stated in the > >> SCSI command: > >> SCSI_PROT_READ_STRIP: should allocate protection protection sg and > >> execute target_submit_cmd_map_sgls(). > >> SCSI_PROT_WRITE_GENERATE: should allocate protection protection sg, > >> compute protection and execute target_submit_cmd_map_sgls(). > >> SCSI_PROT_READ_INSERT: don't know if worth the bother for loopback. > >> SCSI_PROT_WRITE_STRIP: don't know if worth the bother for loopback. > >> SCSI_PROT_READ_PASS: stay the same. > >> SCSI_PROT_WRITE_PASS: stay the same. > >> > >> The alternative is to avoid this patch but then we might miss DIF > >> support when working against legacy initiators. > >> > >> Thoughts? > > Mmmm, not sure about this one.. > > > > Considering that it's only the first two READ_10s that this effects > > before normal SCSI_PROT_*_PASS operation kicks in, I'm not sure if it's > > worth it to add these to tcm_loop.. > > Note that it won't be operational when disabling > read_verify/write_generate via /sys/block/$DEV/integrity/ as well. > Mmm, good point. > > Note that virtio-scsi + vhost-scsi > > would need similar bits as well.. > > sgl allocation can be moved to target_core_base.h and be used by all, > and we can put DIF generation in target_core_sbc.c for all to use. > Given these are SCSI host LLDs, it should most likely go into drivers/scsi/ code somewhere.. > > So I'd prefer to get target-core working for both types of cases, vs. > > adding READ_STRIP + WRITE_GENERATE emulation logic to tcm_loop + > > virtio-scsi LLDs. > > > > MKP, any preference here..? > > I see, I can live with that... > Just thought it would be nice to provide (half-way) protection also > against legacy initiators. > I'll defer to MKP's judgment on these, given that it's a host LLD specific issue. MKP..? --nab -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
