cloudflare / cloudflare/quiche

Empty non-FIN STREAM frames cause connection flow-control double-counting

Closed
#2,696 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
11.8k
Forks
1.1k
Avg merge
3d 7h
Merged PRs (30d)
16

Description

## Background

This was discovered during load/soak testing with 32 parallel `iperf3` streams transferring approximately 700 Mb/s over QUIC through Netty’s quiche integration, with a lossy WAN path between client and server.

The receiver occasionally closed the QUIC connection with `FLOW_CONTROL_ERROR`, typically within 60 minutes under this workload. Instrumentation added to a local quiche build showed `rx_data` exceeding the sum of the streams’ high-water marks by exact whole-frame multiples.

Affected revision tested: [0b1a89af](https://github.com/cloudflare/quiche/tree/0b1a89af341b7e42c4e75fedf7bdda246d8319af).

## Root cause

A zero-length non-`FIN` `STREAM` frame advances the receiver’s largest received offset for that stream (RFC 9000 Section 19.8) and must therefore be counted against connection-level flow control.

The receive path calculates:

```rust
data.max_off().saturating_sub(stream.recv.max_off())
```

before calling `RecvBuf::write()`. It subsequently adds that delta to `rx_data`.

However, `RecvBuf::write()` discards an empty non-`FIN` buffer without advancing the stream’s high-water mark. Consequently:

1. An empty frame advances from offset `X` to `X + N`.
2. The connection-level ledger charges `N` bytes.
3. `RecvBuf::write()` discards the frame while leaving the stream high-water mark at `X`.
4. When real data through `X + N` arrives, the receive path calculates and charges the same `N` bytes again.

This creates permanent connection-level ledger drift. Repetition eventually causes a legal `STREAM` frame to appear to exceed `MAX_DATA`, and the receiver closes the connection with `FLOW_CONTROL_ERROR`, even though the sender has not exceeded its granted credit.

## Why the sender emits these frames

`send_single()` can emit an empty non-`FIN` `STREAM` frame when the remaining packet capacity fits the encoded `STREAM` header exactly, leaving zero bytes for payload.

The stream-packing block is entered only when more than `MAX_STREAM_OVERHEAD` bytes remain. Therefore, this requires an actual header larger than that constant. For example, a stream ID of at least 64 and an offset of at least `2^30` produce a 13-byte header. This explains why the problem primarily appears late in long-running bulk transfers.

The bytes preceding the frame’s offset have already been accounted for in the sender’s `tx_data`. The empty frame is legal but redundant; it exposes the receiver accounting bug without adding payload or `FIN`.

## Minimal receiver reproduction

Given a connection-level receive limit of 30 bytes and a per-stream limit of 15 bytes:

1. Receive five bytes on stream 0 at offsets `0..5`.
2. Receive an empty non-`FIN` frame on stream 0 at offset 15.
3. Receive ten bytes on stream 0 at offsets `5..15`.
4. Receive fifteen bytes on stream 4 at offsets `0..15`.

The true connection-level total is exactly 30 bytes. On the affected code, offsets `5..15` on stream 0 are charged twice, and the final frame is rejected with `Error::FlowControl`.

## Proposed fixes

1. **Receiver (the accounting fix):** In the empty non-`FIN` early-return path in `RecvBuf::write()`, advance:

```rust
self.len = cmp::max(self.len, buf.max_off());
```

This keeps the stream high-water mark synchronized with the connection-level charge.

2. **Sender (defensive optimization):** Do not encode a `STREAM` frame when `emit()` returns `(0, false)`. The frame carries neither payload nor `FIN`. Treat this case as packet-size-limited rather than application-limited so it does not incorrectly mark BBR2 bandwidth samples as app-limited.

Contributor guide

Open the contributing guide

Research direction

Start at the empty non-FIN early-return path in RecvBuf::write() and the stream-packing block in send_single(), then trace how stream high-water marks and rx_data are updated. Reproduce the 30-byte flow-control sequence from the issue and verify that empty frames no longer cause double-counting, while zero-payload non-FIN frames are not emitted or treated as application-limited.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.