Subject: Re: [quicwg/base-drafts] Byte counting at Congestion avoidance (#3917)

From: Junho Choi <[email-masked]>
Date: Tue, 01 Sep 2020 22:51:56 -0700

@junhochoi commented on this pull request.

> +      bytes_acked += acked_packet.sent_bytes
+      if (bytes_acked >= congestion_window):
+        bytes_acked -= congestion_window
+        congestion_window += max_datagram_size

This is a simulated environment, not an public internet. In this testing I used MacOS dummynet. As I said this number is what recovery module detected as loss (which will trigger a congestion event). So may not be an actual loss (e.g. delivered late after a time threshold). I didn't measure actual packet loss.
What's your expectation?

-- 
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/3917#discussion_r481745686

---

From: Kazu Yamamoto <[email-masked]>
Date: Thu, 03 Sep 2020 23:22:56 -0700

So, how to decide including or declining this PR?

-- 
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/3917#issuecomment-686939079

---

From: Jana Iyengar <[email-masked]>
Date: Fri, 04 Sep 2020 17:02:33 -0700

The text currently says that the congestion window MUST NOT increase by more than 1 max datagram size per acknowledged cwnd. The pseudo code currently reflects what the text says. In that sense, the pseudo code is correct.

This PR is an alternative implementation of the text. As such, it doesn't fix a problem in the text, it fixes a problem with implementations that use integers.

The pushback on this PR is that the behavior is different than the existing pseudo code. Broadly, I agree with that, though, as I have said earlier, it shouldn't matter in practice. That said, the difference in behavior exists. The pseudo code in the draft is meant as clarification of the text, and implementations will and do differ from the pseudo code in various places.

My suggestion then would be to drop this PR, and instead add a comment above the concerning line saying something like: "Implementers that use an integer representation for the congestion window should be careful with the following division, and can use the alternative approach suggested in Section 2.1 of RFC 3465."

-- 
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/3917#issuecomment-687481216

---

From: Junho Choi <[email-masked]>
Date: Tue, 08 Sep 2020 15:42:43 -0700

> My suggestion then would be to drop this PR, and instead add a comment above the concerning line saying something like: "Implementers that use an integer representation for the congestion window sh…

To clarify, close this one and open a new PR about text changes only? I'm ok to do that.

-- 
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/3917#issuecomment-689174124

---

From: Junho Choi <[email-masked]>
Date: Tue, 08 Sep 2020 15:54:29 -0700

Opened #4082 for text changes.

-- 
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/3917#issuecomment-689178254

---

From: Jana Iyengar <[email-masked]>
Date: Tue, 08 Sep 2020 15:59:27 -0700

Closed #3917 via #4082.

-- 
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/3917#event-3743130978