huitema
commented
10 months ago The last commit fixes the pacing of small packets by accounting for packet length in picoquic_update_pacing_after_send in sender.c. The effect of that fixes are mixed: it changes the timing of packets, which most of the time results in better performance, but sometimes causes event sequences to change, causing some tests to run slower. Overall it is better, because it does fix pacing issues when the traffic includes lots of small packets, and ensure that the data rate computed by the congestion control algorithm is consistent with the actual pacing rate.
The main part of this PR is a rewrite of the function
BBRUpdateBtlBwinbbr.c, to implement a proper handling of limited applications. The code includes a list of previous bandwidth measurements, and BBR selects the maximum available in that list. The previous version would forget the oldest measurement at the end of each epoch; the new version only does that if the new measurement is larger than the current data rate, or was not captured in limited conditions. This will help solving the issue observed by @alexrabi in #1547.The PR also includes a new test, in which short datagrams are sent each in their own packet instead of being possibly grouped in larger packets. This is an attempt to reproduce issue #1547, but it is still work in progress. There may be a remaining issue caused by the function
picoquic_update_pacing_after_sendinsender.c. That function consumes a fixed amount of pacing credit per packet, which could be problematic if the traffic consists mostly of short packets. We need to address that before merging this PR.