Merge libnvme #2951
Merged
Merge libnvme #2951
+142,263
−13
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The JSON parser is loading the keys into the kernel. This is a privileged operations (root) thus the parser will fail for a normal user. This also makes 'nvme list' etc fail for normal users. Furthermore, we try to insert each time the keys into the keystore when libnvme parses the JSON configuration. Let's move the key import operations to the connect call path where we need the right permission. A nice side benefit is that we also are able to pass in a configured key. The export feature will be added back later. Signed-off-by: Daniel Wagner <[email protected]>
Add a test case for the PSK API to ensure that the generated JSON is correct. Signed-off-by: Daniel Wagner <[email protected]>
The nvme_configure_ctrl function gets a bit big, thus move the dhchap and tls part into separate function. Signed-off-by: Daniel Wagner <[email protected]>
Newer kernels expose the key which is used to setup the connection as tls_configured_key. During operations the kernel is going to update the tls_key thus the tls_key is not going to be useful from user point of view. Also the keyring used to store the keys is exposed via sysfs. Signed-off-by: Daniel Wagner <[email protected]>
The recent change where key import function out of the JSON parser to the connect path, dropped the feature to export the keys back to the config. The keys are only necessary to be loaded back to the user memory when the configuration is dumped. Thus hook the export functionality into the dump code path. Signed-off-by: Daniel Wagner <[email protected]>
The import code assumes tls_key always contains a Configured PSK and did the transformation step again. Only do the transformation when necessary. Signed-off-by: Daniel Wagner <[email protected]>
The first argument of the macros is the nvme_root_t variable. build_options is using 'r' as variable name for this type, thus the macro still worked even though the first argument was called 'o'. Let's rename it so it matches the actual use. Signed-off-by: Daniel Wagner <[email protected]>
The keyclt CLI and /proc/keys show the key ids as hex numbers. This makes debugging unnessary more complex when comparing what the key ids from the connect command line with the key store ids. Signed-off-by: Daniel Wagner <[email protected]>
A controller can already be created by the config, thus it is already in the tree. Adding to the tree is wrong and will lead to double frees when releasing the tree. Signed-off-by: Daniel Wagner <[email protected]>
The LOG_NOTICE level is always set. Only notify the user when at least the INFO level is set. Signed-off-by: Daniel Wagner <[email protected]>
The kernel returns ENOKEY when the PSK TLS is missing, thus add the corresponding ENVME error code. Signed-off-by: Daniel Wagner <[email protected]>
The NVMe TCP spec defines a PSK HMAC type '0', which indicates that the configured key should be used as a retained key with no transformation in between. Signed-off-by: Hannes Reinecke <[email protected]> Signed-off-by: Daniel Wagner <[email protected]>
Added enums for ANSAN and RGCNS bit of OAES field based on NVM Express Base Specification 2.1 Signed-off-by: Arbaz Khan <[email protected]> Reviewed-by: Steven Seungcheol Lee <[email protected]> Reviewed-by: Sathyavathi M <[email protected]>
Signed-off-by: Tokunori Ikegami <[email protected]>
Supported the I/O command set specific indentify controller data structure fields below by NVMe 2.1. 1. version reporting 2. PCIe infrastructure for live migration Signed-off-by: Tokunori Ikegami <[email protected]>
The config output should only contain the entries which were also provided by the user via the command line. Thus we should not lookup the keys used in the keystore when only --tls is used. Signed-off-by: Daniel Wagner <[email protected]>
TLS keys are using a base64 encoding thus the additional slash encoding will corrupt the key. nvme-cli already uses this option for the rest, so it should be fine here too. Signed-off-by: Daniel Wagner <[email protected]>
Add Estimated Time For Post-Verification Deallocation State (ETPVDS) and Sanitize State Information (SSI) fields of sanitize status log. TP4152 - Post-Sanitize Media Verification 2024.04.01 Ratified. Signed-off-by: Francis Pravin <[email protected]>
For nvme_mi_admin_xfer the DOFF should be zero when sending req data and the DLEN should be req_data_size in this case, per spec. Signed-off-by: Chuck Horkin <[email protected]>
…face Ensure that we're setting the dlen and doff values correctly, where the semantics of dlen in particular will change on requests vs. responses. Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
The reordering of the variables broke derive_psk_digest OpenSSL 1.1 version. Fixes: 4cc727d ("linux: reorder variable declarations") Signed-off-by: Daniel Wagner <[email protected]>
The code coverage action is silently failing partly because the required Codecov token is missing. Reading the token from the GitHub secrets. The missing gpg package must be fixed in https://github.com/linux-nvme/ci-containers. Furthermore, fail the coverage workflow if the codecov-action errors out. Signed-off-by: Dennis Maisenbacher <[email protected]>
__nvme_import_keys_from_config is helper to import the TLS keys. When we don't have OpenSSL builds enabled, this should be a no op. Thus don't return an error in this case. Signed-off-by: Daniel Wagner <[email protected]>
Reservations and Host Identifier Interaction (RHII):
This bit indicates the reservations and
Host Identifier interaction support for the controller.
Signed-off-by: Steven Seungcheol Lee <[email protected]>
There is no point in accessing the keyring if we don't have to load a key into the kernel. Signed-off-by: Daniel Wagner <[email protected]>
There is no point in trying to import a key if the TLS option is not enabled. Signed-off-by: Daniel Wagner <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
The check for existing controller is inverted. Signed-off-by: Hannes Reinecke <[email protected]>
Update subsystem definitions to include all attributes from the underlying structure. Signed-off-by: Hannes Reinecke <[email protected]>
Update the discovery-loop.py example to use the tcp transport and fix an issue where the controller wasn't disconnected when the disovery failed. Signed-off-by: Hannes Reinecke <[email protected]>
Add a fallback implementation for nvme_insert_tls_key_compat() if CONFIG_KEYUTILS is not set. Signed-off-by: Hannes Reinecke <[email protected]>
In the example, when ctrl.discover() fails, we should not invoke sys.exit() immediately, but instead allow the ctrl.disconnect() to be invoked. Signed-off-by: Martin Belanger <[email protected]>
Bumps [pypa/gh-action-pypi-publish](https://github.com/pypa/gh-action-pypi-publish) from 1.12.4 to 1.13.0. - [Release notes](https://github.com/pypa/gh-action-pypi-publish/releases) - [Commits](pypa/gh-action-pypi-publish@v1.12.4...v1.13.0) --- updated-dependencies: - dependency-name: pypa/gh-action-pypi-publish dependency-version: 1.13.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]>
nvme_disconnect_ctrl() can fail, and will return an error if so. So update the python binding to properly raise an exception when disconnect fails. Signed-off-by: Hannes Reinecke <[email protected]>
The python bindings implement hosts(), subsystems(), controllers(), and namespaces() as iterators. However, these iterators all fail on empty lists. This hopefully fixes this issue. Signed-off-by: Martin Belanger <[email protected]>
This is just a bit of clean up. After reworking iterators by using actual Python code instead of C++, we don't need to keep these "iter" elements: host_iter, subsystem_iter, ctrl_iter, ns_iter. Signed-off-by: Martin Belanger <[email protected]>
The python 'with' statement allows for correct resource tracking, where the allocated resource will be freed at the end of the statement. This is helpful for 'host', 'subsystem', and 'controller' objects which we might want to clean up (and clear out the internal tree). The nice thing here is that we can do an automatic controller disconnect, and don't have to worry about leaving stale connections or objects around. Signed-off-by: Hannes Reinecke <[email protected]>
Now that we have resource tracking we can use the 'with' statement for discovery and clean up connections and discovery controllers on exit. Signed-off-by: Hannes Reinecke <[email protected]>
As we now have resource tracking we can implement a discovery loop in python without leaving stale controllers around. Signed-off-by: Hannes Reinecke <[email protected]>
Newer GCC complain about 'digest' being an empty argument, which is technically true, but in practice can only happen if version == 0, which we already check. So add a warning to keep GCC happy. Signed-off-by: Hannes Reinecke <[email protected]>
OpenSSL prior to 3.3.1 had an issue with EVP_PKEY_CTX_add1_hkdf_info() where it acted like a 'set1' function instead of an 'add1' as documented. Work around that by building the entire info vector outside of the OpenSSL API and only calling this function once. This is the same workaround used in commit eff0ffe ("linux: fix HKDF TLS key derivation back to OpenSSL 3.0.8"). Signed-off-by: Daniel Wagner <[email protected]>
The EVP_PKEY_CTX_add1_hkdf_info implementation had a bug in the past which made it behave like a set instead of add function. When linking against external builds warn about it. The libnvme implementation works around this problem, but it's better to have this logged during the configure step, so there is chance to debug this. Signed-off-by: Daniel Wagner <[email protected]>
The coverage workflow returns a failure when the coverage goes down. Unfortunately, the test is a bit to error prone and reports too many falls positives. Just ignore these errors. Signed-off-by: Daniel Wagner <[email protected]>
Apple silicon support on Linux uses a separate platform driver called 'apple-nvme', and libnvme doesn't know about this transport. This fixes a "No transport address for 'apple-nvme'" error when running on Apple silicon, where libnvme was unable to enumerate nvme storage attached to this platform. $ nvme list Node Generic SN Model Namespace Usage Format FW Rev --------------------- --------------------- -------------------- ---------------------------------------- ---------- -------------------------- ---------------- -------- /dev/nvme0n1 /dev/ng0n1 xxxxxxxxxxxxxxxx APPLE SSD AP2048Z 0x1 2.00 TB / 2.00 TB 4 KiB + 0 B 532.140. ... Signed-off-by: Clayton Craft <[email protected]> squash
The nvme_get_uuid_list function will check for uuid list support and return the list if supported. Signed-off-by: Jeff Lien <[email protected]> Reviewed-by: Brandon Paupore <[email protected]>
The test/mi-mctp.c fails to compile in certain cases where HAVE_LINUX_MCTP_H is false. There's a workaround for this condition in src/mi-mctp.c but not here. This change makes a common header and includes it in both cases. Signed-off-by: Chuck Horkin <[email protected]>
The fw download and fw commit test fail sometimes because struct args is not properly initiliazed. Signed-off-by: Daniel Wagner <[email protected]>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 5. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v5) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 5 to 6. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v5...v6) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
Commit fe89efa ("tree: do not set dhchap_key to 'none'") and commit a343ed6 ("tree: always set the host key") missed one spot to preserve the existing secret (aka host key). Update nvme_scan_ctrl to only set the secret from sysfs when it's actually there. Reported-by: Puspak Sahu <[email protected]> Signed-off-by: Daniel Wagner <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
Signed-off-by: Daniel Wagner <[email protected]>
To simplify the development process, merge the library into the existing nvme-cli git repo. Signed-off-by: Daniel Wagner <[email protected]>
Since the library is now in the nvme-cli repo, we don't need to use the meson fallback for getting the library. For the time being just softlink link from subprojects to the library directory. Not sure if this is the right approach to do so, but it seems to work. Signed-off-by: Daniel Wagner <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge libnvme into the nvme-cli project. The library stays independent of nvme-cli but it lives in the nvme-cli repo.
Fixes: linux-nvme/libnvme#1081
This merge only updates the minimum. Some of the tooling needs to be updated, e.g. the release script but let's do that later.