Bug 2869

Summary:	code-review: Implementation of TCP BBR in ns-3
Product:	ns-3	Reporter:	Vivek Jain <jain.vivek.anand>
Component:	tcp	Assignee:	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 UTC

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 UTC

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 UTC

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);