From 966ece6e53f2662bd8cf6b1a18544d22609cedf5 Mon Sep 17 00:00:00 2001 From: Lars Eggert Date: Wed, 12 Feb 2025 17:00:22 +0200 Subject: [PATCH] chore: Convert `FrameType` to an enum (#2438) * chore: Convert `FrameType` to an enum * Update neqo-transport/src/ackrate.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-transport/src/connection/tests/resumption.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-transport/src/connection/tests/stream.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson Signed-off-by: Lars Eggert * Fix * `DecodingFrame` -> `FrameEncodingError` --------- Signed-off-by: Lars Eggert Co-authored-by: Martin Thomson --- neqo-http3/src/lib.rs | 1 - neqo-transport/src/ackrate.rs | 4 +- neqo-transport/src/addr_valid.rs | 5 +- neqo-transport/src/cid.rs | 5 +- neqo-transport/src/connection/mod.rs | 22 +- neqo-transport/src/connection/state.rs | 18 +- .../src/connection/tests/datagram.rs | 4 +- .../src/connection/tests/migration.rs | 4 +- neqo-transport/src/connection/tests/mod.rs | 4 +- .../src/connection/tests/recovery.rs | 4 +- .../src/connection/tests/resumption.rs | 4 +- neqo-transport/src/connection/tests/stream.rs | 56 +-- neqo-transport/src/crypto.rs | 3 +- neqo-transport/src/fc.rs | 26 +- neqo-transport/src/frame.rs | 381 +++++++++++------- neqo-transport/src/lib.rs | 1 - neqo-transport/src/packet/mod.rs | 9 +- neqo-transport/src/path.rs | 8 +- neqo-transport/src/pmtud.rs | 4 +- neqo-transport/src/qlog.rs | 2 +- neqo-transport/src/quic_datagrams.rs | 11 +- neqo-transport/src/recv_stream.rs | 4 +- neqo-transport/src/send_stream.rs | 4 +- neqo-transport/src/tracking.rs | 6 +- 24 files changed, 323 insertions(+), 267 deletions(-) diff --git a/neqo-http3/src/lib.rs b/neqo-http3/src/lib.rs index 4b1cde5383..f93e38423d 100644 --- a/neqo-http3/src/lib.rs +++ b/neqo-http3/src/lib.rs @@ -214,7 +214,6 @@ pub enum Error { // Internal errors from here. AlreadyClosed, AlreadyInitialized, - DecodingFrame, FatalError, HttpGoaway, Internal, diff --git a/neqo-transport/src/ackrate.rs b/neqo-transport/src/ackrate.rs index 78ed5071c7..ea096cd1b2 100644 --- a/neqo-transport/src/ackrate.rs +++ b/neqo-transport/src/ackrate.rs @@ -11,7 +11,7 @@ use std::{cmp::max, time::Duration}; use neqo_common::qtrace; use crate::{ - connection::params::ACK_RATIO_SCALE, frame::FRAME_TYPE_ACK_FREQUENCY, packet::PacketBuilder, + connection::params::ACK_RATIO_SCALE, frame::FrameType, packet::PacketBuilder, recovery::RecoveryToken, stats::FrameStats, }; @@ -43,7 +43,7 @@ impl AckRate { pub fn write_frame(&self, builder: &mut PacketBuilder, seqno: u64) -> bool { builder.write_varint_frame(&[ - FRAME_TYPE_ACK_FREQUENCY, + u64::from(FrameType::AckFrequency), seqno, u64::try_from(self.packets + 1).expect("usize fits in u64"), u64::try_from(self.delay.as_micros()).unwrap_or(u64::MAX), diff --git a/neqo-transport/src/addr_valid.rs b/neqo-transport/src/addr_valid.rs index ca43ada2dd..e925c21eee 100644 --- a/neqo-transport/src/addr_valid.rs +++ b/neqo-transport/src/addr_valid.rs @@ -19,7 +19,8 @@ use neqo_crypto::{ use smallvec::SmallVec; use crate::{ - cid::ConnectionId, packet::PacketBuilder, recovery::RecoveryToken, stats::FrameStats, Res, + cid::ConnectionId, frame::FrameType, packet::PacketBuilder, recovery::RecoveryToken, + stats::FrameStats, Res, }; /// A prefix we add to Retry tokens to distinguish them from `NEW_TOKEN` tokens. @@ -423,7 +424,7 @@ impl NewTokenSender { if t.needs_sending && t.len() <= builder.remaining() { t.needs_sending = false; - builder.encode_varint(crate::frame::FRAME_TYPE_NEW_TOKEN); + builder.encode_varint(FrameType::NewToken); builder.encode_vvec(&t.token); tokens.push(RecoveryToken::NewToken(t.seqno)); diff --git a/neqo-transport/src/cid.rs b/neqo-transport/src/cid.rs index 949148f1c9..23dbc6856e 100644 --- a/neqo-transport/src/cid.rs +++ b/neqo-transport/src/cid.rs @@ -19,8 +19,7 @@ use neqo_crypto::{random, randomize}; use smallvec::{smallvec, SmallVec}; use crate::{ - frame::FRAME_TYPE_NEW_CONNECTION_ID, packet::PacketBuilder, recovery::RecoveryToken, - stats::FrameStats, Error, Res, + frame::FrameType, packet::PacketBuilder, recovery::RecoveryToken, stats::FrameStats, Error, Res, }; pub const MAX_CONNECTION_ID_LEN: usize = 20; @@ -306,7 +305,7 @@ impl ConnectionIdEntry<[u8; 16]> { return false; } - builder.encode_varint(FRAME_TYPE_NEW_CONNECTION_ID); + builder.encode_varint(FrameType::NewConnectionId); builder.encode_varint(self.seqno); builder.encode_varint(0u64); builder.encode_vec(1, &self.cid); diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index cd3a0aef64..294ed07622 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -40,10 +40,7 @@ use crate::{ crypto::{Crypto, CryptoDxState, Epoch}, ecn, events::{ConnectionEvent, ConnectionEvents, OutgoingDatagramOutcome}, - frame::{ - CloseError, Frame, FrameType, FRAME_TYPE_CONNECTION_CLOSE_APPLICATION, - FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT, - }, + frame::{CloseError, Frame, FrameType}, packet::{self, DecryptedPacket, PacketBuilder, PacketNumber, PacketType, PublicPacket}, path::{Path, PathRef, Paths}, qlog, @@ -956,7 +953,7 @@ impl Connection { /// For use with `process_input()`. Errors there can be ignored, but this /// needs to ensure that the state is updated. fn absorb_error(&mut self, now: Instant, res: Res) -> Option { - self.capture_error(None, now, 0, res).ok() + self.capture_error(None, now, FrameType::Padding, res).ok() } fn process_timer(&mut self, now: Instant) { @@ -1578,7 +1575,7 @@ impl Connection { ); path.borrow_mut().add_received(d.len()); let res = self.input_path(&path, d, received); - _ = self.capture_error(Some(path), now, 0, res); + _ = self.capture_error(Some(path), now, FrameType::Padding, res); } #[allow(clippy::too_many_lines)] // Will be addressed as part of https://github.com/mozilla/neqo/pull/2396 @@ -2033,7 +2030,7 @@ impl Connection { || Ok(SendOption::default()), |path| { let res = self.output_path(&path, now, None); - self.capture_error(Some(path), now, 0, res) + self.capture_error(Some(path), now, FrameType::Padding, res) }, ), State::Closing { .. } | State::Draining { .. } | State::Closed(_) => { @@ -2051,7 +2048,7 @@ impl Connection { } else { self.output_path(&path, now, Some(&details)) }; - self.capture_error(Some(path), now, 0, res) + self.capture_error(Some(path), now, FrameType::Padding, res) }, ) } @@ -2242,7 +2239,7 @@ impl Connection { if probe { // Nothing ack-eliciting and we need to probe; send PING. debug_assert_ne!(builder.remaining(), 0); - builder.encode_varint(crate::frame::FRAME_TYPE_PING); + builder.encode_varint(FrameType::Ping); let stats = &mut self.stats.borrow_mut().frame_tx; stats.ping += 1; } @@ -2609,7 +2606,8 @@ impl Connection { let error = CloseReason::Application(app_error); let timeout = self.get_closing_period_time(now); if let Some(path) = self.paths.primary() { - self.state_signaling.close(path, error.clone(), 0, msg); + self.state_signaling + .close(path, error.clone(), FrameType::Padding, msg); self.set_state(State::Closing { error, timeout }, now); } else { self.set_state(State::Closed(error), now); @@ -3024,12 +3022,12 @@ impl Connection { // NO_ERROR in this case. ( Error::PeerApplicationError(error_code.code()), - FRAME_TYPE_CONNECTION_CLOSE_APPLICATION, + FrameType::ConnectionCloseApplication, ) } else { ( Error::PeerError(error_code.code()), - FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT, + FrameType::ConnectionCloseTransport, ) }; let error = CloseReason::Transport(detail); diff --git a/neqo-transport/src/connection/state.rs b/neqo-transport/src/connection/state.rs index 2404f8d377..9bdd13687a 100644 --- a/neqo-transport/src/connection/state.rs +++ b/neqo-transport/src/connection/state.rs @@ -14,14 +14,8 @@ use std::{ use neqo_common::Encoder; use crate::{ - frame::{ - FrameType, FRAME_TYPE_CONNECTION_CLOSE_APPLICATION, FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT, - FRAME_TYPE_HANDSHAKE_DONE, - }, - packet::PacketBuilder, - path::PathRef, - recovery::RecoveryToken, - CloseReason, Error, + frame::FrameType, packet::PacketBuilder, path::PathRef, recovery::RecoveryToken, CloseReason, + Error, }; #[derive(Clone, Debug, PartialEq, Eq)] @@ -152,7 +146,7 @@ impl ClosingFrame { Some(Self { path: Rc::clone(&self.path), error: CloseReason::Transport(Error::ApplicationError), - frame_type: 0, + frame_type: FrameType::Padding, reason_phrase: Vec::new(), }) } else { @@ -171,12 +165,12 @@ impl ClosingFrame { } match &self.error { CloseReason::Transport(e) => { - builder.encode_varint(FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT); + builder.encode_varint(FrameType::ConnectionCloseTransport); builder.encode_varint(e.code()); builder.encode_varint(self.frame_type); } CloseReason::Application(code) => { - builder.encode_varint(FRAME_TYPE_CONNECTION_CLOSE_APPLICATION); + builder.encode_varint(FrameType::ConnectionCloseApplication); builder.encode_varint(*code); } } @@ -224,7 +218,7 @@ impl StateSignaling { pub fn write_done(&mut self, builder: &mut PacketBuilder) -> Option { (matches!(self, Self::HandshakeDone) && builder.remaining() >= 1).then(|| { *self = Self::Idle; - builder.encode_varint(FRAME_TYPE_HANDSHAKE_DONE); + builder.encode_varint(FrameType::HandshakeDone); RecoveryToken::HandshakeDone }) } diff --git a/neqo-transport/src/connection/tests/datagram.rs b/neqo-transport/src/connection/tests/datagram.rs index b80a348bca..95059d0da1 100644 --- a/neqo-transport/src/connection/tests/datagram.rs +++ b/neqo-transport/src/connection/tests/datagram.rs @@ -17,7 +17,7 @@ use super::{ use crate::{ connection::tests::DEFAULT_ADDR, events::{ConnectionEvent, OutgoingDatagramOutcome}, - frame::FRAME_TYPE_DATAGRAM, + frame::FrameType, packet::PacketBuilder, quic_datagrams::MAX_QUIC_DATAGRAM, send_stream::{RetransmissionPriority, TransmissionPriority}, @@ -44,7 +44,7 @@ struct InsertDatagram<'a> { impl crate::connection::test_internal::FrameWriter for InsertDatagram<'_> { fn write_frames(&mut self, builder: &mut PacketBuilder) { - builder.encode_varint(FRAME_TYPE_DATAGRAM); + builder.encode_varint(FrameType::Datagram); builder.encode(self.data); } } diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs index 7fa12eb840..30175e6d9c 100644 --- a/neqo-transport/src/connection/tests/migration.rs +++ b/neqo-transport/src/connection/tests/migration.rs @@ -28,7 +28,7 @@ use crate::{ connection::tests::{ assert_path_challenge_min_len, connect, send_something_paced, send_with_extra, }, - frame::FRAME_TYPE_NEW_CONNECTION_ID, + frame::FrameType, packet::PacketBuilder, path::MAX_PATH_PROBES, pmtud::Pmtud, @@ -1049,7 +1049,7 @@ impl crate::connection::test_internal::FrameWriter for RetireAll { const SEQNO: u64 = 100; let cid = self.cid_gen.borrow_mut().generate_cid().unwrap(); builder - .encode_varint(FRAME_TYPE_NEW_CONNECTION_ID) + .encode_varint(FrameType::NewConnectionId) .encode_varint(SEQNO) .encode_varint(SEQNO) // Retire Prior To .encode_vec(1, &cid) diff --git a/neqo-transport/src/connection/tests/mod.rs b/neqo-transport/src/connection/tests/mod.rs index c249909fa3..269b54e206 100644 --- a/neqo-transport/src/connection/tests/mod.rs +++ b/neqo-transport/src/connection/tests/mod.rs @@ -24,7 +24,7 @@ use crate::{ cc::CWND_INITIAL_PKTS, cid::ConnectionIdRef, events::ConnectionEvent, - frame::FRAME_TYPE_PING, + frame::FrameType, packet::PacketBuilder, pmtud::Pmtud, recovery::ACK_ONLY_SIZE_LIMIT, @@ -182,7 +182,7 @@ struct PingWriter {} impl test_internal::FrameWriter for PingWriter { fn write_frames(&mut self, builder: &mut PacketBuilder) { - builder.encode_varint(FRAME_TYPE_PING); + builder.encode_varint(FrameType::Ping); } } diff --git a/neqo-transport/src/connection/tests/recovery.rs b/neqo-transport/src/connection/tests/recovery.rs index c1558fce1d..768ace2781 100644 --- a/neqo-transport/src/connection/tests/recovery.rs +++ b/neqo-transport/src/connection/tests/recovery.rs @@ -22,7 +22,7 @@ use super::{ }; use crate::{ connection::{test_internal::FrameWriter, tests::cwnd_min}, - frame::FRAME_TYPE_ACK, + frame::FrameType, packet::PacketBuilder, recovery::{ FAST_PTO_SCALE, MAX_OUTSTANDING_UNACK, MAX_PTO_PACKET_COUNT, MIN_OUTSTANDING_UNACK, @@ -857,7 +857,7 @@ fn ack_for_unsent() { impl FrameWriter for AckforUnsentWriter { fn write_frames(&mut self, builder: &mut PacketBuilder) { - builder.encode_varint(FRAME_TYPE_ACK); + builder.encode_varint(FrameType::Ack); builder.encode_varint(666u16); // Largest ACKed builder.encode_varint(0u8); // ACK delay builder.encode_varint(0u8); // ACK block count diff --git a/neqo-transport/src/connection/tests/resumption.rs b/neqo-transport/src/connection/tests/resumption.rs index 1a3d650a85..0e62cbd566 100644 --- a/neqo-transport/src/connection/tests/resumption.rs +++ b/neqo-transport/src/connection/tests/resumption.rs @@ -23,7 +23,7 @@ use super::{ }; use crate::{ addr_valid::{AddressValidation, ValidateAddress}, - frame::FRAME_TYPE_PADDING, + frame::FrameType, rtt::INITIAL_RTT, ConnectionParameters, Error, State, Version, MIN_INITIAL_PACKET_SIZE, }; @@ -129,7 +129,7 @@ fn ticket_rtt(rtt: Duration) -> Duration { dec.skip_vvec(); // Skip over the payload. // Replace the ACK frame with PADDING. - plaintext[..ACK_FRAME_1.len()].fill(FRAME_TYPE_PADDING.try_into().unwrap()); + plaintext[..ACK_FRAME_1.len()].fill(u8::from(FrameType::Padding)); // And rebuild a packet. let mut packet = header.clone(); diff --git a/neqo-transport/src/connection/tests/stream.rs b/neqo-transport/src/connection/tests/stream.rs index e775cbb8ec..a306af3813 100644 --- a/neqo-transport/src/connection/tests/stream.rs +++ b/neqo-transport/src/connection/tests/stream.rs @@ -16,10 +16,7 @@ use super::{ }; use crate::{ events::ConnectionEvent, - frame::{ - FRAME_TYPE_MAX_STREAM_DATA, FRAME_TYPE_RESET_STREAM, FRAME_TYPE_STOP_SENDING, - FRAME_TYPE_STREAM_CLIENT_BIDI, FRAME_TYPE_STREAM_DATA_BLOCKED, - }, + frame::FrameType, packet::PacketBuilder, recv_stream::RECV_BUFFER_SIZE, send_stream::{OrderGroup, SendStreamState, SEND_BUFFER_SIZE}, @@ -567,15 +564,15 @@ fn illegal_stream_related_frames() { // 0 = Client-Initiated, Bidirectional; 2 = Client-Initiated, Unidirectional for stream_id in [0, 2] { for frame_type in [ - FRAME_TYPE_RESET_STREAM, - FRAME_TYPE_STOP_SENDING, - FRAME_TYPE_MAX_STREAM_DATA, - FRAME_TYPE_STREAM_DATA_BLOCKED, - FRAME_TYPE_STREAM_CLIENT_BIDI, + FrameType::ResetStream, + FrameType::StopSending, + FrameType::MaxStreamData, + FrameType::StreamDataBlocked, + FrameType::Stream, ] { // The slice contains an extra 0 that is only needed for a RESET_STREAM frame. // It's ignored for the other frame types as PADDING. - test_with_illegal_frame(&[frame_type, stream_id, 0, 0]); + test_with_illegal_frame(&[frame_type.into(), stream_id, 0, 0]); } } } @@ -610,7 +607,12 @@ fn legal_out_of_order_frame_on_remote_initiated_closed_stream() { // Deliver an out-of-order `FRAME_TYPE_MAX_STREAM_DATA` on forgotten stream. let dgram = send_with_extra( &mut client, - Writer(vec![FRAME_TYPE_MAX_STREAM_DATA, stream_id.as_u64(), 0, 0]), + Writer(vec![ + u64::from(FrameType::MaxStreamData), + stream_id.as_u64(), + 0, + 0, + ]), now(), ); server.process_input(dgram, now()); @@ -667,7 +669,7 @@ fn simultaneous_stop_sending_and_reset() { #[test] /// Make a stream data or control frame arrive after the stream has been used and cleared. fn late_stream_related_frames() { - fn late_stream_related_frame(frame_type: u64) { + fn late_stream_related_frame(frame_type: FrameType) { let mut client = default_client(); let mut server = default_server(); connect(&mut client, &mut server); @@ -681,24 +683,24 @@ fn late_stream_related_frames() { // Make the server generate a packet containing the test frame. let before = server.stats().frame_tx; match frame_type { - FRAME_TYPE_RESET_STREAM => { + FrameType::ResetStream => { server.stream_reset_send(stream_id, 0).unwrap(); } - FRAME_TYPE_STOP_SENDING => { + FrameType::StopSending => { server.stream_stop_sending(stream_id, 0).unwrap(); } - FRAME_TYPE_STREAM_CLIENT_BIDI => { + FrameType::Stream => { server.stream_send(stream_id, &[0x00]).unwrap(); server.stream_close_send(stream_id).unwrap(); } - FRAME_TYPE_MAX_STREAM_DATA => { + FrameType::MaxStreamData => { server .streams .get_recv_stream_mut(stream_id) .unwrap() .set_stream_max_data(u32::MAX.into()); } - FRAME_TYPE_STREAM_DATA_BLOCKED => { + FrameType::StreamDataBlocked => { let internal_stream = server.streams.get_send_stream_mut(stream_id).unwrap(); if let SendStreamState::Ready { fc, .. } = internal_stream.state() { fc.blocked(); @@ -711,19 +713,19 @@ fn late_stream_related_frames() { let tester = server.process_output(now()).dgram(); let after = server.stats().frame_tx; match frame_type { - FRAME_TYPE_RESET_STREAM => { + FrameType::ResetStream => { assert_eq!(after.reset_stream, before.reset_stream + 1); } - FRAME_TYPE_STOP_SENDING => { + FrameType::StopSending => { assert_eq!(after.stop_sending, before.stop_sending + 1); } - FRAME_TYPE_STREAM_CLIENT_BIDI => { + FrameType::Stream => { assert_eq!(after.stream, before.stream + 1); } - FRAME_TYPE_MAX_STREAM_DATA => { + FrameType::MaxStreamData => { assert_eq!(after.max_stream_data, before.max_stream_data + 1); } - FRAME_TYPE_STREAM_DATA_BLOCKED => { + FrameType::StreamDataBlocked => { assert_eq!(after.stream_data_blocked, before.stream_data_blocked + 1); } _ => panic!("unexpected frame type"), @@ -740,11 +742,11 @@ fn late_stream_related_frames() { } for frame_type in [ - FRAME_TYPE_RESET_STREAM, - FRAME_TYPE_STOP_SENDING, - FRAME_TYPE_MAX_STREAM_DATA, - FRAME_TYPE_STREAM_DATA_BLOCKED, - FRAME_TYPE_STREAM_CLIENT_BIDI, + FrameType::ResetStream, + FrameType::StopSending, + FrameType::MaxStreamData, + FrameType::StreamDataBlocked, + FrameType::Stream, ] { late_stream_related_frame(frame_type); } diff --git a/neqo-transport/src/crypto.rs b/neqo-transport/src/crypto.rs index 361b3b7104..68c9ab3411 100644 --- a/neqo-transport/src/crypto.rs +++ b/neqo-transport/src/crypto.rs @@ -31,6 +31,7 @@ use neqo_crypto::{ use crate::{ cid::ConnectionIdRef, + frame::FrameType, packet::{PacketBuilder, PacketNumber}, recovery::RecoveryToken, recv_stream::RxStreamOrderer, @@ -1506,7 +1507,7 @@ impl CryptoStreams { Encoder::varint_len(u64::try_from(length).expect("usize fits in u64")) - 1; let length = min(data.len(), builder.remaining() - header_len); - builder.encode_varint(crate::frame::FRAME_TYPE_CRYPTO); + builder.encode_varint(FrameType::Crypto); builder.encode_varint(offset); builder.encode_vvec(&data[..length]); Some((offset, length)) diff --git a/neqo-transport/src/fc.rs b/neqo-transport/src/fc.rs index a0c0dcc389..86733072b4 100644 --- a/neqo-transport/src/fc.rs +++ b/neqo-transport/src/fc.rs @@ -15,11 +15,7 @@ use std::{ use neqo_common::{qtrace, Role}; use crate::{ - frame::{ - FRAME_TYPE_DATA_BLOCKED, FRAME_TYPE_MAX_DATA, FRAME_TYPE_MAX_STREAMS_BIDI, - FRAME_TYPE_MAX_STREAMS_UNIDI, FRAME_TYPE_MAX_STREAM_DATA, FRAME_TYPE_STREAMS_BLOCKED_BIDI, - FRAME_TYPE_STREAMS_BLOCKED_UNIDI, FRAME_TYPE_STREAM_DATA_BLOCKED, - }, + frame::FrameType, packet::PacketBuilder, recovery::{RecoveryToken, StreamRecoveryToken}, stats::FrameStats, @@ -132,7 +128,7 @@ impl SenderFlowControl<()> { stats: &mut FrameStats, ) { if let Some(limit) = self.blocked_needed() { - if builder.write_varint_frame(&[FRAME_TYPE_DATA_BLOCKED, limit]) { + if builder.write_varint_frame(&[FrameType::DataBlocked.into(), limit]) { stats.data_blocked += 1; tokens.push(RecoveryToken::Stream(StreamRecoveryToken::DataBlocked( limit, @@ -152,7 +148,7 @@ impl SenderFlowControl { ) { if let Some(limit) = self.blocked_needed() { if builder.write_varint_frame(&[ - FRAME_TYPE_STREAM_DATA_BLOCKED, + FrameType::StreamDataBlocked.into(), self.subject.as_u64(), limit, ]) { @@ -178,10 +174,10 @@ impl SenderFlowControl { ) { if let Some(limit) = self.blocked_needed() { let frame = match self.subject { - StreamType::BiDi => FRAME_TYPE_STREAMS_BLOCKED_BIDI, - StreamType::UniDi => FRAME_TYPE_STREAMS_BLOCKED_UNIDI, + StreamType::BiDi => FrameType::StreamsBlockedBiDi, + StreamType::UniDi => FrameType::StreamsBlockedUniDi, }; - if builder.write_varint_frame(&[frame, limit]) { + if builder.write_varint_frame(&[frame.into(), limit]) { stats.streams_blocked += 1; tokens.push(RecoveryToken::Stream(StreamRecoveryToken::StreamsBlocked { stream_type: self.subject, @@ -300,7 +296,7 @@ impl ReceiverFlowControl<()> { return; } let max_allowed = self.next_limit(); - if builder.write_varint_frame(&[FRAME_TYPE_MAX_DATA, max_allowed]) { + if builder.write_varint_frame(&[FrameType::MaxData.into(), max_allowed]) { stats.max_data += 1; tokens.push(RecoveryToken::Stream(StreamRecoveryToken::MaxData( max_allowed, @@ -349,7 +345,7 @@ impl ReceiverFlowControl { } let max_allowed = self.next_limit(); if builder.write_varint_frame(&[ - FRAME_TYPE_MAX_STREAM_DATA, + FrameType::MaxStreamData.into(), self.subject.as_u64(), max_allowed, ]) { @@ -403,10 +399,10 @@ impl ReceiverFlowControl { } let max_streams = self.next_limit(); let frame = match self.subject { - StreamType::BiDi => FRAME_TYPE_MAX_STREAMS_BIDI, - StreamType::UniDi => FRAME_TYPE_MAX_STREAMS_UNIDI, + StreamType::BiDi => FrameType::MaxStreamsBiDi, + StreamType::UniDi => FrameType::MaxStreamsUniDi, }; - if builder.write_varint_frame(&[frame, max_streams]) { + if builder.write_varint_frame(&[frame.into(), max_streams]) { stats.max_streams += 1; tokens.push(RecoveryToken::Stream(StreamRecoveryToken::MaxStreams { stream_type: self.subject, diff --git a/neqo-transport/src/frame.rs b/neqo-transport/src/frame.rs index a207459d75..9bcdf0d091 100644 --- a/neqo-transport/src/frame.rs +++ b/neqo-transport/src/frame.rs @@ -18,68 +18,153 @@ use crate::{ AppError, CloseReason, Error, Res, TransportError, }; -pub type FrameType = u64; - -pub const FRAME_TYPE_PADDING: FrameType = 0x0; -pub const FRAME_TYPE_PING: FrameType = 0x1; -pub const FRAME_TYPE_ACK: FrameType = 0x2; -pub const FRAME_TYPE_ACK_ECN: FrameType = 0x3; -pub const FRAME_TYPE_RESET_STREAM: FrameType = 0x4; -pub const FRAME_TYPE_STOP_SENDING: FrameType = 0x5; -pub const FRAME_TYPE_CRYPTO: FrameType = 0x6; -pub const FRAME_TYPE_NEW_TOKEN: FrameType = 0x7; -const FRAME_TYPE_STREAM: FrameType = 0x8; -#[cfg(test)] -pub const FRAME_TYPE_STREAM_CLIENT_BIDI: FrameType = FRAME_TYPE_STREAM; -const FRAME_TYPE_STREAM_MAX: FrameType = 0xf; -pub const FRAME_TYPE_MAX_DATA: FrameType = 0x10; -pub const FRAME_TYPE_MAX_STREAM_DATA: FrameType = 0x11; -pub const FRAME_TYPE_MAX_STREAMS_BIDI: FrameType = 0x12; -pub const FRAME_TYPE_MAX_STREAMS_UNIDI: FrameType = 0x13; -pub const FRAME_TYPE_DATA_BLOCKED: FrameType = 0x14; -pub const FRAME_TYPE_STREAM_DATA_BLOCKED: FrameType = 0x15; -pub const FRAME_TYPE_STREAMS_BLOCKED_BIDI: FrameType = 0x16; -pub const FRAME_TYPE_STREAMS_BLOCKED_UNIDI: FrameType = 0x17; -pub const FRAME_TYPE_NEW_CONNECTION_ID: FrameType = 0x18; -pub const FRAME_TYPE_RETIRE_CONNECTION_ID: FrameType = 0x19; -pub const FRAME_TYPE_PATH_CHALLENGE: FrameType = 0x1a; -pub const FRAME_TYPE_PATH_RESPONSE: FrameType = 0x1b; -pub const FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT: FrameType = 0x1c; -pub const FRAME_TYPE_CONNECTION_CLOSE_APPLICATION: FrameType = 0x1d; -pub const FRAME_TYPE_HANDSHAKE_DONE: FrameType = 0x1e; -// draft-ietf-quic-ack-delay -pub const FRAME_TYPE_ACK_FREQUENCY: FrameType = 0xaf; -// draft-ietf-quic-datagram -pub const FRAME_TYPE_DATAGRAM: FrameType = 0x30; -pub const FRAME_TYPE_DATAGRAM_WITH_LEN: FrameType = 0x31; -const DATAGRAM_FRAME_BIT_LEN: u64 = 0x01; - -const STREAM_FRAME_BIT_FIN: u64 = 0x01; -const STREAM_FRAME_BIT_LEN: u64 = 0x02; -const STREAM_FRAME_BIT_OFF: u64 = 0x04; +#[repr(u64)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FrameType { + Padding = 0x0, + Ping = 0x1, + Ack = 0x2, + AckEcn = 0x3, + ResetStream = 0x4, + StopSending = 0x5, + Crypto = 0x6, + NewToken = 0x7, + Stream = 0x08, // + 0b000 + StreamWithFin = 0x08 + 0b001, + StreamWithLen = 0x08 + 0b010, + StreamWithLenFin = 0x08 + 0b011, + StreamWithOff = 0x08 + 0b100, + StreamWithOffFin = 0x08 + 0b101, + StreamWithOffLen = 0x08 + 0b110, + StreamWithOffLenFin = 0x08 + 0b111, + MaxData = 0x10, + MaxStreamData = 0x11, + MaxStreamsBiDi = 0x12, + MaxStreamsUniDi = 0x13, + DataBlocked = 0x14, + StreamDataBlocked = 0x15, + StreamsBlockedBiDi = 0x16, + StreamsBlockedUniDi = 0x17, + NewConnectionId = 0x18, + RetireConnectionId = 0x19, + PathChallenge = 0x1a, + PathResponse = 0x1b, + ConnectionCloseTransport = 0x1c, + ConnectionCloseApplication = 0x1d, + HandshakeDone = 0x1e, + // draft-ietf-quic-ack-delay + AckFrequency = 0xaf, + // draft-ietf-quic-datagram + Datagram = 0x30, + DatagramWithLen = 0x31, +} -#[derive(PartialEq, Eq, Debug, PartialOrd, Ord, Clone, Copy)] -pub enum CloseError { - Transport(TransportError), - Application(AppError), +impl From for u64 { + fn from(val: FrameType) -> Self { + val as Self + } } -impl CloseError { - const fn frame_type_bit(self) -> u64 { - match self { - Self::Transport(_) => 0, - Self::Application(_) => 1, +impl From for u8 { + fn from(val: FrameType) -> Self { + val as Self + } +} + +impl TryFrom for FrameType { + type Error = Error; + + fn try_from(value: u64) -> Result { + match value { + 0x0 => Ok(Self::Padding), + 0x1 => Ok(Self::Ping), + 0x2 => Ok(Self::Ack), + 0x3 => Ok(Self::AckEcn), + 0x4 => Ok(Self::ResetStream), + 0x5 => Ok(Self::StopSending), + 0x6 => Ok(Self::Crypto), + 0x7 => Ok(Self::NewToken), + 0x8 => Ok(Self::Stream), + 0x9 => Ok(Self::StreamWithFin), + 0xa => Ok(Self::StreamWithLen), + 0xb => Ok(Self::StreamWithLenFin), + 0xc => Ok(Self::StreamWithOff), + 0xd => Ok(Self::StreamWithOffFin), + 0xe => Ok(Self::StreamWithOffLen), + 0xf => Ok(Self::StreamWithOffLenFin), + 0x10 => Ok(Self::MaxData), + 0x11 => Ok(Self::MaxStreamData), + 0x12 => Ok(Self::MaxStreamsBiDi), + 0x13 => Ok(Self::MaxStreamsUniDi), + 0x14 => Ok(Self::DataBlocked), + 0x15 => Ok(Self::StreamDataBlocked), + 0x16 => Ok(Self::StreamsBlockedBiDi), + 0x17 => Ok(Self::StreamsBlockedUniDi), + 0x18 => Ok(Self::NewConnectionId), + 0x19 => Ok(Self::RetireConnectionId), + 0x1a => Ok(Self::PathChallenge), + 0x1b => Ok(Self::PathResponse), + 0x1c => Ok(Self::ConnectionCloseTransport), + 0x1d => Ok(Self::ConnectionCloseApplication), + 0x1e => Ok(Self::HandshakeDone), + 0xaf => Ok(Self::AckFrequency), + 0x30 => Ok(Self::Datagram), + 0x31 => Ok(Self::DatagramWithLen), + _ => Err(Error::UnknownFrameType), } } +} - const fn from_type_bit(bit: u64, code: u64) -> Self { - if (bit & 0x01) == 0 { - Self::Transport(code) - } else { - Self::Application(code) +impl FrameType { + const fn is_stream_with_length(self) -> bool { + matches!( + self, + Self::StreamWithLen + | Self::StreamWithLenFin + | Self::StreamWithOffLen + | Self::StreamWithOffLenFin + ) + } + + const fn is_stream_with_offset(self) -> bool { + matches!( + self, + Self::StreamWithOff + | Self::StreamWithOffFin + | Self::StreamWithOffLen + | Self::StreamWithOffLenFin + ) + } + const fn is_stream_with_fin(self) -> bool { + matches!( + self, + Self::StreamWithFin + | Self::StreamWithLenFin + | Self::StreamWithOffFin + | Self::StreamWithOffLenFin + ) + } +} + +impl TryFrom for StreamType { + type Error = Error; + + fn try_from(value: FrameType) -> Result { + match value { + FrameType::MaxStreamsBiDi | FrameType::StreamsBlockedBiDi => Ok(Self::BiDi), + FrameType::MaxStreamsUniDi | FrameType::StreamsBlockedUniDi => Ok(Self::UniDi), + _ => Err(Error::FrameEncodingError), } } +} +#[derive(PartialEq, Eq, Debug, PartialOrd, Ord, Clone, Copy)] +pub enum CloseError { + Transport(TransportError), + Application(AppError), +} + +impl CloseError { #[must_use] pub const fn code(&self) -> u64 { match self { @@ -207,60 +292,45 @@ pub enum Frame<'a> { } impl<'a> Frame<'a> { - const fn get_stream_type_bit(stream_type: StreamType) -> u64 { - match stream_type { - StreamType::BiDi => 0, - StreamType::UniDi => 1, - } - } - - const fn stream_type_from_bit(bit: u64) -> StreamType { - if (bit & 0x01) == 0 { - StreamType::BiDi - } else { - StreamType::UniDi - } - } - #[must_use] pub const fn get_type(&self) -> FrameType { match self { - Self::Padding { .. } => FRAME_TYPE_PADDING, - Self::Ping => FRAME_TYPE_PING, - Self::Ack { .. } => FRAME_TYPE_ACK, - Self::ResetStream { .. } => FRAME_TYPE_RESET_STREAM, - Self::StopSending { .. } => FRAME_TYPE_STOP_SENDING, - Self::Crypto { .. } => FRAME_TYPE_CRYPTO, - Self::NewToken { .. } => FRAME_TYPE_NEW_TOKEN, + Self::Padding { .. } => FrameType::Padding, + Self::Ping => FrameType::Ping, + Self::Ack { .. } => FrameType::Ack, + Self::ResetStream { .. } => FrameType::ResetStream, + Self::StopSending { .. } => FrameType::StopSending, + Self::Crypto { .. } => FrameType::Crypto, + Self::NewToken { .. } => FrameType::NewToken, Self::Stream { fin, offset, fill, .. } => Self::stream_type(*fin, *offset > 0, *fill), - Self::MaxData { .. } => FRAME_TYPE_MAX_DATA, - Self::MaxStreamData { .. } => FRAME_TYPE_MAX_STREAM_DATA, - Self::MaxStreams { stream_type, .. } => { - FRAME_TYPE_MAX_STREAMS_BIDI + Self::get_stream_type_bit(*stream_type) - } - Self::DataBlocked { .. } => FRAME_TYPE_DATA_BLOCKED, - Self::StreamDataBlocked { .. } => FRAME_TYPE_STREAM_DATA_BLOCKED, - Self::StreamsBlocked { stream_type, .. } => { - FRAME_TYPE_STREAMS_BLOCKED_BIDI + Self::get_stream_type_bit(*stream_type) - } - Self::NewConnectionId { .. } => FRAME_TYPE_NEW_CONNECTION_ID, - Self::RetireConnectionId { .. } => FRAME_TYPE_RETIRE_CONNECTION_ID, - Self::PathChallenge { .. } => FRAME_TYPE_PATH_CHALLENGE, - Self::PathResponse { .. } => FRAME_TYPE_PATH_RESPONSE, - Self::ConnectionClose { error_code, .. } => { - FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT + error_code.frame_type_bit() - } - Self::HandshakeDone => FRAME_TYPE_HANDSHAKE_DONE, - Self::AckFrequency { .. } => FRAME_TYPE_ACK_FREQUENCY, - Self::Datagram { fill, .. } => { - if *fill { - FRAME_TYPE_DATAGRAM - } else { - FRAME_TYPE_DATAGRAM_WITH_LEN - } - } + Self::MaxData { .. } => FrameType::MaxData, + Self::MaxStreamData { .. } => FrameType::MaxStreamData, + Self::MaxStreams { stream_type, .. } => match stream_type { + StreamType::BiDi => FrameType::MaxStreamsBiDi, + StreamType::UniDi => FrameType::MaxStreamsUniDi, + }, + Self::DataBlocked { .. } => FrameType::DataBlocked, + Self::StreamDataBlocked { .. } => FrameType::StreamDataBlocked, + Self::StreamsBlocked { stream_type, .. } => match stream_type { + StreamType::BiDi => FrameType::StreamsBlockedBiDi, + StreamType::UniDi => FrameType::StreamsBlockedUniDi, + }, + Self::NewConnectionId { .. } => FrameType::NewConnectionId, + Self::RetireConnectionId { .. } => FrameType::RetireConnectionId, + Self::PathChallenge { .. } => FrameType::PathChallenge, + Self::PathResponse { .. } => FrameType::PathResponse, + Self::ConnectionClose { error_code, .. } => match error_code { + CloseError::Transport(_) => FrameType::ConnectionCloseTransport, + CloseError::Application(_) => FrameType::ConnectionCloseApplication, + }, + Self::HandshakeDone => FrameType::HandshakeDone, + Self::AckFrequency { .. } => FrameType::AckFrequency, + Self::Datagram { fill, .. } => match fill { + false => FrameType::Datagram, + true => FrameType::DatagramWithLen, + }, } } @@ -281,18 +351,17 @@ impl<'a> Frame<'a> { } #[must_use] - pub const fn stream_type(fin: bool, nonzero_offset: bool, fill: bool) -> u64 { - let mut t = FRAME_TYPE_STREAM; - if fin { - t |= STREAM_FRAME_BIT_FIN; + pub const fn stream_type(fin: bool, nonzero_offset: bool, fill: bool) -> FrameType { + match (nonzero_offset, fill, fin) { + (false, true, false) => FrameType::Stream, + (false, true, true) => FrameType::StreamWithFin, + (false, false, false) => FrameType::StreamWithLen, + (false, false, true) => FrameType::StreamWithLenFin, + (true, true, false) => FrameType::StreamWithOff, + (true, true, true) => FrameType::StreamWithOffFin, + (true, false, false) => FrameType::StreamWithOffLen, + (true, false, true) => FrameType::StreamWithOffLenFin, } - if nonzero_offset { - t |= STREAM_FRAME_BIT_OFF; - } - if !fill { - t |= STREAM_FRAME_BIT_LEN; - } - t } /// If the frame causes a recipient to generate an ACK within its @@ -475,11 +544,12 @@ impl<'a> Frame<'a> { return Err(Error::ProtocolViolation); } + let t = t.try_into()?; match t { - FRAME_TYPE_PADDING => { + FrameType::Padding => { let mut length: u16 = 1; while let Some(b) = dec.peek_byte() { - if u64::from(b) != FRAME_TYPE_PADDING { + if b != u8::from(FrameType::Padding) { break; } length += 1; @@ -487,8 +557,8 @@ impl<'a> Frame<'a> { } Ok(Self::Padding(length)) } - FRAME_TYPE_PING => Ok(Self::Ping), - FRAME_TYPE_RESET_STREAM => Ok(Self::ResetStream { + FrameType::Ping => Ok(Self::Ping), + FrameType::ResetStream => Ok(Self::ResetStream { stream_id: StreamId::from(dv(dec)?), application_error_code: dv(dec)?, final_size: match dec.decode_varint() { @@ -496,13 +566,13 @@ impl<'a> Frame<'a> { _ => return Err(Error::NoMoreData), }, }), - FRAME_TYPE_ACK => decode_ack(dec, false), - FRAME_TYPE_ACK_ECN => decode_ack(dec, true), - FRAME_TYPE_STOP_SENDING => Ok(Self::StopSending { + FrameType::Ack => decode_ack(dec, false), + FrameType::AckEcn => decode_ack(dec, true), + FrameType::StopSending => Ok(Self::StopSending { stream_id: StreamId::from(dv(dec)?), application_error_code: dv(dec)?, }), - FRAME_TYPE_CRYPTO => { + FrameType::Crypto => { let offset = dv(dec)?; let data = d(dec.decode_vvec())?; if offset + u64::try_from(data.len())? > ((1 << 62) - 1) { @@ -510,21 +580,28 @@ impl<'a> Frame<'a> { } Ok(Self::Crypto { offset, data }) } - FRAME_TYPE_NEW_TOKEN => { + FrameType::NewToken => { let token = d(dec.decode_vvec())?; if token.is_empty() { return Err(Error::FrameEncodingError); } Ok(Self::NewToken { token }) } - FRAME_TYPE_STREAM..=FRAME_TYPE_STREAM_MAX => { + FrameType::Stream + | FrameType::StreamWithFin + | FrameType::StreamWithLen + | FrameType::StreamWithLenFin + | FrameType::StreamWithOff + | FrameType::StreamWithOffFin + | FrameType::StreamWithOffLen + | FrameType::StreamWithOffLenFin => { let s = dv(dec)?; - let o = if t & STREAM_FRAME_BIT_OFF == 0 { - 0 - } else { + let o = if t.is_stream_with_offset() { dv(dec)? + } else { + 0 }; - let fill = (t & STREAM_FRAME_BIT_LEN) == 0; + let fill = !t.is_stream_with_length(); let data = if fill { qtrace!("STREAM frame, extends to the end of the packet"); dec.decode_remainder() @@ -536,49 +613,49 @@ impl<'a> Frame<'a> { return Err(Error::FrameEncodingError); } Ok(Self::Stream { - fin: (t & STREAM_FRAME_BIT_FIN) != 0, + fin: t.is_stream_with_fin(), stream_id: StreamId::from(s), offset: o, data, fill, }) } - FRAME_TYPE_MAX_DATA => Ok(Self::MaxData { + FrameType::MaxData => Ok(Self::MaxData { maximum_data: dv(dec)?, }), - FRAME_TYPE_MAX_STREAM_DATA => Ok(Self::MaxStreamData { + FrameType::MaxStreamData => Ok(Self::MaxStreamData { stream_id: StreamId::from(dv(dec)?), maximum_stream_data: dv(dec)?, }), - FRAME_TYPE_MAX_STREAMS_BIDI | FRAME_TYPE_MAX_STREAMS_UNIDI => { + FrameType::MaxStreamsBiDi | FrameType::MaxStreamsUniDi => { let m = dv(dec)?; if m > (1 << 60) { return Err(Error::StreamLimitError); } Ok(Self::MaxStreams { - stream_type: Self::stream_type_from_bit(t), + stream_type: t.try_into()?, maximum_streams: m, }) } - FRAME_TYPE_DATA_BLOCKED => Ok(Self::DataBlocked { + FrameType::DataBlocked => Ok(Self::DataBlocked { data_limit: dv(dec)?, }), - FRAME_TYPE_STREAM_DATA_BLOCKED => Ok(Self::StreamDataBlocked { + FrameType::StreamDataBlocked => Ok(Self::StreamDataBlocked { stream_id: dv(dec)?.into(), stream_data_limit: dv(dec)?, }), - FRAME_TYPE_STREAMS_BLOCKED_BIDI | FRAME_TYPE_STREAMS_BLOCKED_UNIDI => { + FrameType::StreamsBlockedBiDi | FrameType::StreamsBlockedUniDi => { Ok(Self::StreamsBlocked { - stream_type: Self::stream_type_from_bit(t), + stream_type: t.try_into()?, stream_limit: dv(dec)?, }) } - FRAME_TYPE_NEW_CONNECTION_ID => { + FrameType::NewConnectionId => { let sequence_number = dv(dec)?; let retire_prior = dv(dec)?; let connection_id = d(dec.decode_vec(1))?; if connection_id.len() > MAX_CONNECTION_ID_LEN { - return Err(Error::DecodingFrame); + return Err(Error::FrameEncodingError); } let srt = d(dec.decode(16))?; let stateless_reset_token = <&[_; 16]>::try_from(srt)?; @@ -590,27 +667,26 @@ impl<'a> Frame<'a> { stateless_reset_token, }) } - FRAME_TYPE_RETIRE_CONNECTION_ID => Ok(Self::RetireConnectionId { + FrameType::RetireConnectionId => Ok(Self::RetireConnectionId { sequence_number: dv(dec)?, }), - FRAME_TYPE_PATH_CHALLENGE => { + FrameType::PathChallenge => { let data = d(dec.decode(8))?; let mut datav: [u8; 8] = [0; 8]; datav.copy_from_slice(data); Ok(Self::PathChallenge { data: datav }) } - FRAME_TYPE_PATH_RESPONSE => { + FrameType::PathResponse => { let data = d(dec.decode(8))?; let mut datav: [u8; 8] = [0; 8]; datav.copy_from_slice(data); Ok(Self::PathResponse { data: datav }) } - FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT | FRAME_TYPE_CONNECTION_CLOSE_APPLICATION => { - let error_code = CloseError::from_type_bit(t, dv(dec)?); - let frame_type = if t == FRAME_TYPE_CONNECTION_CLOSE_TRANSPORT { - dv(dec)? + FrameType::ConnectionCloseTransport | FrameType::ConnectionCloseApplication => { + let (error_code, frame_type) = if t == FrameType::ConnectionCloseTransport { + (CloseError::Transport(dv(dec)?), dv(dec)?) } else { - 0 + (CloseError::Application(dv(dec)?), 0) }; // We can tolerate this copy for now. let reason_phrase = String::from_utf8_lossy(d(dec.decode_vvec())?).to_string(); @@ -620,8 +696,8 @@ impl<'a> Frame<'a> { reason_phrase, }) } - FRAME_TYPE_HANDSHAKE_DONE => Ok(Self::HandshakeDone), - FRAME_TYPE_ACK_FREQUENCY => { + FrameType::HandshakeDone => Ok(Self::HandshakeDone), + FrameType::AckFrequency => { let seqno = dv(dec)?; let tolerance = dv(dec)?; if tolerance == 0 { @@ -640,8 +716,8 @@ impl<'a> Frame<'a> { ignore_order, }) } - FRAME_TYPE_DATAGRAM | FRAME_TYPE_DATAGRAM_WITH_LEN => { - let fill = (t & DATAGRAM_FRAME_BIT_LEN) == 0; + FrameType::Datagram | FrameType::DatagramWithLen => { + let fill = t == FrameType::Datagram; let data = if fill { qtrace!("DATAGRAM frame, extends to the end of the packet"); dec.decode_remainder() @@ -651,7 +727,6 @@ impl<'a> Frame<'a> { }; Ok(Self::Datagram { data, fill }) } - _ => Err(Error::UnknownFrameType), } } } @@ -663,7 +738,7 @@ mod tests { use crate::{ cid::MAX_CONNECTION_ID_LEN, ecn::Count, - frame::{AckRange, Frame, FRAME_TYPE_ACK}, + frame::{AckRange, Frame, FrameType}, CloseError, Error, StreamId, StreamType, }; @@ -892,7 +967,7 @@ mod tests { enc.encode(&[0x11; 16][..]); assert_eq!( Frame::decode(&mut enc.as_decoder()).unwrap_err(), - Error::DecodingFrame + Error::FrameEncodingError ); } @@ -1028,7 +1103,7 @@ mod tests { fn frame_decode_enforces_bound_on_ack_range() { let mut e = Encoder::new(); - e.encode_varint(FRAME_TYPE_ACK); + e.encode_varint(FrameType::Ack); e.encode_varint(0u64); // largest acknowledged e.encode_varint(0u64); // ACK delay e.encode_varint(u32::MAX); // ACK range count = huge, but maybe available for allocation diff --git a/neqo-transport/src/lib.rs b/neqo-transport/src/lib.rs index a5f3e8ac7f..0ba8d77bcc 100644 --- a/neqo-transport/src/lib.rs +++ b/neqo-transport/src/lib.rs @@ -107,7 +107,6 @@ pub enum Error { ConnectionIdLimitExceeded, ConnectionIdsExhausted, ConnectionState, - DecodingFrame, DecryptError, DisabledVersion, IdleTimeout, diff --git a/neqo-transport/src/packet/mod.rs b/neqo-transport/src/packet/mod.rs index 0e0e617b6d..ba9016c7bf 100644 --- a/neqo-transport/src/packet/mod.rs +++ b/neqo-transport/src/packet/mod.rs @@ -20,7 +20,7 @@ use neqo_crypto::random; use crate::{ cid::{ConnectionId, ConnectionIdDecoder, ConnectionIdRef, MAX_CONNECTION_ID_LEN}, crypto::{CryptoDxState, CryptoStates, Epoch}, - frame::FRAME_TYPE_PADDING, + frame::FrameType, recovery::SendProfile, version::{Version, WireVersion}, Error, Pmtud, Res, @@ -296,12 +296,7 @@ impl PacketBuilder { /// Cannot happen. pub fn pad(&mut self) -> bool { if self.padding && !self.is_long() { - self.encoder.pad_to( - self.limit, - FRAME_TYPE_PADDING - .try_into() - .expect("FRAME_TYPE_PADDING < 256"), - ); + self.encoder.pad_to(self.limit, FrameType::Padding.into()); true } else { false diff --git a/neqo-transport/src/path.rs b/neqo-transport/src/path.rs index 3c27cd174c..049a17dc4a 100644 --- a/neqo-transport/src/path.rs +++ b/neqo-transport/src/path.rs @@ -22,7 +22,7 @@ use crate::{ cc::CongestionControlAlgorithm, cid::{ConnectionId, ConnectionIdRef, ConnectionIdStore, RemoteConnectionIdEntry}, ecn, - frame::{FRAME_TYPE_PATH_CHALLENGE, FRAME_TYPE_PATH_RESPONSE, FRAME_TYPE_RETIRE_CONNECTION_ID}, + frame::FrameType, packet::PacketBuilder, pmtud::Pmtud, recovery::{RecoveryToken, SentPacket}, @@ -381,7 +381,7 @@ impl Paths { self.to_retire.push(seqno); break; } - builder.encode_varint(FRAME_TYPE_RETIRE_CONNECTION_ID); + builder.encode_varint(FrameType::RetireConnectionId); builder.encode_varint(seqno); tokens.push(RecoveryToken::RetireConnectionId(seqno)); stats.retire_connection_id += 1; @@ -768,7 +768,7 @@ impl Path { // Send PATH_RESPONSE. let resp_sent = if let Some(challenge) = self.challenge.take() { qtrace!("[{self}] Responding to path challenge {}", hex(challenge)); - builder.encode_varint(FRAME_TYPE_PATH_RESPONSE); + builder.encode_varint(FrameType::PathResponse); builder.encode(&challenge[..]); // These frames are not retransmitted in the usual fashion. @@ -786,7 +786,7 @@ impl Path { if let ProbeState::ProbeNeeded { probe_count } = self.state { qtrace!("[{self}] Initiating path challenge {probe_count}"); let data = random::<8>(); - builder.encode_varint(FRAME_TYPE_PATH_CHALLENGE); + builder.encode_varint(FrameType::PathChallenge); builder.encode(&data); // As above, no recovery token. diff --git a/neqo-transport/src/pmtud.rs b/neqo-transport/src/pmtud.rs index 114af42deb..9bf8357fbd 100644 --- a/neqo-transport/src/pmtud.rs +++ b/neqo-transport/src/pmtud.rs @@ -13,7 +13,7 @@ use std::{ use neqo_common::{qdebug, qinfo}; use static_assertions::const_assert; -use crate::{frame::FRAME_TYPE_PING, packet::PacketBuilder, recovery::SentPacket, Stats}; +use crate::{frame::FrameType, packet::PacketBuilder, recovery::SentPacket, Stats}; // Values <= 1500 based on: A. Custura, G. Fairhurst and I. Learmonth, "Exploring Usable Path MTU in // the Internet," 2018 Network Traffic Measurement and Analysis Conference (TMA), Vienna, Austria, @@ -120,7 +120,7 @@ impl Pmtud { pub fn send_probe(&mut self, builder: &mut PacketBuilder, stats: &mut Stats) { // The packet may include ACK-eliciting data already, but rather than check for that, it // seems OK to burn one byte here to simply include a PING. - builder.encode_varint(FRAME_TYPE_PING); + builder.encode_varint(FrameType::Ping); stats.frame_tx.ping += 1; stats.pmtud_tx += 1; self.probe_count += 1; diff --git a/neqo-transport/src/qlog.rs b/neqo-transport/src/qlog.rs index 273b3bb8a6..c4d51b1d7f 100644 --- a/neqo-transport/src/qlog.rs +++ b/neqo-transport/src/qlog.rs @@ -560,7 +560,7 @@ impl From> for QuicFrame { Frame::HandshakeDone => Self::HandshakeDone, Frame::AckFrequency { .. } => Self::Unknown { frame_type_value: None, - raw_frame_type: frame.get_type(), + raw_frame_type: frame.get_type().into(), raw: None, }, Frame::Datagram { data, .. } => Self::Datagram { diff --git a/neqo-transport/src/quic_datagrams.rs b/neqo-transport/src/quic_datagrams.rs index e39e7db6e0..25503a27b8 100644 --- a/neqo-transport/src/quic_datagrams.rs +++ b/neqo-transport/src/quic_datagrams.rs @@ -11,11 +11,8 @@ use std::{cmp::min, collections::VecDeque}; use neqo_common::Encoder; use crate::{ - events::OutgoingDatagramOutcome, - frame::{FRAME_TYPE_DATAGRAM, FRAME_TYPE_DATAGRAM_WITH_LEN}, - packet::PacketBuilder, - recovery::RecoveryToken, - ConnectionEvents, Error, Res, Stats, + events::OutgoingDatagramOutcome, frame::FrameType, packet::PacketBuilder, + recovery::RecoveryToken, ConnectionEvents, Error, Res, Stats, }; pub const MAX_QUIC_DATAGRAM: u64 = 65535; @@ -115,10 +112,10 @@ impl QuicDatagrams { Encoder::varint_len(u64::try_from(len).expect("usize fits in u64")); // Include a length if there is space for another frame after this one. if builder.remaining() >= 1 + length_len + len + PacketBuilder::MINIMUM_FRAME_SIZE { - builder.encode_varint(FRAME_TYPE_DATAGRAM_WITH_LEN); + builder.encode_varint(FrameType::DatagramWithLen); builder.encode_vvec(dgram.as_ref()); } else { - builder.encode_varint(FRAME_TYPE_DATAGRAM); + builder.encode_varint(FrameType::Datagram); builder.encode(dgram.as_ref()); builder.mark_full(); } diff --git a/neqo-transport/src/recv_stream.rs b/neqo-transport/src/recv_stream.rs index a92992466e..9f79ba52f5 100644 --- a/neqo-transport/src/recv_stream.rs +++ b/neqo-transport/src/recv_stream.rs @@ -23,7 +23,7 @@ use smallvec::SmallVec; use crate::{ events::ConnectionEvents, fc::ReceiverFlowControl, - frame::FRAME_TYPE_STOP_SENDING, + frame::FrameType, packet::PacketBuilder, recovery::{RecoveryToken, StreamRecoveryToken}, send_stream::SendStreams, @@ -895,7 +895,7 @@ impl RecvStream { } => { if *frame_needed && builder.write_varint_frame(&[ - FRAME_TYPE_STOP_SENDING, + FrameType::StopSending.into(), self.stream_id.as_u64(), *err, ]) diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs index 142eeea3f4..0dbf4f3b1e 100644 --- a/neqo-transport/src/send_stream.rs +++ b/neqo-transport/src/send_stream.rs @@ -26,7 +26,7 @@ use smallvec::SmallVec; use crate::{ events::ConnectionEvents, fc::SenderFlowControl, - frame::{Frame, FRAME_TYPE_RESET_STREAM}, + frame::{Frame, FrameType}, packet::PacketBuilder, recovery::{RecoveryToken, StreamRecoveryToken}, stats::FrameStats, @@ -1062,7 +1062,7 @@ impl SendStream { return false; } if builder.write_varint_frame(&[ - FRAME_TYPE_RESET_STREAM, + FrameType::ResetStream.into(), self.stream_id.as_u64(), err, final_size, diff --git a/neqo-transport/src/tracking.rs b/neqo-transport/src/tracking.rs index e0dfa9af7a..6a36d8d192 100644 --- a/neqo-transport/src/tracking.rs +++ b/neqo-transport/src/tracking.rs @@ -19,7 +19,7 @@ use neqo_crypto::Epoch; use crate::{ ecn, - frame::{FRAME_TYPE_ACK, FRAME_TYPE_ACK_ECN}, + frame::FrameType, packet::{PacketBuilder, PacketNumber, PacketType}, recovery::RecoveryToken, stats::FrameStats, @@ -521,9 +521,9 @@ impl RecvdPackets { } builder.encode_varint(if self.ecn_count.is_some() { - FRAME_TYPE_ACK_ECN + FrameType::AckEcn } else { - FRAME_TYPE_ACK + FrameType::Ack }); let mut iter = ranges.iter(); let Some(first) = iter.next() else { return };