Merge branch 'main' of https://github.com/mozilla/neqo into send-recv…

…-no-alloc

.github/actions/nss/action.yml

@@ -59,35 +59,33 @@ runs:
         echo "System NSS is suitable: $NSS_VERSION"
         echo "BUILD_NSS=0" >> "$GITHUB_ENV"
 
-    # Ideally, we'd use actions/checkout. But things are sufficiently flaky that we're better off
-    # trying both hg and git.
-
     - name: Checkout NSS
-      shell: bash
       if: env.BUILD_NSS == '1'
-      run: |
-        git clone --depth=1 https://github.com/nss-dev/nss "${{ github.workspace }}/nss" || \
-          hg clone https://hg.mozilla.org/projects/nss "${{ github.workspace }}/nss"
+      uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      with:
+        repository: nss-dev/nss
+        ref: c631e9cb5ed1806038f7b85fbbc825f856c9d133 # later revisions hang when building on Windows
+        path: nss
 
     - name: Checkout NSPR
-      shell: bash
       if: env.BUILD_NSS == '1'
-      run: |
-        git clone --depth=1 https://github.com/nss-dev/nspr "${{ github.workspace }}/nspr" || \
-          hg clone https://hg.mozilla.org/projects/nspr "${{ github.workspace }}/nspr"
+      uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
+      with:
+        repository: nss-dev/nspr
+        path: nspr
 
     - name: Install build dependencies (Linux)
       shell: bash
       if: runner.os == 'Linux' && env.BUILD_NSS == '1' && runner.environment == 'github-hosted'
       env:
         DEBIAN_FRONTEND: noninteractive
-      run: sudo apt-get install -y --no-install-recommends git mercurial gyp ninja-build
+      run: sudo apt-get install -y --no-install-recommends gyp ninja-build
 
     - name: Install build dependencies (MacOS)
       shell: bash
       if: runner.os == 'MacOS' && env.BUILD_NSS == '1'
       run: |
-        brew install mercurial ninja
+        brew install ninja
         echo "gyp-next>=0.18.1" > req.txt
         python3 -m pip install --user --break-system-packages -r req.txt
         echo "$(python3 -m site --user-base)/bin" >> "$GITHUB_PATH"
@@ -101,7 +99,7 @@ runs:
           echo C:/msys64/usr/bin
           echo C:/msys64/mingw64/bin
         } >> "$GITHUB_PATH"
-        /c/msys64/usr/bin/pacman -S --noconfirm python3-pip mercurial nsinstall
+        /c/msys64/usr/bin/pacman -S --noconfirm python3-pip nsinstall
         echo "gyp-next>=0.18.1" > req.txt
         python3 -m pip install -r req.txt
 

.github/actions/quic-interop-runner/action.yml

@@ -81,11 +81,16 @@ runs:
             FAILED=1
           fi
         done
-        # Remove all log files > $MAX_SIZE to make the artifacts smaller.
-        MAX_SIZE=5M
+        # Remove all log files > $MAX_SIZE for succeeded tests to make the artifacts smaller.
+        MAX_SIZE=2M
         echo "Removed log file > $MAX_SIZE during GitHub workflow" > note.txt
         echo "Removing these log files > $MAX_SIZE:"
