-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pacing #452
pacing #452
Conversation
…head more than 10mtu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that the pacer is an additional restriction on how much we can send (on top of CWND), maybe we need need to update when the delivery rate estimator starts running.
FWIW, current logic is do_send
is:
if (can_send_stream_data &&
(s->num_datagrams == s->max_datagrams || conn->egress.loss.sentmap.bytes_in_flight >= conn->egress.cc.cwnd)) {
/* as the flow is CWND-limited, start delivery rate estimator */
quicly_ratemeter_in_cwnd_limited(&conn->egress.ratemeter, s->first_packet_number);
} else {
quicly_ratemeter_not_cwnd_limited(&conn->egress.ratemeter, conn->egress.packet_number);
}
As you can see, pacer is not taken into consideration (as the name of the function is becoming weird).
PS. This issue has been addressed in c8ec914.
…ed state; ATM the state being referred to is that when `quicly_send` is called the last time. For Cubic, this is a TODO.
…ultiplier into core
… complete; the pressure being applied to the network is mostly like only 3 MTU
Things to consider:
As of dc13a40, pace is 2x flow rate in slow start phase, 1.2x in congestion avoidance phase. ATM, pace is 2x flow rate regardless of in slow start. Use smaller multiplier for congestion avoidance phase?Pacing rate is 2x for both slow start and congestion avoidance phase; for rationale, see the discussion below.Dispatch resolution isWhen the flow rate is small, two packets are dispatched at once, see comment below.max(packets_per_ms, 1)
, meaning that when the flow rate is small, quicly sends one packet at a time. Should we change that to 2 packets or something?At the moment, the pacer precedes the flow rate by 10mtu to 11mtu. Should we change it to 9mtu to 10mtu? -> done in c9be530. Pacer sends burst of 10mtu, then sends 2 packets in batch so that the pace would precede the flow rate by between 8mtu and 10 mtu.ToDo:
Add end-to-end tests? But what kind of?Done.