chore: Convert FrameType to an enum (#2438)

* chore: Convert `FrameType` to an enum * Update neqo-transport/src/ackrate.rs Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org> * Update neqo-transport/src/connection/tests/resumption.rs Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org> * Update neqo-transport/src/connection/tests/stream.rs Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org> * Update neqo-transport/src/frame.rs Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org> * Fix * `DecodingFrame` -> `FrameEncodingError` --------- Signed-off-by: Lars Eggert <lars@eggert.org> Co-authored-by: Martin Thomson <mt@lowentropy.net>
mozilla · Feb 12, 2025 · 966ece6 · 966ece6
1 parent d21c121
commit 966ece6
Show file tree

Hide file tree

Showing 24 changed files with 323 additions and 267 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<T>(&mut self, now: Instant, res: Res<T>) -> Option<T> {
-        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
@@ -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<RecoveryToken> {
         (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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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();