connector, mux: close four network-management gaps - #28
Merged
Merged
Conversation
Found while tracing ISW/ZONE link instability in a downstream acquirer. Each gap forced a workaround that could not fully fix the problem from outside the library. Reply to an unsolicited frame. Unsolicited handed over the frame and nothing else, and neither Mux nor Connector exposed a raw Send — so a peer-initiated message could not be answered at all. Postilion-family hosts do not only answer echoes, they send them, and a host that echo-tests us and never receives an 0810 concludes the link is dead and tears it down. Adds mux.Reply, WithUnsolicitedReplier, (*Mux).Send, (*Connector).Send and Config.OnUnsolicited. Direction-blind Keyer. One keyer served both directions, and with a single function no keyer can match a response to its request and also separate a peer's request from our own — the two are structurally identical. Network management correlates on the trace alone, so a peer 0800 on a colliding trace was delivered as our 0810 while the real answer arrived unmatched. Adds WithResponseKeyer / Config.ResponseKeyer: the request keyer maps an outgoing frame to the key of the response it expects, the response keyer maps an inbound frame by its own MTI. Keepalive inheriting the request timeout. A peer that dies cleanly unblocks the pending keepalive through the mux, so that case was always handled. The gap is the half-open peer — TCP established, application silent, no FIN — where nothing closes the mux and the echo waits out a timeout sized for a financial round-trip, while the connector goes on reporting Connected() and accepting traffic onto a dead socket. Adds Config.KeepaliveTimeout, bounding one keepalive independently. serve also runs the keepalive off the supervise loop, which does not shrink that window but stops an outstanding keepalive holding Stop hostage. No hook after the mux is published. OnConnect runs before the mux is stored, which is right for sign-on but leaves nowhere for a ceremony that must go through the connector's own request path — it fails ErrNotConnected. Adds Config.OnReady. All additive: existing configurations behave exactly as before. Six tests, including one that fails without KeepaliveTimeout and one documenting why OnReady exists.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Found while tracing ISW/ZONE link instability in a downstream acquirer. Each gap forced a workaround that could not fully fix the problem from outside the library. All four changes are additive — an existing configuration behaves exactly as before.
1. No way to reply to an unsolicited frame
Unsolicitedhanded over the frame and nothing else, and neitherMuxnorConnectorexposed a rawSend, so a peer-initiated message could not be answered at all.Requestis the wrong tool — the frame we would send is the response, so there is nothing to wait for.This matters because Postilion-family hosts do not only answer echoes, they send them: a host that echo-tests us and never receives an 0810 concludes the link is dead and closes it, which is indistinguishable from a flapping network.
Adds
mux.Reply,mux.WithUnsolicitedReplier,(*Mux).Send,(*Connector).Send,connector.Config.OnUnsolicited.2.
Keyeris direction-blindOne keyer served both directions. With a single function no keyer can match a response to its request and separate a peer's request from our own — the two are structurally identical. Network management correlates on the trace alone (an echo carries no terminal id), so a peer 0800 on a colliding trace was delivered as our 0810 while the real answer arrived unmatched and was dropped.
Adds
mux.WithResponseKeyer/connector.Config.ResponseKeyer: the request keyer maps an outgoing frame to the key of the response it expects, the response keyer maps an inbound frame by its own MTI. Our 0800 registers under0810:<trace>, the real 0810 matches, and a peer's 0800 keys to0800:<trace>— no match, so it reaches the unsolicited handler where it belongs.3. A keepalive inherited the request timeout
Worth stating precisely, because the obvious diagnosis is wrong: a peer that dies cleanly (EOF/RST) fails the mux from the read loop, which unblocks the pending keepalive immediately — the synchronous
serveloop handled that case fine.The gap is the half-open peer — TCP established, application silent, no FIN (a NAT idle-out, a wedged host thread). Nothing closes the mux, so the echo waits out
Timeout, a value sized for a financial round-trip. Until it gives up the connector goes on reportingConnected()and accepting requests onto a socket that will never answer: a window ofKeepaliveInterval + Timeout.Adds
connector.Config.KeepaliveTimeout, bounding one keepalive independently (0 falls back toTimeout).servealso runs the keepalive off the supervise loop — that does not shrink the half-open window, but it stops an outstanding keepalive holdingStophostage.4. No hook after the mux is published
OnConnectruns beforec.mux.Store(m). That ordering is right for sign-on — the session must exist before the link is usable — but it leaves nowhere for a ceremony that must go through the connector's own request path (a working-key exchange, a parameter download): fromOnConnectit failsErrNotConnected.Adds
connector.Config.OnReady, invoked in its own goroutine right after the mux is published. UnlikeOnConnectit cannot drop the link — its failure is the caller's to handle.Tests
Six, in
connector/netmgmt_test.go, over a small line-framed peer:Sendon a down link returnsErrNotConnectedResponseKeyer, a peer request on our trace reaches the unsolicited handler and our own request still times outKeepaliveTimeoutStopOnReadycan use the connector;OnConnectcannot (documents whyOnReadyexists)go vet,gofmtand the full suite are clean.