Merge branch 'main' into dependabot/github_actions/actions/checkout-4…

….2.0

Signed-off-by: Lars Eggert <[email protected]>

.github/actions/nss/action.yml

@@ -33,18 +33,18 @@ runs:
       run: |
         if ! command -v pkg-config &> /dev/null; then
           echo "pkg-config: not found"
-          echo "BUILD_NSS=1" >> "$GITHUB_ENV"
+          echo "USE_SYSTEM_NSS=0" >> "$GITHUB_ENV"
           exit 0
         fi
         if ! pkg-config --exists nss; then
           echo "pkg-config: NSS not found"
-          echo "BUILD_NSS=1" >> "$GITHUB_ENV"
+          echo "USE_SYSTEM_NSS=0" >> "$GITHUB_ENV"
           exit 0
         fi
         NSS_VERSION="$(pkg-config --modversion nss)"
         if [ "$?" -ne 0 ]; then
           echo "pkg-config: failed to determine NSS version"
-          echo "BUILD_NSS=1" >> "$GITHUB_ENV"
+          echo "USE_SYSTEM_NSS=0" >> "$GITHUB_ENV"
           exit 0
         fi
         NSS_MAJOR=$(echo "$NSS_VERSION" | cut -d. -f1)
@@ -53,11 +53,11 @@ runs:
         REQ_NSS_MINOR=$(echo "${{ inputs.minimum-version}}" | cut -d. -f2)
         if [[ "$NSS_MAJOR" -lt "$REQ_NSS_MAJOR" || "$NSS_MAJOR" -eq "$REQ_NSS_MAJOR" && "$NSS_MINOR" -lt "$REQ_NSS_MINOR" ]]; then
           echo "System NSS is too old: $NSS_VERSION"
-          echo "BUILD_NSS=1" >> "$GITHUB_ENV"
+          echo "USE_SYSTEM_NSS=0" >> "$GITHUB_ENV"
           exit 0
         fi
         echo "System NSS is suitable: $NSS_VERSION"
-        echo "BUILD_NSS=0" >> "$GITHUB_ENV"
+        echo "USE_SYSTEM_NSS=1" >> "$GITHUB_ENV"
 
     - name: Use sccache
       # Apparently the action can't be installed twice in the same workflow, so check if
@@ -66,11 +66,11 @@ runs:
       #
       # Also, only enable sscache on our self-hosted runner, because the GitHub cache limit
       # is too small for this to be effective there.
-      if: env.SCCACHE_ENABLED != '1' && env.BUILD_NSS == '1' && runner.environment != 'github-hosted'
+      if: env.SCCACHE_ENABLED != '1' && env.USE_SYSTEM_NSS == '0' && runner.environment != 'github-hosted'
       uses: mozilla-actions/sccache-action@2e7f9ec7921547d4b46598398ca573513895d0bd # v0.0.4
 
     - name: Enable sscache
-      if: env.BUILD_NSS == '1' && runner.environment != 'github-hosted'
+      if: env.USE_SYSTEM_NSS == '0' && runner.environment != 'github-hosted'
       shell: bash
       run: |
         echo "SCCACHE_ENABLED=1" >> "$GITHUB_ENV"
@@ -86,21 +86,21 @@ runs:
         fi
 
     - name: Checkout NSS
-      if: env.BUILD_NSS == '1'
+      if: env.USE_SYSTEM_NSS == '0'
       uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
       with:
         repository: nss-dev/nss
         path: nss
 
     - name: Checkout NSPR
-      if: env.BUILD_NSS == '1'
+      if: env.USE_SYSTEM_NSS == '0'
       uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
       with:
         repository: nss-dev/nspr
         path: nspr
 
     - name: Get head revisions
-      if: env.BUILD_NSS == '1'
+      if: env.USE_SYSTEM_NSS == '0'
       shell: bash
       run: |
         NSS_HEAD=$(git -C nss rev-parse HEAD)
@@ -110,21 +110,22 @@ runs:
 
     - name: Cache NSS
       id: cache
