Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
Back to the original question, I don't think Sagi was asking if it was valid to do a legacy/unprotected READ, it was what to expect with a protected READ on unwritten blocks: So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? So the precise answer is: the PI will be all 0xff bytes, unless logical provisioning is enabled, LBPRZ=0 and the device's compliance predates sbc3r34.pdf (November 2012). Doug Gilbert Thanks Doug, This answer translates directly into a fix in my T10-PI offload API. Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 1/10/2014 10:46 PM, Martin K. Petersen wrote: Andy == Andy Grover agro...@redhat.com writes: Andy Yes, don't you need FORMAT UNIT because protection information is Andy going to mean the pi-enabled lun will need to report less blocks? Modern disk drives won't shrink when you reformat them with PI. This is a result of an IDEMA agreement about LBA counts. And if you create a 10GB PI LUN on an array you'll get 10GB for data. Andy The ramdisk backstore changes in this series allocate extra space Andy for PI info, but my understanding was that especially for Andy emulation with block and fileio backstores, everything needs to go Andy in the same amount of space. For both file and block I'd recommend we store the PI in a separate block device or file unless the backing device is PI-capable. Andy Furthermore, if we want PI info stored along with the blocks, then Andy block and fileio backstore formats are no longer going to be 1:1 Andy -- requiring offset calculations, non-aligned read-modify-write, Andy and all that unpleasantness to be handled? I only think interleaved makes sense if you're passing the PI through instead of emulating. I agree, I implemented interleaved mode just as a proof of concept that our HW can perform offload in that manner. I assume we can stick with non-interleaved, although it can be added as a user option. Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 1/10/2014 10:39 PM, Martin K. Petersen wrote: Sagi == Sagi Grimberg sa...@mellanox.com writes: Sagi What about FORMAT_UNIT emulation? The backstore protection Sagi configuration is done at the target side via configfs/targetcli, I don't know of any non-disk devices that actually implement FORMAT UNIT. Usually such configuration is done using the array management interface. Hmm, So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? I mean, SCSI login executes some reads from several LBAs which will probably fail as blocks are unwritten. What is the usage model? perform Initiator login and then format the LUN on the target node? This is why I thought FORMAT_UNIT should be implemented. I understand this corner will disappear in DIF v2 (following DIX1.1 draft) with ESCAPE flags. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 1/10/2014 10:39 PM, Martin K. Petersen wrote: Sagi == Sagi Grimberg sa...@mellanox.com writes: Sagi What about FORMAT_UNIT emulation? The backstore protection Sagi configuration is done at the target side via configfs/targetcli, I don't know of any non-disk devices that actually implement FORMAT UNIT. Usually such configuration is done using the array management interface. Hmm, So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? I mean, SCSI login executes some reads from sevel LBAs which will probably fail as blocks are unwritten. What is the usage model? perform Initiator login and then format the LUN on the target node? This is why I thought FORMAT_UNIT should be implemented. I understand this corner will disappear in DIF v2 (following DIX1.1 draft) with ESCAPE flags. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
Sagi == Sagi Grimberg sa...@dev.mellanox.co.il writes: I don't know of any non-disk devices that actually implement FORMAT UNIT. Usually such configuration is done using the array management interface. Sagi So this takes me to a corner I still don't understand, if a LUN is Sagi pre-formatted as T10-protected, what happens to unwritten blocks Sagi read? I mean, SCSI login executes some reads from sevel LBAs Sagi which will probably fail as blocks are unwritten. Per SBC, PI must be initialized to 0x. Since an app tag value of 0x is an escape, this will prevent both target and initiator from performing PI-verification when that block is read. If a block is subsequently written and no PI is sent from the host (WRPROTECT=0), the target must generate valid PI for each block. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 1/12/2014 2:33 PM, Martin K. Petersen wrote: Sagi == Sagi Grimberg sa...@dev.mellanox.co.il writes: I don't know of any non-disk devices that actually implement FORMAT UNIT. Usually such configuration is done using the array management interface. Sagi So this takes me to a corner I still don't understand, if a LUN is Sagi pre-formatted as T10-protected, what happens to unwritten blocks Sagi read? I mean, SCSI login executes some reads from sevel LBAs Sagi which will probably fail as blocks are unwritten. Per SBC, PI must be initialized to 0x. Since an app tag value of 0x is an escape, this will prevent both target and initiator from performing PI-verification when that block is read. OK, so this is an implicit escape (which will become explicit in DIX1.1?). So I will open that in DIF RDMA verbs. If a block is subsequently written and no PI is sent from the host (WRPROTECT=0), the target must generate valid PI for each block. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
Doug == Douglas Gilbert dgilb...@interlog.com writes: So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? I mean, SCSI login executes some reads from sevel LBAs which will probably fail as blocks are unwritten. Doug Some observations: I haven't seen any disks pre-formatted with Doug T10-protection, they usually come pre-formatted without Doug T10-protection. Depends where you buy them. All the drives we ship arrive formatted with T10 PI from the manufacturer. However, nobody expects you to format a LUN on an array. When you create a LUN on a PI-capable storage array, T10 PI is a storage management interface option just like size, RAID level, etc. Upon creation, arrays zero out newly provisioned LUN blocks and write parity, etc. If PI is enabled on a LUN, blocks are written with all zeroes in the data block and all ones in the trailing PI tuple. This is all part of the regular LUN setup procedure. In any case. The usage model is that you never format a disk unless you are a tinkerer and bought a retail SAS drive at Fry's. Drives will be delivered by your server vendor formatted with PI and ready to go. If you use a non-disk storage device, whether or not to enable PI is part of the LUN provisioning procedure (array management console, RAID or flash controller card config utility, etc.) Doug So a tentative READ, for example checking if a disk has a Doug partition table, could be preceded by a GET LBA STATUS Doug command. IMO, if provisioning is enabled, LBPRZ=0 then the GET LBA Doug STATUS command should be mandatory. Otherwise a tentative READ is Doug a lucky dip. It's perfectly valid to do a legacy/unprotected READ from a T10 PI device. Doesn't matter whether the blocks are unwritten or not. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 14-01-12 12:21 PM, Martin K. Petersen wrote: Doug == Douglas Gilbert dgilb...@interlog.com writes: So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? I mean, SCSI login executes some reads from sevel LBAs which will probably fail as blocks are unwritten. Doug Some observations: I haven't seen any disks pre-formatted with Doug T10-protection, they usually come pre-formatted without Doug T10-protection. Depends where you buy them. All the drives we ship arrive formatted with T10 PI from the manufacturer. However, nobody expects you to format a LUN on an array. When you create a LUN on a PI-capable storage array, T10 PI is a storage management interface option just like size, RAID level, etc. Upon creation, arrays zero out newly provisioned LUN blocks and write parity, etc. If PI is enabled on a LUN, blocks are written with all zeroes in the data block and all ones in the trailing PI tuple. This is all part of the regular LUN setup procedure. In any case. The usage model is that you never format a disk unless you are a tinkerer and bought a retail SAS drive at Fry's. Drives will be delivered by your server vendor formatted with PI and ready to go. Only tinkerers would contribute to something like the scsi_debug driver. After all, the pros could rely on their T10 compliant vendor equipment :-) And you are right, I do like to test sg_format against something real. Perhaps sg_format is the utility those server vendors use. Another tinkerer called James Bottomley wrote the original sg_format code, according to my notes. If you use a non-disk storage device, whether or not to enable PI is part of the LUN provisioning procedure (array management console, RAID or flash controller card config utility, etc.) Doug So a tentative READ, for example checking if a disk has a Doug partition table, could be preceded by a GET LBA STATUS Doug command. IMO, if provisioning is enabled, LBPRZ=0 then the GET LBA Doug STATUS command should be mandatory. Otherwise a tentative READ is Doug a lucky dip. It's perfectly valid to do a legacy/unprotected READ from a T10 PI device. Doesn't matter whether the blocks are unwritten or not. Ah, the current SBC-3 draft (sbc3r36.pdf) does say if one does a READ from a disk with logical provisioning enabled and LBPRZ=0 then the block data is vendor specific and the PI, if any, is all 0xff bytes. That last bit was added in sbc3r34.pdf (and it was any value prior to that). Back to the original question, I don't think Sagi was asking if it was valid to do a legacy/unprotected READ, it was what to expect with a protected READ on unwritten blocks: So this takes me to a corner I still don't understand, if a LUN is pre-formatted as T10-protected, what happens to unwritten blocks read? So the precise answer is: the PI will be all 0xff bytes, unless logical provisioning is enabled, LBPRZ=0 and the device's compliance predates sbc3r34.pdf (November 2012). Doug Gilbert -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 01/09/2014 10:21 PM, Nicholas A. Bellinger wrote: What about FORMAT_UNIT emulation? Would certainly be useful to have.. The backstore protection configuration is done at the target side via configfs/targetcli, if you publish DIF support in INQUERY_EVPD/READ_CAPACITY you need to accept protection information format? Mmmm, these two bits bits are following what scsi_debug is currently exposing minus FORMAT_UNIT support..? MKP..? Yes, don't you need FORMAT UNIT because protection information is going to mean the pi-enabled lun will need to report less blocks? The ramdisk backstore changes in this series allocate extra space for PI info, but my understanding was that especially for emulation with block and fileio backstores, everything needs to go in the same amount of space. Furthermore, if we want PI info stored along with the blocks, then block and fileio backstore formats are no longer going to be 1:1 -- requiring offset calculations, non-aligned read-modify-write, and all that unpleasantness to be handled? I've looked at the patchset and the code looks good to me, so I guess I'm trying to understand the scope of future work needed on the backstore side for this support to be functional. Regards -- Andy -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On Fri, 2014-01-10 at 11:50 -0800, Andy Grover wrote: On 01/09/2014 10:21 PM, Nicholas A. Bellinger wrote: What about FORMAT_UNIT emulation? Would certainly be useful to have.. The backstore protection configuration is done at the target side via configfs/targetcli, if you publish DIF support in INQUERY_EVPD/READ_CAPACITY you need to accept protection information format? Mmmm, these two bits bits are following what scsi_debug is currently exposing minus FORMAT_UNIT support..? MKP..? Yes, don't you need FORMAT UNIT because protection information is going to mean the pi-enabled lun will need to report less blocks? FORMAT_UNIT is simply a mechanism that allows the client to setup the protection information remotely, to complement the per device configfs attribute that does the same thing from the target side. The ramdisk backstore changes in this series allocate extra space for PI info, but my understanding was that especially for emulation with block and fileio backstores, everything needs to go in the same amount of space. No, that's only for the interleaved case. Furthermore, if we want PI info stored along with the blocks, then block and fileio backstore formats are no longer going to be 1:1 -- requiring offset calculations, non-aligned read-modify-write, and all that unpleasantness to be handled? I'm currently not intending to support interleaved mode into the backend, given that backends not doing emulation expect these to be in seperate SGLs to start. --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
nab == Nicholas A Bellinger n...@daterainc.com writes: nab This patch updates sbc_emulate_readcapacity_16() to set P_TYPE and nab PROT_EN bits when DIF emulation is enabled by the backend device. Reviewed-by: Martin K. Petersen martin.peter...@oracle.com -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
Sagi == Sagi Grimberg sa...@mellanox.com writes: Sagi What about FORMAT_UNIT emulation? The backstore protection Sagi configuration is done at the target side via configfs/targetcli, I don't know of any non-disk devices that actually implement FORMAT UNIT. Usually such configuration is done using the array management interface. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
nab == Nicholas A Bellinger n...@linux-iscsi.org writes: The backstore protection configuration is done at the target side via configfs/targetcli, if you publish DIF support in INQUERY_EVPD/READ_CAPACITY you need to accept protection information format? nab Mmmm, these two bits bits are following what scsi_debug is nab currently exposing minus FORMAT_UNIT support..? PROT_EN and P_TYPE should be fine for now. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
Andy == Andy Grover agro...@redhat.com writes: Andy Yes, don't you need FORMAT UNIT because protection information is Andy going to mean the pi-enabled lun will need to report less blocks? Modern disk drives won't shrink when you reformat them with PI. This is a result of an IDEMA agreement about LBA counts. And if you create a 10GB PI LUN on an array you'll get 10GB for data. Andy The ramdisk backstore changes in this series allocate extra space Andy for PI info, but my understanding was that especially for Andy emulation with block and fileio backstores, everything needs to go Andy in the same amount of space. For both file and block I'd recommend we store the PI in a separate block device or file unless the backing device is PI-capable. Andy Furthermore, if we want PI info stored along with the blocks, then Andy block and fileio backstore formats are no longer going to be 1:1 Andy -- requiring offset calculations, non-aligned read-modify-write, Andy and all that unpleasantness to be handled? I only think interleaved makes sense if you're passing the PI through instead of emulating. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On 1/8/2014 10:36 PM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch updates sbc_emulate_readcapacity_16() to set P_TYPE and PROT_EN bits when DIF emulation is enabled by the backend device. Cc: Martin K. Petersen martin.peter...@oracle.com Cc: Christoph Hellwig h...@lst.de Cc: Hannes Reinecke h...@suse.de Cc: Sagi Grimberg sa...@mellanox.com Cc: Or Gerlitz ogerl...@mellanox.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- drivers/target/target_core_sbc.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 366b9bb..22599e8 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -106,6 +106,11 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd) buf[9] = (dev-dev_attrib.block_size 16) 0xff; buf[10] = (dev-dev_attrib.block_size 8) 0xff; buf[11] = dev-dev_attrib.block_size 0xff; + /* +* Set P_TYPE and PROT_EN bits for DIF support +*/ + if (dev-dev_attrib.pi_prot_type) + buf[12] = (dev-dev_attrib.pi_prot_type - 1) 1 | 0x1; if (dev-transport-get_lbppbe) buf[13] = dev-transport-get_lbppbe(dev) 0x0f; Hey Nic, What about FORMAT_UNIT emulation? The backstore protection configuration is done at the target side via configfs/targetcli, if you publish DIF support in INQUERY_EVPD/READ_CAPACITY you need to accept protection information format? Did I miss that one? or is it still under WIP? Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/14] target/sbc: Add P_TYPE + PROT_EN bits to READ_CAPACITY_16
On Thu, 2014-01-09 at 12:24 +0200, Sagi Grimberg wrote: On 1/8/2014 10:36 PM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch updates sbc_emulate_readcapacity_16() to set P_TYPE and PROT_EN bits when DIF emulation is enabled by the backend device. Cc: Martin K. Petersen martin.peter...@oracle.com Cc: Christoph Hellwig h...@lst.de Cc: Hannes Reinecke h...@suse.de Cc: Sagi Grimberg sa...@mellanox.com Cc: Or Gerlitz ogerl...@mellanox.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- drivers/target/target_core_sbc.c |5 + 1 file changed, 5 insertions(+) diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c index 366b9bb..22599e8 100644 --- a/drivers/target/target_core_sbc.c +++ b/drivers/target/target_core_sbc.c @@ -106,6 +106,11 @@ sbc_emulate_readcapacity_16(struct se_cmd *cmd) buf[9] = (dev-dev_attrib.block_size 16) 0xff; buf[10] = (dev-dev_attrib.block_size 8) 0xff; buf[11] = dev-dev_attrib.block_size 0xff; + /* +* Set P_TYPE and PROT_EN bits for DIF support +*/ + if (dev-dev_attrib.pi_prot_type) + buf[12] = (dev-dev_attrib.pi_prot_type - 1) 1 | 0x1; if (dev-transport-get_lbppbe) buf[13] = dev-transport-get_lbppbe(dev) 0x0f; Hey Nic, What about FORMAT_UNIT emulation? Would certainly be useful to have.. The backstore protection configuration is done at the target side via configfs/targetcli, if you publish DIF support in INQUERY_EVPD/READ_CAPACITY you need to accept protection information format? Mmmm, these two bits bits are following what scsi_debug is currently exposing minus FORMAT_UNIT support..? MKP..? --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
