Repository navigation
feat(sm): DialContext and DialTLSContext; DialNetworkTLS binds laddr - #76
Conversation
sm.Client could not abandon a dial: the timeout argument bounded only the transport connect, and the CER/CEA exchange ran until its retransmissions were exhausted. DialNetworkTLS also dropped its laddr argument, so the connection used an ephemeral source address. - DialContext and DialTLSContext bound the whole dial with a context: name resolution (SCTP through go-sctp's ResolveAddrContext), the transport connect, the TLS handshake and the CER/CEA exchange with its retransmissions (RFC 6733 §5.3). When the context ends, the unfinished connection is aborted (an SCTP association with ABORT, RFC 9260 §9.1, instead of waiting for a graceful shutdown), no watchdog starts, a CEA that has not yet been accepted is refused, and the error matches ctx.Err(). Once the dial has returned, the context no longer affects the connection. - diam.Server gains DialContext and DialTLSContext, and every existing dial function, in diam and in sm.Client, is a wrapper over the context-aware path with its previous timeout semantics: over TCP the timeout covers name resolution and the connect, over SCTP it starts after name resolution, and it never covers TLS negotiation or CER/CEA. - DialNetworkTLS passes laddr through. - go-sctp moves to da2f7fb, which adds ResolveAddrContext. Tests cancel during the TCP and SCTP CER wait, a blocked CER write, the TLS handshake, SCTP name resolution and a full-backlog TCP connect, and race a late CEA against cancellation; check deadlines, an already-cancelled context, that a successful dial survives cancellation, and that no goroutine is left behind; and check the bound source address for DialNetworkTLS and DialTLSExt. Fixes #71 Fixes #69
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The lint failure is not from this change: staticcheck 2026.2.1 cannot read the export data of Go 1.27.2, the stable release CI now installs, so it stopped before analysing any package. Under Go 1.27.2, a staticcheck built from go-tools master (f1838cc3, which carries the fix) and golangci-lint both report no issues on this branch. The workflow pin is updated in a separate change. |
Two issues on
sm.Client's dial path:timeoutargument bounded only the transport connect, and the CER/CEA exchange ran until its retransmissions were exhausted.DialNetworkTLSdropped itsladdrargument, so the connection used an ephemeral source address.Change
New context-aware dials.
sm.Client.DialContext(ctx, network, addr, laddr)andDialTLSContext(ctx, network, addr, certFile, keyFile, laddr)bound the whole dial with the context:ResolveAddrContext);When the context ends:
ctx.Err().Once the dial has returned, the context no longer affects the connection. One race is documented: if the CEA was accepted just before cancellation,
OnHandshakemay already have run.One dial path.
diam.ServergainsDialContextandDialTLSContext. Every existing dial function indiamandsm.Clientis a wrapper over the context-aware path, and keeps its previous timeout semantics:sm.Client.DialNetworkTLS ignores its laddr argument #69:
DialNetworkTLSpassesladdrthrough.Dependency: go-sctp moves to
da2f7fb, which addsResolveAddrContext.Tests
smanddiam.Server);DialNetworkTLSandDialTLSExt.DialNetworkTLSwas seen on the wrong source port;OnHandshakeafter cancellation.-tags sctpblackhole, which needs a privileged container; see the README) removes the peer's address after the CER. On the old code, cancellation returned after 3.0 s of graceful shutdown. With this change it returns within about 0.2 ms, for CER and TLS over SCTP alike, and tshark shows ABORT with no SHUTDOWN wait.Resetremoved;Validation
Under Go 1.27.2:
go build ./...,go test ./... -count=1,go test -race ./diam/... -count=1andgo vet ./...(also with-tags sctpblackhole) pass;golangci-lint run ./...report no issues, andgofmt -l .is empty;examples/middlewarebuild, vet and test pass;Fixes #71
Fixes #69
Summary by cubic
Adds context-aware dialing to
sm.Clientanddiam.Server, so a dial can be cancelled or time out during transport connect, TLS handshake, and CER/CEA exchange instead of waiting for retransmissions to exhaust.sm.Client.DialContext(ctx, network, addr, laddr)andDialTLSContextbound the whole dial; on cancellation, an unfinished SCTP association is aborted (RFC 9260 §9.1) rather than closed gracefully, no watchdog starts, a CEA not yet accepted is refused, and the error matchesctx.Err().OnHandshakemay still run.diam.ServergainsDialContextandDialTLSContext; all existing dial functions indiamandsm.Clientare wrappers over the context-aware path, keeping their previous timeout semantics (TCP: name resolution and connect; SCTP: after name resolution; never TLS or CER/CEA).DialNetworkTLSnow passesladdrthrough, fixing the dropped source address (issue sm.Client.DialNetworkTLS ignores its laddr argument #69).go-sctpis bumped to a version that addsResolveAddrContext, covering SCTP name resolution with cancellation.sctpblackholeharness that verifies an abort takes about 0.2 ms against an unresponsive SCTP peer.Written for commit 4eef779. Summary will update on new commits.