[PATCH] nvme: allow ioctl to submit io command with dif for NVMeoF

[PATCH] nvme: allow ioctl to submit io command with dif for NVMeoF

[Chao Leng]

If nvme over fabrics support t10dif, NVME_NS_EXT_LBAS and
NVME_NS_METADATA_SUPPORTED of ns->features will be set to 1.
Now nvme_submit_io do not allow the metadata if NVME_NS_EXT_LBAS of
ns->features is setted. Indeed It should allow the metadata if both
NVME_NS_EXT_LBAS and NVME_NS_METADATA_SUPPORTED of ns->features are setted.

Signed-off-by: Chao Leng <lengchao@huawei.com>
---
 drivers/nvme/host/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index a2e89db1cd63..408ba0cc111f 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -199,7 +199,8 @@ static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 		metadata = nvme_to_user_ptr(io.metadata);
 	}
 
-	if (ns->features & NVME_NS_EXT_LBAS) {
+	if ((ns->features & (NVME_NS_EXT_LBAS | NVME_NS_METADATA_SUPPORTED)) ==
+		NVME_NS_EXT_LBAS) {
 		length += meta_len;
 		meta_len = 0;
 	} else if (meta_len) {
-- 
2.16.4

Re: [PATCH] nvme: allow ioctl to submit io command with dif for NVMeoF

[Keith Busch]

On Wed, Aug 03, 2022 at 05:44:36PM +0800, Chao Leng wrote:
> If nvme over fabrics support t10dif, NVME_NS_EXT_LBAS and
> NVME_NS_METADATA_SUPPORTED of ns->features will be set to 1.
> Now nvme_submit_io do not allow the metadata if NVME_NS_EXT_LBAS of
> ns->features is setted. Indeed It should allow the metadata if both
> NVME_NS_EXT_LBAS and NVME_NS_METADATA_SUPPORTED of ns->features are setted.

I'm not sure. As a passthrough interface, don't you want to set up the host
buffers with the inline metadata instead of relying on the HBA to splice
separate ones?

If you really want to control this kind of thing, it would be better to use
NVME_IOCTL_IO_CMD instead of NVME_IOCTL_SUBMIT_IO.

Re: [PATCH] nvme: allow ioctl to submit io command with dif for NVMeoF

[Chao Leng]

On 2022/8/3 23:44, Keith Busch wrote:
> On Wed, Aug 03, 2022 at 05:44:36PM +0800, Chao Leng wrote:
>> If nvme over fabrics support t10dif, NVME_NS_EXT_LBAS and
>> NVME_NS_METADATA_SUPPORTED of ns->features will be set to 1.
>> Now nvme_submit_io do not allow the metadata if NVME_NS_EXT_LBAS of
>> ns->features is setted. Indeed It should allow the metadata if both
>> NVME_NS_EXT_LBAS and NVME_NS_METADATA_SUPPORTED of ns->features are setted.
> 
> I'm not sure. As a passthrough interface, don't you want to set up the host
> buffers with the inline metadata instead of relying on the HBA to splice
> separate ones?
If HBA support t10DIF, we need the HBA to verify the metadata for end-to-end DIF.
So we need set up the metadata separately.
Two scenarios:
1.Test whether the HBA verification meets expectations when specify data and metadata.
2.The user wants to use the capability of end-to-end DIF through IOCTL.
If the HBA does not support T10DIF, Keep the old behavior.
> 
> If you really want to control this kind of thing, it would be better to use
> NVME_IOCTL_IO_CMD instead of NVME_IOCTL_SUBMIT_IO.
NVME_IOCTL_IO_CMD is a universal interface, of course, it can work well.
But nvme-cli do not support customized commands, and it is not friendly to use.
We can use nvme write/read simply, it looks better.
> .
> 

Re: [PATCH] nvme: allow ioctl to submit io command with dif for NVMeoF

[Chao Leng]

Hi, Keith
     Any other comments on this patch?
     Is there any chance that this patch can be accepted?

On 2022/8/4 11:00, Chao Leng wrote:
> 
> 
> On 2022/8/3 23:44, Keith Busch wrote:
>> On Wed, Aug 03, 2022 at 05:44:36PM +0800, Chao Leng wrote:
>>> If nvme over fabrics support t10dif, NVME_NS_EXT_LBAS and
>>> NVME_NS_METADATA_SUPPORTED of ns->features will be set to 1.
>>> Now nvme_submit_io do not allow the metadata if NVME_NS_EXT_LBAS of
>>> ns->features is setted. Indeed It should allow the metadata if both
>>> NVME_NS_EXT_LBAS and NVME_NS_METADATA_SUPPORTED of ns->features are setted.
>>
>> I'm not sure. As a passthrough interface, don't you want to set up the host
>> buffers with the inline metadata instead of relying on the HBA to splice
>> separate ones?
> If HBA support t10DIF, we need the HBA to verify the metadata for end-to-end DIF.
> So we need set up the metadata separately.
> Two scenarios:
> 1.Test whether the HBA verification meets expectations when specify data and metadata.
> 2.The user wants to use the capability of end-to-end DIF through IOCTL.
> If the HBA does not support T10DIF, Keep the old behavior.
>>
>> If you really want to control this kind of thing, it would be better to use
>> NVME_IOCTL_IO_CMD instead of NVME_IOCTL_SUBMIT_IO.
> NVME_IOCTL_IO_CMD is a universal interface, of course, it can work well.
> But nvme-cli do not support customized commands, and it is not friendly to use.
> We can use nvme write/read simply, it looks better.
>> .
>>
> 
> .