ip/tcp: propagate peer close to spliced loopback writer#1
Open
rafael2knokia wants to merge 2 commits into
Open
ip/tcp: propagate peer close to spliced loopback writer#1rafael2knokia wants to merge 2 commits into
rafael2knokia wants to merge 2 commits into
Conversation
When two TCP conversations on the same kernel connect to each other on loopback, tcpincoming() splices them via tcpsplice(), installing tcpbypass() on each side's wq to copy blocks directly into the other side's rq. When one side closes, tcpsetstate(Closed) clears its own bypass but leaves the peer's wq with a kick that still points at us; the next user write reaches tcpbypass(), sees the missing peer and silently drops the block, and qbwrite() returns blocklen(b). The writer therefore sees writes "succeeding" forever and never observes the peer close. Hang up the peer's wq from the Closed arm of tcpsetstate() so its next user write returns "connection closed" via qbwrite() instead of running the now-stale bypass. Reproducer: misc/plan9/arm64/loopback-close-probe.go in the Go plan9-arm64 port (https://go-review.googlesource.com/c/go/+/719643) runs three scenarios and reports the writer Write() failing within a few milliseconds after the peer's Close(), as on Linux and the BSDs, instead of accepting 1 GiB of dropped data. The same bug should exist in 9legacy since the splice logic predates the fork.
c95f856 ("ip/tcp: propagate peer close to spliced loopback writer") clears the surviving end's bypass pointer and hangs up its wq so that a write into a now-dead splice partner fails fast. However, when that surviving end later runs tcpclose() the Established arm sees bypass==nil, falls through to the Syn_received clause and calls tcpoutput(), which sends a FIN to the peer's old (l, r)addr/port tuple. The peer is already iphtrem'd, so iphtlook() fails and we sndrst() back. Normally the RST harmlessly dies in iphtlook() too, but under SMP it can race against a concurrent active connect from another conv that just happens to be assigned the same ephemeral lport (1/32768 chance), and that fresh conv suddenly observes Econrefused on its first read - visible in Go's net.TestVariousDeadlines as a "Copy = 0, <nil>; want timeout" flake at sub-millisecond deadlines. Mark the surviving end with a new Tcpctl.bypeerclosed flag from tcpsetstate(Closed) before clearing its bypass (the order matters so that on TSO a tcpclose() seeing bypass==nil is guaranteed to also see bypeerclosed==1), and teach tcpclose() to localclose() locally instead of sending the doomed FIN. Verified with the new misc/plan9/amd64/guest-deadline-stress.rc harness in the Go plan9-arm64 port: 30 -count iterations of net.TestVariousDeadlines (= 1710 deadline cycles) on -smp 4 pass cleanly, where the previous patched kernel reproduced 5-6 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When two TCP conversations on the same kernel connect to each other on loopback, tcpincoming() splices them via tcpsplice(), installing tcpbypass() on each side's wq to copy blocks directly into the other side's rq. When one side closes, tcpsetstate(Closed) clears its own bypass but leaves the peer's wq with a kick that still points at us; the next user write reaches tcpbypass(), sees the missing peer and silently drops the block, and qbwrite() returns blocklen(b). The writer therefore sees writes "succeeding" forever and never observes the peer close.
Hang up the peer's wq from the Closed arm of tcpsetstate() so its next user write returns "connection closed" via qbwrite() instead of running the now-stale bypass.
Reproducer: misc/plan9/arm64/loopback-close-probe.go in the Go plan9-arm64 port (https://go-review.googlesource.com/c/go/+/719643) runs three scenarios and reports the writer Write() failing within a few milliseconds after the peer's Close(), as on Linux and the BSDs, instead of accepting 1 GiB of dropped data.
The same bug should exist in 9legacy since the splice logic predates the fork.