Skip to content

MI: add command timeouts#432

Merged
igaw merged 3 commits intolinux-nvme:masterfrom
CodeConstruct:mi-timeout
Jul 18, 2022
Merged

MI: add command timeouts#432
igaw merged 3 commits intolinux-nvme:masterfrom
CodeConstruct:mi-timeout

Conversation

@jk-ozlabs
Copy link
Copy Markdown
Collaborator

This series implements a facility for endpoint-specific timeouts for the NVMe-MI transports. We add a simple API to get/set timeout values:

/**
 * nvme_mi_ep_set_timeout - set a timeout for NVMe-MI responses
 * @ep: MI endpoint object
 * @timeout_ms: Timeout for MI responses, given in milliseconds
 */
int nvme_mi_ep_set_timeout(nvme_mi_ep_t ep, unsigned int timeout_ms);

/**
 * nvme_mi_ep_set_mprt_max - set the maximum wait time for a More Processing
 * Required response
 * @ep: MI endpoint object
 * @mprt_max_ms: Maximum more processing required wait time
 *
 * NVMe-MI endpoints may respond to a request with a "More Processing Required"
 * response; this also includes a hint on the worst-case processing time for
 * the eventual response data, with a specification-defined maximum of 65.535
 * seconds.
 *
 * This function provides a way to limit the maximum time we're prepared to
 * wait for the final response. Specify zero in @mprt_max_ms for no limit.
 * This should be larger than the command/response timeout set in
 * &nvme_mi_ep_set_timeout().
 */
void nvme_mi_ep_set_mprt_max(nvme_mi_ep_t ep, unsigned int mprt_max_ms);

/**
 * nvme_mi_ep_get_timeout - get the current timeout value for NVMe-MI responses
 * @ep: MI endpoint object
 *
 * Returns the current timeout value, in milliseconds, for this endpoint.
 */
unsigned int nvme_mi_ep_get_timeout(nvme_mi_ep_t ep);

- and implement this for the MCTP transport layer, plus a bunch of tests.

This means that we can recover from device/transport/address errors, instead of potentially blocking forever waiting for a response.

This change introduces an API for querying and modifying the endpoint
timeout. Transports may provide a function for valiating a new timeout
value before setting.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
This change adds a function to set the maximum time we're prepared to
wait for a response after a More Processing Required message.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Now that we have timeout values in the endpoint, implement a per-command
timeout using poll(), and clamp the maximum MPR time.

Add tests to suit.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #432 (6a08780) into master (6c5aedd) will increase coverage by 1.75%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   16.43%   18.19%   +1.75%     
==========================================
  Files          31       31              
  Lines        5068     5206     +138     
  Branches      973      997      +24     
==========================================
+ Hits          833      947     +114     
- Misses       3982     3988       +6     
- Partials      253      271      +18     
Impacted Files Coverage Δ
src/nvme/mi.h 36.84% <ø> (ø)
src/nvme/mi.c 52.23% <61.53%> (+0.29%) ⬆️
src/nvme/mi-mctp.c 71.03% <80.76%> (+0.63%) ⬆️
test/mi-mctp.c 83.96% <86.66%> (+1.80%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c5aedd...6a08780. Read the comment docs.

@igaw igaw merged commit 1133776 into linux-nvme:master Jul 18, 2022
@jk-ozlabs jk-ozlabs deleted the mi-timeout branch July 28, 2022 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants