From 8e794f3949993b5cd87aa205f23c81ad9f068bca Mon Sep 17 00:00:00 2001 From: Dorjoy Chowdhury Date: Mon, 8 Jul 2024 23:44:39 +0600 Subject: [PATCH 1/2] vhost-device-vsock: introduce queue-size option The vring queue size was hardcoded to 256 which is problematic for vsock devices that use a larger queue size. For example, when using vhost-device-vsock with QEMU's vhost-user-vsock-device, the message to set the vring queue size to 1024 causes an "InvalidParameter" error from vhost-device-vsock. Making the queue size configurable via an option makes it easy to use vhost-device-vsock where a larger queue size is required. Signed-off-by: Dorjoy Chowdhury --- vhost-device-vsock/README.md | 11 +++--- vhost-device-vsock/src/main.rs | 46 +++++++++++++++++++----- vhost-device-vsock/src/thread_backend.rs | 3 ++ vhost-device-vsock/src/vhu_vsock.rs | 20 ++++++++--- 4 files changed, 63 insertions(+), 17 deletions(-) diff --git a/vhost-device-vsock/README.md b/vhost-device-vsock/README.md index 8716c0390..c58f3d699 100644 --- a/vhost-device-vsock/README.md +++ b/vhost-device-vsock/README.md @@ -44,18 +44,19 @@ vhost-device-vsock --guest-cid= \ --socket= \ --uds-path= \ [--tx-buffer-size=host packets)>] \ + [--queue-size=] \ [--groups=] ``` or ``` -vhost-device-vsock --vm guest_cid=,socket=,uds-path=[,tx-buffer-size=host packets)>][,groups=] +vhost-device-vsock --vm guest_cid=,socket=,uds-path=[,tx-buffer-size=host packets)>][,queue-size=][,groups=] ``` Specify the `--vm` argument multiple times to specify multiple devices like this: ``` vhost-device-vsock \ --vm guest-cid=3,socket=/tmp/vhost3.socket,uds-path=/tmp/vm3.vsock,groups=group1+groupA \ ---vm guest-cid=4,socket=/tmp/vhost4.socket,uds-path=/tmp/vm4.vsock,tx-buffer-size=32768 +--vm guest-cid=4,socket=/tmp/vhost4.socket,uds-path=/tmp/vm4.vsock,tx-buffer-size=32768,queue-size=256 ``` Or use a configuration file: @@ -70,11 +71,13 @@ vms: socket: /tmp/vhost3.socket uds_path: /tmp/vm3.sock tx_buffer_size: 65536 + queue_size: 1024 groups: group1+groupA - guest_cid: 4 socket: /tmp/vhost4.socket uds_path: /tmp/vm4.sock tx_buffer_size: 32768 + queue_size: 256 groups: group2+groupB ``` @@ -94,9 +97,9 @@ qemu-system-x86_64 \ ```sh shell1$ vhost-device-vsock --vm guest-cid=4,uds-path=/tmp/vm4.vsock,socket=/tmp/vhost4.socket ``` -or if you want to configure the TX buffer size +or if you want to configure the TX buffer size and vring queue size ```sh -shell1$ vhost-device-vsock --vm guest-cid=4,uds-path=/tmp/vm4.vsock,socket=/tmp/vhost4.socket,tx-buffer-size=65536 +shell1$ vhost-device-vsock --vm guest-cid=4,uds-path=/tmp/vm4.vsock,socket=/tmp/vhost4.socket,tx-buffer-size=65536,queue-size=1024 ``` ```sh diff --git a/vhost-device-vsock/src/main.rs b/vhost-device-vsock/src/main.rs index f58da2345..b69e2533f 100644 --- a/vhost-device-vsock/src/main.rs +++ b/vhost-device-vsock/src/main.rs @@ -27,6 +27,7 @@ use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap}; const DEFAULT_GUEST_CID: u64 = 3; const DEFAULT_TX_BUFFER_SIZE: u32 = 64 * 1024; +const DEFAULT_QUEUE_SIZE: usize = 256; const DEFAULT_GROUP_NAME: &str = "default"; #[derive(Debug, ThisError)] @@ -84,6 +85,10 @@ struct VsockParam { #[clap(long, default_value_t = DEFAULT_TX_BUFFER_SIZE, conflicts_with = "config", conflicts_with = "vm")] tx_buffer_size: u32, + /// The size of the vring queue + #[clap(long, default_value_t = DEFAULT_QUEUE_SIZE, conflicts_with = "config", conflicts_with = "vm")] + queue_size: usize, + /// The list of group names to which the device belongs. /// A group is a set of devices that allow sibling communication between their guests. #[arg( @@ -102,6 +107,7 @@ struct ConfigFileVsockParam { socket: String, uds_path: String, tx_buffer_size: Option, + queue_size: Option, groups: Option, } @@ -112,9 +118,9 @@ struct VsockArgs { param: Option, /// Device parameters corresponding to a VM in the form of comma separated key=value pairs. - /// The allowed keys are: guest_cid, socket, uds_path, tx_buffer_size and group. + /// The allowed keys are: guest_cid, socket, uds_path, tx_buffer_size, queue_size and group. /// Example: - /// --vm guest-cid=3,socket=/tmp/vhost3.socket,uds-path=/tmp/vm3.vsock,tx-buffer-size=65536,groups=group1+group2 + /// --vm guest-cid=3,socket=/tmp/vhost3.socket,uds-path=/tmp/vm3.vsock,tx-buffer-size=65536,queue-size=1024,groups=group1+group2 /// Multiple instances of this argument can be provided to configure devices for multiple guests. #[arg(long, conflicts_with = "config", verbatim_doc_comment, value_parser = parse_vm_params)] vm: Option>, @@ -129,6 +135,7 @@ fn parse_vm_params(s: &str) -> Result { let mut socket = None; let mut uds_path = None; let mut tx_buffer_size = None; + let mut queue_size = None; let mut groups = None; for arg in s.trim().split(',') { @@ -145,6 +152,9 @@ fn parse_vm_params(s: &str) -> Result { "tx_buffer_size" | "tx-buffer-size" => { tx_buffer_size = Some(val.parse().map_err(VmArgsParseError::ParseInteger)?) } + "queue_size" | "queue-size" => { + queue_size = Some(val.parse().map_err(VmArgsParseError::ParseInteger)?) + } "groups" => groups = Some(val.split('+').map(String::from).collect()), _ => return Err(VmArgsParseError::InvalidKey(key.to_string())), } @@ -155,6 +165,7 @@ fn parse_vm_params(s: &str) -> Result { socket.ok_or_else(|| VmArgsParseError::RequiredKeyNotFound("socket".to_string()))?, uds_path.ok_or_else(|| VmArgsParseError::RequiredKeyNotFound("uds-path".to_string()))?, tx_buffer_size.unwrap_or(DEFAULT_TX_BUFFER_SIZE), + queue_size.unwrap_or(DEFAULT_QUEUE_SIZE), groups.unwrap_or(vec![DEFAULT_GROUP_NAME.to_string()]), )) } @@ -176,6 +187,7 @@ impl VsockArgs { p.socket.trim().to_string(), p.uds_path.trim().to_string(), p.tx_buffer_size.unwrap_or(DEFAULT_TX_BUFFER_SIZE), + p.queue_size.unwrap_or(DEFAULT_QUEUE_SIZE), p.groups.map_or(vec![DEFAULT_GROUP_NAME.to_string()], |g| { g.trim().split('+').map(String::from).collect() }), @@ -209,6 +221,7 @@ impl TryFrom for Vec { p.socket.trim().to_string(), p.uds_path.trim().to_string(), p.tx_buffer_size, + p.queue_size, p.groups.trim().split('+').map(String::from).collect(), )]) }), @@ -323,6 +336,7 @@ mod tests { socket: &str, uds_path: &str, tx_buffer_size: u32, + queue_size: usize, groups: &str, ) -> Self { VsockArgs { @@ -331,6 +345,7 @@ mod tests { socket: socket.to_string(), uds_path: uds_path.to_string(), tx_buffer_size, + queue_size, groups: groups.to_string(), }), vm: None, @@ -352,7 +367,7 @@ mod tests { let socket_path = test_dir.path().join("vhost4.socket").display().to_string(); let uds_path = test_dir.path().join("vm4.vsock").display().to_string(); - let args = VsockArgs::from_args(3, &socket_path, &uds_path, 64 * 1024, "group1"); + let args = VsockArgs::from_args(3, &socket_path, &uds_path, 64 * 1024, 1024, "group1"); let configs = Vec::::try_from(args); assert!(configs.is_ok()); @@ -365,6 +380,7 @@ mod tests { assert_eq!(config.get_socket_path(), socket_path); assert_eq!(config.get_uds_path(), uds_path); assert_eq!(config.get_tx_buffer_size(), 64 * 1024); + assert_eq!(config.get_queue_size(), 1024); assert_eq!(config.get_groups(), vec!["group1".to_string()]); test_dir.close().unwrap(); @@ -386,8 +402,8 @@ mod tests { ]; let params = format!( "--vm socket={vhost3_socket},uds_path={vm3_vsock} \ - --vm socket={vhost4_socket},uds-path={vm4_vsock},guest-cid=4,tx_buffer_size=65536,groups=group1 \ - --vm groups=group2+group3,guest-cid=5,socket={vhost5_socket},uds_path={vm5_vsock},tx-buffer-size=32768", + --vm socket={vhost4_socket},uds-path={vm4_vsock},guest-cid=4,tx_buffer_size=65536,queue_size=1024,groups=group1 \ + --vm groups=group2+group3,guest-cid=5,socket={vhost5_socket},uds_path={vm5_vsock},tx-buffer-size=32768,queue_size=256", vhost3_socket = socket_paths[0].display(), vhost4_socket = socket_paths[1].display(), vhost5_socket = socket_paths[2].display(), @@ -415,6 +431,7 @@ mod tests { ); assert_eq!(config.get_uds_path(), uds_paths[0].display().to_string()); assert_eq!(config.get_tx_buffer_size(), 65536); + assert_eq!(config.get_queue_size(), 1024); assert_eq!(config.get_groups(), vec![DEFAULT_GROUP_NAME.to_string()]); let config = configs.get(1).unwrap(); @@ -425,6 +442,7 @@ mod tests { ); assert_eq!(config.get_uds_path(), uds_paths[1].display().to_string()); assert_eq!(config.get_tx_buffer_size(), 65536); + assert_eq!(config.get_queue_size(), 1024); assert_eq!(config.get_groups(), vec!["group1".to_string()]); let config = configs.get(2).unwrap(); @@ -435,6 +453,7 @@ mod tests { ); assert_eq!(config.get_uds_path(), uds_paths[2].display().to_string()); assert_eq!(config.get_tx_buffer_size(), 32768); + assert_eq!(config.get_queue_size(), 256); assert_eq!( config.get_groups(), vec!["group2".to_string(), "group3".to_string()] @@ -459,6 +478,7 @@ mod tests { socket: {} uds_path: {} tx_buffer_size: 32768 + queue_size: 256 groups: group1+group2", socket_path.display(), uds_path.display(), @@ -476,6 +496,7 @@ mod tests { assert_eq!(config.get_socket_path(), socket_path.display().to_string()); assert_eq!(config.get_uds_path(), uds_path.display().to_string()); assert_eq!(config.get_tx_buffer_size(), 32768); + assert_eq!(config.get_queue_size(), 256); assert_eq!( config.get_groups(), vec!["group1".to_string(), "group2".to_string()] @@ -504,6 +525,7 @@ mod tests { assert_eq!(config.get_socket_path(), socket_path.display().to_string()); assert_eq!(config.get_uds_path(), uds_path.display().to_string()); assert_eq!(config.get_tx_buffer_size(), DEFAULT_TX_BUFFER_SIZE); + assert_eq!(config.get_queue_size(), DEFAULT_QUEUE_SIZE); assert_eq!(config.get_groups(), vec![DEFAULT_GROUP_NAME.to_string()]); std::fs::remove_file(&config_path).unwrap(); @@ -514,6 +536,7 @@ mod tests { fn test_vsock_server() { const CID: u64 = 3; const CONN_TX_BUF_SIZE: u32 = 64 * 1024; + const QUEUE_SIZE: usize = 1024; let test_dir = tempdir().expect("Could not create a temp test directory."); @@ -533,6 +556,7 @@ mod tests { vhost_socket_path, vsock_socket_path, CONN_TX_BUF_SIZE, + QUEUE_SIZE, vec![DEFAULT_GROUP_NAME.to_string()], ); @@ -567,6 +591,7 @@ mod tests { #[test] fn test_start_backend_servers_failure() { const CONN_TX_BUF_SIZE: u32 = 64 * 1024; + const QUEUE_SIZE: usize = 1024; let test_dir = tempdir().expect("Could not create a temp test directory."); @@ -584,6 +609,7 @@ mod tests { .display() .to_string(), CONN_TX_BUF_SIZE, + QUEUE_SIZE, vec![DEFAULT_GROUP_NAME.to_string()], ), VsockConfig::new( @@ -599,6 +625,7 @@ mod tests { .display() .to_string(), CONN_TX_BUF_SIZE, + QUEUE_SIZE, vec![DEFAULT_GROUP_NAME.to_string()], ), ]; @@ -635,20 +662,21 @@ mod tests { assert_matches!(error, CliError::NoArgsProvided); assert_eq!(format!("{error:?}"), "NoArgsProvided"); - let args = VsockArgs::from_args(0, "", "", 0, ""); - assert_eq!(format!("{args:?}"), "VsockArgs { param: Some(VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: \"\" }), vm: None, config: None }"); + let args = VsockArgs::from_args(0, "", "", 0, 0, ""); + assert_eq!(format!("{args:?}"), "VsockArgs { param: Some(VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, queue_size: 0, groups: \"\" }), vm: None, config: None }"); let param = args.param.unwrap().clone(); - assert_eq!(format!("{param:?}"), "VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: \"\" }"); + assert_eq!(format!("{param:?}"), "VsockParam { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, queue_size: 0, groups: \"\" }"); let config = ConfigFileVsockParam { guest_cid: None, socket: String::new(), uds_path: String::new(), tx_buffer_size: None, + queue_size: None, groups: None, } .clone(); - assert_eq!(format!("{config:?}"), "ConfigFileVsockParam { guest_cid: None, socket: \"\", uds_path: \"\", tx_buffer_size: None, groups: None }"); + assert_eq!(format!("{config:?}"), "ConfigFileVsockParam { guest_cid: None, socket: \"\", uds_path: \"\", tx_buffer_size: None, queue_size: None, groups: None }"); } } diff --git a/vhost-device-vsock/src/thread_backend.rs b/vhost-device-vsock/src/thread_backend.rs index 2b9c4aa89..1cf8f9aed 100644 --- a/vhost-device-vsock/src/thread_backend.rs +++ b/vhost-device-vsock/src/thread_backend.rs @@ -356,6 +356,7 @@ mod tests { const DATA_LEN: usize = 16; const CONN_TX_BUF_SIZE: u32 = 64 * 1024; + const QUEUE_SIZE: usize = 1024; const GROUP_NAME: &str = "default"; #[test] @@ -471,6 +472,7 @@ mod tests { sibling_vhost_socket_path, sibling_vsock_socket_path, CONN_TX_BUF_SIZE, + QUEUE_SIZE, vec!["group1", "group2", "group3"] .into_iter() .map(String::from) @@ -482,6 +484,7 @@ mod tests { sibling2_vhost_socket_path, sibling2_vsock_socket_path, CONN_TX_BUF_SIZE, + QUEUE_SIZE, vec!["group1"].into_iter().map(String::from).collect(), ); diff --git a/vhost-device-vsock/src/vhu_vsock.rs b/vhost-device-vsock/src/vhu_vsock.rs index 547fbbb9a..0a57d6cd8 100644 --- a/vhost-device-vsock/src/vhu_vsock.rs +++ b/vhost-device-vsock/src/vhu_vsock.rs @@ -28,7 +28,6 @@ pub(crate) type CidMap = HashMap>, Arc>>, EventFd)>; const NUM_QUEUES: usize = 3; -const QUEUE_SIZE: usize = 256; // New descriptors pending on the rx queue const RX_QUEUE_EVENT: u16 = 0; @@ -151,6 +150,7 @@ pub(crate) struct VsockConfig { socket: String, uds_path: String, tx_buffer_size: u32, + queue_size: usize, groups: Vec, } @@ -162,6 +162,7 @@ impl VsockConfig { socket: String, uds_path: String, tx_buffer_size: u32, + queue_size: usize, groups: Vec, ) -> Self { Self { @@ -169,6 +170,7 @@ impl VsockConfig { socket, uds_path, tx_buffer_size, + queue_size, groups, } } @@ -194,6 +196,10 @@ impl VsockConfig { self.tx_buffer_size } + pub fn get_queue_size(&self) -> usize { + self.queue_size + } + pub fn get_groups(&self) -> Vec { self.groups.clone() } @@ -229,6 +235,7 @@ unsafe impl ByteValued for VirtioVsockConfig {} pub(crate) struct VhostUserVsockBackend { config: VirtioVsockConfig, + queue_size: usize, pub threads: Vec>, queues_per_thread: Vec, pub exit_event: EventFd, @@ -249,6 +256,7 @@ impl VhostUserVsockBackend { config: VirtioVsockConfig { guest_cid: From::from(config.get_guest_cid()), }, + queue_size: config.get_queue_size(), threads: vec![thread], queues_per_thread, exit_event: EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?, @@ -265,7 +273,7 @@ impl VhostUserBackend for VhostUserVsockBackend { } fn max_queue_size(&self) -> usize { - QUEUE_SIZE + self.queue_size } fn features(&self) -> u64 { @@ -376,6 +384,7 @@ mod tests { use vm_memory::GuestAddress; const CONN_TX_BUF_SIZE: u32 = 64 * 1024; + const QUEUE_SIZE: usize = 1024; #[test] fn test_vsock_backend() { @@ -401,6 +410,7 @@ mod tests { vhost_socket_path.to_string(), vsock_socket_path.to_string(), CONN_TX_BUF_SIZE, + QUEUE_SIZE, groups_list, ); @@ -487,6 +497,7 @@ mod tests { "/sys/not_allowed.socket".to_string(), "/sys/not_allowed.vsock".to_string(), CONN_TX_BUF_SIZE, + QUEUE_SIZE, groups.clone(), ); @@ -500,6 +511,7 @@ mod tests { vhost_socket_path.to_string(), vsock_socket_path.to_string(), CONN_TX_BUF_SIZE, + QUEUE_SIZE, groups, ); @@ -542,9 +554,9 @@ mod tests { #[test] fn test_vhu_vsock_structs() { - let config = VsockConfig::new(0, String::new(), String::new(), 0, vec![String::new()]); + let config = VsockConfig::new(0, String::new(), String::new(), 0, 0, vec![String::new()]); - assert_eq!(format!("{config:?}"), "VsockConfig { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, groups: [\"\"] }"); + assert_eq!(format!("{config:?}"), "VsockConfig { guest_cid: 0, socket: \"\", uds_path: \"\", tx_buffer_size: 0, queue_size: 0, groups: [\"\"] }"); let conn_map = ConnMapKey::new(0, 0); assert_eq!( From 35a945b04ec5e98c7692a1603a0339e92934c127 Mon Sep 17 00:00:00 2001 From: Dorjoy Chowdhury Date: Wed, 10 Jul 2024 19:26:32 +0600 Subject: [PATCH 2/2] vhost-device-vsock: change default vring queue size to 1024 Although increasing queue size from 256 to 1024 increases the memory usage of vhost-device-vsock, 1024 is a reasonable default which should make vhost-device-vsock work by default (i.e., without using the queue-size option) with devices that use vring queue size up to 1024 such as QEMU's vhost-user-vsock-device. If Users require greater or smaller queue size, they can use the 'queue-size' option to configure the queue size. Signed-off-by: Dorjoy Chowdhury --- vhost-device-vsock/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vhost-device-vsock/src/main.rs b/vhost-device-vsock/src/main.rs index b69e2533f..897112e78 100644 --- a/vhost-device-vsock/src/main.rs +++ b/vhost-device-vsock/src/main.rs @@ -27,7 +27,7 @@ use vm_memory::{GuestMemoryAtomic, GuestMemoryMmap}; const DEFAULT_GUEST_CID: u64 = 3; const DEFAULT_TX_BUFFER_SIZE: u32 = 64 * 1024; -const DEFAULT_QUEUE_SIZE: usize = 256; +const DEFAULT_QUEUE_SIZE: usize = 1024; const DEFAULT_GROUP_NAME: &str = "default"; #[derive(Debug, ThisError)]