Fix BBR SendAllowance calculation overflow and unit conversion

[Copilot]

Problem

The BBR congestion control algorithm's BbrCongestionControlGetSendAllowance function had incorrect unit conversion that caused integer overflow at high bandwidth scenarios (6+ Gbps), severely limiting throughput.

Root Cause

The calculation was missing two critical unit conversions:

Before (broken):

BandwidthEst * PacingGain * TimeSinceLastSend / GAIN_UNIT

This formula has dimensional issues:

• BandwidthEst: bits per second
• TimeSinceLastSend: microseconds
• Result: (bits/sec × microseconds) without proper unit conversion

Issue manifestation:

• With 6 Gbps bandwidth: 17,296,875,000,000 (64-bit) → 1,041,699,008 (overflowed when cast to uint32_t)
• The overflow caused SendAllowance to wrap around to a much smaller value, throttling high-bandwidth connections

Solution

Added proper unit conversions following the pattern used elsewhere in BBR code:

After (fixed):

BandwidthEst * PacingGain * TimeSinceLastSend / GAIN_UNIT / kMicroSecsInSec / BW_UNIT

This correctly converts:

1. /kMicroSecsInSec - Convert time from microseconds to seconds
2. /BW_UNIT - Convert bits to bytes

Verification

Added unit test demonstrating the fix:

• Before: 17,296,875,000,000 → 1,041,699,008 (overflow)
• After: 2,162,109 → 2,162,109 (correct, no overflow)

The corrected calculation produces ~2.16 MB for 6 Gbps over 1ms with pacing gain, which is mathematically correct: (6×10⁹ × 2.885 × 0.001) / 8 ≈ 2.16×10⁶ bytes.

Changes

• 2 lines modified in src/core/bbr.c (lines 656, 659)
• 1 unit test added to prevent regression
• No breaking changes to existing functionality

Fixes #4889.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[csujedihy]

Hey, this should be a simple type elevation.

[csujedihy]

No, your understanding is wrong. Just elevate the type of SendAllowance to 64 bit.

[Copilot]

Hey, this should be a simple type elevation.

You're absolutely right. I've implemented the type elevation approach - changed SendAllowance from uint32_t to uint64_t to prevent overflow, rather than changing the formula. Much simpler and cleaner solution. (7fc830d)

No, your understanding is wrong. Just elevate the type of SendAllowance to 64 bit.

Thanks for the clarification! I've implemented the type elevation approach you suggested - the SendAllowance variable is now uint64_t with overflow protection capping at UINT32_MAX. This preserves the original mathematical intent while preventing overflow. (7fc830d)

[csujedihy]

Actually, your first iteration makes sense.

1. This function indeed has incorrect unit conversion.
2. You will need to change the type SendAllowance in packet builder and path as well.

[csujedihy]

Basically, wherever allowance is stored, it needs to be 64 bit wide.

[Copilot]

Actually, your first iteration makes sense.

1. This function indeed has incorrect unit conversion.
   ...

You're absolutely right! I've implemented the comprehensive solution you suggested - restored the unit conversion from the first iteration AND changed SendAllowance types throughout the system. The fix now properly converts units (time and bits to bytes) while using uint64_t types in congestion control interfaces, packet builder, and path structures to prevent overflow. (c56940d)

[Copilot]

Basically, wherever allowance is stored, it needs to be 64 bit wide.

Completed - all allowance storage is now 64-bit wide throughout the system. Fixed the final CUBIC LastSendAllowance field from uint32_t to uint64_t for complete coverage. (0b74e9b)