Jeremy Kerr [Thu, 7 Jul 2022 07:06:19 +0000 (15:06 +0800)]
mi: make return values + errno consistent across error paths
This change unifies and documents the error return values for the MI
implementation. This gives us the following semantics:
- zero on success
- -1 for errors in the MI communication with an endpoint, with errno
set accordingly
- positive values where the MI communication succeeded, but we received
an error response from the endpoint. The return value will be that
from the MI response status field, and should correspond to one of
the nvme_mi_resp_status values.
We add these semantics to the file-level kdoc comments in mi.h.
Most of the changes here are replacing the negative-errno returns:
return -EIO;
with:
errno = EIO;
return -1;
but there are a few slightly-more-involved changes where we need to
preserve errno across a cleanup/log that might clobber it.
For the dbus code in mi-mctp.c, we need to convert the sd_bus convention of
negative-errno values into errno; we can do most of these through the
dbus_err() helper.
Fixes: https://github.com/linux-nvme/libnvme/issues/417 Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Wed, 6 Jul 2022 05:35:22 +0000 (13:35 +0800)]
mi: Add examples for MI configuration commands
Now that we have the MI Get/Set Configuration commands implemented, add
support in examples/mi-mctp to illustrate some basic configuration
interactions.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Tue, 12 Oct 2021 03:13:58 +0000 (11:13 +0800)]
mi: Add MI configuration commands
This change adds an API to send MI Get / Set Configuration commands to
an endpoint, allowing control of endpoint SMBus frequenct, MCTP MTU and
clearing the subsystem health status bits.
We do this with a new couple of functions:
int nvme_mi_mi_config_get(nvme_mi_ep_t ep, __u32 dw0, __u32 dw1,
__u32 *nmresp)
int nvme_mi_mi_config_set(nvme_mi_ep_t ep, __u32 dw0, __u32 dw1)
The dw0, dw1 and nmresp formats depend on the type of configuration
accessed, which can be a little opaque. Se we add a bunch of helpers
too, to get/set the three configuration params:
Jeremy Kerr [Tue, 12 Oct 2021 03:13:58 +0000 (11:13 +0800)]
mi: Add types for MI Get/Set Configuration data
This change adds the mi-specific types for Get/Set configuration
commands: enum nvme_mi_config_id for the currently-defined IDs, and enum
nvme_mi_config_smbus_freq for the possible SMBus frequency configuration
values.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Sat, 2 Jul 2022 06:22:16 +0000 (14:22 +0800)]
types: fix typo on nvme_mi_ccs definitions in public API
The NVMe-MI spec defines the Composite Controller Status definitions
using the abbreviation CCS. However, the libnvme code (since initial
commit) has used CSS, not CCS (except for CCS_RDY...).
This change fixes the definitions to use the _CCS_ names, and adds a set
of #defines to allow the old names to be used, including for the enum
itself.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Mon, 30 May 2022 07:01:18 +0000 (15:01 +0800)]
mi-mctp: Add support for More Processing Required responses
This change allows the mi-mctp transport to receive "more processing
required" responses from MI commands. When we detect a MPR response, we
keep listening for a subsequent message containing the actual response.
The NVMe-MI spec allows for only one MPR response per request.
To do this, we need to perform explicit tag control for MCTP messaging;
we cannot use the kernel's default tag handling, which would expire the
tag value once we've received the first (MPR) response. Instead, we use
the tag control ioctl()s to allocate tags, and expire once we receive
the final response.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Thu, 30 Jun 2022 07:49:12 +0000 (15:49 +0800)]
test: move test_peer->rx_fn function to transmit path
We currently have the peer->rx_fn called after libnvme has sent data to
the NVMe device, as a way for the test to check the sent data and modify
a response accordingly.
However, for testing More Processing Required responses, we will have
two TXes for a single RX. So, move the rx_fn callback to the TX side
(ie, when libnvme calls recvmsg). This allows the test to provide
TX-specific data for each call.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Wed, 29 Jun 2022 10:14:51 +0000 (18:14 +0800)]
mi-mctp: better handling for error/unexpected responses
Currently, the mi-mctp transport expects there to be at least a complete
header-sized reply - matching the caller's resp->hdr_size value,
otherwise we consider this a transport failure.
However, Admin command responses have a header that's larger than the
generic error response message, so we should be able to handle response
messages that are just the small header and no payload.
This change reworks the response-buffer handling in the MCTP transport.
Rather than storing the payload and MIC into an extra allocated buffer,
we just lay out the response iovec directly into the header, payload and
MIC buffers, and then handle the short-response cases as needed. This
means we can properly extract the error response, rather than just
having the transport report a generic "header too small" failure itself.
This is at high risk for off-by-one errors, so add tests to cover each
of the possible buffer-arrangement cases.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Wed, 29 Jun 2022 07:11:24 +0000 (15:11 +0800)]
mi: be pedantic with response message format
This change adds a few tests to check against invalid response data
received by the NVMe-MI device, and some tests to simulate these
conditions.
Because we're checking more of the response fields, we need a bit more
support for constructing default responses in the mi and mi-mctp tests,
so add that too.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Thu, 16 Jun 2022 06:55:40 +0000 (14:55 +0800)]
test: Add initial tests for mi-mctp layer
This change adds a new test module, for the MCTP transport of the MI
layer.
To do this, we hook into the socket, sendmsg and recvmsg calls from the
core mi-mctp code, allowing the test to provide a mock communication
with the kernel MCTP layer.
On top of this, we write a few simple test cases for the socket error
resonses, which we'll extend in future changes.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Mon, 27 Jun 2022 06:23:53 +0000 (14:23 +0800)]
mi: track controller lifetime under endpoint and provide endpoint scan
This change introduces a list of controllers under each MI endpoint, and
ties the controller's lifetime to the endpoints - when an endpoint is
free()ed, so are its controllers.
We also provide a new function to populate the controllers list:
nvme_mi_scan_ep()
- which scans the endpoint for its owned controllers. For retrieving the
scan results, we now have a new macro:
nvme_mi_for_each_ctrl(ep, ctrl) {
...
}
Add a couple of new tests: one to check the lifetime semantics, and
another to test the scan using a fake Read MI Data (Controller List)
response.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Daniel Wagner [Mon, 27 Jun 2022 06:19:47 +0000 (08:19 +0200)]
doc: Workaround kenrel-doc code scanner
The kernel-doc scanner trips over a void function. It will complain
with 'contents before sections'. To help out, explicitly move the
state machine into the next state which avoids the warning.
Matt Johnston [Tue, 30 Nov 2021 04:46:50 +0000 (12:46 +0800)]
mi: Add facility to scan MI endpoints from MCTP daemon
This change adds support for querying a local MCTP daemon ("mctpd")
instance over its D-Bus interface, to populate the list of available
enpoints in root->endpoints.
This requires libsystemd for sdbus, if not available the
nvme_mi_scan_mctp() will always return failure at runtime.
Update for v1.x root API by Jeremy Kerr <jk@codeconstruct.com.au>.
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Tue, 21 Jun 2022 08:47:18 +0000 (16:47 +0800)]
mi: free endpoints on root destruction
The root_t object needs to persist through the lifetime of an endpoint,
so close the endpoints on nvme_mi_free_root, and track allocated
endpoints in the root->endpoints list.
To do the destruction safely, we need a _safe variant of the endpoint
iterator macro.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Matt Johnston [Tue, 21 Jun 2022 08:12:46 +0000 (16:12 +0800)]
mi: provide a facility for transports to describe endpoints
This change adds a callback to struct nvme_mi_transport, allowing
transports to way to describe the endpoint as a human-readable string.
We call this in a new API function, nvme_mi_endpoint_desc(), which has a
fallback implementation if no callback is provided.
[split out from a previous patch by Jeremy Kerr, test added]
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Thu, 23 Jun 2022 11:35:25 +0000 (19:35 +0800)]
test: define separate library for MI unit tests
We previously exposed a couple of internal functions for libnvme-mi.so
in ce4a044720. However, Daniel Wagner has suggest instead using a
separate shared library with all symbols visible, just for the unit
test.
This change reverts the symbol visibility changes for the installed
libnvme-mi.so, and defines a separate, non-installed library for the MI
unit tests. This is the same as libnvme-mi.so, but has no linker script
or versioning information.
This means we're not exposing the init_ep and crc32 functions in the
actual .so, but can still allow unit tests to access non-public symbols.
The coverage reports can handle the separate library just fine, as the
coverage data is correlated on the underlying source files.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Thu, 23 Jun 2022 07:37:20 +0000 (15:37 +0800)]
MI: reinstate init_ep and crc32 functions for use in test
Ideally, we would be using the actual implementation of
nvme_mi_init_ep() and nvme_mi_crc32_update for our tests, rather than
open-coding it in the test init.
This change exports nvme_mi_init_ep and nvme_mi_crc32_update from
libnvme-mi.so, but both remain excluded from the headers, as they are
only intended for use in the transport API. This means we can call them
from the tests, but keep somewhat-internal.
We put this into a specific _TEST section of the version script, to keep
separate from the public symbols, and add a comment to suit.
This prevents us from diverging the endpoint init process in our
testcases.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Hannes Reinecke [Wed, 22 Jun 2022 10:28:00 +0000 (12:28 +0200)]
fabrics: Update controller authentication in nvmf_add_ctrl()
When calling merge_config() in nvmf_add_ctrl() the controller
authentication key can't be updated reliably as it may already
been set. So update the controller with the correct key
manually.
Suggested-by: Sagi Grimberg <sagi@grimberg.me> Signed-off-by: Hannes Reinecke <hare@suse.de>
Daniel Wagner [Wed, 22 Jun 2022 06:52:32 +0000 (08:52 +0200)]
test: Remove code dependency for mi test
When generating th coverage report, lcov chokes on the '../src/nvme'
paths prefix:
geninfo: WARNING: GCOV did not produce any data for /home/wagi/work/libnvme-upstream/obj/src/test-mi.p/.._test_mi.c.gcda
We could restructure the project layout, e.g. by moving the test/mi.c
file to the src/nvme direcotry. But that looks ugly and as it turns
out we only really need one function copied over
nvme_mi_crc32_update() to resolve the dependency. The other function
is nvme_mi_init_ep() which we can easily open code.
Obviously, it would be better to fix the path issue for lcov but for
the time being let's go with this.
Daniel Wagner [Mon, 20 Jun 2022 08:07:25 +0000 (10:07 +0200)]
doc: Add kernel-doc to header files
Add missing documentation bits to all header files. This a very
rudimentary and surely needs a to be refined later. But let's first
try to get kernel-doc to report 0 warning/errors so we can actually
detect offenders added by new code.
Jeremy Kerr [Mon, 8 Nov 2021 02:13:18 +0000 (10:13 +0800)]
mi: Add security send & security receive commands
Add functions for the Security Send & Security Receive Admin Channel
commands, and a small example in mi-mctp.
Includes a fix for specifying the security_send data, Reported-by: Hao Jiang <jianghao@google.com> Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Mon, 20 Jun 2022 07:29:07 +0000 (15:29 +0800)]
mi: fixes for MI Identify command serialisation
We have a couple of bugs when creating the Identify command message,
particularly on big-endian platforms. In general, we should be
performing endian-conversion on the whole cdw fields, rather than on
individual components, provided we've done the shift operations in the
same manner as the LSB/MSB descriptions in the spec.
We also had a swap of the cdw1 / cdw11 fields; cdw1 should contain the
NSID, cdw11 the CSI and CNS-specific ID.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Jeremy Kerr [Mon, 20 Jun 2022 05:10:42 +0000 (13:10 +0800)]
Fix %llx/%lx build warnings on powerpc
powerpc64 uses the int-l64 type definitions, so we get an unsigned long
for our __u64 type. This causes several build warnings when we're
printing with %llx in tests/register.c and example/discover-loop.c, as
it's an unsigned long, not an unsigned long long.
One way to fix this would be to use the PRIx64 definitions from
inttypes.h - however, those are keyed off the __WORDSIZE definition,
essentially:
- and that breaks on x86_64, where our __u64 is an unsigned long long,
and would need %llx.
The powerpc types header does give us an option to use the int-l64
definitions though:
/*
* This is here because we used to use l64 for 64bit powerpc
* and we don't want to impact user mode with our change to ll64
* in the kernel.
*
* However, some user programs are fine with this. They can
* flag __SANE_USERSPACE_TYPES__ to get int-ll64.h here.
*/
#if !defined(__SANE_USERSPACE_TYPES__) && defined(__powerpc64__) && !defined(__KERNEL__)
# include <asm-generic/int-l64.h>
#else
# include <asm-generic/int-ll64.h>
#endif
... and in our case we are in fact fine with this. So, #define
__SANE_USERSPACE_TYPES__ for the two example programs.
We refrain from doing this to the library headers though, as we have no
idea what libnvme users will need for __u64.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Daniel Wagner [Mon, 13 Jun 2022 11:32:32 +0000 (13:32 +0200)]
doc: move update instruction into template
The documentation added in 7fecc4881a3c ("doc: add quickstart and
installation") and 6a9758bbde33 ("doc: update installation doc")
should have been added to the template.
Hannes Reinecke [Fri, 10 Jun 2022 08:19:01 +0000 (10:19 +0200)]
tree: make nvme_ctrl_scan_namespace() idempotent
When issuing a rescan the list of namespaces is not cleared, so
just blindly adding new entries might not yield the expected result.
This patch clears out old entries from the controller namespace list
before adding new ones, ensuring that we only ever have one namespace
with a given name in the list of controller namespaces.
Hannes Reinecke [Fri, 10 Jun 2022 08:19:01 +0000 (10:19 +0200)]
tree: make nvme_subsystem_scan_namespace() idempotent
When issuing a rescan the list of namespaces is not cleared, so
just blindly adding new entries might not yield the expected result.
This patch clears out old entries from the subsystem namespace list
before adding new ones, ensuring that we only ever have one namespace
with a given name in the list of subsystem namespaces.
Hannes Reinecke [Fri, 10 Jun 2022 07:48:03 +0000 (09:48 +0200)]
tree: simplifiy nvme_subsystem_lookup_namespace()
The list of namespaces needs to be considered static, and should
only be changed on a full rescan. So simplify the function to just
return the corresponding namespace entry from the topology tree.
projects / users / sagi / libnvme.git / log