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

Implement retransmit backoff according to 4.2.4.1 #448

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eh-steve
Copy link

@eh-steve eh-steve commented Apr 14, 2022

Description

As per RFC4347 4.2.4.1 (excluding reset after long period of idleness)

Though timer values are the choice of the implementation, mishandling
of the timer can lead to serious congestion problems; for example, if
many instances of a DTLS time out early and retransmit too quickly on
a congested link. Implementations SHOULD use an initial timer value
of 1 second (the minimum defined in RFC 2988 [RFC2988]) and double
the value at each retransmission, up to no less than the RFC 2988
maximum of 60 seconds. Note that we recommend a 1-second timer
rather than the 3-second RFC 2988 default in order to improve latency
for time-sensitive applications. Because DTLS only uses
retransmission for handshake and not dataflow, the effect on
congestion should be minimal.

Implementations SHOULD retain the current timer value until a
transmission without loss occurs, at which time the value may be
reset to the initial value. After a long period of idleness, no less
than 10 times the current timer value, implementations may reset the
timer to the initial value. One situation where this might occur is
when a rehandshake is used after substantial data transfer.

@eh-steve eh-steve force-pushed the backoff-retry-RFC4347-4.2.4.1 branch from 53edba5 to 44c5cbe Compare April 14, 2022 20:40
@codecov
Copy link

codecov bot commented Apr 16, 2022

Codecov Report

Base: 76.62% // Head: 75.85% // Decreases project coverage by -0.77% ⚠️

Coverage data is based on head (44c5cbe) compared to base (9e922d5).
Patch coverage: 72.72% of modified lines in pull request are covered.

❗ Current head 44c5cbe differs from pull request most recent head eb8069f. Consider uploading reports for the commit eb8069f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #448      +/-   ##
==========================================
- Coverage   76.62%   75.85%   -0.77%     
==========================================
  Files          96       96              
  Lines        5762     5584     -178     
==========================================
- Hits         4415     4236     -179     
- Misses       1005     1021      +16     
+ Partials      342      327      -15     
Flag Coverage Δ
go 75.88% <72.72%> (-0.77%) ⬇️
wasm 66.19% <72.72%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
handshaker.go 74.48% <71.42%> (-1.47%) ⬇️
conn.go 81.56% <100.00%> (+0.22%) ⬆️
errors_errno.go 62.50% <0.00%> (-37.50%) ⬇️
pkg/crypto/elliptic/elliptic.go 55.88% <0.00%> (-20.87%) ⬇️
errors.go 69.69% <0.00%> (-15.67%) ⬇️
pkg/protocol/recordlayer/recordlayer.go 69.56% <0.00%> (-13.05%) ⬇️
crypto.go 50.64% <0.00%> (-8.48%) ⬇️
pkg/crypto/prf/prf.go 63.57% <0.00%> (-6.43%) ⬇️
.../protocol/handshake/message_certificate_request.go 64.81% <0.00%> (-3.73%) ⬇️
certificate.go 81.81% <0.00%> (-2.52%) ⬇️
... and 16 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@eh-steve
Copy link
Author

I can’t really tell from the logs whether the CI is failing due to my changes…

@eh-steve eh-steve force-pushed the backoff-retry-RFC4347-4.2.4.1 branch from 44c5cbe to eb8069f Compare February 15, 2023 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant