Re: [PATCH 00/19 V4] nvme-rdma/nvmet-rdma: Add metadata/T10-PI support

[James Smart <james.smart@broadcom.com>]

On 2/24/2020 8:45 AM, Max Gurtovoy wrote:
> Hello Sagi, Christoph, Keith, Martin and Co
>
> This patchset adds metadata (T10-PI) support for NVMeoF/RDMA host side and
> target side, using signature verbs API. This set starts with a few preparation
> commits to the NVMe host core layer. It continues with NVMeoF/RDMA host
> implementation + few preparation commits to the RDMA/rw API and to NVMe target
> core layer. The patchset ends with NVMeoF/RDMA target implementation.
>
> Configuration:
> Host:
>   - nvme connect --pi_enable --transport=rdma --traddr=10.0.1.1 --nqn=test-nvme
>
> Target:
>   - echo 1 > /config/nvmet/subsystems/${NAME}/attr_pi_enable
>   - echo 1 > /config/nvmet/ports/${PORT_NUM}/param_pi_enable
>
> The code was tested using Mellanox's ConnectX-4/ConnectX-5 HCAs.
> This series applies on top of nvme_5.7 branch cleanly.
>
> Changes from v3:
>   - Added Reviewed-by signatures
>   - New RDMA/rw patch (Patch 17/19)
>   - Add mdts setting op for controllers (Patches 14/19, 18/19)
>   - Rename NVME_NS_DIX_SUPPORTED to NVME_NS_MD_HOST_SUPPORTED and
>     NVME_NS_DIF_SUPPORTED to NVME_NS_MD_CTRL_SUPPORTED (Patch 01/19)
>   - Split "nvme: Introduce namespace features flag" patch (patch 02/19)
>   - Rename nvmet_rdma_set_diff_domain to nvmet_rdma_set_sig_domain
>     and nvme_rdma_set_diff_domain to nvme_rdma_set_sig_domain
>     (Patches 08/19, 19/19)
>   - Remove ns parameter from nvme_rdma_set_sig_domain/nvmet_rdma_set_sig_domain
>     functions (patch 08/19, 19/19)
>   - Rebase over nvme-5.7 branch
>
> Changes from v2:
>   - Convert the virtual start sector (which passed to bip_set_seed function)
>     to be in integrity interval units (Patch 14/15)
>   - Clarify some commit messages
>
> Changes from v1:
>   - Added Reviewed-by signatures
>   - Added namespace features flag (Patch 01/15)
>   - Remove nvme_ns_has_pi function (Patch 01/15)
>   - Added has_pi field to struct nvme_request (Patch 01/15)
>   - Subject change for patch 02/15
>   - Fix comment for PCI metadata (Patch 03/15)
>   - Rebase over "nvme: Avoid preallocating big SGL for data" patchset
>   - Introduce NVME_INLINE_PROT_SG_CNT flag (Patch 05/15)
>   - Introduce nvme_rdma_sgl structure (Patch 06/15)
>   - Remove first_sgl pointer from struct nvme_rdma_request (Patch 06/15)
>   - Split nvme-rdma patches (Patches 06/15, 07/15)
>   - Rename is_protected to use_pi (Patch 07/15)
>   - Refactor nvme_rdma_get_max_fr_pages function (Patch 07/15)
>   - Added ifdef CONFIG_BLK_DEV_INTEGRITY (Patches 07/15, 09/15, 13/15,
>     14/15, 15/15)
>   - Added port configfs pi_enable (Patch 14/15)
>
> Israel Rukshin (13):
>    nvme: Introduce namespace features flag
>    nvme: Add has_pi field to the nvme_req structure
>    nvme-fabrics: Allow user enabling metadata/T10-PI support
>    nvme: Introduce NVME_INLINE_PROT_SG_CNT
>    nvme-rdma: Introduce nvme_rdma_sgl structure
>    nvmet: Prepare metadata request
>    nvmet: Add metadata characteristics for a namespace
>    nvmet: Rename nvmet_rw_len to nvmet_rw_data_len
>    nvmet: Rename nvmet_check_data_len to nvmet_check_transfer_len
>    nvme: Add Metadata Capabilities enumerations
>    nvmet: Add metadata/T10-PI support
>    nvmet: Add metadata support for block devices
>    nvmet-rdma: Add metadata/T10-PI support
>
> Max Gurtovoy (6):
>    nvme: Enforce extended LBA format for fabrics metadata
>    nvme: Introduce max_integrity_segments ctrl attribute
>    nvme-rdma: Add metadata/T10-PI support
>    nvmet: Add mdts setting op for controllers
>    RDMA/rw: Expose maximal page list for a device per 1 MR
>    nvmet-rdma: Implement set_mdts controller op
>
>   drivers/infiniband/core/rw.c      |  14 +-
>   drivers/nvme/host/core.c          |  76 +++++---
>   drivers/nvme/host/fabrics.c       |  11 ++
>   drivers/nvme/host/fabrics.h       |   3 +
>   drivers/nvme/host/nvme.h          |   9 +-
>   drivers/nvme/host/pci.c           |   7 +
>   drivers/nvme/host/rdma.c          | 367 +++++++++++++++++++++++++++++++++-----
>   drivers/nvme/target/admin-cmd.c   |  41 +++--
>   drivers/nvme/target/configfs.c    |  61 +++++++
>   drivers/nvme/target/core.c        |  54 ++++--
>   drivers/nvme/target/discovery.c   |   8 +-
>   drivers/nvme/target/fabrics-cmd.c |  19 +-
>   drivers/nvme/target/io-cmd-bdev.c | 113 +++++++++++-
>   drivers/nvme/target/io-cmd-file.c |   6 +-
>   drivers/nvme/target/nvmet.h       |  39 +++-
>   drivers/nvme/target/rdma.c        | 252 ++++++++++++++++++++++++--
>   include/linux/nvme.h              |   2 +
>   include/rdma/rw.h                 |   1 +
>   18 files changed, 955 insertions(+), 128 deletions(-)
>

I found the first 6 patches confusing and in some cases unnecessary. 
They also missed some things. I've been working on a different set that 
I'll post shortly. I had hoped to get these back to you sooner for review.

-- james


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