Martin Belanger [Tue, 15 Aug 2023 19:16:24 +0000 (15:16 -0400)]
meson: Don't hard-code path to "internal/config.h"
When building libnvme as a subproject of another project
(e.g. nvme-stas), the hard-coded absolute path to config.h, i.e.
"-include internal/config.h" does not resolve properly and fails
to build. Instead, use the real path calculated by meson and
saved to variable "config_h".
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Add unit tests for nvmf_get_discovery_log().
They provide coverage of the logic in nvme_discovery_log(),
nvme_get_log_page(), and nvme_get_log() too.
The tests use the mock ioctl() infra to validate the Get Log Page
commands issued and inject responses triggering different code paths.
Functions issuing admin/IO passthru ioctls sorely lack unit tests.
It would be great for unit tests not to need a real NVMe controller.
It's also useful to be able to test responses to commands
that might be impossible to trigger with real controllers.
To that end, implement infrastructure for mocking ioctl(),
allowing tests to set expectations for the NVMe passthru ioctls
that will be issued and control the corresponding responses.
The mock library can be used with LD_PRELOAD so that libnvme's ioctl()
calls are redirected from libc. No changes in libnvme itself are needed.
Hannes Reinecke [Tue, 8 Aug 2023 11:32:53 +0000 (13:32 +0200)]
src/nvme/tree.c: make __nvme_scan_subsystem() returning bool
__nvme_scan_subsystem() will free the 's' argument when the filter
triggers, so it needs a return value to inform the caller that the
argument has been freed.
Martin George [Wed, 26 Jul 2023 13:31:29 +0000 (19:01 +0530)]
nvme-tree: avoid warning in 'list-subsys'
With the recent change to scan all subsystems, 'nvme list-subsys
/dev/nvmeXnY' now displays an annoying warning for the NQN mismatch
for all other subsystems that don't match during the subsystem
scan. For e.g.
NQN mismatch for subsystem 'nvme-subsys1'
NQN mismatch for subsystem 'nvme-subsys4'
nvme-subsys3 - NQN=nqn.1992-08.com.netapp:sn.48391d66c0a611ecaaa5d039ea165514:subsystem.subsys_CLIENT116_1
hostnqn=nqn.2014-08.org.nvmexpress:uuid:e6550026-173e-4959-ba74-be367844bd8a
\
+- nvme3 tcp traddr=192.168.1.116,trsvcid=4420,host_traddr=192.168.1.16,host_iface=eth5 live optimized
+- nvme7 tcp traddr=192.168.2.116,trsvcid=4420,host_traddr=192.168.2.16 live optimized
+- nvme8 tcp traddr=192.168.1.116,trsvcid=4420,host_traddr=192.168.2.16 live optimized
Avoid this warning by displaying it only under debug level.
Fixes: fbd45f1 ("tree: Scan all subsystems") Signed-off-by: Martin George <marting@netapp.com>
Daniel Wagner [Mon, 17 Jul 2023 11:54:30 +0000 (13:54 +0200)]
tree: Scan all subsystems
We need to scan all subsystems because a subsystem might show up on
different hosts, e.g 'nvme connect' with different hostnqn and the
target reports the same namespace.
Jeremy Kerr [Mon, 3 Jul 2023 08:14:47 +0000 (16:14 +0800)]
mi: allow non-4-byte-aligned responses
We currently assume that a MI response will be a multiple of four bytes
in length. However, this may not be the case: for example, a Read MI
Data (Controller List) with an even number of controllers, and with an
unpadded response, may only be aligned on a two-byte boundary.
The NVMe-MI spec states, for the MIC field:
This field is byte aligned.
So, relax the requirement for alignment on the response sizes, and the
expected response size values. We only do this for the mi commands; the
Admin commands still require an aligned value for DLEN.
In doing so, drop the explicit alignment tests, and add a couple that
check that the Controller List example above will work.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au> Reported-by: Klaus Jensen <its@irrelevant.dk>
Jeremy Kerr [Mon, 3 Jul 2023 08:06:34 +0000 (16:06 +0800)]
mi-mctp: use a linear response buffer
Currently, we're passing a 3-entry iovec to the MCTP resvmsg()
interface:
- header
- payload
- MIC
This is fine if the response comes back eaxctly the size we expect, but
causes complexity if we get a smaller response (for example, as an error
or a More Processing Required response), as we need to extract the MIC
from somewhere in those buffers.
At the moment, since we're enforcing 4-byte alignment, that isn't too
complex - we know the MIC will be entirely in one of the buffers. The
MPR code is a bit awkward, but still manageable.
However: we now want to allow unaligned responses from MI messages,
which is about to make that a lot more complex; in the worst case, the
MIC could be split over all three buffers!
This change uses a fixed linear buffer for the MCTP response instead. We
allocate 4k for this by default, but expand if necessary. We use this as
the sendmsg() buffer, so get a linear message back from the MCTP
endpoint. Once we have verified the format (and extracted the MIC), we
copy this into the actual response header/payload buffers as required.
This makes the response handing code simpler, at the cost of one extra
response buffer per endpoint.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Mon, 3 Jul 2023 07:55:04 +0000 (15:55 +0800)]
mi: implement length and offset alignment checks in admin_xfer()
We're about to relax some alignment requirements in the generic
(internal) nvme_mi_submit function. To ensure that the raw admin
interface continues to enfore the required alignment on DOFST and DLEN
fields, implement checks in the Admin command interface.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Martin Belanger [Wed, 5 Jul 2023 14:59:25 +0000 (10:59 -0400)]
tree: missing closedir() causes fd leak for "/sys/bus/pci/slots"
In nvme_ctrl_lookup_phy_slot(), we are missing a closedir(), which
causes file descriptors to leak. Also, there was a missing free()
when the function returns with ENOMEM.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Jeremy Kerr [Wed, 24 May 2023 09:29:33 +0000 (17:29 +0800)]
mi: don't return from mi_mctp_submit with a tag held
If the poll() times-out or fails, we'll exit early from
nvme_mi_mctp_submit still holding the tag reservation. When using an i2c
transport, this may mean we hold a lock on the i2c bus with no way to
release.
Instead, always drop the tag on function exit.
Fixes: 6a08780 ("mi-mctp: Add timeout support to MCTP transport") Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Daniel Wagner [Fri, 23 Jun 2023 13:29:15 +0000 (15:29 +0200)]
build: Move CI build steps into a scripts
Move the build instruction into a script. This allows to run these steps
also locally.
Also disable the fallback static library build as it is clearly not
working because in the dependencies rely to link against a dynamic
glibc. Instead just add a minimal static build without fallbacks.
Daniel Wagner [Thu, 22 Jun 2023 12:13:13 +0000 (14:13 +0200)]
fabrics: Relax match on well known disc ctrl lookup
In case nvmf_add_ctrl() is called to add a well known discovery
controller we also need to verify if we should ignore it (see --context
command line argument of nvme-cli). Though we have to be careful not to
overmatch on the lookup.
That means the host_traddr and host_iface might be different for the
discovery controller than the normal controllers. For example this can
happen when the discovery controller is reached via different interface
than the data controllers.
Thus only consider the transport type, target address and trsvcid only
when looking up well known discovery controllers.
Daniel Wagner [Wed, 14 Jun 2023 12:19:10 +0000 (14:19 +0200)]
fabrics: Filter discovery ctrls out during application context check
We also need to filter out the well known discovery controllers when
using the execution context filtering. Obviously, we can't use the
subsystem name, thus match on the host and target address instead.
Daniel Wagner [Wed, 14 Jun 2023 12:17:41 +0000 (14:17 +0200)]
util: Add ignored error code
When libnvme ignores a connection attempt via the 'application' context
tracking return an unique error code to allow proper filtering on the
caller side.
Umer Saleem [Mon, 12 Jun 2023 14:45:49 +0000 (19:45 +0500)]
tree: Add PCI physical slot number for controller
This commit introduces a physical slot field for controller, that
contains the PCI physcial slot number for controller device.
In case, there are multiple NVME drives present on the platform,
it's hard to identify which NVME drive is present in which slot.
The slot number is usually helpful in determining the location.
It is cross reference-able from lspci, but it would be nice to
have a direct option.
Hannes Reinecke [Thu, 20 Apr 2023 10:38:10 +0000 (12:38 +0200)]
fabrics: filter out subsystems with non-matching application string
If the nvme root has an application string set any subsystem lookup
should ignore subsystems which either have no application string set
or which have a non-matching application string.
Caleb Sander [Fri, 12 May 2023 15:43:22 +0000 (09:43 -0600)]
ioctl: fix RAE bit on last Get Log Page command
If nvme_get_log_page() requires multiple Get Log Page commands
because the total log length exceeds the transfer length,
args->rae is overwritten, causing the RAE bit to be set in all commands.
Retrieve the value of args->rae before overwriting it
so the RAE bit is set as requested in the last command.
Fixes: c23dbd4 ("linux: Change nvme_get_log_page to use nvme_get_log_args parm") Signed-off-by: Caleb Sander <csander@purestorage.com>
Caleb Sander [Fri, 12 May 2023 16:49:46 +0000 (10:49 -0600)]
fabrics: check genctr after getting discovery entries
From the NVMe base spec (version 2.0c, section 5.16.1.23):
If the host reads the Discovery Log Page using multiple Get Log Page
commands the host should ensure that there has not been a change in the
contents of the data. The host should read the Discovery Log Page
contents in order (i.e., with increasing Log Page Offset values) and
then re-read the Generation Counter after the entire log page is
transferred. If the Generation Counter does not match the original value
read, the host should discard the log page read as the entries may be
inconsistent.
nvme_get_log_page() will issue multiple Get Log Page commands
to fetch the discovery log page if it exceeds 4 KB.
Since GENCTR is at the start of the log page, this ordering is possible:
- GENCTR is read by a Get Log Page command for the first 4 KB
- The log page is modified, changing GENCTR
- Other Get Log Page commands read the remainder of the log page
So the check that GENCTR hasn't changed will incorrectly pass,
despite the log page having been modified.
This can lead to inconsistent, missing, or duplicate log page entries.
Ensure a GENCTR update is not missed
by fetching log page header again after all entries.
Also use NVME_LOG_PAGE_PDU_SIZE to match other nvme_get_log_page() calls
instead of hard-coding the 4 KB max transfer length.
And ensure LPO is correctly reset if the log page is read again.
Caleb Sander [Fri, 12 May 2023 00:40:26 +0000 (18:40 -0600)]
fabrics: handle /dev/nvme-fabrics read failure
The ability to read from /dev/nvme-fabrics to find supported options
is a newer Linux kernel feature added in f18ee3d988157 (5.17-rc1).
On earlier kernels, this read returns EINVAL,
preventing the controller from being added:
$ nvme discover --transport tcp --traddr 192.168.1.62
Failed to read from /dev/nvme-fabrics: Invalid argument
failed to add controller, error Invalid argument
So don't treat EINVAL as a fatal error, and instead fall back
to a default set of supported options.
With this change, controllers can be created successfully:
$ nvme discover --transport tcp --traddr 192.168.1.62
Discovery Log Number of Records 4, Generation counter 125
...
Fixes: d123131f2e ("fabrics: Do not pass unsupported options to kernel") Signed-off-by: Caleb Sander <csander@purestorage.com>
Maurizio Lombardi [Mon, 8 May 2023 15:47:00 +0000 (17:47 +0200)]
fabrics: fix potential invalid memory access in __nvmf_supported_option()
In __nvmf_supported_option(), len is declared as size_t (unsigned)
"len = read()" may return a negative number;
the check "if (len < 0)" will always be false and therefore
"buf[len]" will dereference an invalid memory address.
len should be declared as a signed size_t (ssize_t)
Prevent Garbage Collector (GC) from deleting host and root objects
before all controller objects under that root/host have been GCed.
This time, it's the init() method that needed the fix. Previously
we had only fixed the connect() method.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
To suppress the warnings generated by swig when parsing anonymous
struct/union, we simply wrap the offending struct/union in a
"#ifndef SWIG" statement. This is an acceptable workaround because
we don't need to generate Python bindings for these structs. In
fact, we specifically tell swig to not generate wrappers for all
structs in types.h. Although swig does not generate wrappers for
those structs, it still warns when a struct doesn't have a name
and therefore we need to use this workaround.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
examples: fix incorrect controller status in MI info output
In the mi-mctp example, we're incorrectly reporting the percent drive
life used as the controller status. Fix the controller status output
to use the correct (ccs) field.
Signed-off-by: Lior Weintraub <liorw@pliops.com> Reviewed-by: Jeremy Kerr <jk@codeconstruct.com.au>
Martin Belanger [Tue, 18 Apr 2023 13:19:32 +0000 (09:19 -0400)]
Python: make NBFT data more pythonic
I made the nfollowing changes so that the data is more Pythonic.
1) For boolean values, set them to True/False instead of 1/0.
2) NBFT data contains ordered lists. In the raw NBFT data the
position of each element in the list is indicated by a 1-based
index. When converting to Python lists, make sure that each
element is inserted in the list at the right position. This is
done by converting the 1-based index to a 0-based index.
3) For objects that contain index variables that refer to items in
a list, make sure to convert the 1-based index to 0-based so that
it can be used directly to access the python lists (e.g. list[index]).
4) Since Python lists are ordered (per 2 above), there is no
need to keep an explicit 1-based index in each of the list items.
Therefore those 1-based indexes were removed.
5) No need to keep explicit variables representing the length of
a list. In Python one need only use len(list) to get the length.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Daniel Wagner [Mon, 17 Apr 2023 08:25:55 +0000 (10:25 +0200)]
python: Update test data
Since commit 1617d1a3f42a ("nbft: Parse the {HOSTID,HOSTNQN}_CONFIGURED
flags") host_id_configured and host_nqn_configured are parsed. Thus we
need to update the test case accordingly.
Tomas Bzatek [Tue, 11 Apr 2023 16:04:55 +0000 (18:04 +0200)]
nbft: Add a simple unit test
A simple table dump utility, a set of real ACPI NBFT table files
and corresponding set of reference dumps, compared against each
other as part of the meson test run.
Daniel Wagner [Thu, 13 Apr 2023 10:41:31 +0000 (12:41 +0200)]
build: Disable fallback on default
meson's default setting for wrap mode is to attempt to download missing
dependencies. Disable this feature as the community is unhappy with
this default behavior.
Daniel Wagner [Wed, 12 Apr 2023 13:43:18 +0000 (15:43 +0200)]
tree: Fix offset argument check in nvme_bytes_to_lba
Also offset modulo blocksize needs to be 0. Commit 01c6055e5602 ("tree:
Fix argument check in nvme_bytes_to_lba") missed to update this, thus do
it now.
Stuart Hayes [Thu, 31 Mar 2022 18:47:11 +0000 (13:47 -0500)]
nbft: add NBFT v1.0 table support
Added support for parsing and printing the contents
of the NBFT table (per NVMe-oF boot specification v1.0).
Signed-off-by: Stuart Hayes <stuart_hayes@dell.com> Signed-off-by: Martin Belanger <martin.belanger@dell.com> Signed-off-by: Martin Wilck <mwilck@suse.com> Signed-off-by: Tomas Bzatek <tbzatek@redhat.com> Signed-off-by: John Meneghini <jmeneghi@redhat.com>
Daniel Wagner [Wed, 5 Apr 2023 06:37:38 +0000 (08:37 +0200)]
tree: Fix argument check in nvme_bytes_to_lba
nvme_bytes_to_lba() argument checker is ensuring that all passed in
values are valid. That means we have at least one block to write, the
offset is aligned to a block starting address and the number of blocks
is a multiple of the block size
Hannes Reinecke [Tue, 28 Mar 2023 10:28:21 +0000 (12:28 +0200)]
Print out correct TREQ strings for discovery
The TREQ field in the discover log page is actually a bit field,
so we need to update the string mapping to print out all possible
combinations correctly.
Minwoo Im [Wed, 22 Mar 2023 22:59:03 +0000 (07:59 +0900)]
tree: fix generic device open failure
If scan_namespace is called with a generic device (e.g., /dev/ng0n1)
given, it fails to scan a namespace based on the generic device because
there's no entry point in /sys/block/ for the generic device.
This patch provides two helpers to change the given generic device name
to a block device name based on the instances:
ng0n1 -> nvme0n1
This patch fixes command failure:
root@vm:~/work/nvme-cli.git# nvme show-regs /dev/ng0n1
Unable to find ng0n1
get-property: Invalid argument
Signed-off-by: Minwoo Im <minwoo.im@samsung.com>
[dwagner: also catch blkdev allocation failures] Signed-off-by: Daniel Wagner <dwagner@suse.de>
Hannes Reinecke [Thu, 23 Mar 2023 09:44:02 +0000 (10:44 +0100)]
fabrics: add configuration option 'tls_key'
Add a fabrics configuration option to specify the TLS PSK for a
connection. The PSK is referenced by its serial number, but stored
with its description in the JSON configuration file.
Hannes Reinecke [Thu, 23 Mar 2023 09:14:03 +0000 (10:14 +0100)]
fabrics: add configuration option 'keyring'
Add a fabrics configuation option 'keyring' to set the keyring
for storing and looking up keys.
As the keyring serial number is ephemeral we cannot store it
in the JSON configuration file, so store the keyring description
instead.
Hannes Reinecke [Thu, 23 Mar 2023 13:46:13 +0000 (14:46 +0100)]
linux: add key helper functions
Add helper functions for key handling.
Signed-off-by: Hannes Reinecke <hare@suse.de>
[dwagner:
- set errno on failure and updated documentation accordingly
- fix return check of nvme_lookup_key in nvme_insert_tls_key] Signed-off-by: Daniel Wagner <dwagner@suse.de>
Hannes Reinecke [Thu, 23 Mar 2023 13:18:44 +0000 (14:18 +0100)]
linux: add nvme_lookup_keyring()
Add a function to lookup a keyring by its description.
Signed-off-by: Hannes Reinecke <hare@suse.de>
[dwagner:
- pass in command line option to dependency requirement argument
- drop log message, find_key_by_type_and_desc sets errno] Signed-off-by: Daniel Wagner <dwagner@suse.de>
Daniel Wagner [Fri, 24 Mar 2023 15:50:48 +0000 (16:50 +0100)]
build: Update OpenSSL wrap
Update to OpenSSL 3.0.7.
When the fallback was using v1 we never went into the v3 testing. With
updating to v3 we are going to figure out if we have LibreSSL spin of
the API. But cc.has_header_symbol() and cc.has_header() only work on
external library and not on fallbacks.
Thus if we use the fallback (internal) we just know it is API version 3
and don't do any crazy testing.
Martin Belanger [Fri, 17 Mar 2023 15:56:26 +0000 (11:56 -0400)]
python: Add setter property for controller DHCHAP Key
Similar to the way we allow setting/getting the Host DHCHAP key,
we need to allow setting/getting the Controller DHCHAP Key.
There was already a "getter" property, but the "setter" property
was missing.
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
projects / users / sagi / libnvme.git / log