From fe8cc56e2e7b81adcdf72cf31ab19eb6112ca329 Mon Sep 17 00:00:00 2001 From: Diwakar Sharma Date: Mon, 26 Aug 2024 16:58:55 +0000 Subject: [PATCH] feat: use transport specific nvmf uri scheme Signed-off-by: Diwakar Sharma --- io-engine/src/bdev/dev.rs | 7 ++++++- io-engine/src/bdev_api.rs | 4 ++-- io-engine/src/core/bdev.rs | 2 +- io-engine/src/subsys/nvmf/transport.rs | 11 ++++++++++- io-engine/src/target/nvmf.rs | 13 ++++++++++--- io-engine/tests/nvmf.rs | 5 +++-- libnvme-rs/src/nvme_uri.rs | 3 +++ test/grpc/test_common.js | 2 +- test/grpc/test_replica.js | 6 +++--- 9 files changed, 39 insertions(+), 14 deletions(-) diff --git a/io-engine/src/bdev/dev.rs b/io-engine/src/bdev/dev.rs index 727a9e0a3..c8f87909d 100644 --- a/io-engine/src/bdev/dev.rs +++ b/io-engine/src/bdev/dev.rs @@ -66,7 +66,12 @@ pub(crate) mod uri { } "malloc" => Ok(Box::new(malloc::Malloc::try_from(&url)?)), "null" => Ok(Box::new(null_bdev::Null::try_from(&url)?)), - "nvmf" => Ok(Box::new(nvmx::NvmfDeviceTemplate::try_from(&url)?)), + // keeping nvmf scheme so existing tests(if any, setting this + // scheme) work. The replicas and nexus however should + // always be exposing nvmf+tcp or nvmf+rdma now. + "nvmf" | "nvmf+tcp" | "nvmf+rdma+tcp" => { + Ok(Box::new(nvmx::NvmfDeviceTemplate::try_from(&url)?)) + } "pcie" => Ok(Box::new(nvme::NVMe::try_from(&url)?)), "uring" => Ok(Box::new(uring::Uring::try_from(&url)?)), "nexus" => Ok(Box::new(nx::Nexus::try_from(&url)?)), diff --git a/io-engine/src/bdev_api.rs b/io-engine/src/bdev_api.rs index cfbcc05e8..9c39d892c 100644 --- a/io-engine/src/bdev_api.rs +++ b/io-engine/src/bdev_api.rs @@ -126,7 +126,7 @@ where Ok(device) if device.get_name() == bdev.name() => { bdev.driver() == match uri.scheme() { - "nvmf" | "pcie" => "nvme", + "nvmf" | "nvmf+tcp" | "nvmf+rdma+tcp" | "pcie" => "nvme", scheme => scheme, } } @@ -143,7 +143,7 @@ where Ok(device) if device.get_name() == bdev.name() => { bdev.driver() == match uri.scheme() { - "nvmf" | "pcie" => "nvme", + "nvmf" | "nvmf+tcp" | "nvmf+rdma+tcp" | "pcie" => "nvme", scheme => scheme, } } diff --git a/io-engine/src/core/bdev.rs b/io-engine/src/core/bdev.rs index 19e4a8abd..8d2814076 100644 --- a/io-engine/src/core/bdev.rs +++ b/io-engine/src/core/bdev.rs @@ -203,7 +203,7 @@ where type Error = CoreError; type Output = String; - /// share the bdev over NVMe-OF TCP + /// share the bdev over NVMe-OF TCP(and RDMA if enabled) async fn share_nvmf( self: Pin<&mut Self>, props: Option, diff --git a/io-engine/src/subsys/nvmf/transport.rs b/io-engine/src/subsys/nvmf/transport.rs index 56e47e05b..1f3be04bd 100644 --- a/io-engine/src/subsys/nvmf/transport.rs +++ b/io-engine/src/subsys/nvmf/transport.rs @@ -154,9 +154,18 @@ impl TransportId { impl Display for TransportId { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + // If an rdma transport is found in transport id, we modify the + // trstring for uri scheme to explicitly indicate the tcp support + // also by default when there is rdma available. + let trstring = match self.0.trstring.as_str() { + "RDMA" => "rdma+tcp".to_string(), + _else => _else.to_lowercase(), + }; + write!( f, - "nvmf://{}:{}", + "nvmf+{}://{}:{}", + trstring, self.0.traddr.as_str(), self.0.trsvcid.as_str() ) diff --git a/io-engine/src/target/nvmf.rs b/io-engine/src/target/nvmf.rs index c92c0331c..f247d821a 100644 --- a/io-engine/src/target/nvmf.rs +++ b/io-engine/src/target/nvmf.rs @@ -35,9 +35,16 @@ pub async fn unshare(uuid: &str) -> Result<(), NvmfError> { pub fn get_uri(uuid: &str) -> Option { if let Some(ss) = NvmfSubsystem::nqn_lookup(uuid) { - // for now we only pop the first but we can share a bdev - // over multiple nqn's - ss.uri_endpoints().unwrap().pop() + // If there is rdma capable uri available, return that. Otherwise, + // for now we only pop the most relevant, but we can share a bdev + // over multiple nqn's. + let mut uris = ss.uri_endpoints().expect("no uri endpoints"); + let rdma_uri = uris + .iter() + .find(|u| u.starts_with("nvmf+rdma+tcp")) + .cloned(); + + rdma_uri.or(uris.pop()) } else { None } diff --git a/io-engine/tests/nvmf.rs b/io-engine/tests/nvmf.rs index b2f31e657..37310cb2e 100644 --- a/io-engine/tests/nvmf.rs +++ b/io-engine/tests/nvmf.rs @@ -156,9 +156,10 @@ async fn nvmf_set_target_interface() { .into_inner() .uri; - let re = Regex::new(r"^nvmf://([0-9.]+):[0-9]+/.*$").unwrap(); + let re = Regex::new(r"^nvmf(\+rdma\+tcp|\+tcp)://([0-9.]+):[0-9]+/.*$") + .unwrap(); let cap = re.captures(&bdev_uri).unwrap(); - let shared_ip = cap.get(1).unwrap().as_str(); + let shared_ip = cap.get(2).unwrap().as_str(); hdl.bdev .unshare(CreateReply { diff --git a/libnvme-rs/src/nvme_uri.rs b/libnvme-rs/src/nvme_uri.rs index 328b0e74d..7298011dc 100644 --- a/libnvme-rs/src/nvme_uri.rs +++ b/libnvme-rs/src/nvme_uri.rs @@ -42,12 +42,14 @@ impl Drop for NvmeStringWrapper { #[derive(Debug, PartialEq)] enum NvmeTransportType { Tcp, + Rdma, } impl NvmeTransportType { fn to_str(&self) -> &str { match self { NvmeTransportType::Tcp => "tcp", + NvmeTransportType::Rdma => "rdma", } } } @@ -83,6 +85,7 @@ impl TryFrom<&str> for NvmeTarget { let trtype = match url.scheme() { "nvmf" | "nvmf+tcp" => Ok(NvmeTransportType::Tcp), + "nvmf+rdma+tcp" => Ok(NvmeTransportType::Rdma), _ => Err(NvmeError::UrlError { source: ParseError::IdnaError, }), diff --git a/test/grpc/test_common.js b/test/grpc/test_common.js index 0d8bd0e7b..4583d88e8 100644 --- a/test/grpc/test_common.js +++ b/test/grpc/test_common.js @@ -21,7 +21,7 @@ const CSI_ID = 'test-node-id'; const LOCALHOST = '127.0.0.1'; const NVME_MODEL_ID = 'Mayastor NVMe controller'; const NVME_NQN_PREFIX = 'nqn.2019-05.io.openebs'; -const NVMF_URI = /^nvmf:\/\/(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}):\d{1,5}\/nqn.2019-05.io.openebs:/; +const NVMF_URI = /^nvmf\+(tcp|rdma\+tcp):\/\/(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}):\d{1,5}\/nqn.2019-05.io.openebs:/; const testPort = process.env.TEST_PORT || GRPC_PORT; const myIp = getMyIp() || LOCALHOST; diff --git a/test/grpc/test_replica.js b/test/grpc/test_replica.js index a26cc0af1..94a1c7cc2 100644 --- a/test/grpc/test_replica.js +++ b/test/grpc/test_replica.js @@ -290,7 +290,7 @@ describe('replica', function () { (err, res) => { if (err) return done(err); assert.match(res.uri, NVMF_URI); - assert.equal(res.uri.match(NVMF_URI)[1], common.getMyIp()); + assert.equal(res.uri.match(NVMF_URI)[2], common.getMyIp()); client.listReplicas({}, (err, res) => { if (err) return done(err); @@ -548,7 +548,7 @@ describe('replica', function () { assert.equal(res.size, 96 * 1024 * 1024); assert.equal(res.share, 'REPLICA_NVMF'); assert.match(res.uri, NVMF_URI); - assert.equal(res.uri.match(NVMF_URI)[1], common.getMyIp()); + assert.equal(res.uri.match(NVMF_URI)[2], common.getMyIp()); uri = res.uri; done(); } @@ -635,7 +635,7 @@ describe('replica', function () { if (err) return done(err); assert.match(res.uri, NVMF_URI); - assert.equal(res.uri.match(NVMF_URI)[1], common.getMyIp()); + assert.equal(res.uri.match(NVMF_URI)[2], common.getMyIp()); uri = res.uri; done(); }