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);
| ^~~~~~~~~~
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.
projects / users / sagi / libnvme.git / log