Various cleanup and fixes

[edaniels]

Description

I've spent the past week debugging what's going on with throughput and stalling when sending a lot of small and large data through SCTP. I have draft work in my fork that adds the modified Nagle's algorithm as well as a missing sender side implementation of addressing Silly Window Syndrome (this is a MUST in the RFC). Both of these help considerably on increasing throughput and stability but I need to verify it doesn't cause any unexpected regressions on throughput before moving forward. They also are hiding what I think may be some bug in this library that causes no progress to be made, even when RTXs are happening, so I want to solve that first.

Changes

• Add comments referring back to RFCs
• Allow associations to be named
• Fix some typos
• Fix a bug where we unmarshal a packet twice, doubling it's size when we are doing mandatory checksumming
• Add some more stats for debugging
• Make useZeroChecksum atomic
• Add locks to queues to avoid caught races

[codecov]

Codecov Report

Attention: Patch coverage is 93.59606% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 81.17%. Comparing base (5359da5) to head (224c60f).

Files	Patch %	Lines
association.go	90.72%	7 Missing and 2 partials ⚠️
payload_queue.go	90.24%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #302      +/-   ##
==========================================
+ Coverage   80.61%   81.17%   +0.55%     
==========================================
  Files          49       49              
  Lines        4121     4254     +133     
==========================================
+ Hits         3322     3453     +131     
- Misses        653      656       +3     
+ Partials      146      145       -1     

Flag	Coverage Δ
go	81.17% <93.59%> (+0.55%)	⬆️
wasm	67.55% <82.26%> (+0.65%)	⬆️

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.

[enobufs]

I've spent the past week debugging what's going on with throughput and stalling when sending a lot of small and large data through SCTP

What was the cause of stalling?

[enobufs]

Add locks to queues to avoid caught races

I see you added mutex to payloadQueue and reassemblyQueue. Were these really necessary when we already have mutex at Association (and also Stream) level? Mutexes are not computationally free. (Where were the data races?)

[edaniels]

I've spent the past week debugging what's going on with throughput and stalling when sending a lot of small and large data through SCTP

What was the cause of stalling?

Still undetermined and need a consistent repro but where I left off was the reassembly queue getting SSNs that are less than the ones it currently expects since too many messages were sent and the SSN overflowed.

[edaniels]

Add locks to queues to avoid caught races

I see you added mutex to payloadQueue and reassemblyQueue. Were these really necessary when we already have mutex at Association (and also Stream) level? Mutexes are not computationally free. (Where were the data races?)

Running with -race wold have these occasionally fail. Do you think I should go back and try to use the locks already in the association/streams? Although not computationally free, the I think the cost is similar to our use of sync.Cond/channels for the wait/signal/broadcast system on reads and writes.

[edaniels]

@enobufs, it seems like a good idea if I just split this PR up into more agreeable chunks so that we can discuss changes individually. I'll do that soon!

[edaniels]

@enobufs good call on the mutexes. Whatever messing around I was doing was causing races to happen but I cannot get them to reproduce, so I'm excluding those in other PRs!

[enobufs]

One minor comment I forgot to hit 'submit' ...

[enobufs]

As we are moving to go 1.19, we should use atomic.Bool.