From 89429598bf897a34d61027cf7f68f5e35fcfff4b Mon Sep 17 00:00:00 2001 From: Toby Date: Fri, 6 Oct 2023 18:27:30 -0700 Subject: [PATCH] fix: BBR bandwidth estimation edge case --- core/internal/congestion/bbr/bbr_sender.go | 18 ++++++++++++++---- core/internal/congestion/common/pacer.go | 3 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/core/internal/congestion/bbr/bbr_sender.go b/core/internal/congestion/bbr/bbr_sender.go index 48e6e4b..9cf9cf6 100644 --- a/core/internal/congestion/bbr/bbr_sender.go +++ b/core/internal/congestion/bbr/bbr_sender.go @@ -21,6 +21,8 @@ import ( // const ( + minBps = 65536 // 64 kbps + invalidPacketNumber = -1 initialCongestionWindowPackets = 32 @@ -283,10 +285,7 @@ func newBbrSender( maxCongestionWindowWithNetworkParametersAdjusted: initialMaxCongestionWindow, maxDatagramSize: initialMaxDatagramSize, } - b.pacer = common.NewPacer(func() congestion.ByteCount { - // Pacer wants bytes per second, but Bandwidth is in bits per second. - return congestion.ByteCount(float64(b.bandwidthEstimate()) * b.congestionWindowGain / float64(BytesPerSecond)) - }) + b.pacer = common.NewPacer(b.bandwidthForPacer) /* if b.tracer != nil { @@ -536,6 +535,17 @@ func (b *bbrSender) bandwidthEstimate() Bandwidth { return b.maxBandwidth.GetBest() } +func (b *bbrSender) bandwidthForPacer() congestion.ByteCount { + bps := congestion.ByteCount(float64(b.bandwidthEstimate()) * b.congestionWindowGain / float64(BytesPerSecond)) + if bps < minBps { + // We need to make sure that the bandwidth value for pacer is never zero, + // otherwise it will go into an edge case where HasPacingBudget = false + // but TimeUntilSend is before, causing the quic-go send loop to go crazy and get stuck. + return minBps + } + return bps +} + // Returns the current estimate of the RTT of the connection. Outside of the // edge cases, this is minimum RTT. func (b *bbrSender) getMinRtt() time.Duration { diff --git a/core/internal/congestion/common/pacer.go b/core/internal/congestion/common/pacer.go index b2b301b..4e089a3 100644 --- a/core/internal/congestion/common/pacer.go +++ b/core/internal/congestion/common/pacer.go @@ -43,6 +43,9 @@ func (p *Pacer) Budget(now time.Time) congestion.ByteCount { return p.maxBurstSize() } budget := p.budgetAtLastSent + (p.getBandwidth()*congestion.ByteCount(now.Sub(p.lastSentTime).Nanoseconds()))/1e9 + if budget < 0 { // protect against overflows + budget = congestion.ByteCount(1<<62 - 1) + } return minByteCount(p.maxBurstSize(), budget) }