Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvme/opts: parse timeouts in humantime #1548

Merged
merged 8 commits into from
Nov 24, 2023
12 changes: 12 additions & 0 deletions .github/workflows/staging-dco.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Staging DCO
on:
push:
branches:
- staging

jobs:
DCO:
runs-on: ubuntu-latest
steps:
- name: DCO
run: echo "DCO"
19 changes: 14 additions & 5 deletions io-engine/src/bdev/nexus/nexus_bdev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1265,11 +1265,20 @@ impl<'n> BdevOps for Nexus<'n> {
| IoType::WriteZeros => {
let supported = self.io_is_supported(io_type);
if !supported {
debug!(
"{:?}: I/O type '{:?}' not supported by at least \
one of child devices",
self, io_type
);
if io_type == IoType::Flush {
trace!(
"{:?}: I/O type '{:?}' not supported by at least \
one of child devices",
self,
io_type
);
} else {
debug!(
"{:?}: I/O type '{:?}' not supported by at least \
one of child devices",
self, io_type
);
}
}
supported
}
Expand Down
1 change: 0 additions & 1 deletion io-engine/src/bin/io-engine-client/v0/jsonrpc_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use tracing::debug;

pub fn subcommands() -> Command {
Command::new("jsonrpc")
.subcommand_required(true)
.arg_required_else_help(true)
.about("Call a json-rpc method with a raw JSON payload")
.arg(
Expand Down
1 change: 0 additions & 1 deletion io-engine/src/bin/io-engine-client/v1/jsonrpc_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ pub fn subcommands() -> Command {
.index(2)
.help("Parameters (JSON string) to pass to method call"),
)
.subcommand_required(true)
.arg_required_else_help(true)
}

Expand Down
2 changes: 1 addition & 1 deletion io-engine/src/subsys/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,6 @@ impl Config {
assert!(self.socket_opts.set());
assert!(self.iobuf_opts.set());

debug!("{:#?}", self);
info!("{:#?}", self);
Abhinandan-Purkait marked this conversation as resolved.
Show resolved Hide resolved
}
}
117 changes: 103 additions & 14 deletions io-engine/src/subsys/config/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use spdk_rs::{
};

use std::{
convert::TryFrom,
fmt::{Debug, Display},
str::FromStr,
};
Expand Down Expand Up @@ -166,14 +167,82 @@ where
val
},
Err(e) => {
error!("Invalid value: {} (error {}) specified for {}. Reverting to default ({})", v, e, name, default);
error!("Invalid value: {} (error {}) specified for {}. Reverting to default value ({})", v, e, name, default);
default
}
}
},
)
}

enum TimeUnit {
MilliSeconds,
MicroSeconds,
}
impl TimeUnit {
/// A backwards comptabile env variable name, which contains the
/// time units appended to the name.
fn backcompat_name(&self, name: &str) -> String {
format!(
"{name}{}",
match self {
TimeUnit::MilliSeconds => "_MS",
TimeUnit::MicroSeconds => "_US",
}
)
}
/// Collects a raw value in `Self` units for a given `Duration`.
fn value(&self, value: std::time::Duration) -> u128 {
match self {
TimeUnit::MilliSeconds => value.as_millis(),
TimeUnit::MicroSeconds => value.as_micros(),
}
}
fn units(&self) -> &str {
match self {
TimeUnit::MilliSeconds => "ms",
TimeUnit::MicroSeconds => "us",
}
}
}

/// Try to read an env variable in humantime, and if not found reverts back to
/// old format to keep backward compatibility.
fn time_try_from_env<T>(name: &str, default: T, unit: TimeUnit) -> T
where
T: FromStr + Display + Copy + TryFrom<u128>,
<T as FromStr>::Err: Debug + Display,
<T as TryFrom<u128>>::Error: Display,
{
match std::env::var(name) {
Ok(value) => {
let result = match humantime::parse_duration(&value) {
Ok(value) => {
let in_units = unit.value(value);
if in_units == 0 && !value.is_zero() {
Err(format!("must be at least 1{}", unit.units()))
} else {
T::try_from(unit.value(value))
.map_err(|error| error.to_string())
}
}
Err(error) => Err(error.to_string()),
};
match result {
Ok(value) => {
info!("Overriding {} value to '{}'", name, value);
value
}
Err(e) => {
error!("Invalid value: {} (error {}) specified for {}. Reverting to default value ({}{})", value, e, name, default, unit.units());
default
}
}
}
Err(_) => try_from_env(&unit.backcompat_name(name), default),
}
}

