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. Like I said, I'm fairly indifferent on this issue, and leave it up to MKP's judgment. ;) --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
