Skip to content
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

Persistent congestion pseudocode to match text #4010

Closed
wants to merge 7 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 38 additions & 28 deletions draft-ietf-quic-recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,10 @@ ssthresh:
first_rtt_sample:
: The time that the first RTT sample was obtained.

pc_lost:
: Lost packets from all packet number spaces that are used in determining
persistent congestion.


## Initialization

Expand Down Expand Up @@ -1544,16 +1548,16 @@ newly acked_packets from sent_packets.
return sent_time <= congestion_recovery_start_time

OnPacketsAcked(acked_packets):
if (first_rtt_sample == 0):
first_rtt_sample = now()

for acked_packet in acked_packets:
OnPacketAcked(acked_packet)

OnPacketAcked(acked_packet):
// Remove from bytes_in_flight.
bytes_in_flight -= acked_packet.sent_bytes

if (first_rtt_sample == 0):
first_rtt_sample = now()

// Do not increase congestion_window if application
// limited or flow control limited.
if (IsAppOrFlowControlLimited())
Expand Down Expand Up @@ -1614,31 +1618,37 @@ Invoked when an ACK frame with an ECN section is received from the peer.
Invoked when DetectAndRemoveLostPackets deems packets lost.

~~~
InPersistentCongestion(largest_lost):
pto = smoothed_rtt + max(4 * rttvar, kGranularity) +
max_ack_delay
pc_period = pto * kPersistentCongestionThreshold

// Only look for persistent congestion if the period
// starts after getting the first RTT sample.
assert(first_rtt_sample != 0)
if (largest_lost.time_sent - pc_period > first_rtt_sample):
return false

// Determine if all packets in the time period before the
// largest newly lost packet, including the edges and
// across all packet number spaces, are marked lost.
return AreAllPacketsLost(largest_lost, pc_period)

OnPacketsLost(lost_packets):
// Remove lost packets from bytes_in_flight.
for lost_packet in lost_packets:
bytes_in_flight -= lost_packet.sent_bytes
OnCongestionEvent(lost_packets.largest().time_sent)

// Collapse congestion window if persistent congestion
if (InPersistentCongestion(lost_packets.largest())):
congestion_window = kMinimumWindow
InPersistentCongestion(lost_packets):
// Consider lost packets across all packet number spaces.
pc_lost += lost_packets
martinthomson marked this conversation as resolved.
Show resolved Hide resolved

// Disregard packets sent prior to getting an RTT sample.
assert(first_rtt_sample != 0)
for lost in pc_lost:
if lost.time_sent <= first_rtt_sample:
pc_lost.remove(lost)

// Find the largest contiguous set of lost packets that
// starts and ends with an ack-eliciting packet.
(first, last) = FindLargestContiguousLoss(pc_lost)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot imagine how to detect contiguity for all packet spaces.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kazu-yamamoto : you can use time indexing for this purpose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You want the (first, last) around the largest period of loss, not the largest set of lost packets. Perhaps FindLargestLossPeriod ?


// Declare persistent congestion if these packets span
// a period longer than the persistent congestion period.
pto = smoothed_rtt + max(4 * rttvar, kGranularity) +
max_ack_delay
pc_period = pto * kPersistentCongestionThreshold
return (last.time_sent - first.time_sent) > pc_period
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the recovery draft, it is not certain which is correct, > or >=.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3961 says "exceed". So, > should be correct.


OnPacketsLost(lost_packets):
// Remove lost packets from bytes_in_flight.
for lost_packet in lost_packets:
bytes_in_flight -= lost_packet.sent_bytes
OnCongestionEvent(lost_packets.largest().time_sent)

// Collapse congestion window on persistent congestion.
if (InPersistentCongestion(lost_packets)):
pc_lost.clear()
congestion_window = kMinimumWindow
~~~

## Upon dropping Initial or Handshake keys
Expand Down