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);
| ^~~~~~~~~~
Daniel Wagner [Wed, 20 Dec 2023 09:48:31 +0000 (10:48 +0100)]
build: handle patch level versioning
Use the patch level identifier from the version string when it is
provided instead blindly extending the version string with '.0'. This
prevents patch level releases.
Daniel Wagner [Wed, 20 Dec 2023 07:26:16 +0000 (08:26 +0100)]
tree: do no free ns on error in nvme_ns_init
The ns pointer is owned by the caller not by nvme_ns_init, thus we can't
just free it on error.
Fixes: 7959f52960fd ("tree: read all attributes from sysfs when available") Reported-by: Tomasz Kłoczko <kloczek@fedoraproject.org> Signed-off-by: Daniel Wagner <dwagner@suse.de>
Daniel Wagner [Tue, 19 Dec 2023 09:40:21 +0000 (10:40 +0100)]
build: construct path to config.h manually
meson reports:
internal/meson.build:25: DEPRECATION: Project uses feature that was
always broken, and is now deprecated since '1.3.0': str.format: Value
other than strings, integers, bools, options, dictionaries and lists
thereof.
Thus just hardcode the config file path by using current_build_dir. Note
this changes the path from a relative one to an absolute one.
Daniel Wagner [Thu, 7 Dec 2023 12:52:59 +0000 (13:52 +0100)]
tree: do not open blk device on default
The fd is not needed anymore if the kernel exposes all necessary sysfs
entries to fully scan the nvme subsystem. Thus do not alwyas open the
blk device and do it only when necessary.
Daniel Wagner [Fri, 1 Dec 2023 09:23:29 +0000 (10:23 +0100)]
tree: read all attributes from sysfs when available
The kernel already exposes parts or all attributes we are looking up
with the ns id command. By reading these from the sysfs we can remove
the last command we are issuing during the topology scan.
Chris Patterson [Fri, 1 Dec 2023 06:21:15 +0000 (22:21 -0800)]
types: fix regression for vendor-specific field in nvme_id_ns
Recent versions of nvme-cli have started reading vs from offset
392 instead of 384. Previous PRs coupled the use of nvme_id_ns
for use in namespace management (create_ns). However, the NVMe
spec has a different structure for namespace management, with
only a subset of the fields allowed/shared and some additional
fields.
To fix this, remove lbstm and restore the proper length for vs
from nvme_id_ns.
I expect that create_ns() should fully switch over to the
nvme_ns_mgmt_host_sw_specified struct which seems aligned with
the spec (though has some newer fields than what is available in
the latest NVMe Command Set Specification Revision 1.0c). This
will have to be addressed separately in nvme-cli.
Caleb Sander [Tue, 28 Nov 2023 21:32:21 +0000 (14:32 -0700)]
tree: use cleanup functions
Use cleanup attributes from cleanup.h to avoid boilerplate cleanup code.
Introduce struct dirents and the corresponding _cleanup_dirents_
to clean up arrays of directory entries.
Caleb Sander [Tue, 28 Nov 2023 20:35:01 +0000 (13:35 -0700)]
cleanup: add cleanup functions
Introduce _cleanup_free_ from nvme-cli to call free() on cleanup,
_cleanup_file_ to call fclose(), _cleanup_dir_ to call closedir(),
and _cleanup_fd_ to call close().
_cleanup_free_ generalizes __cleanup__(cleanup_charp),
so remove it along with cleanup.c.
Caleb Sander [Thu, 30 Nov 2023 18:58:22 +0000 (11:58 -0700)]
tree: fix incorrect return value
nvme_scan_subsystem() accidentally sets errno = -EINVAL
if the matching subsystem is filtered out.
This errno value will be overwritten by the out_free path with -ret,
where ret is the length of the path string.
Set ret = -EINVAL instead, so errno will be set to EINVAL.
This matches the behavior in the case where a new subsystem is allocated
and fails the filter function.
Hannes Reinecke [Thu, 16 Nov 2023 06:41:37 +0000 (07:41 +0100)]
libnvme: support NVMe TLS identities version 1
With NVMe TP8018 a new version '1' for generating NVMe TLS identities
was specified; identities generated for this version require a PSK hash
to be attached to the version '0' identifier.
This patch implements a new function 'nvme_insert_tls_keys_versioned()'
to support this functionality and makes the original function
'nvme_insert_tls_keys()' a wrapper for the new function.
Daniel Wagner [Thu, 16 Nov 2023 14:28:25 +0000 (15:28 +0100)]
build: do not rebuild muon/samu every time
There is not need to build the build tools everytime. Thus stage the
samurai and muon build into a new top level directory .build-tools and
only build them on demand.
Caleb Sander [Tue, 8 Aug 2023 01:21:12 +0000 (19:21 -0600)]
fabrics: avoid redundant args in nvme_discovery_log()
nvme_discovery_log() takes nvme_get_log_args as input
and then overwrites many of its fields.
This leads to wasted code in the caller setting up unused fields.
Instead pass nvme_get_discovery_args, which more accurately
expresses the possible discovery log arguments.
As an added benefit, it also subsumes the other arguments.
And nvmf_get_discovery_wargs() can pass its arguments straight through.
Caleb Sander [Tue, 8 Aug 2023 00:59:10 +0000 (18:59 -0600)]
fabrics: have nvmf_get_discovery_log() call nvmf_get_discovery_wargs()
There is a lot of duplicated code between nvmf_get_discovery_log()
and nvmf_get_discovery_wargs().
Since nvmf_get_discovery_wargs() is more general,
use it to implement nvmf_get_discovery_log().
Caleb Sander [Tue, 1 Aug 2023 17:24:15 +0000 (11:24 -0600)]
fabrics: fetch smaller Discovery Log Page header
Most of the 1 KB Discovery Log Page header is reserved.
Only the first 18 bytes are currently defined.
So avoid transfering more data from the controller than necessary.
Caleb Sander [Tue, 1 Aug 2023 01:57:54 +0000 (19:57 -0600)]
fabrics: avoid redundant Get Log Page on retry
In case of a GENCTR mismatch, nvme_discovery_log() currently fetches
the Discovery Log Page header again before fetching the entries.
This is unnecessary since it was just fetched to determine GENCTR.
So only fetch the header once and re-use it for the next loop iteration.
Remove a couple of unnecessary variables in the process.
Caleb Sander [Tue, 1 Aug 2023 01:29:03 +0000 (19:29 -0600)]
fabrics: clear RAE for discovery log page commands
In several circumstances, nvme_discovery_log() leaves the RAE bit set
for all Get Log Page commands it issues.
This happens if the header indicates the log page has no records,
or if a command other than the header re-fetch errors out.
If the RAE bit is never cleared, the controller will not send more
Discovery Asynchronous Event Notifications.
Setting the RAE bit is not necessary to avoid missed events since
the use of GENCTR ensures the log page is fetched atomically.
It is better to risk receiving additional AENs
than for the discovery controller to never send more AENs
because it is waiting for a Get Log Page command that never comes.
So clear the RAE bit when fetching each piece of the log page.
Caleb Sander [Thu, 2 Nov 2023 13:29:56 +0000 (14:29 +0100)]
test: Cast values to u32 if shift overflows int
Bit shifts that overflow the resulting type are undefined behavior in C.
C arithmetic promotes to ints all smaller integer types.
There are several places where a 32-bit unsigned value
is constructed by shifting a u8 or u16 to the most significant bits.
Since this overflows a signed 32-bit integer,
explicitly cast to u32 to avoid the UB.
Technically, an int is allowed to only be 16 bits,
so any shift that could set bit 15 or higher is UB.
But platforms where int is s16 are not very common,
so it's likely not worth the effort to fix the code.
Caleb Sander [Thu, 2 Nov 2023 13:29:46 +0000 (14:29 +0100)]
mi: Cast values to u32 if shift overflows int
Bit shifts that overflow the resulting type are undefined behavior in C.
C arithmetic promotes to ints all smaller integer types.
There are several places where a 32-bit unsigned value
is constructed by shifting a u8 or u16 to the most significant bits.
Since this overflows a signed 32-bit integer,
explicitly cast to u32 to avoid the UB.
Technically, an int is allowed to only be 16 bits,
so any shift that could set bit 15 or higher is UB.
But platforms where int is s16 are not very common,
so it's likely not worth the effort to fix the code.
Caleb Sander [Sat, 14 Oct 2023 01:28:33 +0000 (19:28 -0600)]
types: Cast values to u32 if shift overflows int
Bit shifts that overflow the resulting type are undefined behavior in C.
C arithmetic promotes to ints all smaller integer types.
There are several places where a 32-bit unsigned value
is constructed by shifting a u8 or u16 to the most significant bits.
Since this overflows a signed 32-bit integer,
explicitly cast to u32 to avoid the UB.
Technically, an int is allowed to only be 16 bits,
so any shift that could set bit 15 or higher is UB.
But platforms where int is s16 are not very common,
so it's likely not worth the effort to fix the code.
Caleb Sander [Thu, 2 Nov 2023 13:26:36 +0000 (14:26 +0100)]
test: Avoid unaligned pointer dereferences
Avoid casting byte-aligned pointers to pointers with higher alignment.
Loading or storing values with higher alignment is undefined behavior,
since some processors don't allow unaligned memory accesses
and compilers may assume pointers of different types don't alias.
Perform an explicit memcpy(), which an optimizing compiler
can easily replace with a single load/store on supported architectures.
Caleb Sander [Sat, 14 Oct 2023 04:35:43 +0000 (22:35 -0600)]
nbft: Avoid unaligned pointer dereferences
Avoid casting byte-aligned pointers to pointers with higher alignment.
Loading or storing values with higher alignment is undefined behavior,
since some processors don't allow unaligned memory accesses
and compilers may assume pointers of different types don't alias.
Perform an explicit memcpy() in two places, which an optimizing compiler
can easily replace with a single load/store on supported architectures.
While we're touching this code, also use IN6_IS_ADDR_V4MAPPED()
instead of hand-rolling it.
Caleb Sander [Sat, 14 Oct 2023 01:28:09 +0000 (19:28 -0600)]
test: don't pass NULL to memcmp() or memset()
According to the C standard, it is undefined behavior
to pass NULL pointers to standard library functions.
This includes the mem*() family of functions,
even they are passed a length of 0.
So avoid calling these functions when the length is 0.
Caleb Sander [Sat, 14 Oct 2023 01:13:56 +0000 (19:13 -0600)]
test: make LD_PRELOAD tests work with ASAN
Several tests mock libc functions by setting LD_PRELOAD
to a shared library containing mock implementations of the functions.
Currently this prevents the tests from running with ASAN.
ASAN complains that libasan doesn't come first in LD_PRELOAD and aborts.
But in practice this doesn't seem to be an issue.
Sample ASAN issues added to libnvme, the test cases, and the mocks
are all correctly reported.
So suppress this warning by setting the environment variable
ASAN_OPTIONS=verify_asan_link_order=0.
In case the user does want to inject a specific libasan,
change the tests' use of LD_PRELOAD to append the mock shared library
rather than overwriting LD_PRELOAD entirely.
Caleb Sander [Fri, 13 Oct 2023 23:41:30 +0000 (17:41 -0600)]
readme: clarify that LD_PRELOAD is not needed for ASAN
The README currently suggests setting LD_PRELOAD to the libasan path.
However, this seems to be unnecessary (at least on my gcc 9 setup).
The test executables request to link with libasan.so (before libc.so),
so it is automatically loaded.
The recommended suggestion also ends up running ninja with ASAN
instrumentation, which causes it to fail with leaks detected.
Joy Gu [Wed, 25 Oct 2023 20:47:42 +0000 (13:47 -0700)]
types: add cross-namespace copy formats, status codes, ONCS bits
Add support for NVMe TP4130 ("Cross-Namespace Copy"):
- Add Copy Descriptor Formats 2h and 3h
- Add new status codes for Copy: Incompatible Namespace or Format, Fast
Copy Not Possible, Overlapping I/O Range, and Insufficient Resources
- Add two new ONCS bits NVMCSA and NVMAFC
- Add Copy Descriptor Formats Enable (CDFE) to Host Behavior Support
Data Structure
[dwagner: whitespace cleanups
moved new functions to 1.7 linker section] Signed-off-by: Daniel Wagner <dwagner@suse.de>
Caleb Sander [Tue, 20 Jun 2023 03:17:16 +0000 (21:17 -0600)]
fabrics: use SECTYPE to determine whether to use TLS
The NVMe specfications are clear that the discovery log page entry
SECTYPE field indicates whether TLS is supported.
Currently the TREQ field is used, which results in enabling TLS
even when SECTYPE = "No Security" and TREQ = "Not required".
Only enable TLS if SECTYPE indicates a TLS version is enabled.
From the NVMe/TCP transport specification, version 1.0c:
Security Type (SECTYPE): Specifies the type of security used by the
NVMe/TCP port. If SECTYPE is a value of 0h (No Security), then the host
shall set up a normal TCP connection.
From TP 8018:
The SECTYPE value of 0h (i.e., No Security) specifies that TLS is not
supported.
From TP 8025 (the "TLS Permitted" host case):
If the SECTYPE field in the TSAS field in the discovery log entry for
the remote entity is cleared to zero and the TSC field is not set to 01b
(i.e., Required), then initiate TCP connections without TLS.
projects / users / sagi / libnvme.git / log