Bug 2869 - code-review: Implementation of TCP BBR in ns-3
code-review: Implementation of TCP BBR in ns-3
Product: ns-3
Classification: Unclassified
Component: tcp
All All
: P3 enhancement
Assigned To: natale.patriciello
: feature-request
Depends on: 2868
  Show dependency treegraph
Reported: 2018-02-08 08:07 EST by Vivek Jain
Modified: 2018-06-01 12:24 EDT (History)
3 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Vivek Jain 2018-02-08 08:07:11 EST
This is code review request for the implementation of TCP BBR model in ns-3.

Following is the link for the discussion on developers mailing list:


Link to Github Repo:

Note: TCP BBR uses CongControl and Delivery rate estimation, which are already being reviewed (Bug Id: 2868).
Comment 1 Vivek Jain 2018-02-16 09:53:19 EST
1. Designed and added a preliminary test-suite for BBR called tcp-bbr-test.cc.
2. Updated documentation (tcp.rst) for TCP BBR.
3. Addressed review comments given by Tom Sir for BBR code.

The code is available here for the review:
Comment 2 Menglei Zhang 2018-04-23 17:17:43 EDT
I was using this BBR implementation, and spot 2 bugs.

In the TcpTxBuffer class, the m_priorDelivered parameter of struct RateSample should be initialized to 0. The original line 44
uint32_t m_priorDelivered; //!< The delivered count of the most recent packet delivered
should be changed to
uint32_t m_priorDelivered = 0; //!< The delivered count of the most recent packet delivered

Sometime the congestion window goes to a very large value, I figure out the problem was from the TcpBbr::ModulateCwndForRecovery() function line 429
tcb->m_cWnd = std::max (tcb->m_cWnd.Get () - rs->m_packetLoss, tcb->m_segmentSize);
should change be to
tcb->m_cWnd = std::max ((int)tcb->m_cWnd.Get () - (int)rs->m_packetLoss, (int)tcb->m_segmentSize);