Subject: [quicwg/base-drafts] Reference RFC6928 (#1245)

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 17:38:11 +0000 (UTC)

Partially fixes #592
You can view, comment on, or merge this pull request online at:

  https://github.com/quicwg/base-drafts/pull/1245

-- Commit Summary --

  * Reference RFC6928

-- File Changes --

    M draft-ietf-quic-recovery.md (5)

-- Patch Links --

https://github.com/quicwg/base-drafts/pull/1245.patch
https://github.com/quicwg/base-drafts/pull/1245.diff

-- 
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/1245

---

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 17:44:43 +0000 (UTC)

@ianswett pushed 1 commit.

1099e08  Reference RFC5681

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1245/files/672a0b7e01fbfd001bf066b71a2edfff4f4387d8..1099e081073b330e2cd5ec32a46fb172af4c73b8

---

From: ianswett <[email-masked]>
Date: Tue, 20 Mar 2018 17:47:48 +0000 (UTC)

bdd2bfe  Update pseudocode

-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/quicwg/base-drafts/pull/1245/files/1099e081073b330e2cd5ec32a46fb172af4c73b8..bdd2bfe33e9d57edfb4ebfb64f58d7d6e022ac67

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 22 Mar 2018 11:49:23 +0000 (UTC)

gloinul commented on this pull request.

Please resolve the inconsistencies when MSS is not default size. Both large and small MSS causes unintended behaviour here. 

> @@ -955,15 +955,9 @@ Constants used in congestion control are based on a combination of RFCs,
 papers, and common practice.  Some may need to be changed or negotiated
 in order to better suit a variety of environments.
 
-kDefaultMss (default 1460 bytes):
-: The default max packet size used for calculating default and minimum
-  congestion windows.
-
-kInitialWindow (default 10 * kDefaultMss):
-: Default limit on the amount of outstanding data in bytes.
-
-kMinimumWindow (default 2 * kDefaultMss):
-: Default minimum congestion window.
+kInitialWindow (default 14600 bytes):
+: Default limit on the initial amount of outstanding data in bytes.

I would note that RFC 6928 states the following as the TCP initial window:

   More precisely, the upper bound for the initial window will be

         min (10*MSS, max (2*MSS, 14600))                            (1)

I would recommend that one do calculate this value for the Initial value, as MSS may be based on a 9K MTU, thus allowing two full packets in those cases. 

> @@ -955,15 +955,9 @@ Constants used in congestion control are based on a combination of RFCs,
 papers, and common practice.  Some may need to be changed or negotiated
 in order to better suit a variety of environments.
 
-kDefaultMss (default 1460 bytes):
-: The default max packet size used for calculating default and minimum
-  congestion windows.
-
-kInitialWindow (default 10 * kDefaultMss):
-: Default limit on the amount of outstanding data in bytes.
-
-kMinimumWindow (default 2 * kDefaultMss):
-: Default minimum congestion window.
+kInitialWindow (default 14600 bytes):
+: Default limit on the initial amount of outstanding data in bytes.

Also if mss is smaller than 1460 then more than 10 packets can be sent. Is that intentional? I am slightly worried about packet bursts. Are there a pacing requirement?

-- 
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/1245#pullrequestreview-106079821

---

From: janaiyengar <[email-masked]>
Date: Tue, 27 Mar 2018 12:09:00 -0700

janaiyengar commented on this pull request.

> @@ -974,6 +968,13 @@ kLossReductionFactor (default 0.5):
 Variables required to implement the congestion control mechanisms
 are described in this section.
 
+sender_mss:

I don't think we should use MSS, since the idea of a segment is TCP-specific. How about max_datagram_size?

-- 
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/1245#pullrequestreview-106043989