Re: [PATCH 09/16] nvme-rdma: add metadata/T10-PI support

From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Max Gurtovoy <maxg@mellanox.com>
Cc: axboe@kernel.dk, jsmart2021@gmail.com, sagi@grimberg.me,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	shlomin@mellanox.com, israelr@mellanox.com,
	vladimirk@mellanox.com, linux-nvme@lists.infradead.org,
	idanb@mellanox.com, oren@mellanox.com, kbusch@kernel.org,
	nitzanc@mellanox.com, hch@lst.de
Subject: Re: [PATCH 09/16] nvme-rdma: add metadata/T10-PI support
Date: Thu, 14 May 2020 18:40:02 -0400	[thread overview]
Message-ID: <yq1o8qqta2l.fsf@oracle.com> (raw)
In-Reply-To: <c7f3151e-a401-56f7-735f-476b01337b17@mellanox.com> (Max Gurtovoy's message of "Thu, 14 May 2020 11:48:18 +0300")

Hi Max!

> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 05dbed3..9b9d11b 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -1338,9 +1338,10 @@ static void nvme_rdma_set_sig_domain(struct
> blk_integrity *bi,
>         domain->sig.dif.ref_tag = le32_to_cpu(cmd->rw.reftag);
>         domain->sig.dif.apptag_check_mask = 0xffff;

Also, this has me wondering. Not sure how it works in your case?

There are basically two modes of operation:

 1. The app tag is opaque storage and may be different for each LBA in
    the I/O. The only thing the HBA should do is to bypass all PI checks
    if the app tag value for a given block is 0xffff (if app_escape is
    true).

 2. The app tag, or part of it, is constant for an entire I/O. And for
    that model one specifies the mask identifying which bits in the app
    tag should be verified against a constant value. The HBA should
    still bypass PI checking if the app tag value for a given block is
    0xffff and app_escape is true.

Linux belongs to the school #1 of the app tag being opaque storage. I'm
not entirely sure how that works given apptag_check_mask = 0xffff which
sounds to me like you want to verify all bits in each app tag against a
constant value. But you don't set the constant value to check against.
What's going on there?

>         domain->sig.dif.app_escape = true;
> -       domain->sig.dif.ref_escape = true;
>         if (control & NVME_RW_PRINFO_PRCHK_REF)
>                 domain->sig.dif.ref_remap = true;
> +       else
> +               domain->sig.dif.ref_escape = true;
>  }

ref_escape should really only be true if the namespace is formatted with
Type 3. Whether the ref tag should be checked is orthogonal.

-- 
Martin K. Petersen	Oracle Linux Engineering

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme