On Thu, 2014-03-13 at 12:57 -0700, Nicholas A. Bellinger wrote: > On Thu, 2014-03-13 at 21:28 +0200, Sagi Grimberg wrote: > > On 3/13/2014 9:13 PM, Nicholas A. Bellinger wrote: > > > On Thu, 2014-03-13 at 10:03 +0200, Sagi Grimberg wrote: > > >> On 3/12/2014 4:16 AM, Martin K. Petersen wrote: > > >>>>>>>> "Sagi" == Sagi Grimberg <sagig@xxxxxxxxxxxxxxxxxx> writes: > > >>>>> The real question is whether there is actually an I/O path to > > >>>>> protect? It seems somewhat pointless to generate CRCs and then hand > > >>>>> the resulting buffer to a "target" function call that then does a > > >>>>> pass to verify it without any real data movement taking place in > > >>>>> between. The corruption window in that case is fairly small. > > >>> Sagi> I agree, it does seem too pedantic, but ignoring scsi_cmnd prot_op > > >>> Sagi> feels somewhat wrong to me. > > >>> > > >>> I'm not talking about ignoring the prot_op. The kernel is not going to > > >>> request PI transfers (prot_op > 0) unless both initiator and target > > >>> agree on the protection mode. > > >>> > > >>> And if you are both initiator and target you are also in control over > > >>> the host's prot_capabilities mask and whether you report PROT_EN=1 in > > >>> READ CAPACITY(16) for the target. > > >>> > > >> The kernel may also request the LLD to WRITE_INSERT/READ_STRIP > > >> protection. if I turn > > >> off write_generate/read_verify integrity sysfs attributes. For "real" > > >> device LLDs we know > > >> what to expect but what do we expect from the vhost_scsi LLD to do in > > >> this case? > > >> > > > So the vhost-scsi fabric driver will be receiving a virtio header from > > > the virtio-scsi LLD to signal that a protection buffer is available or > > > not available. > > > > > > AFAICT for the two READ_10s that come down with prot_op == READ_STRIP, > > > there is no associated scsi_prot_sglist() or scsi_prot_sg_count(), so > > > from vhost-scsi's perspective, the virtio header will signal no > > > protection SGLs are available, and that the operation should function > > > like a normal unprotected operation. > > > > In my opinion that is a miss-interpretation. READ_STRIP means "validate > > protection and strip it". > > Is WRITE_GENERATE also a normal unprotected operation? > > > > As MKP said, it's a matter of how pedantic we want to be. we can just > > say that for vhost_scsi/loopback LLDs > > there is no real justification of doing these operations, but the > > meaning is that we are performing a violation. > > Probably a minor one, but still a violation. > > > > That's true. IMHO the important thing is to leave open the possibility > for a backend fabric below vhost-scsi / tcm-loop LUN's to still be able > to perform the READ_STRIP / WRITE_INSERT in hardware if command will be > going to a remote machine. > > Right now in this case the only way a backend fabric can determine if > this should happen is by looking at RDPROTECT / WRPROTECT bits in the > CDB, and doing the READ_STRIP + WRITE_INSERT when no protection > information was passed from the target layer. Hardly ideal, but without > the ability to pass prot_op between target + initiator that's what we're > stuck up with. > > The other option would be to add target-core support for doing the > READ_STRIP / WRITE_INSERT emulation for these cases, preferably using > common code from sd_dif.c. > That reminds me, one other case where it would be useful for the target to able to do READ_STRIP + WRITE_INSERT ops would be for locally generated EXTENDED_COPY READ / WRITE commands. --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