impl Default for NvmfTcpTransportOpts {
fn default() -> Self {
Self {
Expand All @@ -185,7 +254,7 @@ impl Default for NvmfTcpTransportOpts {
"NVMF_TCP_MAX_QPAIRS_PER_CTRL",
32,
),
num_shared_buf: try_from_env("NVMF_TCP_NUM_SHARED_BUF", 2048),
num_shared_buf: try_from_env("NVMF_TCP_NUM_SHARED_BUF", 2047),
buf_cache_size: try_from_env("NVMF_TCP_BUF_CACHE_SIZE", 64),
dif_insert_or_strip: false,
max_aq_depth: 32,
Expand Down Expand Up @@ -265,8 +334,11 @@ pub struct NvmeBdevOpts {
pub fast_io_fail_timeout_sec: u32,
/// TODO
pub disable_auto_failback: bool,
/// enable creation of submission and completion queues asynchronously.
pub async_mode: bool,
/// Enable generation of unique identifiers for NVMe bdevs only if they
/// do not provide UUID themselves.
/// These strings are based on device serial number and namespace ID and
/// will always be the same for that device.
pub generate_uuids: bool,
}

impl GetOpts for NvmeBdevOpts {
Expand Down Expand Up @@ -294,19 +366,36 @@ impl Default for NvmeBdevOpts {
fn default() -> Self {
Self {
action_on_timeout: 4,
timeout_us: try_from_env("NVME_TIMEOUT_US", 5_000_000),
timeout_admin_us: try_from_env("NVME_TIMEOUT_ADMIN_US", 5_000_000),
keep_alive_timeout_ms: try_from_env("NVME_KATO_MS", 1_000),
timeout_us: time_try_from_env(
"NVME_TIMEOUT",
5_000_000,
TimeUnit::MicroSeconds,
),
timeout_admin_us: time_try_from_env(
"NVME_TIMEOUT_ADMIN",
5_000_000,
TimeUnit::MicroSeconds,
),
keep_alive_timeout_ms: time_try_from_env(
"NVME_KATO",
10_000,
TimeUnit::MilliSeconds,
),
transport_retry_count: try_from_env("NVME_RETRY_COUNT", 0),
arbitration_burst: 0,
low_priority_weight: 0,
medium_priority_weight: 0,
high_priority_weight: 0,
nvme_adminq_poll_period_us: try_from_env(
"NVME_ADMINQ_POLL_PERIOD_US",
nvme_adminq_poll_period_us: time_try_from_env(
"NVME_ADMINQ_POLL_PERIOD",
1_000,
TimeUnit::MilliSeconds,
),
nvme_ioq_poll_period_us: time_try_from_env(
"NVME_IOQ_POLL_PERIOD",
0,
TimeUnit::MicroSeconds,
),
nvme_ioq_poll_period_us: try_from_env("NVME_IOQ_POLL_PERIOD_US", 0),
io_queue_requests: 0,
delay_cmd_submit: true,
bdev_retry_count: try_from_env("NVME_BDEV_RETRY_COUNT", 0),
Expand All @@ -315,7 +404,7 @@ impl Default for NvmeBdevOpts {
reconnect_delay_sec: 0,
fast_io_fail_timeout_sec: 0,
disable_auto_failback: false,
async_mode: try_from_env("NVME_QPAIR_CONNECT_ASYNC", false),
generate_uuids: try_from_env("NVME_GENERATE_UUIDS", true),
}
}
}
Expand All @@ -342,7 +431,7 @@ impl From<spdk_bdev_nvme_opts> for NvmeBdevOpts {
reconnect_delay_sec: o.reconnect_delay_sec,
fast_io_fail_timeout_sec: o.fast_io_fail_timeout_sec,
disable_auto_failback: o.disable_auto_failback,
async_mode: NvmeBdevOpts::default().async_mode,
generate_uuids: o.generate_uuids,
}
}
}
Expand All @@ -369,7 +458,7 @@ impl From<&NvmeBdevOpts> for spdk_bdev_nvme_opts {
reconnect_delay_sec: o.reconnect_delay_sec,
fast_io_fail_timeout_sec: o.fast_io_fail_timeout_sec,
disable_auto_failback: o.disable_auto_failback,
generate_uuids: false,
generate_uuids: o.generate_uuids,
transport_tos: 0,
nvme_error_stat: false,
rdma_srq_size: 0,
Expand Down Expand Up @@ -582,7 +671,7 @@ impl Default for IoBufOpts {
fn default() -> Self {
Self {
small_pool_count: try_from_env("IOBUF_SMALL_POOL_COUNT", 8192),
large_pool_count: try_from_env("IOBUF_LARGE_POOL_COUNT", 1024),
large_pool_count: try_from_env("IOBUF_LARGE_POOL_COUNT", 2048),
dsharma-dc marked this conversation as resolved.
Show resolved Hide resolved
small_bufsize: try_from_env("IOBUF_SMALL_BUFSIZE", 8 * 1024),
large_bufsize: try_from_env("IOBUF_LARGE_BUFSIZE", 132 * 1024),
}
Expand Down
Loading