Skip to content

Commit aaf1558

Browse files
Merge pull request #212 from arctic-alpaca/keepalive-new-base
Backport upstream timeout/keep-alive handling
2 parents 7ead85f + a874ba9 commit aaf1558

7 files changed

Lines changed: 85 additions & 94 deletions

File tree

‎Cargo.lock‎

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vm-migration/src/keep_alive_stream.rs‎

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::{result, thread};
1313
use vm_memory::bitmap::BitmapSlice;
1414
use vm_memory::{ReadVolatile, VolatileMemoryError, VolatileSlice, WriteVolatile};
1515

16-
use crate::protocol::{Request, Response};
16+
use crate::protocol::Response;
1717

1818
/// The `KeepAliveStream` is a stream that is intended to be used for the main
1919
/// connection of live migrations. If the `KeepAliveStream` does not read or
@@ -51,16 +51,14 @@ enum KeepAliveStreamAnswer {
5151

5252
struct KeepAliveWorker<S: Read + Write + AsFd> {
5353
stream: S,
54-
/// Is this running on the sender or receiver side?
55-
is_sender: bool,
5654
}
5755

5856
impl<S> KeepAliveWorker<S>
5957
where
6058
S: Read + Write + AsFd,
6159
{
62-
pub fn new(stream: S, is_sender: bool) -> Self {
63-
Self { stream, is_sender }
60+
pub fn new(stream: S) -> Self {
61+
Self { stream }
6462
}
6563

6664
pub fn read(&mut self, mut buf: Vec<u8>, len: usize) -> io::Result<(Vec<u8>, usize)> {
@@ -74,8 +72,11 @@ where
7472

7573
pub fn write(&mut self, buf: Vec<u8>, len: usize) -> io::Result<(Vec<u8>, usize)> {
7674
debug_assert!(len <= buf.len());
77-
let n = Write::write(&mut self.stream, &buf[..len])?;
78-
Ok((buf, n))
75+
// If only 6 of 16 response bytes are written, the caller's retry could
76+
// follow a keepalive: 6 response bytes, keepalive, then 10 bytes.
77+
// Finish this write before the timer can run again.
78+
Write::write_all(&mut self.stream, &buf[..len])?;
79+
Ok((buf, len))
7980
}
8081

8182
pub fn flush(&mut self) -> io::Result<()> {
@@ -103,7 +104,6 @@ impl KeepAliveStream {
103104
pub fn new<T: Read + Write + Send + AsFd + 'static>(
104105
stream: T,
105106
timeout: Duration,
106-
is_sender: bool,
107107
) -> result::Result<Self, io::Error> {
108108
let fd = stream.as_fd().try_clone_to_owned()?;
109109

@@ -114,7 +114,7 @@ impl KeepAliveStream {
114114
let thread = thread::Builder::new()
115115
.name("migration_keep_alive_thread".to_string())
116116
.spawn(move || {
117-
let mut worker = KeepAliveWorker::new(stream, is_sender);
117+
let mut worker = KeepAliveWorker::new(stream);
118118
loop {
119119
// The idea is to always send a keep alive message when this times out.
120120
match message_rx.recv_timeout(timeout) {
@@ -148,13 +148,8 @@ impl KeepAliveStream {
148148
KeepAliveStreamMessage::Disconnect => break,
149149
},
150150
Err(RecvTimeoutError::Timeout) => {
151-
if worker.is_sender {
152-
let keep_alive = Request::keep_alive();
153-
let _ = keep_alive.write_to(&mut worker.stream);
154-
} else {
155-
let keep_alive = Response::keep_alive();
156-
let _ = keep_alive.write_to(&mut worker.stream);
157-
}
151+
let keep_alive = Response::keep_alive();
152+
let _ = keep_alive.write_to(&mut worker.stream);
158153
}
159154
Err(RecvTimeoutError::Disconnected) => break,
160155
}

‎vm-migration/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use crate::protocol::MemoryRangeTable;
1818

1919
mod bitpos_iterator;
2020
mod context;
21+
// TODO(keepalive): remove this module when gardenlinux-release-26-09-18 compatibility ends.
2122
pub mod keep_alive_stream;
2223
pub mod progress;
2324
pub mod protocol;

‎vm-migration/src/protocol.rs‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,7 @@ pub enum Command {
127127
/// Finalizes the migration without resuming the VM on the destination.
128128
/// Sent when the source VM was paused at migration time.
129129
CompletePaused = 9,
130-
// We introduced this with discriminant eight but in the meantime,
131-
// upstream introduced a new command with discriminant 8. For
132-
// migration-compatibility we stick to this temporarily, until we have
133-
// a solution for the discriminant collision.
130+
// TODO(keepalive): remove this command when gardenlinux-release-26-09-18 compatibility ends.
134131
KeepAlive = 8,
135132
}
136133

@@ -185,10 +182,6 @@ impl Request {
185182
Self::new(Command::Abandon, 0)
186183
}
187184

188-
pub fn keep_alive() -> Self {
189-
Self::new(Command::KeepAlive, 0)
190-
}
191-
192185
pub fn command(&self) -> Command {
193186
self.command
194187
}
@@ -216,6 +209,7 @@ impl Request {
216209
.map_err(|error| MigratableError::DeserializeError(anyhow!("{error:?}")))?;
217210

218211
// If we read a keep alive message, we throw it away and keep reading.
212+
// TODO(keepalive): remove this branch when gardenlinux-release-26-09-18 compatibility ends.
219213
if request.command() == Command::KeepAlive {
220214
*request = Request::default();
221215
continue;
@@ -237,6 +231,7 @@ pub enum Status {
237231
Invalid,
238232
Ok,
239233
Error,
234+
// TODO(keepalive): remove this status when gardenlinux-release-26-09-18 compatibility ends.
240235
KeepAlive,
241236
}
242237

@@ -265,6 +260,7 @@ impl Response {
265260
Self::new(Status::Error, 0)
266261
}
267262

263+
// TODO(keepalive): remove this constructor when gardenlinux-release-26-09-18 compatibility ends.
268264
pub fn keep_alive() -> Self {
269265
Self::new(Status::KeepAlive, 0)
270266
}
@@ -296,6 +292,7 @@ impl Response {
296292
.map_err(|error| MigratableError::DeserializeError(anyhow!("{error:?}")))?;
297293

298294
// If we read a keep alive message, we throw it away and keep reading.
295+
// TODO(keepalive): remove this branch when gardenlinux-release-26-09-18 compatibility ends.
299296
if response.status() == Status::KeepAlive {
300297
*response = Response::default();
301298
continue;

‎vmm/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ serde_json = { workspace = true }
8686
serial_buffer = { path = "../serial_buffer" }
8787
sha2 = { workspace = true }
8888
signal-hook = { workspace = true }
89+
socket2 = { version = "0.6.5", features = ["all"] }
8990
thiserror = { workspace = true }
9091
tracer = { path = "../tracer" }
9192
uuid = { workspace = true }

‎vmm/src/lib.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,17 +1734,10 @@ impl Vmm {
17341734
let mut ctx = OngoingMigrationContext::new();
17351735

17361736
// Set up the socket connection
1737-
let mut socket = if send_data_migration.local {
1738-
transport::send_migration_socket(
1739-
&send_data_migration.destination_url,
1740-
send_data_migration.tls_dir.as_deref(),
1741-
)?
1742-
} else {
1743-
transport::send_migration_socket_with_keep_alive(
1744-
&send_data_migration.destination_url,
1745-
send_data_migration.tls_dir.as_deref(),
1746-
)?
1747-
};
1737+
let mut socket = transport::send_migration_socket(
1738+
&send_data_migration.destination_url,
1739+
send_data_migration.tls_dir.as_deref(),
1740+
)?;
17481741

17491742
// Start the migration
17501743
transport::send_request_expect_ok(

‎vmm/src/migration/transport.rs‎

Lines changed: 52 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use std::time::Duration;
2020
use anyhow::{Context, anyhow};
2121
use log::{debug, error, info, warn};
2222
use serde_json;
23+
use socket2::{SockRef, TcpKeepalive};
2324
use vm_memory::bitmap::BitmapSlice;
2425
use vm_memory::{
2526
Bytes, GuestAddress, GuestAddressSpace, GuestMemoryAtomic, ReadVolatile, VolatileMemoryError,
@@ -39,33 +40,10 @@ use crate::{GuestMemoryMmap, VmMigrationConfig};
3940
/// receiver side.
4041
pub(crate) const MAX_MIGRATION_CONNECTIONS: u32 = 128;
4142

42-
/// The time a writer may block on a socket until it throws an error.
43-
///
44-
/// Also the interval at which the [`KeepAliveStream`] sends keep alive messages.
45-
///
46-
/// # Relation with [`MIGRATION_READ_TIMEOUT_DURATION`]
47-
///
48-
/// This timeout has to be smaller than [`MIGRATION_READ_TIMEOUT_DURATION`],
49-
/// otherwise spurious timeouts may happen.
50-
const MIGRATION_WRITE_TIMEOUT_DURATION: Duration = Duration::from_secs(5);
51-
52-
/// The time a reader may block on a socket until it throws an error.
53-
///
54-
/// # Relation with [`MIGRATION_WRITE_TIMEOUT_DURATION`]
55-
///
56-
/// This timeout has to be larger than [`MIGRATION_WRITE_TIMEOUT_DURATION`],
57-
/// otherwise spurious timeouts may happen.
58-
const MIGRATION_READ_TIMEOUT_DURATION: Duration = {
59-
let migration_read_timeout_duration = Duration::from_secs(10);
60-
61-
// This timeout has to be larger than [`MIGRATION_WRITE_TIMEOUT_DURATION`],
62-
// otherwise spurious timeouts may happen.
63-
assert!(
64-
MIGRATION_WRITE_TIMEOUT_DURATION.as_millis() < migration_read_timeout_duration.as_millis(),
65-
"MIGRATION_WRITE_TIMEOUT_DURATION must be smaller than MIGRATION_READ_TIMEOUT_DURATION",
66-
);
67-
migration_read_timeout_duration
68-
};
43+
/// Interval at which the [`KeepAliveStream`] sends keep alive messages.
44+
/// This must remain shorter than the old sender's 10-second read timeout.
45+
// TODO(keepalive): remove this interval when gardenlinux-release-26-09-18 compatibility ends.
46+
const MIGRATION_KEEP_ALIVE_INTERVAL: Duration = Duration::from_secs(5);
6947

7048
/// The timeout of the migration-receiver.
7149
///
@@ -76,16 +54,6 @@ const MIGRATION_READ_TIMEOUT_DURATION: Duration = {
7654
/// if the management software doesn't kill the process correctly.
7755
const MIGRATION_ACCEPT_TIMEOUT_DURATION: Duration = Duration::from_secs(60);
7856

79-
fn set_migration_socket_timeouts(socket: &TcpStream) -> anyhow::Result<()> {
80-
socket
81-
.set_read_timeout(Some(MIGRATION_READ_TIMEOUT_DURATION))
82-
.context("Error setting read timeout on TCP socket")?;
83-
socket
84-
.set_write_timeout(Some(MIGRATION_WRITE_TIMEOUT_DURATION))
85-
.context("Error setting write timeout on TCP socket")?;
86-
Ok(())
87-
}
88-
8957
/// Transport-agnostic listener used to receive connections.
9058
#[derive(Debug)]
9159
pub(crate) enum ReceiveListener {
@@ -98,6 +66,7 @@ impl ReceiveListener {
9866
/// Block until a connection is accepted.
9967
pub(crate) fn accept(
10068
&mut self,
69+
// TODO(keepalive): remove this parameter when gardenlinux-release-26-09-18 compatibility ends.
10170
main_connection: bool,
10271
) -> Result<SocketStream, MigratableError> {
10372
match self {
@@ -109,11 +78,16 @@ impl ReceiveListener {
10978
let (socket, _) = accept_with_timeout(listener, MIGRATION_ACCEPT_TIMEOUT_DURATION)
11079
.context("Failed to accept TCP migration connection")
11180
.map_err(MigratableError::MigrateReceive)?;
112-
set_migration_socket_timeouts(&socket).map_err(MigratableError::MigrateReceive)?;
81+
82+
set_tcp_keepalive_and_user_timeout(&socket)
83+
.context("Failed to set socket options")
84+
.map_err(MigratableError::MigrateReceive)?;
11385

11486
let socket = SocketStream::Tcp(socket);
87+
88+
// TODO(keepalive): remove this wrapper when gardenlinux-release-26-09-18 compatibility ends.
11589
if main_connection {
116-
KeepAliveStream::new(socket, MIGRATION_WRITE_TIMEOUT_DURATION, false)
90+
KeepAliveStream::new(socket, MIGRATION_KEEP_ALIVE_INTERVAL)
11791
.map(SocketStream::KeepAlive)
11892
.context("Error creating keep-alive migration stream")
11993
.map_err(MigratableError::MigrateReceive)
@@ -134,16 +108,20 @@ impl ReceiveListener {
134108
let (socket, _) = accept_with_timeout(listener, MIGRATION_ACCEPT_TIMEOUT_DURATION)
135109
.context("Failed to accept TCP connection")
136110
.map_err(MigratableError::MigrateReceive)?;
137-
set_migration_socket_timeouts(&socket).map_err(MigratableError::MigrateReceive)?;
111+
112+
set_tcp_keepalive_and_user_timeout(&socket)
113+
.context("Failed to set socket options")
114+
.map_err(MigratableError::MigrateReceive)?;
138115

139116
let socket = TlsStream::new_server(socket, config)
140117
.map(Box::new)
141118
.map(SocketStream::Tls)
142119
.context("Failed to accept TLS migration connection")
143120
.map_err(MigratableError::MigrateReceive)?;
144121

122+
// TODO(keepalive): remove this wrapper when gardenlinux-release-26-09-18 compatibility ends.
145123
if main_connection {
146-
KeepAliveStream::new(socket, MIGRATION_WRITE_TIMEOUT_DURATION, false)
124+
KeepAliveStream::new(socket, MIGRATION_KEEP_ALIVE_INTERVAL)
147125
.map(SocketStream::KeepAlive)
148126
.context("Error creating keep-alive migration stream")
149127
.map_err(MigratableError::MigrateReceive)
@@ -228,6 +206,7 @@ pub(crate) enum SocketStream {
228206
Unix(UnixStream),
229207
Tcp(TcpStream),
230208
Tls(Box<TlsStream>),
209+
// TODO(keepalive): remove this variant when gardenlinux-release-26-09-18 compatibility ends.
231210
KeepAlive(KeepAliveStream),
232211
}
233212

@@ -282,6 +261,7 @@ impl ReadVolatile for SocketStream {
282261
SocketStream::Unix(s) => s.read_volatile(buf),
283262
SocketStream::Tcp(s) => s.read_volatile(buf),
284263
SocketStream::Tls(s) => s.read_volatile(buf),
264+
285265
SocketStream::KeepAlive(s) => s.read_volatile(buf),
286266
}
287267
}
@@ -940,6 +920,33 @@ fn tcp_address_to_server_name(address: &str) -> Result<&str, anyhow::Error> {
940920
}
941921
}
942922

923+
/// Enables `SO_KEEPALIVE` and `TCP_USER_TIMEOUT` for the `tcp_stream`'s socket.
924+
///
925+
/// The set options target a failure detection time of two to three minutes.
926+
fn set_tcp_keepalive_and_user_timeout(tcp_stream: &TcpStream) -> io::Result<()> {
927+
/// [`TcpKeepalive`] config for migration TCP sockets.
928+
///
929+
/// After 60 seconds, the kernel starts sending keepalive probes.
930+
/// Every 15 seconds another keepalive probe is sent.
931+
/// If 4 probes are sent without response, the connection is dropped.
932+
const MIGRATION_TCP_KEEPALIVE: TcpKeepalive = TcpKeepalive::new()
933+
.with_retries(4)
934+
.with_time(Duration::from_secs(60))
935+
.with_interval(Duration::from_secs(15));
936+
937+
let socket_ref = SockRef::from(&tcp_stream);
938+
939+
// `TCP_USER_TIMEOUT` overrides `SO_KEEPALIVE` partially as per documentation:
940+
// "[...] TCP_USER_TIMEOUT will override keepalive to determine when
941+
// to close a connection due to keepalive failure."
942+
// https://man7.org/linux/man-pages/man7/tcp.7.html
943+
//TODO: Look into setting the keepalive via `std` once
944+
// https://github.com/rust-lang/rust/issues/155889 is stabilized.
945+
socket_ref.set_tcp_keepalive(&MIGRATION_TCP_KEEPALIVE)?;
946+
947+
socket_ref.set_tcp_user_timeout(Some(Duration::from_secs(120)))
948+
}
949+
943950
/// Connect to a migration endpoint and return the established stream.
944951
pub(crate) fn send_migration_socket(
945952
destination_url: &str,
@@ -951,7 +958,10 @@ pub(crate) fn send_migration_socket(
951958
let socket = TcpStream::connect(address).map_err(|e| {
952959
MigratableError::MigrateSend(anyhow!("Error connecting to TCP socket: {e}"))
953960
})?;
954-
set_migration_socket_timeouts(&socket).map_err(MigratableError::MigrateSend)?;
961+
962+
set_tcp_keepalive_and_user_timeout(&socket)
963+
.context("Failed to set socket options")
964+
.map_err(MigratableError::MigrateSend)?;
955965

956966
if let Some(tls_dir) = tls_dir {
957967
let server_name = tcp_address_to_server_name(address)
@@ -977,23 +987,6 @@ pub(crate) fn send_migration_socket(
977987
}
978988
}
979989

980-
/// Connect to the main migration endpoint and keep the connection active while
981-
/// memory is transferred over additional streams.
982-
pub(crate) fn send_migration_socket_with_keep_alive(
983-
destination_url: &str,
984-
tls_dir: Option<&Path>,
985-
) -> Result<SocketStream, MigratableError> {
986-
match send_migration_socket(destination_url, tls_dir)? {
987-
socket @ (SocketStream::Tcp(_) | SocketStream::Tls(_)) => {
988-
KeepAliveStream::new(socket, MIGRATION_WRITE_TIMEOUT_DURATION, true)
989-
.map(SocketStream::KeepAlive)
990-
.context("Error creating keep-alive migration stream")
991-
.map_err(MigratableError::MigrateSend)
992-
}
993-
socket => Ok(socket),
994-
}
995-
}
996-
997990
/// Bind a migration listener for the receiver side.
998991
pub(crate) fn receive_migration_listener(
999992
receiver_url: &str,

0 commit comments

Comments
 (0)