-        find ../logs -type f -size +$MAX_SIZE -ls -exec cp note.txt {} \;
+        SUCCEEDED=$(jq < ../result.json '. | .results[][] | select(.result == "succeeded").name' | tr -d '"')
+        for run in ../logs/*; do
+          for test in $SUCCEEDED; do
+            find "$run/$test" -type f -size +$MAX_SIZE -ls -exec cp note.txt {} \;
+          done
+        done
         exit $FAILED
       shell: bash
 

.github/actions/rust/action.yml

@@ -14,6 +14,9 @@ inputs:
   token:
     description: 'A Github PAT'
     required: true
+  targets:
+    description: Comma-separated list of target triples to install for this toolchain
+    required: false
 
 runs:
   using: composite
@@ -28,6 +31,7 @@ runs:
       with:
         toolchain: ${{ inputs.version }}
         components: ${{ inputs.components }}
+        targets: ${{ inputs.targets }}
 
     - name: Set up MSVC (Windows)
       if: runner.os == 'Windows'

.github/workflows/bench.yml

@@ -10,7 +10,7 @@ env:
   CARGO_PROFILE_RELEASE_DEBUG: true
   CARGO_TERM_COLOR: always
   RUST_BACKTRACE: 1
-  TOOLCHAIN: nightly
+  TOOLCHAIN: nightly-2024-09-01
   RUSTFLAGS: -C link-arg=-fuse-ld=lld -C link-arg=-Wl,--no-rosegment, -C force-frame-pointers=yes
   PERF_OPT: record -F997 --call-graph fp -g
 

.github/workflows/check.yml

@@ -28,7 +28,7 @@ jobs:
         # Don't increase beyond what Firefox is currently using:
         # https://searchfox.org/mozilla-central/search?q=MINIMUM_RUST_VERSION&path=python/mozboot/mozboot/util.py
         # Keep in sync with Cargo.toml
-        rust-toolchain: [1.76.0, stable, nightly]
+        rust-toolchain: [1.76.0, stable, nightly-2024-09-01]
         type: [debug]
         include:
           - os: ubuntu-latest
@@ -92,7 +92,7 @@ jobs:
 
       - name: Check formatting
         run: |
-          if [ "${{ matrix.rust-toolchain }}" != "nightly" ]; then
+          if [ "${{ startsWith(matrix.rust-toolchain, 'nightly') && 'nightly' }}" != "nightly" ]; then
             CONFIG_PATH="--config-path=$(mktemp)"
           fi
           # shellcheck disable=SC2086
@@ -112,7 +112,7 @@ jobs:
           # workspace. See e.g. https://github.com/mozilla/neqo/pull/1695.
           cargo +${{ matrix.rust-toolchain }} hack clippy --all-targets --feature-powerset --exclude-features gecko -- -D warnings || ${{ matrix.rust-toolchain == 'nightly' }}
           # Check that the fuzz targets also build
-          if [ ${{ matrix.rust-toolchain }} == 'nightly' ]; then
+          if [ ${{ startsWith(matrix.rust-toolchain, 'nightly') && 'nightly' }} == 'nightly' ]; then
             cargo +${{ matrix.rust-toolchain }} fuzz check
           fi
         if: success() || failure()

.github/workflows/firefox.yml

@@ -63,6 +63,12 @@ jobs:
         if: runner.os == 'Windows'
         run: choco install -y mozillabuild
 
+      - name: Install Rust
+        uses: ./.github/actions/rust
+        with:
+          version: stable
+          token: ${{ secrets.GITHUB_TOKEN }}
+
       - name: Bootstrap Firefox
         run: |
           cd mozilla-unified

fuzz/Cargo.toml

@@ -22,7 +22,7 @@ neqo-transport = { path = "../neqo-transport" }
 test-fixture = { path = "../test-fixture" }
 
 [target.'cfg(not(windows))'.dependencies]
-libfuzzer-sys = { version = "0.4" }
+libfuzzer-sys = { version = "0.4", default-features = false }
 
 [lints]
 workspace = true

neqo-bin/Cargo.toml

@@ -44,7 +44,7 @@ tokio = { version = "1", default-features = false, features = ["net", "time", "m
 url = { version = "2.5", default-features = false }
 
 [dev-dependencies]
-criterion = { version = "0.5", default-features = false, features = ["html_reports", "async_tokio"] }
+criterion = { version = "0.5", default-features = false, features = ["async_tokio"] }
 tokio = { version = "1", default-features = false, features = ["sync"] }
 
 [features]

neqo-transport/Cargo.toml

@@ -27,7 +27,7 @@ smallvec = { version = "1.11", default-features = false }
 static_assertions = { version = "1.1", default-features = false }
 
 [dev-dependencies]
-criterion = { version = "0.5", default-features = false, features = ["html_reports"] }
+criterion = { version = "0.5", default-features = false }
 test-fixture = { path = "../test-fixture" }
 
 [features]

neqo-transport/src/connection/mod.rs

@@ -2200,7 +2200,6 @@ impl Connection {
             builder.encode_varint(crate::frame::FRAME_TYPE_PING);
             let stats = &mut self.stats.borrow_mut().frame_tx;
             stats.ping += 1;
-            stats.all += 1;
         }
         probe
     }
@@ -2287,13 +2286,11 @@ impl Connection {
         let stats = &mut self.stats.borrow_mut().frame_tx;
         let padded = if ack_eliciting && full_mtu && builder.pad() {
             stats.padding += 1;
-            stats.all += 1;
             true
         } else {
             false
         };
 
-        stats.all += tokens.len();
         (tokens, ack_eliciting, padded)
     }
 
@@ -2864,7 +2861,6 @@ impl Connection {
             qinfo!("frame not allowed: {:?} {:?}", frame, packet_type);
             return Err(Error::ProtocolViolation);
         }
-        self.stats.borrow_mut().frame_rx.all += 1;
         let space = PacketNumberSpace::from(packet_type);
         if frame.is_stream() {
             return self

neqo-transport/src/connection/params.rs

@@ -44,6 +44,7 @@ pub enum PreferredAddressConfig {
 /// `ConnectionParameters` use for setting intitial value for QUIC parameters.
 /// This collects configuration like initial limits, protocol version, and
 /// congestion control algorithm.
+#[allow(clippy::struct_excessive_bools)] // We need that many, sorry.
 #[derive(Debug, Clone)]
 pub struct ConnectionParameters {
     versions: VersionConfig,
@@ -78,6 +79,7 @@ pub struct ConnectionParameters {
     incoming_datagram_queue: usize,
     fast_pto: u8,
     grease: bool,
+    disable_migration: bool,
     pacing: bool,
     /// Whether the connection performs PLPMTUD.
     pmtud: bool,
@@ -102,6 +104,7 @@ impl Default for ConnectionParameters {
             incoming_datagram_queue: MAX_QUEUED_DATAGRAMS_DEFAULT,
             fast_pto: FAST_PTO_SCALE,
             grease: true,
+            disable_migration: false,
             pacing: true,
             pmtud: false,
         }
@@ -336,6 +339,12 @@ impl ConnectionParameters {
         self
     }
 
+    #[must_use]
+    pub const fn disable_migration(mut self, disable_migration: bool) -> Self {
+        self.disable_migration = disable_migration;
+        self
+    }
+
     #[must_use]
     pub const fn pacing_enabled(&self) -> bool {
         self.pacing
@@ -373,8 +382,12 @@ impl ConnectionParameters {
             tparams::ACTIVE_CONNECTION_ID_LIMIT,
             u64::try_from(LOCAL_ACTIVE_CID_LIMIT)?,
         );
-        tps.local.set_empty(tparams::DISABLE_MIGRATION);
-        tps.local.set_empty(tparams::GREASE_QUIC_BIT);
+        if self.disable_migration {
+            tps.local.set_empty(tparams::DISABLE_MIGRATION);
+        }
+        if self.grease {
+            tps.local.set_empty(tparams::GREASE_QUIC_BIT);
+        }
         tps.local.set_integer(
             tparams::MAX_ACK_DELAY,
             u64::try_from(DEFAULT_ACK_DELAY.as_millis())?,

neqo-transport/src/connection/tests/handshake.rs

@@ -36,7 +36,7 @@ use crate::{
     events::ConnectionEvent,
     server::ValidateAddress,
     stats::FrameStats,
-    tparams::{TransportParameter, MIN_ACK_DELAY},
+    tparams::{self, TransportParameter, MIN_ACK_DELAY},
     tracking::DEFAULT_ACK_DELAY,
     CloseReason, ConnectionParameters, EmptyConnectionIdGenerator, Error, Pmtud, StreamType,
     Version,
@@ -798,13 +798,13 @@ fn anti_amplification() {
     client.process_input(&s_init1, now);
     client.process_input(&s_init2, now);
     let ack_count = client.stats().frame_tx.ack;
-    let frame_count = client.stats().frame_tx.all;
+    let frame_count = client.stats().frame_tx.all();
     let ack = client.process_alloc(Some(&s_init3), now).dgram().unwrap();
     assert!(!maybe_authenticate(&mut client)); // No need yet.
 
     // The client sends a padded datagram, with just ACK for Handshake.
     assert_eq!(client.stats().frame_tx.ack, ack_count + 1);
-    assert_eq!(client.stats().frame_tx.all, frame_count + 1);
+    assert_eq!(client.stats().frame_tx.all(), frame_count + 1);
     assert_ne!(ack.len(), client.plpmtu()); // Not padded (it includes Handshake).
 
     now += DEFAULT_RTT / 2;
@@ -1210,7 +1210,6 @@ fn client_initial_retransmits_identical() {
             client.stats().frame_tx,
             FrameStats {
                 crypto: i,
-                all: i,
                 ..Default::default()
             }
         );
@@ -1241,7 +1240,6 @@ fn server_initial_retransmits_identical() {
             FrameStats {
                 crypto: i * 2,
                 ack: i,
-                all: i * 3,
                 ..Default::default()
             }
         );
@@ -1257,3 +1255,27 @@ fn server_initial_retransmits_identical() {
         total_ptos += pto;
     }
 }
+
+#[test]
+fn grease_quic_bit_transport_parameter() {
+    fn get_remote_tp(conn: &Connection) -> bool {
+        conn.tps
+            .borrow()
+            .remote
+            .as_ref()
+            .unwrap()
+            .get_empty(tparams::GREASE_QUIC_BIT)
+    }
+
+    for client_grease in [true, false] {
+        for server_grease in [true, false] {
+            let mut client = new_client(ConnectionParameters::default().grease(client_grease));
+            let mut server = new_server(ConnectionParameters::default().grease(server_grease));
+
+            connect(&mut client, &mut server);
+
+            assert_eq!(client_grease, get_remote_tp(&server));
+            assert_eq!(server_grease, get_remote_tp(&client));
+        }
+    }
+}

neqo-transport/src/connection/tests/migration.rs

@@ -191,11 +191,11 @@ fn migrate_immediate() {
     assert_v6_path(&server2, true);
 
     // The second packet has no real effect, it just elicits an ACK.
-    let all_before = server.stats().frame_tx.all;
+    let all_before = server.stats().frame_tx.all();
     let ack_before = server.stats().frame_tx.ack;
     let server3 = server.process_alloc(Some(&client2), now).dgram();
     assert!(server3.is_some());
-    assert_eq!(server.stats().frame_tx.all, all_before + 1);
+    assert_eq!(server.stats().frame_tx.all(), all_before + 1);
     assert_eq!(server.stats().frame_tx.ack, ack_before + 1);
 
     // Receiving a packet sent by the server before migration doesn't change path.
@@ -249,14 +249,14 @@ fn migrate_immediate_fail() {
         let after = client.stats().frame_tx;
         assert_eq!(after.path_challenge, before.path_challenge + 1);
         assert_eq!(after.padding, before.padding + 1);
-        assert_eq!(after.all, before.all + 2);
+        assert_eq!(after.all(), before.all() + 2);
 
         // This might be a PTO, which will result in sending a probe.
         if let Some(probe) = client.process_alloc(None, now).dgram() {
             assert_v4_path(&probe, false); // Contains PATH_CHALLENGE.
             let after = client.stats().frame_tx;
             assert_eq!(after.ping, before.ping + 1);
-            assert_eq!(after.all, before.all + 3);
+            assert_eq!(after.all(), before.all() + 3);
         }
     }
 
@@ -325,14 +325,14 @@ fn migrate_same_fail() {
         let after = client.stats().frame_tx;
         assert_eq!(after.path_challenge, before.path_challenge + 1);
         assert_eq!(after.padding, before.padding + 1);
-        assert_eq!(after.all, before.all + 2);
+        assert_eq!(after.all(), before.all() + 2);
 
         // This might be a PTO, which will result in sending a probe.
         if let Some(probe) = client.process_alloc(None, now).dgram() {
             assert_v6_path(&probe, false); // Contains PATH_CHALLENGE.
             let after = client.stats().frame_tx;
             assert_eq!(after.ping, before.ping + 1);
-            assert_eq!(after.all, before.all + 3);
+            assert_eq!(after.all(), before.all() + 3);
         }
     }
 

neqo-transport/src/connection/tests/mod.rs

@@ -28,6 +28,7 @@ use crate::{
     pmtud::Pmtud,
     recovery::ACK_ONLY_SIZE_LIMIT,
     stats::{FrameStats, Stats, MAX_PTO_COUNTS},
+    tparams::{DISABLE_MIGRATION, GREASE_QUIC_BIT},
     ConnectionIdDecoder, ConnectionIdGenerator, ConnectionParameters, Error, StreamId, StreamType,
     Version,
 };
@@ -681,3 +682,21 @@ fn create_server() {
     // Server won't have a default path, so no RTT.
     assert_eq!(stats.rtt, Duration::from_secs(0));
 }
+
+#[test]
+fn tp_grease() {
+    for enable in [true, false] {
+        let client = new_client(ConnectionParameters::default().grease(enable));
+        let grease = client.tps.borrow_mut().local.get_empty(GREASE_QUIC_BIT);
+        assert_eq!(enable, grease);
+    }
+}
+
+#[test]
+fn tp_disable_migration() {
+    for disable in [true, false] {
+        let client = new_client(ConnectionParameters::default().disable_migration(disable));
+        let disable_migration = client.tps.borrow_mut().local.get_empty(DISABLE_MIGRATION);
+        assert_eq!(disable, disable_migration);
+    }
+}