Bug 2869

Summary: code-review: Implementation of TCP BBR in ns-3
Product: ns-3 Reporter: Vivek Jain <jain.vivek.anand>
Component: tcpAssignee: natale.patriciello
Status: PATCH PENDING ---    
Severity: enhancement CC: mengleizhang.mz, ns-bugs, tomh
Priority: P3 Keywords: feature-request
Version: ns-3-dev   
Hardware: All   
OS: All   
Bug Depends on: 2868    
Bug Blocks:    

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:

http://mailman.isi.edu/pipermail/ns-developers/2018-January/014270.html


Link to Github Repo:
https://github.com/Vivek-anand-jain/ns-3-dev-git/tree/bbr-dev

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
Update:
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:
https://github.com/Vivek-anand-jain/ns-3-dev-git/tree/bbr-dev
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);