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 RFC 8888 Congestion Control Feedback #117

Merged
merged 1 commit into from
Feb 2, 2022
Merged

Conversation

mengelbart
Copy link
Contributor

Implements Congestion Control Feedback Reports as defined in RFC 8888.

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #117 (35d9cc1) into master (7d61e80) will decrease coverage by 0.27%.
The diff coverage is 71.42%.

❗ Current head 35d9cc1 differs from pull request most recent head 680c5bf. Consider uploading reports for the commit 680c5bf to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
- Coverage   76.56%   76.28%   -0.28%     
==========================================
  Files          20       21       +1     
  Lines        2227     2353     +126     
==========================================
+ Hits         1705     1795      +90     
- Misses        435      461      +26     
- Partials       87       97      +10     
Flag Coverage Δ
go 76.28% <71.42%> (-0.28%) ⬇️
wasm 76.28% <71.42%> (-0.28%) ⬇️

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

Impacted Files Coverage Δ
rfc8888.go 71.42% <71.42%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d61e80...680c5bf. Read the comment docs.

@Sean-Der
Copy link
Member

Sean-Der commented Feb 1, 2022

Some of the best Go code I have read in a while :) LGTM!

rfc8888.go Show resolved Hide resolved
rfc8888.go Outdated Show resolved Hide resolved
rfc8888.go Outdated Show resolved Hide resolved
for i := uint16(0); i < numReports; i++ {
var mb CCFeedbackMetricBlock
offset := reportsOffset + 2*i
if err := mb.unmarshal(rawPacket[offset : offset+2]); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't, thanks! I added another check.

Implements Congestion Control Feedback Reports as defined in
https://www.rfc-editor.org/rfc/rfc8888.html
@mengelbart mengelbart merged commit 11478d1 into master Feb 2, 2022
@mengelbart mengelbart deleted the feat/rfc-8888 branch February 2, 2022 20:08
@kevmo314
Copy link
Contributor

kevmo314 commented Feb 2, 2022

Nice! Excited to use this :)

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

3 participants