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 draft-ietf-tsvwg-sctp-zero-checksum-01 #284

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

mengelbart
Copy link
Contributor

Description

Implementation of draft-ietf-tsvwg-sctp-zero-checksum-01.

I currently don't have a specific use case for this, but implementing the draft was an interesting exercise. I suggest keeping this as draft PR, at least until the document is finalized.

@mengelbart mengelbart marked this pull request as draft July 20, 2023 10:02
@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (2927025) 80.59% compared to head (b168526) 80.56%.

Files Patch % Lines
association.go 89.09% 4 Missing and 2 partials ⚠️
param_zero_checksum.go 60.00% 4 Missing and 2 partials ⚠️
packet.go 76.92% 2 Missing and 1 partial ⚠️
paramtype.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
- Coverage   80.59%   80.56%   -0.04%     
==========================================
  Files          48       49       +1     
  Lines        4056     4121      +65     
==========================================
+ Hits         3269     3320      +51     
- Misses        644      654      +10     
- Partials      143      147       +4     
Flag Coverage Δ
go 80.56% <80.45%> (-0.04%) ⬇️
wasm 66.90% <44.82%> (-0.41%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tuexen
Copy link
Contributor

tuexen commented Aug 9, 2023

Question, maybe also for @Sean-Der : What possibilities of collaboration do exist? Only by commenting or is there a way to propose changes to a PR?

@mengelbart
Copy link
Contributor Author

Hi @tuexen! You can add suggestions in the comments (there's a little icon in the comments UI where you can directly suggest code changes), or I think you can also open a PR against this branch. I also don't mind if you directly push to this branch, but I think that only works if you are a member of the pion org. (if not, I think @Sean-Der can add you as a member?)

@Sean-Der
Copy link
Member

Sean-Der commented Aug 9, 2023

Done! You can directly push to branches now @tuexen

Everyone just needs one approval to merge to master :)

@tuexen
Copy link
Contributor

tuexen commented Aug 9, 2023

Done! You can directly push to branches now @tuexen

Great thanks. This makes things much simpler.

Everyone just needs one approval to merge to master :)

Great. I'll be conservative, since I'm new to go...

@mengelbart mengelbart force-pushed the feat/zero-checksum-support branch from 2227146 to 1a0c73c Compare November 9, 2023 09:24
@Sean-Der
Copy link
Member

Sean-Der commented Nov 9, 2023

Anything I can do to help @mengelbart ? Seems like this is really close, would love to get it in :)

@mengelbart
Copy link
Contributor Author

I think it is almost done. The IETF draft changed a bit since I first implemented it. I don't think it requires changes in this patch but someone should probably review that again. And I think the code needs to be reviewed in general :)

@mengelbart mengelbart marked this pull request as ready for review November 9, 2023 16:55
@daweifeng
Copy link

I have been using this branch in my project for a month and I haven't seen any issues so far. It is compatible with the C++ zero checksum implementation in libdatachannel

@matthewfarstad
Copy link

Very excited for this to be merged. Until then I will be using the PR.

@Sean-Der Sean-Der force-pushed the feat/zero-checksum-support branch 3 times, most recently from b3c21fb to a3a3a63 Compare February 9, 2024 16:36
@Sean-Der Sean-Der force-pushed the feat/zero-checksum-support branch from a3a3a63 to b168526 Compare February 9, 2024 16:38
@Sean-Der Sean-Der merged commit 932b71a into master Feb 9, 2024
14 checks passed
@Sean-Der Sean-Der deleted the feat/zero-checksum-support branch February 9, 2024 16:41
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.

5 participants