diff --git a/apps/src/args.rs b/apps/src/args.rs index 9ff02998b9..3cd8e0ecae 100644 --- a/apps/src/args.rs +++ b/apps/src/args.rs @@ -297,7 +297,7 @@ Options: --multipath-old Enable (old v4) multipath support. -A --address ADDR ... Specify addresses to be used instead of the unspecified address. Non-routable addresses will lead to connectivity issues. -R --rm-addr TIMEADDR ... Specify addresses to stop using after the provided time (format time,addr). - -S --status TIMEADDRSTAT ... Specify status to advertise to the peer after the provided time (format time,addr,status). + -S --status TIMEADDRSTAT ... Specify availability status to advertise to the peer after the provided time (format time,addr,available). -H --header HEADER ... Add a request header. -n --requests REQUESTS Send the given number of identical requests [default: 1]. --send-priority-update Send HTTP/3 priority updates if the query string params 'u' or 'i' are present in URLs @@ -329,7 +329,7 @@ pub struct ClientArgs { pub send_priority_update: bool, pub addrs: Vec, pub rm_addrs: Vec<(std::time::Duration, SocketAddr)>, - pub status: Vec<(std::time::Duration, SocketAddr, u64)>, + pub status: Vec<(std::time::Duration, SocketAddr, bool)>, } impl Args for ClientArgs { @@ -450,7 +450,8 @@ impl Args for ClientArgs { Err(_) => return None, }; let status = match s[2].parse::() { - Ok(s) => s, + Ok(0) => false, + Ok(_) => true, Err(_) => return None, }; Some((std::time::Duration::from_secs(secs), addr, status)) diff --git a/apps/src/client.rs b/apps/src/client.rs index dc7e0c2522..9217a893ea 100644 --- a/apps/src/client.rs +++ b/apps/src/client.rs @@ -36,7 +36,6 @@ use std::rc::Rc; use std::cell::RefCell; -use quiche::PathStatus; use ring::rand::*; use slab::Slab; @@ -513,14 +512,10 @@ pub fn connect( } }); - status.retain(|(d, addr, s)| { + status.retain(|(d, addr, available)| { if app_data_start.elapsed() >= *d { - info!("Advertising path status {s} to {addr:?}"); - let status = match s { - 1 => PathStatus::Standby, - 2 => PathStatus::Available, - s => PathStatus::Unknown(*s), - }; + let status = (*available).into(); + info!("Advertising path status {status:?} to {addr:?}"); conn.set_path_status(*addr, peer_addr, status, true) .is_err() } else { diff --git a/qlog/src/events/quic.rs b/qlog/src/events/quic.rs index da39fe3d7e..74ab29c0b2 100644 --- a/qlog/src/events/quic.rs +++ b/qlog/src/events/quic.rs @@ -515,10 +515,14 @@ pub enum QuicFrame { reason: Option, }, - PathStatus { + PathStandby { + dcid_seq_num: u64, + seq_num: u64, + }, + + PathAvailable { dcid_seq_num: u64, seq_num: u64, - status: u64, }, Unknown { diff --git a/quiche/src/frame.rs b/quiche/src/frame.rs index 0817088eeb..19b5ce08af 100644 --- a/quiche/src/frame.rs +++ b/quiche/src/frame.rs @@ -191,10 +191,14 @@ pub enum Frame { reason: Vec, }, - PathStatus { + PathStandby { + dcid_seq_num: u64, + seq_num: u64, + }, + + PathAvailable { dcid_seq_num: u64, seq_num: u64, - status: u64, }, } @@ -334,10 +338,14 @@ impl Frame { reason: b.get_bytes_with_varint_length()?.to_vec(), }, - 0x15228c06 => Frame::PathStatus { + 0x15228c07 => Frame::PathStandby { + dcid_seq_num: b.get_varint()?, + seq_num: b.get_varint()?, + }, + + 0x15228c08 => Frame::PathAvailable { dcid_seq_num: b.get_varint()?, seq_num: b.get_varint()?, - status: b.get_varint()?, }, _ => return Err(Error::InvalidFrame), @@ -359,7 +367,8 @@ impl Frame { (packet::Type::ZeroRTT, Frame::ConnectionClose { .. }) => false, (packet::Type::ZeroRTT, Frame::ACKMP { .. }) => false, (packet::Type::ZeroRTT, Frame::PathAbandon { .. }) => false, - (packet::Type::ZeroRTT, Frame::PathStatus { .. }) => false, + (packet::Type::ZeroRTT, Frame::PathStandby { .. }) => false, + (packet::Type::ZeroRTT, Frame::PathAvailable { .. }) => false, // ACK, CRYPTO and CONNECTION_CLOSE can be sent on all other packet // types. @@ -609,16 +618,24 @@ impl Frame { b.put_bytes(reason.as_ref())?; }, - Frame::PathStatus { + Frame::PathStandby { + dcid_seq_num, + seq_num, + } => { + b.put_varint(0x15228c07)?; + + b.put_varint(*dcid_seq_num)?; + b.put_varint(*seq_num)?; + }, + + Frame::PathAvailable { dcid_seq_num, seq_num, - status, } => { - b.put_varint(0x15228c06)?; + b.put_varint(0x15228c08)?; b.put_varint(*dcid_seq_num)?; b.put_varint(*seq_num)?; - b.put_varint(*status)?; }, } @@ -831,15 +848,22 @@ impl Frame { reason.len() }, - Frame::PathStatus { + Frame::PathStandby { dcid_seq_num, seq_num, - status, } => { 4 + // frame size octets::varint_len(*dcid_seq_num) + - octets::varint_len(*seq_num) + - octets::varint_len(*status) + octets::varint_len(*seq_num) + }, + + Frame::PathAvailable { + dcid_seq_num, + seq_num, + } => { + 4 + // frame size + octets::varint_len(*dcid_seq_num) + + octets::varint_len(*seq_num) }, } } @@ -1095,14 +1119,20 @@ impl Frame { reason: Some(String::from_utf8_lossy(reason).into_owned()), }, - Frame::PathStatus { + Frame::PathStandby { dcid_seq_num, seq_num, - status, - } => QuicFrame::PathStatus { + } => QuicFrame::PathStandby { + dcid_seq_num: *dcid_seq_num, + seq_num: *seq_num, + }, + + Frame::PathAvailable { + dcid_seq_num, + seq_num, + } => QuicFrame::PathAvailable { dcid_seq_num: *dcid_seq_num, seq_num: *seq_num, - status: *status, }, } } @@ -1297,16 +1327,24 @@ impl std::fmt::Debug for Frame { )?; }, - Frame::PathStatus { + Frame::PathStandby { dcid_seq_num, seq_num, - status, - .. } => { write!( f, - "PATH_STATUS dcid_seq_num={dcid_seq_num:x} seq_num={seq_num:x} status={status:x}", - )?; + "PATH_STANDBY dcid_seq_num={dcid_seq_num:x} seq_num={seq_num:x}", + )? + }, + + Frame::PathAvailable { + dcid_seq_num, + seq_num, + } => { + write!( + f, + "PATH_AVAILABLE dcid_seq_num={dcid_seq_num:x} seq_num={seq_num:x}", + )? }, } @@ -2432,17 +2470,15 @@ mod tests { } #[test] - fn path_status() { + fn path_standby() { let mut d = [42; 128]; let dcid_seq_num = 0xabcdef00; let seq_num = 0x42; - let status = 1; - let frame = Frame::PathStatus { + let frame = Frame::PathStandby { dcid_seq_num, seq_num, - status, }; let wire_len = { @@ -2451,7 +2487,40 @@ mod tests { }; assert_eq!(frame.wire_len(), wire_len); - assert_eq!(wire_len, 15); + assert_eq!(wire_len, 14); + + let mut b = octets::Octets::with_slice(&d); + assert_eq!(Frame::from_bytes(&mut b, packet::Type::Short), Ok(frame)); + + let mut b = octets::Octets::with_slice(&d); + assert!(Frame::from_bytes(&mut b, packet::Type::Initial).is_err()); + + let mut b = octets::Octets::with_slice(&d); + assert!(Frame::from_bytes(&mut b, packet::Type::ZeroRTT).is_err()); + + let mut b = octets::Octets::with_slice(&d); + assert!(Frame::from_bytes(&mut b, packet::Type::Handshake).is_err()); + } + + #[test] + fn path_available() { + let mut d = [42; 128]; + + let dcid_seq_num = 0xabcdef00; + let seq_num = 0x42; + + let frame = Frame::PathAvailable { + dcid_seq_num, + seq_num, + }; + + let wire_len = { + let mut b = octets::OctetsMut::with_slice(&mut d); + frame.to_bytes(&mut b).unwrap() + }; + + assert_eq!(frame.wire_len(), wire_len); + assert_eq!(wire_len, 14); let mut b = octets::Octets::with_slice(&d); assert_eq!(Frame::from_bytes(&mut b, packet::Type::Short), Ok(frame)); diff --git a/quiche/src/lib.rs b/quiche/src/lib.rs index c2685b3039..f7c063f49b 100644 --- a/quiche/src/lib.rs +++ b/quiche/src/lib.rs @@ -4117,16 +4117,23 @@ impl Connection { } } - // Create PATH_STATUS frames as needed. - while let Some((path_id, seq_num, status)) = self.paths.path_status() + // Create PATH_AVAILABLE/PATH_STANDBY frames as needed. + while let Some((path_id, seq_num, available)) = + self.paths.path_status() { let status_path = self.paths.get(path_id)?; let dcid_seq_num = status_path.active_dcid_seq.ok_or(Error::InvalidState)?; - let frame = frame::Frame::PathStatus { - dcid_seq_num, - seq_num, - status, + let frame = if available { + frame::Frame::PathAvailable { + dcid_seq_num, + seq_num, + } + } else { + frame::Frame::PathStandby { + dcid_seq_num, + seq_num, + } }; if push_frame_to_pkt!(b, frames, frame, left) { self.paths.on_path_status_sent(); @@ -7641,11 +7648,24 @@ impl Connection { )?; }, - frame::Frame::PathStatus { + frame::Frame::PathStandby { + dcid_seq_num, + seq_num, + } => { + if !self.use_path_pkt_num_space(epoch) { + return Err(Error::MultiPathViolation); + } + let pid = self + .ids + .get_dcid(dcid_seq_num)? + .path_id + .ok_or(Error::InvalidFrame)?; + self.paths.on_path_status_received(pid, seq_num, false); + }, + + frame::Frame::PathAvailable { dcid_seq_num, seq_num, - status, - .. } => { if !self.use_path_pkt_num_space(epoch) { return Err(Error::MultiPathViolation); @@ -7655,7 +7675,7 @@ impl Connection { .get_dcid(dcid_seq_num)? .path_id .ok_or(Error::InvalidFrame)?; - self.paths.on_path_status_received(pid, seq_num, status); + self.paths.on_path_status_received(pid, seq_num, true); }, }; Ok(()) @@ -8465,7 +8485,7 @@ impl TransportParams { tp.max_datagram_frame_size = Some(val.get_varint()?); }, - 0x0f739bbc1b666d05 => { + 0x0f739bbc1b666d06 => { tp.enable_multipath = true; }, @@ -8632,7 +8652,7 @@ impl TransportParams { } if tp.enable_multipath { - TransportParams::encode_param(&mut b, 0x0f739bbc1b666d05, 0)?; + TransportParams::encode_param(&mut b, 0x0f739bbc1b666d06, 0)?; } let out_len = b.off(); diff --git a/quiche/src/path.rs b/quiche/src/path.rs index cdf683a79b..6a78b8c202 100644 --- a/quiche/src/path.rs +++ b/quiche/src/path.rs @@ -115,27 +115,19 @@ pub enum PathStatus { /// The host should consider this path to send non-probing packets. Available, - - /// Other values, might be application-specific. - Unknown(u64), } -impl From for u64 { +impl From for bool { fn from(s: PathStatus) -> Self { - match s { - PathStatus::Standby => 1, - PathStatus::Available => 2, - PathStatus::Unknown(v) => v, - } + matches!(s, PathStatus::Available) } } -impl From for PathStatus { - fn from(v: u64) -> Self { +impl From for PathStatus { + fn from(v: bool) -> Self { match v { - 1 => PathStatus::Standby, - 2 => PathStatus::Available, - u => PathStatus::Unknown(u), + false => PathStatus::Standby, + true => PathStatus::Available, } } } @@ -687,7 +679,8 @@ pub struct PathMap { path_abandon: VecDeque, /// Whether a connection-wide PATH_STATUS frame should be sent. - path_status_to_advertise: VecDeque<(usize, u64, u64)>, + /// Send a PATH_AVAILABLE is true, PATH_STANDBY else. + path_status_to_advertise: VecDeque<(usize, u64, bool)>, /// The sequence number for the next PATH_STATUS. next_path_status_seq_num: u64, } @@ -1139,27 +1132,27 @@ impl PathMap { !self.path_status_to_advertise.is_empty() } - /// Returns the Path ID, the sequence number and the status value that - /// should be advertised in the next PATH_STATUS frame. - pub fn path_status(&self) -> Option<(usize, u64, u64)> { + /// Returns the Path ID, the sequence number and the availability + /// status (PATH_STANDBY or PATH_AVAILABLE) that should be advertised next. + pub fn path_status(&self) -> Option<(usize, u64, bool)> { self.path_status_to_advertise.front().copied() } - /// Handles the sending of PATH_STATUSes. + /// Handles the sending of PATH_STANDBY/PATH_AVAILABLE. pub fn on_path_status_sent(&mut self) { self.path_status_to_advertise.pop_front(); } - /// Handles the reception of PATH_STATUSes. + /// Handles the reception of PATH_STANDBY/PATH_AVAILABLE. pub fn on_path_status_received( - &mut self, path_id: usize, seq_num: u64, status: u64, + &mut self, path_id: usize, seq_num: u64, available: bool, ) { if let Ok(p) = self.get_mut(path_id) { if seq_num >= p.expected_path_status_seq_num { p.expected_path_status_seq_num = seq_num.saturating_add(1); let addr = (p.local_addr(), p.peer_addr()); self.events - .push_back(PathEvent::PeerPathStatus(addr, status.into())); + .push_back(PathEvent::PeerPathStatus(addr, available.into())); } } } @@ -1496,10 +1489,10 @@ mod tests { paths.advertise_path_status(pid_3).unwrap(); assert_eq!(paths.has_path_status(), true); - assert_eq!(paths.path_status(), Some((pid_2, 0, 1))); + assert_eq!(paths.path_status(), Some((pid_2, 0, false))); paths.on_path_status_sent(); assert_eq!(paths.has_path_status(), true); - assert_eq!(paths.path_status(), Some((pid_3, 1, 2))); + assert_eq!(paths.path_status(), Some((pid_3, 1, true))); paths.on_path_status_sent(); assert_eq!(paths.has_path_status(), false); assert_eq!(paths.path_status(), None); @@ -1509,10 +1502,10 @@ mod tests { paths.advertise_path_status(pid_2).unwrap(); paths.advertise_path_status(pid_3).unwrap(); assert_eq!(paths.has_path_status(), true); - assert_eq!(paths.path_status(), Some((pid_2, 2, 2))); + assert_eq!(paths.path_status(), Some((pid_2, 2, true))); paths.on_path_status_sent(); assert_eq!(paths.has_path_status(), true); - assert_eq!(paths.path_status(), Some((pid_3, 3, 1))); + assert_eq!(paths.path_status(), Some((pid_3, 3, false))); paths.on_path_status_sent(); assert_eq!(paths.has_path_status(), false); assert_eq!(paths.path_status(), None);