Skip to content
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

Two reasons to exceed bytes in flight #4004

Closed
martinthomson opened this issue Aug 17, 2020 · 6 comments · Fixed by #4006
Closed

Two reasons to exceed bytes in flight #4004

martinthomson opened this issue Aug 17, 2020 · 6 comments · Fixed by #4006
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@martinthomson
Copy link
Member

It might pay to reconcile:

An endpoint MUST NOT send a packet if it would cause bytes_in_flight (see
{{vars-of-interest}}) to be larger than the congestion window, unless the packet
is sent on a PTO timer expiration; see {{pto}}.

And

When entering recovery, a single packet MAY be sent even if bytes in flight
now exceeds the recently reduced congestion window.

Now, you might implement the latter using PTO machinery, but that text doesn't really suggest that in any way.

@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -recovery labels Aug 17, 2020
@marten-seemann
Copy link
Contributor

Where does this second sentence come from? I'm pretty sure I haven't implemented anything like this in quic-go. Should I?

@martinthomson
Copy link
Member Author

That sentence is the the definition of the recovery period. We haven't implemented that either, and I was wondering if it truly helps.

@mirjak
Copy link
Contributor

mirjak commented Aug 17, 2020

For TCP this second case if important because as long as you don't retransmit the lost packet you will not get any ACK that acknowledges new data. I guess we don't need that for QUIC.

@ianswett
Copy link
Contributor

ianswett commented Aug 17, 2020

The second sentence was added later in #3443, and the MUST wasn't updated, so I'll fix that.

Chromium currently implements PRR(RFC6937), which is a more complex variant of this which is designed to keep the ACK clock going. I thought we discussed adding an informative reference to PRR when we added this text, and I'm not sure why we didn't.

@mirjak
Copy link
Contributor

mirjak commented Aug 17, 2020

#3483 seems to be the wrong issue you are pointed to...

@ianswett
Copy link
Contributor

Good catch, it's #3443

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants