-
Notifications
You must be signed in to change notification settings - Fork 204
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
Reference RFC5681 and RFC6928 #1245
Conversation
Partially fixes #592
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the inconsistencies when MSS is not default size. Both large and small MSS causes unintended behaviour here.
draft-ietf-quic-recovery.md
Outdated
kMinimumWindow (default 2 * kDefaultMss): | ||
: Default minimum congestion window. | ||
kInitialWindow (default 14600 bytes): | ||
: Default limit on the initial amount of outstanding data in bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
draft-ietf-quic-recovery.md
Outdated
@@ -974,6 +968,13 @@ kLossReductionFactor (default 0.5): | |||
Variables required to implement the congestion control mechanisms | |||
are described in this section. | |||
|
|||
sender_mss: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should use MSS, since the idea of a segment is TCP-specific. How about max_datagram_size?
@ianswett, I leave this with you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this works for me.
Fixes #592 and #1102