Re: [PATCH v4 00/18] NVMe PCI endpoint target driver

Manivannan Sadhasivam
2024-12-16T06:07:47Z

On Thu, Dec 12, 2024 at 08:34:22PM +0900, Damien Le Moal wrote:
> This patch series implements an NVMe target driver for the PCI transport
> using the PCI endpoint framework.
>
> The first 4 patches of this series move and cleanup some nvme code that
> will be reused in following patches.
>
> Patch 5 introduces the PCI transport type to allow setting up ports for
> the new PCI target controller driver. Patch 6 to 9 are improvements of
> the target core code to allow creating the PCI controller and processing
> its nvme commands without the need to rely on fabrics commands like the
> connect command to create the admin and I/O queues.
>
> Patch 10 relaxes the SGL check in nvmet_req_init() to allow for PCI
> admin commands (which must use PRPs).
>
> Patches 11 to 15 improve the set/get feature support of the target code
> to get closer to achieving NVMe specification compliance. These patches
> though do not implement support for some mandatory features.
>
> Patch 16 is the main patch which introduces the NVMe PCI endpoint target
> driver. This patch commit message provides and overview of the driver
> design and operation.
>
> Finally, patch 17 documents the NVMe PCI endpoint target driver and
> provides a user guide explaning how to setup an NVMe PCI endpoint
> device.
>
> The patches are base on Linus 6.13-rc2 tree.
>
> This driver has been extensively tested using a Radxa Rock5B board
> (RK3588 Arm SoC). Some tests have also been done using a Pine Rockpro64
> board. However, this board does not support DMA channels for the PCI
> endpoint controller, leading to very poor performance.
>
> Using the Radxa Rock5b board and setting up a 4 queue-pairs controller
> with a null-blk block device loop target, performance was measured using
> fio as follows:
>
> +----------------------------------+------------------------+
> | Workload | IOPS (BW) |
> +----------------------------------+------------------------+
> | Rand read, 4KB, QD=1, 1 job | 14.3k IOPS |
> | Rand read, 4KB, QD=32, 1 job | 80.8k IOPS |
> | Rand read, 4KB, QD=32, 4 jobs | 131k IOPS |
> | Rand read, 128KB, QD=32, 1 job | 16.7k IOPS (2.18 GB/s) |
> | Rand read, 128KB, QD=32, 4 jobs | 17.4k IOPS (2.27 GB/s) |
> | Rand read, 512KB, QD=32, 1 job | 5380 IOPS (2.82 GB/s) |
> | Rand read, 512KB, QD=32, 4 jobs | 5206 IOPS (2.27 GB/s) |
> | Rand write, 128KB, QD=32, 1 job | 9617 IOPS (1.26 GB/s) |
> | Rand write, 128KB, QD=32, 4 jobs | 8405 IOPS (1.10 GB/s) |
> +----------------------------------+------------------------+
>
> These results use the default MDTS of the NVMe enpoint driver of 512 KB.
>
> This driver is not intended for production use but rather to be a
> playground for learning NVMe and exploring/testing new NVMe features
> while providing reasonably good performance.
>

Damien, thanks for the work! Please wait for my review comments before posting
next version. And sorry about the delay.

- Mani

> Changes from v3:
> - Added patch 1 which was missing from v3 and caused the 0day build
> failure
> - Corrected a few typos in the documentation (patch 18)
> - Added Christoph's review tag and Rick's tested tag
>
> Changes from v2:
> - Changed all preparatory patches before patch 16 to move more NVMe
> generic code out of the PCI endpoint target driver and into the
> target core.
> - Changed patch 16 to use directly a target controller instead of a
> host controller. Many aspects of the command management and DMA
> transfer management have also been simplified, leading to higher
> performance.
> - Change the documentation patch to match the above changes
>
> Changes from v1:
> - Added review tag to patch 1
> - Modified patch 4 to:
> - Add Rick's copyright notice
> - Improve admin command handling (set_features command) to handle the
> number of queues feature (among others) to enable Windows host
> - Improved SQ and CQ work items handling
>
> Damien Le Moal (18):
> nvme: Move opcode string helper functions declarations
> nvmet: Add vendor_id and subsys_vendor_id subsystem attributes
> nvmet: Export nvmet_update_cc() and nvmet_cc_xxx() helpers
> nvmet: Introduce nvmet_get_cmd_effects_admin()
> nvmet: Add drvdata field to struct nvmet_ctrl
> nvme: Add PCI transport type
> nvmet: Improve nvmet_alloc_ctrl() interface and implementation
> nvmet: Introduce nvmet_req_transfer_len()
> nvmet: Introduce nvmet_sq_create() and nvmet_cq_create()
> nvmet: Add support for I/O queue management admin commands
> nvmet: Do not require SGL for PCI target controller commands
> nvmet: Introduce get/set_feature controller operations
> nvmet: Implement host identifier set feature support
> nvmet: Implement interrupt coalescing feature support
> nvmet: Implement interrupt config feature support
> nvmet: Implement arbitration feature support
> nvmet: New NVMe PCI endpoint target driver
> Documentation: Document the NVMe PCI endpoint target driver
>
> Documentation/PCI/endpoint/index.rst | 1 +
> .../PCI/endpoint/pci-nvme-function.rst | 14 +
> Documentation/nvme/index.rst | 12 +
> .../nvme/nvme-pci-endpoint-target.rst | 365 +++
> Documentation/subsystem-apis.rst | 1 +
> drivers/nvme/host/nvme.h | 39 -
> drivers/nvme/target/Kconfig | 10 +
> drivers/nvme/target/Makefile | 2 +
> drivers/nvme/target/admin-cmd.c | 388 ++-
> drivers/nvme/target/configfs.c | 49 +
> drivers/nvme/target/core.c | 266 +-
> drivers/nvme/target/discovery.c | 17 +
> drivers/nvme/target/fabrics-cmd-auth.c | 14 +-
> drivers/nvme/target/fabrics-cmd.c | 101 +-
> drivers/nvme/target/nvmet.h | 110 +-
> drivers/nvme/target/pci-ep.c | 2626 +++++++++++++++++
> include/linux/nvme.h | 42 +
> 17 files changed, 3897 insertions(+), 160 deletions(-)
> create mode 100644 Documentation/PCI/endpoint/pci-nvme-function.rst
> create mode 100644 Documentation/nvme/index.rst
> create mode 100644 Documentation/nvme/nvme-pci-endpoint-target.rst
> create mode 100644 drivers/nvme/target/pci-ep.c
>
>
> base-commit: 231825b2e1ff6ba799c5eaf396d3ab2354e37c6b
> --
> 2.47.1
>

--
மணிவண்ணன் சதாசிவம்