-      if: env.BUILD_NSS == '1' && runner.environment == 'github-hosted'
+      if: env.USE_SYSTEM_NSS == '0' && runner.environment == 'github-hosted'
       uses: actions/cache@0c45773b623bea8c8e75f6c82b208c3cf94ea4f9 # v4.0.2
       with:
         path: dist
         key: nss-${{ runner.os }}-${{ inputs.type }}-${{ env.NSS_HEAD }}-${{ env.NSPR_HEAD }}
 
     - name: Check if build is needed
-      if: env.BUILD_NSS == '1' && runner.environment == 'github-hosted'
+      if: env.USE_SYSTEM_NSS == '0'
       shell: bash
       run: |
-        if [ "${{ steps.cache.outputs.cache-hit }}" == "true" ]; then
+        if [ "${{ runner.environment }}" != "github-hosted" ] || [ "${{ steps.cache.outputs.cache-hit }}" == "false" ]; then
+          echo "Building NSS from source"
+          echo "BUILD_NSS=1" >> "$GITHUB_ENV"
+        else
           echo "Using cached prebuilt NSS"
           echo "BUILD_NSS=0" >> "$GITHUB_ENV"
-        else
-          echo "Building NSS from source"
         fi
 
     - name: Install build dependencies (Linux)
@@ -176,6 +177,7 @@ runs:
 
     - name: Set up environment
       shell: bash
+      if: env.USE_SYSTEM_NSS == '0'
       run: |
         NSS_TARGET="${{ inputs.type }}"
         echo "NSS_TARGET=$NSS_TARGET" >> "$GITHUB_ENV"
@@ -187,7 +189,6 @@ runs:
         echo "NSS_PREBUILT=1" >> "$GITHUB_ENV"
       env:
         NSS_DIR: ${{ github.workspace }}/nss
-        NSPR_DIR: ${{ github.workspace }}/nspr
 
     - name: Build
       shell: bash

.github/workflows/check.yml

@@ -8,6 +8,12 @@ on:
     paths-ignore: ["*.md", "*.png", "*.svg", "LICENSE-*"]
   merge_group:
   workflow_dispatch:
+    inputs:
+      run_benchmarks:
+        description: 'Run benchmarks'
+        type: boolean
+        required: false
+        default: false
 env:
   CARGO_TERM_COLOR: always
   RUST_BACKTRACE: 1
@@ -99,4 +105,8 @@ jobs:
 
   bench:
     needs: [check]
+    if: >
+      (github.event_name == 'workflow_dispatch' && github.event.inputs.run_benchmarks) ||
+      (github.event_name == 'pull_request' && !github.event.pull_request.draft) ||
+      (github.event_name != 'workflow_dispatch' && github.event_name != 'pull_request')
     uses: ./.github/workflows/bench.yml

.github/workflows/qns.yml

@@ -55,7 +55,7 @@ jobs:
             # set latest tag for default branch
             type=raw,value=latest,enable={{is_default_branch}}
 
-      - uses: docker/build-push-action@5cd11c3a4ced054e52742c5fd54dca954e0edd85 # v6.7.0
+      - uses: docker/build-push-action@32945a339266b759abcbdc89316275140b0fc960 # v6.8.0
         if: github.event_name != 'pull_request'
         with:
           push: true
@@ -66,7 +66,7 @@ jobs:
           cache-to: type=gha,mode=max
           platforms: 'linux/amd64, linux/arm64'
 
-      - uses: docker/build-push-action@5cd11c3a4ced054e52742c5fd54dca954e0edd85 # v6.7.0
+      - uses: docker/build-push-action@32945a339266b759abcbdc89316275140b0fc960 # v6.8.0
         id: docker_build_and_push
         with:
           tags: ${{ steps.meta.outputs.tags }}

neqo-http3/src/connection_client.rs

@@ -2620,7 +2620,7 @@ mod tests {
         force_idle(&mut client, &mut server);
 
         let idle_timeout = ConnectionParameters::default().get_idle_timeout();
-        assert_eq!(client.process_output(now()).callback(), idle_timeout);
+        assert_eq!(client.process_output(now()).callback(), idle_timeout / 2);
     }
 
     // Helper function: read response when a server sends HTTP_RESPONSE_2.
@@ -5114,7 +5114,7 @@ mod tests {
         assert!(!fin);
 
         force_idle(&mut client, &mut server);
-        assert_eq!(client.process_output(now()).callback(), idle_timeout);
+        assert_eq!(client.process_output(now()).callback(), idle_timeout / 2);
     }
 
     #[test]

neqo-transport/src/connection/idle.rs

@@ -96,6 +96,21 @@ impl IdleTimeout {
         self.start(now) + max(self.timeout / 2, pto)
     }
 
+    pub fn next_keep_alive(&self, now: Instant, pto: Duration) -> Option<Instant> {
+        if self.keep_alive_outstanding {
+            return None;
+        }
+
+        let timeout = self.keep_alive_timeout(now, pto);
+        // Timer is in the past, i.e. we should have sent a keep alive,
+        // but we were unable to, e.g. due to CC.
+        if timeout <= now {
+            return None;
+        }
+
+        Some(timeout)
+    }
+
     pub fn send_keep_alive(
         &mut self,
         now: Instant,

neqo-transport/src/connection/mod.rs

@@ -629,13 +629,17 @@ impl Connection {
             .unwrap()
     }
 
+    fn confirmed(&self) -> bool {
+        self.state == State::Confirmed
+    }
+
     /// Get the simplest PTO calculation for all those cases where we need
     /// a value of this approximate order.  Don't use this for loss recovery,
     /// only use it where a more precise value is not important.
     fn pto(&self) -> Duration {
         self.paths.primary().map_or_else(
-            || RttEstimate::default().pto(PacketNumberSpace::ApplicationData),
-            |p| p.borrow().rtt().pto(PacketNumberSpace::ApplicationData),
+            || RttEstimate::default().pto(self.confirmed()),
+            |p| p.borrow().rtt().pto(self.confirmed()),
         )
     }
 
@@ -1049,7 +1053,7 @@ impl Connection {
             return timeout.duration_since(now);
         }
 
-        let mut delays = SmallVec::<[_; 6]>::new();
+        let mut delays = SmallVec::<[_; 7]>::new();
         if let Some(ack_time) = self.acks.ack_time(now) {
             qtrace!([self], "Delayed ACK timer {:?}", ack_time);
             delays.push(ack_time);
@@ -1058,12 +1062,19 @@ impl Connection {
         if let Some(p) = self.paths.primary() {
             let path = p.borrow();
             let rtt = path.rtt();
-            let pto = rtt.pto(PacketNumberSpace::ApplicationData);
+            let pto = rtt.pto(self.confirmed());
 
             let idle_time = self.idle_timeout.expiry(now, pto);
-            qtrace!([self], "Idle/keepalive timer {:?}", idle_time);
+            qtrace!([self], "Idle timer {:?}", idle_time);
             delays.push(idle_time);
 
+            if self.streams.need_keep_alive() {
+                if let Some(keep_alive_time) = self.idle_timeout.next_keep_alive(now, pto) {
+                    qtrace!([self], "Keep alive timer {:?}", keep_alive_time);
+                    delays.push(keep_alive_time);
+                }
+            }
+
             if let Some(lr_time) = self.loss_recovery.next_timeout(&path) {
                 qtrace!([self], "Loss recovery timer {:?}", lr_time);
                 delays.push(lr_time);
@@ -1525,7 +1536,7 @@ impl Connection {
         let mut dcid = None;
 
         qtrace!([self], "{} input {}", path.borrow(), hex(&**d));
-        let pto = path.borrow().rtt().pto(PacketNumberSpace::ApplicationData);
+        let pto = path.borrow().rtt().pto(self.confirmed());
 
         // Handle each packet in the datagram.
         while !slc.is_empty() {
@@ -2141,7 +2152,7 @@ impl Connection {
             // or the PTO timer fired: probe.
             true
         } else {
-            let pto = path.borrow().rtt().pto(PacketNumberSpace::ApplicationData);
+            let pto = path.borrow().rtt().pto(self.confirmed());
             if !builder.packet_empty() {
                 // The packet only contains an ACK.  Check whether we want to
                 // force an ACK with a PING so we can stop tracking packets.