Re: [LTP] [PATCH] testcases/nvme: Add NVMe device discovery and identification test

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> Signed-off-by: priyama2 <priyama2@linux.ibm.com>
> ---
>  testcases/kernel/device-drivers/nvme/Makefile |  42 ++++
>  testcases/kernel/device-drivers/nvme/README   | 155 ++++++++++++
>  testcases/kernel/device-drivers/nvme/nvme01.c | 236 ++++++++++++++++++
>  3 files changed, 433 insertions(+)
>  create mode 100644 testcases/kernel/device-drivers/nvme/Makefile
>  create mode 100644 testcases/kernel/device-drivers/nvme/README
>  create mode 100644 testcases/kernel/device-drivers/nvme/nvme01.c
> 
> diff --git a/testcases/kernel/device-drivers/nvme/Makefile b/testcases/kernel/device-drivers/nvme/Makefile
> new file mode 100644
> index 000000000..ace6696f6
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/nvme/Makefile
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024 IBM Corporation
> +#
> +# Standalone Makefile for NVMe LTP test suite
> +
> +# Compiler and flags
> +CC		= gcc
> +CFLAGS		= -Wall -O2 -I/opt/ltp/include
> +LDFLAGS		= -L/opt/ltp/lib
> +LDLIBS		= -lltp -lrt
> +
> +# Test binaries
> +TARGETS		= nvme01 nvme02 nvme03 nvme04
> +
> +# Build all tests
> +all: $(TARGETS)
> +
> +nvme01: nvme01.c
> +	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LDLIBS)
> +
> +nvme02: nvme02.c
> +	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LDLIBS)
> +
> +nvme03: nvme03.c
> +	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LDLIBS)
> +
> +nvme04: nvme04.c
> +	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) $(LDLIBS)
> +
> +# Install targets
> +install: $(TARGETS)
> +	@mkdir -p /opt/ltp/testcases/bin
> +	@for target in $(TARGETS); do \
> +		install -m 0755 $$target /opt/ltp/testcases/bin/; \
> +	done
> +	@echo "Tests installed to /opt/ltp/testcases/bin/"
> +
> +# Clean
> +clean:
> +	rm -f $(TARGETS) *.o
> +
> +.PHONY: all install clean

We have a buildsystem in LTP, all you need to do is to include the right
file in the Makefile, do that instead.

> diff --git a/testcases/kernel/device-drivers/nvme/README b/testcases/kernel/device-drivers/nvme/README
> new file mode 100644
> index 000000000..6ee5796fe
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/nvme/README
> @@ -0,0 +1,155 @@
> +NVMe Test Suite for Linux Test Project (LTP)
> +==============================================
> +
> +This directory contains test cases for NVMe (Non-Volatile Memory Express) 
> +device functionality testing.
> +
> +Test Cases
> +----------
> +
> +1. nvme01 - Device Discovery & Identification
> +   Tests:
> +   - Detect NVMe controllers
> +   - Enumerate namespaces
> +   - Verify PCI enumeration
> +   - Check driver binding
> +
> +2. nvme02 - Namespace Management
> +   Tests:
> +   - Create namespaces (various sizes)
> +   - Delete namespaces
> +   - Attach/detach namespaces to controllers
> +   - Resize namespaces
> +   - Format namespaces (different block sizes: 512B, 4KB)
> +   - Namespace sharing across controllers
> +
> +3. nvme03 - I/O Operations
> +   Tests:
> +   - Sequential read/write operations
> +   - Random read/write operations
> +   - Mixed workloads
> +   - Different block sizes (512B, 4KB, 8KB)
> +   - Queue depth variations
> +   - Direct I/O vs buffered I/O
> +
> +4. nvme04 - Multipath & Redundancy
> +   Tests:
> +   - Path failover testing
> +   - Load balancing across paths
> +   - Path recovery
> +   - ANA (Asymmetric Namespace Access) states
> +   - Controller failover
> +
> +Requirements
> +------------
> +
> +Hardware:
> +- System with NVMe storage device(s)
> +- For multipath tests: NVMe device with multiple paths configured
> +
> +Software:
> +- Linux kernel with NVMe support (CONFIG_BLK_DEV_NVME=y)
> +- nvme-cli tools (for namespace management tests)
> +- Root/sudo privileges
> +
> +Building
> +--------
> +
> +From the LTP root directory:
> +  make -C testcases/kernel/device-drivers/nvme
> +
> +Or from this directory:
> +  make
> +
> +Running Tests
> +-------------
> +
> +Individual test execution:
> +  ./nvme01
> +  ./nvme02
> +  ./nvme03
> +  ./nvme04
> +
> +Using LTP runtest:
> +  runltp -f nvme
> +
> +Using runtest file:
> +  runltp -f /opt/ltp/runtest/nvme
> +
> +Test Results
> +------------
> +
> +Tests use standard LTP result codes:
> +- TPASS: Test passed
> +- TFAIL: Test failed
> +- TCONF: Test not configured (e.g., no NVMe device found)
> +- TBROK: Test broken (setup failure)
> +- TWARN: Test warning
> +
> +Notes
> +-----
> +
> +1. Some tests require specific hardware configurations:
> +   - nvme02: May require namespace management support
> +   - nvme04: Requires multipath configuration for full testing
> +
> +2. Destructive tests:
> +   - nvme02 includes namespace creation/deletion which may affect data
> +   - Always run on test systems or with proper backups
> +
> +3. Performance tests (nvme03):
> +   - Results vary based on hardware and system load
> +   - Tests measure relative performance, not absolute benchmarks
> +
> +4. Root privileges:
> +   - All tests require root access for device operations
> +
> +Test Coverage
> +-------------
> +
> +The test suite covers:
> +- Basic NVMe device detection and enumeration
> +- Namespace lifecycle management
> +- I/O performance characteristics
> +- Multipath and redundancy features
> +- Driver and controller state verification
> +
> +Known Limitations
> +-----------------
> +
> +1. Namespace management tests may not work on all NVMe devices
> +   (depends on controller capabilities)
> +
> +2. Multipath tests require specific hardware and kernel configuration
> +
> +3. Some tests are informational and may show TCONF on systems
> +   without full NVMe feature support
> +
> +Contributing
> +------------
> +
> +When adding new tests:
> +1. Follow LTP coding standards
> +2. Use tst_test.h framework
> +3. Include proper error handling
> +4. Document test purpose and requirements
> +5. Test on multiple NVMe device types if possible
> +
> +References
> +----------
> +
> +- NVMe Specification: https://nvmexpress.org/specifications/
> +- Linux NVMe Driver: Documentation/block/nvme.rst
> +- nvme-cli: https://github.com/linux-nvme/nvme-cli
> +- LTP Documentation: https://linux-test-project.github.io/
> +
> +Authors
> +-------
> +
> +Copyright (c) 2024 IBM Corporation
> +LTP NVMe Test Suite
> +
> +License
> +-------
> +

