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.

Sign up for GitHub

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

Jump to bottom

draft-ietf-quic-recovery-13 ssthresh never set #1598

Closed
aep opened this issue Jul 24, 2018 · 1 comment · Fixed by #1602
Closed

draft-ietf-quic-recovery-13 ssthresh never set #1598

aep opened this issue Jul 24, 2018 · 1 comment · Fixed by #1602
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.

Comments

@aep
Copy link
Contributor

aep commented Jul 24, 2018 •
edited
Loading

in 4.3. Congestion Avoidance

and sets the slow start threshold to the new congestion window.

however, the pseudo code doesnt do that

i'm guessing that CongestionEvent should do that?

      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 *= kMarkReductionFactor
          congestion_window = max(congestion_window, kMinimumWindow)
+         ssthresh = congestion_window
@ianswett
Copy link
Contributor

ianswett commented Jul 24, 2018 •
edited
Loading

Your suggestion looks correct to me. PR?

aep added a commit to aep/base-drafts that referenced this issue Jul 24, 2018
@aep
adds missing ssthresh = congestion_window
3937b5c 
fixes quicwg#1598
aep added a commit to aep/base-drafts that referenced this issue Jul 24, 2018
@aep
adds missing ssthresh = congestion_window
36a22bd 
fixes quicwg#1598
@aep aep mentioned this issue Jul 24, 2018
Merged
ianswett pushed a commit that referenced this issue Jul 24, 2018
@aep @ianswett
adds missing ssthresh = congestion_window (#1602)
c13ae7e 
fixes #1598
@mnot mnot added -recovery editorial An issue that does not affect the design of the protocol; does not require consensus. labels Mar 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

adds missing ssthresh = congestion_window aep/base-drafts
3 participants
@mnot @aep @ianswett