Skip to content

node:quic: report 'acknowledged' after 'lost' when a late ACK covers the packet - #46056

Draft
robobun wants to merge 2 commits into
robobun/a366d935/quic-datagram-status-by-idfrom
robobun/6397a9c1/quic-datagram-ack-after-loss
Draft

robobun wants to merge 2 commits into
robobun/a366d935/quic-datagram-status-by-idfrom
robobun/6397a9c1/quic-datagram-ack-after-loss

Conversation

@robobun

@robobun robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #44680 and #46055 (the first commit).

Problem

  • ondatagramstatus reports only 'lost' for a datagram whose packet is declared lost and acknowledged later. Node v26.3.0 reports 'lost', then 'acknowledged'.
  • lsquic forgets the id when it reports the loss (send_ctl_handle_regular_lost_packet, patches/lsquic/node-quic-accessors.patch:1872). The late ACK finds no frame.

Fix

  • lsquic keeps (packet number, id) of the last 32 DATAGRAM frames that it reports lost and checks each ACK against them. A match calls the new on_datagram_late_ack hook: node:quic reports 'acknowledged'.
  • Correct because lsquic never reuses a packet number: an ACK proves that the peer has it.
  • Verified: test/js/node/quic/quic-datagram.test.ts (10 tests fail without the hook), 235 vendored test-quic-* files, the HTTP/3 suites.
  • Self-reviewed: 32 concerns raised, 30 addressed. Not done: issues for three follow-ups (a maintainer has to file them), a test of the 256-packet expiry (too slow).

Background

  • A packet declared lost can still arrive. ngtcp2 keeps it for 3 probe timeouts and calls ack_datagram on its ACK.
  • Considered ids in lsquic's loss record: a late ACK would write congestion state. The list only reads ACK ranges.

Downsides

  • One id can get two callbacks and count in both datagramsAcknowledged and datagramsLost, as in Node.
  • Not as Node: lsquic's timers report 'lost' with no ACK. After a stall Bun reports 'lost', then 'acknowledged', Node only 'acknowledged'. A Bun server can drop a late packet, so two Bun endpoints can still end at 'lost'.
  • Each ACK on any QUIC connection costs 3 instructions (486 to 489). With quic: keep every acknowledged packet when lsquic merges two ACK frames #46055, release .text grows 672 bytes.
Notes

One script on both runtimes. A UDP relay holds the packet of datagram 4 of 10 and delivers it when the client reports a status for datagram 4. Each row is 3 of 3 runs, release builds, Linux x64. Node.js is v26.3.0 built from source with --experimental-quic.

statuses of datagram 4 sent / acknowledged / lost
Node.js v26.3.0 lost, acknowledged 10 / 10 / 1
Bun, #44680 lost 10 / 9 / 1
Bun, this PR lost, acknowledged 10 / 10 / 1
late-datagram.mjs (run it from the root of the repository)
// A UDP relay between a node:quic client and server holds the packet of datagram 4 of 10.
// It delivers that packet as soon as the client reports a status for datagram 4.
//   bun late-datagram.mjs
//   node --experimental-quic late-datagram.mjs
import { createPrivateKey } from "node:crypto";
import { createSocket } from "node:dgram";
import { once } from "node:events";
import { readFileSync } from "node:fs";
import { connect, listen } from "node:quic";
const keys = "test/js/node/test/fixtures/keys/";
const key = createPrivateKey(readFileSync(keys + "agent1-key.pem"));
const cert = readFileSync(keys + "agent1-cert.pem");
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));

const received = [];
const server = await listen(
  session => {
    session.onerror = () => {};
    session.closed.catch(() => {});
    session.onstream = async stream => {
      stream.closed.catch(() => {});
      try {
        for await (const _ of stream);
      } catch {}
    };
  },
  {
    alpn: ["sl"],
    sni: { "*": { keys: [key], certs: [cert] } },
    ondatagram: datagram => void received.push(datagram[0]),
  },
);
const front = createSocket("udp4");
const up = createSocket("udp4");
let clientPort = 0;
let hold = false;
let held;
up.on("message", packet => front.send(packet, clientPort, "127.0.0.1"));
front.on("message", (packet, from) => {
  clientPort = from.port;
  // A short-header packet of 700 bytes or more carries one of the datagrams.
  if (hold && !(packet[0] & 0x80) && packet.length >= 700) {
    hold = false;
    held = packet;
  } else up.send(packet, server.address.port, "127.0.0.1");
});
front.bind(0, "127.0.0.1");
up.bind(0, "127.0.0.1");
await Promise.all([once(front, "listening"), once(up, "listening")]);

const statuses = {};
const client = await connect(
  { address: "127.0.0.1", port: front.address().port },
  {
    alpn: "sl",
    servername: "localhost",
    verifyPeer: "manual",
    reuseEndpoint: false,
    onerror() {},
    ondatagramstatus(id, status) {
      (statuses[id] ??= []).push(status);
      if (Number(id) === 4 && held) {
        up.send(held, server.address.port, "127.0.0.1");
        held = undefined;
      }
    },
  },
);
client.closed.catch(() => {});
await client.opened;
const stream = await client.createBidirectionalStream();
stream.closed.catch(() => {});
for (let i = 1; i <= 10; i++) {
  hold = i === 4;
  const id = client.sendDatagram(Buffer.alloc(700, i));
  // Node sends a queued datagram together with other data, so each one goes with a byte of stream data.
  stream.writer.writeSync(new Uint8Array(1));
  await id;
  await sleep(10);
}
await sleep(500);
const { datagramsSent: sent, datagramsAcknowledged: acknowledged, datagramsLost: lost } = client.stats;
console.log(JSON.stringify({ serverReceived: received, statusesOf4: statuses[4], stats: `${sent}/${acknowledged}/${lost}` }));
process.exit(0);

Where the result is still 'lost' alone. The change is on the sender. It reports 'acknowledged' when the peer acknowledges the late packet, and a peer does not acknowledge every late packet.

  • A Bun receiver drops a late packet when the sender has acknowledged a newer ACK of the receiver. lsquic then takes the packet for a duplicate (lsquic_rechist_received below the cutoff that process_ack moves). This needs no traffic from the receiver: a receiver that sends only ACKs adds a PING every 12 to 27 packets, and the ACK of that PING moves the cutoff. With a server that writes stream data every 2 ms, two Bun endpoints give lost and 10 / 9 / 1 before and after this PR, and the server application never gets datagram 4. A Node.js client against a Bun server gives the same.
  • ngtcp2 does not acknowledge a late packet once the sender has acknowledged a newer ACK frame. With a Bun client and a Node.js server in two processes, the result is lost, acknowledged in 9 of 9 runs when the relay delivers the packet after the tenth datagram, and in 3 of 9 runs when it delivers the packet right at the loss. A Node.js client against that server ends with lost alone in 5 of 6 such runs.
  • lsquic ignores an ACK frame in a packet that is older than the last packet with an ACK frame (process_ack_frame, "Ignore old ack"). So a return path that reorders packets can hide the late ACK.

What Node.js does. ngtcp2 keeps the entry of a lost packet with its frames (rtb_on_pkt_lost). An ACK that covers the entry calls ack_datagram (process_acked_pkt). Node.js has no filter for each id (Session::DatagramStatus).

Bounds of the list. The list holds the last 32 frames. An ACK whose largest number is more than 256 above the packet removes the frame. Both numbers are choices of this PR. ngtcp2 keeps a lost packet for 3 probe timeouts (ngtcp2_conn_remove_lost_pkt), and at most the newest max(3, min(256, packets in flight / 2)) lost packets (rtb_detect_lost_pkt, trim). So Bun can report a late 'acknowledged' where Node.js has forgotten the packet: with 5 late packets, Node.js reports it for the newest 3 (13 / 11 / 5) and this PR for all 5 (13 / 13 / 5), 3 of 3 runs each. The list is longer than 3 because a retransmission timeout of lsquic reports every datagram in flight at once. The list exists only while the on_datagram_late_ack hook is set. Only the node:quic shim sets it, so Bun.serve HTTP/3 and fetch never allocate it.

Order. The late frames of an ACK are reported before the frames that the same ACK acknowledges for the first time, also when one of those is in an older packet. ngtcp2 reports in ascending packet order.

The timers. lsquic and ngtcp2 declare a loss by different rules. lsquic has five: packet threshold, early retransmit, sent time, tail loss probe and retransmission timeout. ngtcp2 has two: packet threshold and time threshold, both after an ACK of a later packet. The two timers of lsquic need no ACK. After a stall of 2 s with no packet lost:

statuses sent / acknowledged / lost
Node.js v26.3.0 acknowledged 13 / 13 / 0
Bun, #44680 lost 13 / 12 / 1
Bun, this PR lost, acknowledged 13 / 13 / 1

Each row is 3 of 3 runs. Two new tests pin the Bun row, one for each timer.

timer-lost.mjs (run it from the root of the repository)
// A FIFO relay between a node:quic client and server keeps every packet of both directions for two seconds
// and then forwards them in the order in which they arrived. No packet is lost or reordered.
// The client sends datagram 13, with one byte of stream data, right before the stall.
//   bun timer-lost.mjs
//   node --experimental-quic timer-lost.mjs
import { createPrivateKey } from "node:crypto";
import { createSocket } from "node:dgram";
import { once } from "node:events";
import { readFileSync } from "node:fs";
import { connect, listen } from "node:quic";
const keys = "test/js/node/test/fixtures/keys/";
const key = createPrivateKey(readFileSync(keys + "agent1-key.pem"));
const cert = readFileSync(keys + "agent1-cert.pem");
const sleep = ms => new Promise(resolve => setTimeout(resolve, ms));

const received = [];
const server = await listen(
  session => {
    session.onerror = () => {};
    session.closed.catch(() => {});
    session.onstream = async stream => {
      stream.closed.catch(() => {});
      try {
        for await (const _ of stream);
      } catch {}
    };
  },
  {
    alpn: ["sl"],
    sni: { "*": { keys: [key], certs: [cert] } },
    ondatagram: datagram => void received.push(datagram[0]),
  },
);
const front = createSocket("udp4");
const up = createSocket("udp4");
let clientPort = 0;
let stalled = false;
const kept = [];
const forward = (toServer, packet) =>
  toServer ? up.send(packet, server.address.port, "127.0.0.1") : front.send(packet, clientPort, "127.0.0.1");
up.on("message", packet => (stalled ? kept.push([false, packet]) : forward(false, packet)));
front.on("message", (packet, from) => {
  clientPort = from.port;
  stalled ? kept.push([true, packet]) : forward(true, packet);
});
front.bind(0, "127.0.0.1");
up.bind(0, "127.0.0.1");
await Promise.all([once(front, "listening"), once(up, "listening")]);

const statuses = {};
const client = await connect(
  { address: "127.0.0.1", port: front.address().port },
  {
    alpn: "sl",
    servername: "localhost",
    verifyPeer: "manual",
    reuseEndpoint: false,
    onerror() {},
    ondatagramstatus: (id, status) => void (statuses[id] ??= []).push(status),
  },
);
client.closed.catch(() => {});
await client.opened;
const stream = await client.createBidirectionalStream();
stream.closed.catch(() => {});
// Node sends a queued datagram together with other data, so each one goes with a byte of stream data.
const send = async i => {
  const id = client.sendDatagram(Buffer.alloc(700, i));
  stream.writer.writeSync(new Uint8Array(1));
  return await id;
};
// Warm-up: twelve round trips, so that the RTT estimate is a real one.
for (let i = 1; i <= 12; i++) {
  const id = await send(i);
  while (!statuses[id]) await sleep(1);
}
stalled = true;
const id = await send(13);
await sleep(2000);
const duringStall = (statuses[id] ?? []).slice();
stalled = false;
for (const [toServer, packet] of kept.splice(0)) forward(toServer, packet);
await sleep(800);
const { datagramsSent: sent, datagramsAcknowledged: acknowledged, datagramsLost: lost, pktLost } = client.stats;
console.log(
  JSON.stringify({
    serverReceived13: received.includes(13),
    duringStall,
    statusesOf13: statuses[id],
    stats: `${sent}/${acknowledged}/${lost}`,
    pktLost: `${pktLost}`,
  }),
);
process.exit(0);

Follow-ups, with no issue yet.

  1. A status for each DATAGRAM frame that follows the rules of ngtcp2. It needs a record for each sent frame beside the recovery of lsquic, and it replaces this list. A smaller step holds a timer-declared 'lost' until an ACK decides. To let the retransmission timeout skip a packet with only DATAGRAM frames does not work: with 10 such packets in flight and no ACK, lsquic's send controller then sends nothing after the timeout (12000 bytes in flight, window 2920, alarm not set). lsquic as it is sends 1 packet for each timeout.
  2. The receive side: lsquic drops a late packet that ngtcp2 delivers (first bullet above).
  3. 'abandoned' is not counted in datagramsLost. With maxPendingDatagrams = 2 and 3 sends, Node.js reads 0 / 0 / 1 and Bun 0 / 0 / 0. This PR does not touch it.

Measurements.
Release flags and ThinLTO. The base is #44680 at 0840e16. The numbers are from the code of this PR before the last change, which touched comments and tests only.

  • lsquic_send_ctl_got_ack, no lost datagram: 486 -> 489 instructions after quiescence, 492 -> 495 busy (gdb single-step of one call that acknowledges one packet in order). With the hook set and DATAGRAM packets: 528 -> 531 and 534 -> 537.
  • struct lsquic_send_ctl 1208 -> 1208 bytes: sc_retx_frames moves into a 4-byte hole, and the new pointer takes its place and the hole behind it. struct ietf_full_conn 10232 -> 10232. struct lsquic_stream_if 168 -> 176 (clang record layouts).
  • List allocations: 0 in the HTTP/3 suites (79 processes, 9,892 calls of lsquic_send_ctl_got_ack, the list check true 0 times), 0 in 1,000 lossless datagrams, 1 (one block of 520 bytes) in the reproduction (counters in a debug build).
  • ACK with 1 / 32 remembered ids: +74 / +849 instructions (16 ranges: +77 / +2,694). 300 packets after the losses: +3 (gdb).
  • Timer path: retx_alarm_rings 199 -> 199 static instructions. The timeout loop over 8 packets with a retransmittable frame and no DATAGRAM frame: 1,293 -> 1,293 executed (objdump, gdb).
  • Release binary, this PR together with quic: keep every acknowledged packet when lsquic merges two ACK frames #46055: .text +672 bytes, .rodata +64 (size -A. The base is a relink of the same objects with the lsquic and the shim of node:quic: report a datagram's status under the id kept in its lsquic frame #44680). lsquic_send_ctl_got_ack 0x110f -> 0x1130, send_ctl_handle_regular_lost_packet 0x5a1 -> 0x697, new send_ctl_ack_lost_datagrams 0x161 and nq_on_datagram_late_ack 0x1e (nm -S). The share of quic: keep every acknowledged packet when lsquic merges two ACK frames #46055 is 20 bytes (lsquic_merge_acks 0x1b7 -> 0x1cb).

Checks of the list on lsquic's send controller (C harness, ASAN, SC_SANITY_CHECK): 11 scenarios (loss by ACK evidence and by each timer, packets with and without stream data, several ids in one packet, both early exits of lsquic_send_ctl_got_ack, the cap, the reach). A model check with 200 seeds of 400 steps: 20,000 losses, 11,071 late ACKs, 1,928 ids dropped at the cap, 1,850 by the reach. No difference from the model.

Tests.

  • 10 tests reach the list. With the shim of node:quic: report a datagram's status under the id kept in its lsquic frame #44680 (no hook) all 10 fail on an expect() line, and the other 10 tests of the file pass.
  • What the new tests pin, checked with mutants of the patch on a debug build:
    • remembers the last 32 lost datagrams when a late packet carried 33: without the drop of the oldest entry, ASAN reports a heap-buffer-overflow in send_ctl_keep_lost_datagram.
    • remembers a lost datagram while ACKs that do not cover its packet arrive: fails when such an ACK removes the entry.
    • reports the late acknowledgement once: fails when a reported entry stays in the list.
    • reports a late acknowledgement when no other packet is in flight: fails when the check runs after the exit for an empty queue.
  • Not covered by a test in tree: the removal of an entry 256 packets after the loss. A test for it needs 270 packets and takes 2 to 5 s on a debug build. The model check above covers it.
  • The late-ACK tests need quic: keep every acknowledged packet when lsquic merges two ACK frames #46055. Without the merge fix, arrives late failed in 4 of 6 debug runs.
  • In these tests the relay keeps the ACKs of the client from the server, so that the Bun server does not drop the late packet.
  • The file passed 50 of 50 runs on an idle machine and 20 of 20 runs under 24 busy loops on 12 cores (debug, ASAN, long timeout). Under that load the slowest new test took 2.3 s.

Suites.
Debug (ASAN) build of the final commit, Linux x64.


[policy-decision:dep] gate passed · iteration 0 · 7 files touched

fails on main (without fix)
ASAN without fix: 10 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/quic/quic-datagram.test.ts test/js/node/quic/quic-endpoint.test.ts
bun test v1.4.4 (367d939d9)

test/js/node/quic/quic-endpoint.test.ts:
(node:24351) ExperimentalWarning: quic is an experimental feature and might change at any time
(Use `bun-debug --trace-warnings ...` to show where the warning was created)
(pass) QuicEndpoint client-engine mode > an explicit endpoint rejects a connect() in the other mode [600.38ms]
(pass) endpoint blockList > applies to packets forwarded from a sibling endpoint on the same loop [380.06ms]
(pass) server ALPN list > rejects a list mixing HTTP/3 and non-HTTP/3 protocols [118.55ms]
(pass) node:quic under --isolate > a second test file in the same process gets its own callbacks [5115.02ms]
(pass) dual-mode endpoint > does not stateless-reset its own client connection [235.95ms]
(pass) transportParams.maxIdleTimeout > 0 disables the idle timeout instead of falling back to the default [129.06ms]
(pass) endpoint.close() while a session is live > stops accepting new sessions so closed can resolve [
... (truncated)

release without fix: 19 FAILED
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/quic/quic-endpoint.test.ts:
(node:24402) ExperimentalWarning: quic is an experimental feature and might change at any time
(Use `bun --trace-warnings ...` to show where the warning was created)
(pass) QuicEndpoint client-engine mode > an explicit endpoint rejects a connect() in the other mode [199.33ms]
(pass) endpoint blockList > applies to packets forwarded from a sibling endpoint on the same loop [61.82ms]
(pass) server ALPN list > rejects a list mixing HTTP/3 and non-HTTP/3 protocols [4.71ms]
(pass) node:quic under --isolate > a second test file in the same process gets its own callbacks [144.85ms]
(pass) dual-mode endpoint > does not stateless-reset its own client connection [12.06ms]
(pass) transportParams.maxIdleTimeout > 0 disables the idle timeout instead of falling back to the default [13.00ms]
(pass) endpoint.close() while a session is live > stops accepting new sessions so closed can resolve [11.11ms]
460 |       sendBatch(upstream, serverPort, otherClientPackets.splice(0));
461 |       await until(() => statuses.length === sent);
462 |     }
463 | 
464 |     const { datagramsAcknowledged, datagramsLost 
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/quic/quic-datagram.test.ts test/js/node/quic/quic-endpoint.test.ts
rustup spent 8s installing the pinned toolchain (nightly-2026-09-15); it was missing or incomplete on this machine
bun test v1.4.4 (367d939d9)

test/js/node/quic/quic-endpoint.test.ts:
(node:24747) ExperimentalWarning: quic is an experimental feature and might change at any time
(Use `bun-debug --trace-warnings ...` to show where the warning was created)
(pass) QuicEndpoint client-engine mode > an explicit endpoint rejects a connect() in the other mode [349.02ms]
(pass) endpoint blockList > applies to packets forwarded from a sibling endpoint on the same loop [322.43ms]
(pass) server ALPN list > rejects a list mixing HTTP/3 and non-HTTP/3 protocols [79.21ms]
(pass) node:quic under --isolate > a second test file in the same process gets its own callbacks [3630.38ms]
(pass) dual-mode endpoint > does not stateless-reset its own client connection [171.33ms]
(pass) transportParams.maxIdleTimeout > 0 disables the idle timeout instead of falling back to the default
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 3370ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/8] cc obj/packages/bun-usockets/src/node_quic_shim.c.o
[2/8] rustc bun_lint_driver 
[3/8] rustc bun_runtime 
[4/8] link bun-profile
ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_lint_react-8f596b753a9d2596.rlib(bun_lint_react-8f596b753a9d2596.bun_lint_react.154138952866d2bb-cgu.0.rcgu.o at 568720)' is 'x86_64-unknown-linux-gnu'


ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_lint_typescript-7b8aee6562313049.rlib(bun_lint_typescript-7b8aee6562313049.bun_lint_typescript.920e32a118723f61-cgu.0.rcgu.o at 525508)' is 'x86_64-unknown-linux-gnu'


ld.lld: warning: Linking two modules of different target triples: 'obj/src/simdutf_sys/bun-simdutf.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/li
... (truncated)
diff hotspot
docs/runtime/nodejs-compat.mdx                  |   2 +-
 packages/bun-usockets/src/node_quic_shim.c      |  11 +
 patches/lsquic/datagram-ack-after-loss.patch    | 219 +++++++++++++
 patches/lsquic/merge-acks-contained-range.patch |  16 +
 scripts/build/deps/lsquic.ts                    |  14 +
 test/js/node/quic/quic-datagram.test.ts         | 389 +++++++++++++++++++++---
 test/js/node/quic/quic-endpoint.test.ts         | 161 +++++++++-
 7 files changed, 775 insertions(+), 37 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                             reads  edits  tests
docs/runtime/nodejs-compat.mdx                       1      1     96
packages/bun-usockets/src/node_quic_shim.c           1      1     98
patches/lsquic/datagram-ack-after-loss.patch         0      0     96
patches/lsquic/merge-acks-contained-range.patch      0      0     96
scripts/build/deps/lsquic.ts                         1      1     99
test/js/node/quic/quic-datagram.test.ts              4      1     55
test/js/node/quic/quic-endpoint.test.ts              0      0     45

@robobun

robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: draft, stacked on #44680 and #46055.

Reproduced on the branch of #44680 with one script on Bun and on Node.js v26.3.0 (built with QUIC). A UDP relay holds the packet of datagram 4 of 10 and delivers it when the client reports a status for that datagram. Bun prints ["lost"] and 10 / 9 / 1 (sent / acknowledged / lost). Node.js prints ["lost","acknowledged"] and 10 / 10 / 1. With this PR Bun prints the same as Node.js. The script is in the Notes of the PR body.

CI does not build the current commit: the branch of #44680 is behind the commit of main that CI requires. The previous commit passed CI (181 of 181 jobs). Since then only the test of #46055 in the first commit changed. I will rebase this PR when #44680 takes main or merges.

PR: #46056

@robobun

robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:36 AM PT - Oct 22nd, 2026

❌ @robobun, your commit 7c9dbbc has 1 failures in Build #130841 (All Failures):

  • step failed outside runner - exit 1 on ⚙️

🧪   To try this PR locally:

bunx bun-pr 46056

That installs a local version of the PR into your bun-46056 executable, so you can run:

bun-46056 --bun

lsquic_merge_acks() merges the ACK frames of one batch of packets into
one. A range of one frame that lay inside a range of the other cut that
range short: [16-8] merged with a saved [15-12][10-9][7-3] gave
[16-12][10-9][7-3]. The packets that the newer frame acknowledged in the
gaps of the saved one (a late or reordered packet) were not acknowledged,
and the send controller then declared them lost.

Only a lower end now extends the merged range.

The test hands a node:quic client two such ACK frames in one batch and
expects every datagram to be acknowledged.
…the packet

lsquic reported a DATAGRAM frame lost and then dropped its packet, or
cut the frame out of the packet that it sent again. When the peer
acknowledged the packet after all, the ACK found no frame to report, so
`ondatagramstatus` said 'lost' and never 'acknowledged', and
`stats.datagramsAcknowledged` did not count the datagram. Node reports
both for one id.

The send controller now keeps the (packet number, id) pairs of the last
32 DATAGRAM frames that it reported lost, and compares them with the
ranges of each ACK. A match goes to the new `on_datagram_late_ack` hook,
which the node:quic shim turns into an 'acknowledged' status. An entry
leaves the list when an ACK is 256 packets past it. lsquic keeps nothing
for an engine that does not set the hook.

lsquic declares losses by other rules than ngtcp2. Its retransmission
timers give up on a packet with no ACK of a later packet, where Node
reports 'acknowledged' alone. Bun reports 'lost', then 'acknowledged'
for such a packet. The comments, the docs and two tests say so.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant