Re: [quicwg/base-drafts] Persistent Congestion Time Threshold (#2365)

[Jana Iyengar <[e-mail]>]

janaiyengar commented on this pull request.



> +  rttvar = 0
+  max_ack_delay = 0
+  kPersistentCongestionThreshold = 2
+
+If an eck-eliciting packet is sent at time = 0, the following scenario would
+illustrate persistent congestion:
+
+  t=0 | Send Pkt #1 (App Data)
+  t=1 | Send Pkt #2 (PTO 1)
+  t=3 | Send Pkt #3 (PTO 2)
+  t=7 | Send Pkt #4 (PTO 3)
+  t=8 | Recv ACK of Pkt #4
+
+The first three packets are determined to be lost when the ACK of packet 4 is
+received at t=8.  The congestion period is calculated as the time between the
+oldest and newest lost packets: (3 - 0) = 3.  The duration for persistent

I think @marten-seemann is right, we don't want to have differences between the text and the example at least. We don't need the pseudo code to be descriptive in how the datastructures are used, but It can specify what needs to be done. The problem here is that the example and the pseudo code are suggesting that this is simpler than it actually is. 

I think we may need to slightly alter the mechanism anyway. It's not the time difference between the newest and the oldest that's interesting, it's that packets sent in the 3 x PTO duration prior to the newest lost packet were all marked as lost. So (i) the latest packet sent >= 3 x PTO before the newest lost packet should be the oldest packet that we are interested in (not the oldest packet that is marked as lost), and (ii) we need to then check that all packets between that packet and the newest are marked lost.

So, I propose the following changes:
1. Change the example to show packets that were sent earlier than 3xPTO and marked lost, but they aren't used in the detection of PersistentCongestion
2. Add to the text below (proposed below)
3. Add to the pseudocode (proposed below)


> +  kPersistentCongestionThreshold = 2
+
+If an eck-eliciting packet is sent at time = 0, the following scenario would
+illustrate persistent congestion:
+
+  t=0 | Send Pkt #1 (App Data)
+  t=1 | Send Pkt #2 (PTO 1)
+  t=3 | Send Pkt #3 (PTO 2)
+  t=7 | Send Pkt #4 (PTO 3)
+  t=8 | Recv ACK of Pkt #4
+
+The first three packets are determined to be lost when the ACK of packet 4 is
+received at t=8.  The congestion period is calculated as the time between the
+oldest and newest lost packets: (3 - 0) = 3.  The duration for persistent
+congestion is equal to: (1 * ((2 ^ kPersistentCongestionThreshold) - 1)) = 3.
+Because the threshold was reached, the network is considered to have experienced

Suggestion: "Because the threshold was reached and because none of the packets between the oldest and the newest packets are acknowledged, the network is considered ..."

> @@ -1282,15 +1314,27 @@ Invoked by loss detection from DetectLostPackets when new packets
 are detected lost.
 
 ~~~
+   InPersistentCongestion(congestion_period):

Suggested rework of this function:
```
InPersistentCongestion(newest_lost_packet):
    pto = smoothed_rtt + 4 * rttvar + max_ack_delay
    congestion_period = pto * (2 ^ kPersistentCongestionThreshold - 1)
    // Find the latest packet that was sent at least pto time before the newest lost packet
    oldest_sent_packet = FindPacketSentBefore(newest_lost_packet, congestion_period)
    // Determine if all packets in the window, including the  edges, are marked lost
    return IsWindowLost(oldest_sent_packet, newest_lost_packet)
```
       

>  
      // Start a new congestion epoch if the last lost packet
      // is past the end of the previous recovery epoch.
-     CongestionEvent(largest_lost_packet.time_sent)
+     CongestionEvent(newest_lost_packet.time_sent)
+
+     // Collapse congestion window if persistent congestion
+     if (InPersistentCongestion(

Change params to InPersistentCongestion to only have newest_lost_packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/2365#pullrequestreview-205554955