No README should be added. Instead the test description should be added
into a special doc comment at the top of the test. That description is
then extracted into online documentation:

https://linux-test-project.readthedocs.io/en/latest/users/test_catalog.html

Also this is too verbose, there is a lot of useless information. Just
keep the part that describes what the test actually does. No need to
describe how to compile the tests or how to write new tests, we have
plenty of online docs for LTP now.

> diff --git a/testcases/kernel/device-drivers/nvme/nvme01.c b/testcases/kernel/device-drivers/nvme/nvme01.c
> new file mode 100644
> index 000000000..82fbd315b
> --- /dev/null
> +++ b/testcases/kernel/device-drivers/nvme/nvme01.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2024 IBM Corporation
> + * Author: LTP NVMe Test Suite
                    ^
		    This does not make any sense.


And the part below should be the special doc comment that is exported
into docs.

> + * Test: NVMe Device Discovery & Identification
> + *
> + * This test verifies:
> + * - Detection of NVMe controllers
> + * - Enumeration of namespaces
> + * - PCI enumeration verification
> + * - Driver binding check
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <dirent.h>
> +#include <errno.h>
> +
> +#include "tst_test.h"
> +
> +#define NVME_DEV_PATH "/dev"
> +#define NVME_SYS_PATH "/sys/class/nvme"
> +#define PCI_DEVICES_PATH "/sys/bus/pci/devices"
> +
> +static int nvme_device_found = 0;
> +static char nvme_dev_name[256];
> +
> +/*
> + * Test Case 1: Detect NVMe Controllers
> + * Verifies that NVMe controllers are present in the system
> + */

No obvious comments like this one please. It's pretty clear from the
function name what it does.

> +static void test_detect_nvme_controllers(void)
> +{
> +	DIR *dir;
> +	struct dirent *entry;
> +	int found = 0;
> +
> +	dir = opendir(NVME_DEV_PATH);
> +	if (!dir) {
> +		tst_brk(TBROK | TERRNO, "Failed to open %s", NVME_DEV_PATH);
> +		return;
> +	}

Use SAFE_MACROS() whenever possible. In this case SAFE_OPENDIR().

> +	tst_res(TINFO, "Scanning for NVMe devices in %s", NVME_DEV_PATH);
> +
> +	while ((entry = readdir(dir)) != NULL) {
> +		if (strncmp(entry->d_name, "nvme", 4) == 0 &&
> +		    strlen(entry->d_name) == 5) {  /* nvme0, nvme1, etc. */
                                                  ^
                   No comments commenting obvious like this.

> +			tst_res(TINFO, "Found NVMe controller: %s", entry->d_name);
> +			strncpy(nvme_dev_name, entry->d_name, sizeof(nvme_dev_name) - 1);
> +			found = 1;
> +			nvme_device_found = 1;
> +		}
> +	}
> +
> +	closedir(dir);

SAFE_CLOSEDIR()


And similar comments apply for the rest of the code.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp