fix(proto): re-derive BBRv3's initial pacing rate from the first RTT … - #802
fix(proto): re-derive BBRv3's initial pacing rate from the first RTT …#802richinsley wants to merge 1 commit into
Conversation
…sample At construction no SRTT exists, so `Bbr3::new` computes the initial pacing rate as `startup_pacing_gain * InitialCwnd / 1 ms` (the draft's placeholder when SRTT is unknown): about 266 Mbps for a 12 kB window. Until startup exits, `BBRSetPacingRateWithGain` only ever raises the rate. A flow that stays application-limited never exits startup (its delivery-rate samples never show a plateau), so that placeholder stays for the life of the flow and the flow is effectively unpaced: every burst goes out at line rate. This shows up on any secondary multipath path that is opened after the handshake and fed a rate-limited source. Observed with a paced media source over two links: the second path reported a constant 266 Mbps pacing rate for entire sessions, and frame-sized bursts overflowed a shallow queue with 28% loss. Linux BBR handles this with `has_seen_rtt`: once the first RTT sample exists, the pacing rate is re-initialised from `InitialCwnd / SRTT` (`bbr_init_pacing_rate_from_rtt`) and grows from there. Do the same on the first acknowledgement that arrives with an RTT sample; the estimator gains `has_sample()` so the configured initial RTT is not mistaken for one. Startup's bandwidth-driven growth is unchanged. Test: after one packet acknowledged with a 40 ms RTT the pacing rate is `gain * 12000 / 0.040` (832 kB/s) instead of the 33 MB/s placeholder.
I highly doubt this is the case, given you took your agent's instructions and copied them into your PR description:
I'm tempted to just close this PR. In any case, the PR description needs to be fixed otherwise we can't accept this change. |
|
Sorry about that. The code was me and the review was me, but the PR text claude (I'm not particularly good at PR write ups). |
Description
At construction no SRTT exists, so
Bbr3::newcomputes the initial pacing rate asstartup_pacing_gain * InitialCwnd / 1 ms(~266 Mbps for a 12 kB window). Until startup exits,BBRSetPacingRateWithGainonly ever raises the rate. A flow that stays application-limited never exits startup — its delivery-rate samples never plateau — so that placeholder stays for the life of the flow and the flow is effectively unpaced: every burst leaves at line rate.This change re-derives the pacing rate from
InitialCwnd / SRTTon the first acknowledgement that arrives with an RTT sample, as Linux BBR does (has_seen_rtt/bbr_init_pacing_rate_from_rtt) and as the draft'sBBRInitPacingRateintends (InitialCwnd / (SRTT ? SRTT : 1ms), which is only ever evaluated once, before an SRTT exists). Startup's bandwidth-driven growth is unchanged; the rate simply starts from a value the path can carry.Bbr3::has_seen_rtt, and the re-initialisation inon_ack(withset_send_quantumso the quantum matches the corrected rate).RttEstimator::has_sample()so the configured initial RTT is not mistaken for a measurement.initial_pacing_rate_follows_first_rtt_sample: after one packet acknowledged with a 40 ms RTT,pacing_rateisgain * 12000 / 0.040(832 kB/s), not the 33 MB/s placeholder.Fixes #800.
API Changes
RttEstimator::has_sample(&self) -> bool— additive, on a public type.Notes & open questions
Change checklist
proposed change and wrote an as clear and concise description as
they could.
intented effect.
cargo makepasses locally.