Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_get_features_host_id2()
All 'get features' commands have an 'result' field as the last
argument, so add an alternative function nvme_get_features_host_id2()
to follow the same calling convention.
Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_get_features_timestamp2()
All 'get features' commands have an 'result' field as the last
argument, so add an alternative function nvme_get_features_timestamp2()
to follow the same calling convention.
Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_set_features_iocs_profile2()
All 'set features' commands have an 'result' field as the last
argument, so add an alternative function nvme_set_features_iocs_profile2()
to follow the same calling convention.
Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_set_features_host_id2()
All 'set features' commands have an 'result' field as the last
argument, so add an alternative function nvme_set_features_host_id2()
to follow the same calling convention.
Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_set_features_host_behavior2()
All 'set features' commands have an 'result' field as the last
argument, so add an alternative function nvme_set_features_host_behavior2()
to follow the same calling convention.
Hannes Reinecke [Fri, 5 Apr 2024 14:31:25 +0000 (16:31 +0200)]
ioctl: add nvme_set_features_timestamp2()
All 'set features' commands have an 'result' field as the last
argument, so add an alternative function nvme_set_features_timestamp2()
to follow the same calling convention.
Hannes Reinecke [Thu, 4 Apr 2024 07:07:33 +0000 (09:07 +0200)]
ioctl: return EPROTO when an NVMe status occurred
The ioctl might return 0, but the NVMe status might indicate an
error. In these cases we should return EPROTO to indicate that
the command did not succeed.
The sysfs tests are currently hand-rolling file comparison.
Use diff -u instead to get a rich comparison if the comparison fails.
Create a single shell script that combines extracting the sysfs files,
invoking the test program to generate the tree output,
and diffing the result against the expected output.
This reduces the C and Meson code needed for the test.
Nilay Shroff [Tue, 26 Mar 2024 06:57:23 +0000 (12:27 +0530)]
tree: Add NVM subsystem controller identifier
This commit introduces a field "cntlid" for controller,
that contains the NVM subsystem unique identifier assigned
to each controller device in an NVM subsystem.
While attaching a namespace, typically user needs to specify the
controller identifier (cntlid). The cntlid could be referenced from
sysfs (/sys/class/nvme/nvmeX/cntlid) but it would be nice to have
a direct option.
Nilay Shroff [Thu, 21 Mar 2024 13:16:17 +0000 (18:46 +0530)]
libnvme : record the nvme pci adapter physical slot
There is a subtle bug in the nvme controller physical
slot lookup code due to which the readdir() prematurely
aborts the lookup and hence libnvme fails to record the
pci slot address of the nvme controller. This patch helps
to fix this issue.
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> Signed-off-by: Daniel Wagner <dwagner@suse.de>
Tomas Bzatek [Wed, 20 Mar 2024 14:03:21 +0000 (15:03 +0100)]
build: Switch default meson buildtype to 'debugoptimized'
The meson 'debug' buildtype defaults to '-O0 -g' which misses
some important compiler warnings (as of gcc-13). While there's
no universal solution, the 'debugoptimized' buildtype supplies
'-O2 -g' that appears to be a reasonable compromise for having
important compiler warnings by default.
11a0918a9972 ("nvme: allow to overwrite base sysfs path")
added support for changing the sysfs path via an environment variable.
Unfortunately, it added a heap allocation
every time a sysfs path was requested.
Modify the callers to not free the paths, which allows a string constant
to be returned if the path isn't overridden, avoiding an allocation.
Cache the path in a static variable so that if it is overridden,
the heap-allocated string only needs to be constructed once
and afterwards can be reused.
Create a file sysfs.c to consolidate this logic
instead of spreading them across 3 files.
Also introduce a helper to factor out the duplicated code.
Fixes: 11a0918a9972 ("nvme: allow to overwrite base sysfs path") Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Tomas Bzatek [Tue, 19 Mar 2024 15:33:05 +0000 (16:33 +0100)]
json: Fix uninitialized variables
In file included from ../src/nvme/json.c:17:
In function ‘freep’,
inlined from ‘json_export_nvme_tls_key’ at ../src/nvme/json.c:70:24:
../src/nvme/cleanup.h:24:9: warning: ‘tls_str’ may be used uninitialized [-Wmaybe-uninitialized]
24 | free(*(void **)p);
| ^~~~~~~~~~~~~~~~~
../src/nvme/json.c: In function ‘json_export_nvme_tls_key’:
../src/nvme/json.c:70:38: note: ‘tls_str’ was declared here
70 | _cleanup_free_ char *tls_str;
| ^~~~~~~
In function ‘freep’,
inlined from ‘json_export_nvme_tls_key’ at ../src/nvme/json.c:66:32:
../src/nvme/cleanup.h:24:9: warning: ‘key_data’ may be used uninitialized [-Wmaybe-uninitialized]
24 | free(*(void **)p);
| ^~~~~~~~~~~~~~~~~
../src/nvme/json.c: In function ‘json_export_nvme_tls_key’:
../src/nvme/json.c:66:39: note: ‘key_data’ was declared here
66 | _cleanup_free_ unsigned char *key_data;
| ^~~~~~~~
In function ‘freep’,
inlined from ‘json_import_nvme_tls_key’ at ../src/nvme/json.c:37:32,
inlined from ‘json_update_attributes’ at ../src/nvme/json.c:147:3,
inlined from ‘json_parse_port’ at ../src/nvme/json.c:177:2,
inlined from ‘json_parse_subsys’ at ../src/nvme/json.c:212:4,
inlined from ‘json_parse_host’ at ../src/nvme/json.c:246:4,
inlined from ‘json_read_config’ at ../src/nvme/json.c:316:4:
../src/nvme/cleanup.h:24:9: warning: ‘key_data’ may be used uninitialized [-Wmaybe-uninitialized]
24 | free(*(void **)p);
| ^~~~~~~~~~~~~~~~~
../src/nvme/json.c: In function ‘json_read_config’:
../src/nvme/json.c:37:39: note: ‘key_data’ was declared here
37 | _cleanup_free_ unsigned char *key_data;
| ^~~~~~~~
Tomas Bzatek [Mon, 19 Feb 2024 15:19:57 +0000 (10:19 -0500)]
tests: Add complex NBFT table from Dell R660
Two HFIs, four discovery records (only two visible in the
table) and some form of multipath. Two SSNS records marked
as unavailable due to subnet mismatch.
Tomas Bzatek [Fri, 9 Feb 2024 17:11:35 +0000 (18:11 +0100)]
nbft: Add SSNS 'unavailable' flag
Certain pre-OS driver implementations (i.e. UEFI) may opt
to include SSNS records that failed to connect, caused either
by a temporary target inaccessibility or an invalid
configuration. Such reason is further indicated by TP8029
extended information.
This commit adds a flag indicating namespace availability
so that clients (nvme-cli) may either decide to skip such
record or make another connection attempt.
Tomas Bzatek [Thu, 15 Feb 2024 15:33:20 +0000 (10:33 -0500)]
doc: Document the NBFT API
Added an explicit note about unstable API, that is in fact
semi-unstable whereas existing structures are stable and
new API will possibly be added at the end of the structs.
Hannes Reinecke [Wed, 21 Feb 2024 14:31:00 +0000 (15:31 +0100)]
json: import TLS key from PSK interchange format
As now the JSON configuration file holds the TLS key in PSK interchange
format we should be parsing that key and inserting it into the kernel
keystore to make it available for TLS.
Hannes Reinecke [Wed, 21 Feb 2024 14:25:45 +0000 (15:25 +0100)]
json: export TLS key in PSK interchange format
Rather than printing the key serial number (which will only be valid
for this session) we should be exporting the actual key in PSK
interchange format to make it persistent across reboots.
Tomas Bzatek [Thu, 15 Feb 2024 14:21:20 +0000 (09:21 -0500)]
log: Respect DEFAULT_LOGLEVEL on uninitialized logging
When logging has not been initialized and no root available,
logging via nvme_msg() would just print everything. Better
to check against DEFAULT_LOGLEVEL in such cases.
Tomas Bzatek [Wed, 14 Feb 2024 16:25:01 +0000 (11:25 -0500)]
log: Introduce nvme_get_logging_level()
This is essentially a getter for nvme_init_logging() since
nvme_root_t is an opaque struct. Takes optional pointer
to bool args to retrieve PID and timestamp logging values.
Do not clutter the code base with debugging code. Because
the low level passthru functions are now exposed as weak symbol, the
user application can do it by itself.
Daniel Wagner [Tue, 5 Mar 2024 14:32:59 +0000 (15:32 +0100)]
ioctl: export nvme_submit_passthru{64} as weak symbol
Export the two low level ioctl function as weak symbol. This allows the
user application to provide their own version of this function, e.g. in
order to allow meassuring the time spend in the function or printing the
struct nvme_passthru_cmd{64}.
Jian Zhang [Wed, 7 Feb 2024 11:06:17 +0000 (19:06 +0800)]
example: fix mi identify failed with error cntid
This command failed when we try to identify a controller that the
controller id is 1.
Refer to the `Figure 273: Identify - CNS Values`:
+-----------+-------+
| CNS Value | CNTID |
+-----------+-------+
| 01h | N |
+-----------+-------+
When CNS is 01h, the CNTID field is ignored.
See `Figure 270: Identify - Command Dword 10`:
If this field is not used as part of the Identify operation, then
* host software shall clear this field to 0h for backwards compatibility
(0h is a valid controller identifier);
* and the controller shall ignore this field.
This filed is set to controller id in the example code, but it should be
0 when CNS is 1.
PS: The NVMe that we are testing does not ignore the CNTID field and
returns an error when the CNTID field is not 0.
Daniel Wagner [Tue, 6 Feb 2024 13:06:10 +0000 (14:06 +0100)]
tree: do not issue an error when subsys lookup fails during scanning
The scan operation is not atomically done and the sysfs might change
while we are iterating over it. Thus, it's possible that we find a
controller but when we try to lookup the corresponding subsystem it might
already destroyed and removed.
This error makes blktests fail because it finds controllers controller
which are not under control of blktests, instead they are created and
destroyed by the udev auto connect rules.
These resources appear and disappear while the test runs but when we
scan sysfs we issue errors for unrelated resources. Thus just do not
issue a error, turn this into debug log message.
Anyway, we already do just return error codes for other reason in this
function anyway without logging.
Tomas Bzatek [Tue, 23 Jan 2024 16:42:38 +0000 (17:42 +0100)]
tests: Add sample NBFT table from Dell PowerEdge R660
What's special on this table is the second SSNS record that
is roughly equal to the first one except of the 'nsid' and
'nid' values, although only a single subsystem has been
configured and enabled in the EFI Setup.
Tomas Bzatek [Fri, 5 Jan 2024 13:49:15 +0000 (14:49 +0100)]
tests: Fix diffs output for duplicate HFI entries
With commit "nbft: avoid duplicate entries in ssns->hfis" applied,
nbft-dump will not see any duplicate HFI indices any more.
Fix the reference output for generating the diffs.
Signed-off-by: Martin Wilck <mwilck@suse.com> Signed-off-by: Tomas Bzatek <tbzatek@redhat.com>
Martin Wilck [Thu, 11 Jan 2024 10:02:11 +0000 (11:02 +0100)]
nbft: avoid duplicate entries in ssns->hfis
The NVMe boot specification does not disallow listing the primary
HFI index again in the secondary HFI list, or listing the same
index multiple times in the secondary HFI list. But such duplicate
entries aren't helpful for consumers of this data. In the worst
case, they might lead to confusion and misconfiguration.
Suppress them.
The num_hfis field only reflected the number of Secondary HFI
Associations, resulting in the last parsed HFI being ignored
by users (nvme-cli).
According to the NVM Express Boot Specification, Revision 1.0,
the Primary HFI Descriptor Index in the Subsystem Namespace
(SSNS) Descriptor contains this note:
"If multiple HFIs are associated with this record, subsequent
interfaces should be populated in the Secondary HFI
Associations field."
As both the primary and secondary HFIs are parsed into a single
array, it makes sense to reflect the proper number of elements.
Daniel Wagner [Tue, 30 Jan 2024 17:28:40 +0000 (18:28 +0100)]
nvme: allow to overwrite base sysfs path
In order to be able to test the topology scan code, make the
lookup paths flexible. Check if the LIBNVME_SYSFS_PATH environment
variable is set and if so use this as base path.
We could also introduce a setter for this, but this is really
a debugging interface and thus I don't really want it to be
visible in the public API.
Jinliang Wang [Fri, 19 Jan 2024 07:47:04 +0000 (23:47 -0800)]
mi: set correct rc and errno when crc mismatch
Before the fix, when we meet crc mismatch response, the errno is 0
and rc is 1. This combination will be mistaken as Admin Generic Command
Status code value 0x1 (Invalid Command Opcode):
$ nvme id-ctrl mctp:1,20
crc mismatch
NVMe status: Invalid Command Opcode: A reserved coded value or an
unsupported value in the command opcode field(0x1
After the fix, the rc is -1, and errno is set to Bad message.
$ nvme id-ctrl mctp:1,20
crc mismatch
identify controller: Bad message
Signed-off-by: Jinliang Wang <jinliangw@google.com>
This is because the strlen check on subsysnqn or hostnqn crashes at
src/nvme/linux.c due to either of them being null. Avoid this segfault
by checking these strings before running a strlen on them.
gcc complains about potentially uninitialized variables used
in the cleanup function even though such scenario is unlikely
to happen. Still, an explicit initializer makes the warnings
go away.
E.g.
../src/nvme/fabrics.c: In function ‘nvmf_hostnqn_generate’:
../src/nvme/fabrics.c:1297:30: note: ‘stream’ was declared here
1297 | _cleanup_file_ FILE *stream;
| ^~~~~~
In function ‘cleanup_fd’,
inlined from ‘uuid_from_device_tree’ at ../src/nvme/fabrics.c:1189:19,
inlined from ‘nvmf_hostnqn_generate’ at ../src/nvme/fabrics.c:1350:9:
../src/nvme/cleanup.h:37:17: warning: ‘f’ may be used uninitialized [-Wmaybe-uninitialized]
37 | close(*fd);
| ^~~~~~~~~~