Subject: [quicwg/base-drafts] Need to clarify whether end_of_recovery have to be tracked on each PN spaces separately (#1928)

From: Masakazu Kitajo <[email-masked]>
Date: Sun, 28 Oct 2018 21:56:35 -0700

On section 5.8.2 and 5.8.6, the spec suggests that you should track the largest packet number sent when QUIC detects a loss, and the pseudo code checks the packet number in `CongestionEvent`. Since there are multiple packet number spaces, it seems like `end_of_recovery` should be a vector and largest packet numbers should be tracked separately on each PN spaces. If it was true, it should be clarified on 5.8.2, otherwise it's hard to interpret I think.

However, if we managed them separately, `congestion_window` which is shared with all PN spaces will be decreased on every congestion event on each PN spaces. Although it still works, the result (behavior) would be different from one packet number space scenario.

If this change is not intentional and we want to keep the original behavior, we probably need to introduce one more packet numbering system that would be shared with all PN spaces to track the **last** packet sent.

5.8.2.  Variables of interest
```
end_of_recovery:  The largest packet number sent when QUIC detects a
      loss.  When a larger packet is acknowledged, QUIC exits recovery.
```

5.8.6.  On New Congestion Event
```
   Invoked from ProcessECN and OnPacketsLost when a new congestion event
   is detected.  Starts a new recovery period and reduces the congestion
   window.

      CongestionEvent(packet_number):
        // Start a new congestion event if packet_number
        // is larger than the end of the previous recovery epoch.
        if (!InRecovery(packet_number)):
          end_of_recovery = largest_sent_packet
          congestion_window *= kLossReductionFactor
          congestion_window = max(congestion_window, kMinimumWindow)
          ssthresh = congestion_window

```

-- 
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/issues/1928

---

From: ianswett <[email-masked]>
Date: Mon, 29 Oct 2018 06:08:14 -0700

Good catch, I think this needs to be unified across the spaces, otherwise as you said you could inadvertently reduce the CWND multiple times.

I don't want to introduce another unified packet number space.  If we're going to do that, we should just go back to a single PN space because maintaining 1 unified one and 3 separate ones is becoming too crazy.

I can add a start_of_recovery_sent_time field that's the time of the start of recovery, and if a packet sent after that is acknowledged, recovery is exited.  I think that's the simplest solution to this issue.

-- 
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/issues/1928#issuecomment-433903021

---

From: Masakazu Kitajo <[email-masked]>
Date: Mon, 29 Oct 2018 06:31:31 -0700

`start_of_recovery_sent_time` sounds like a good idea.

-- 
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/issues/1928#issuecomment-433910536

---

From: MikkelFJ <[email-masked]>
Date: Mon, 29 Oct 2018 08:03:36 -0700

I think this captures the nature of the problem more accurately and simpler, even if implementations might use a vector of packet numbers instead.

-- 
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/issues/1928#issuecomment-433943943

---

From: ianswett <[email-masked]>
Date: Tue, 30 Oct 2018 07:07:35 -0700

Closed #1928 via #1930.

-- 
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/issues/1928#event-1935155928