Skip to content

Commit ebdb9ca

Browse files
committed
Perf and stability improve for lib/srv/regular
Parallelized and increased frequency of checks for many tests. Stabilized the following tests. * TestAllowedLabels * TestDirectTCPIP * TestLockInForce * TestTCPIPForward * TestAgentForward * TestX11Forward Original flaky behavior for the above tests can be reproduced using [stress](https://pkg.go.dev/golang.org/x/tools/cmd/stress) with the following command: ``` go test -o flaky.test ./... && \ stress -p 30 -failfast ./flaky.test -test.run="TestName" ``` The following tests still show up when running the full test suite for `lib/srv/regular` under `stress`. * TestMultipleExecCommands * TestSessionAuditLog
1 parent c64bb4c commit ebdb9ca

5 files changed

Lines changed: 325 additions & 115 deletions

File tree

lib/client/client.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,14 +931,26 @@ func (c *NodeClient) remoteListenAndForward(ctx context.Context, ln net.Listener
931931
"remote_addr", remoteAddr,
932932
)
933933
log.InfoContext(ctx, "Starting remote port forwarding")
934+
defer log.InfoContext(ctx, "Shutting down remote port forwarding", "error", ctx.Err())
934935

935-
for ctx.Err() == nil {
936+
for {
936937
conn, err := acceptWithContext(ctx, ln)
937938
if err != nil {
938-
if ctx.Err() == nil {
939+
switch {
940+
// Caller closed context to stop forwarding. For example the user
941+
// ran "tsh ssh -N -R" then hit Ctrl-C.
942+
case ctx.Err() != nil:
943+
return
944+
// The remote server closed the connection. For example, client_idle_timeout
945+
// was hit.
946+
case errors.Is(err, io.EOF):
947+
return
948+
// Accepting forwarded connection failed, but listener may still
949+
// accept. For example, a transient network issue.
950+
default:
939951
log.ErrorContext(ctx, "Remote port forwarding failed", "error", err)
952+
continue
940953
}
941-
continue
942954
}
943955

944956
go func() {
@@ -947,7 +959,6 @@ func (c *NodeClient) remoteListenAndForward(ctx context.Context, ln net.Listener
947959
}
948960
}()
949961
}
950-
log.InfoContext(ctx, "Shutting down remote port forwarding", "error", ctx.Err())
951962
}
952963

953964
// GetRemoteTerminalSize fetches the terminal size of a given SSH session.

lib/srv/regular/sshserver.go

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2460,20 +2460,49 @@ func (s *Server) handleTCPIPForwardRequest(ctx context.Context, ccx *sshutils.Co
24602460
if err != nil {
24612461
return trace.Wrap(err)
24622462
}
2463-
defer scx.Close()
24642463
listener, err := s.listenTCPIP(ctx, scx, scx.SrcAddr)
24652464
if err != nil {
2465+
if serr := scx.Close(); serr != nil {
2466+
s.logger.DebugContext(ctx, "Failed while cleaning up request",
2467+
"request_type", teleport.TCPIPForwardRequest,
2468+
"server_context_close_error", serr,
2469+
"error", err)
2470+
}
24662471
return trace.Wrap(err)
24672472
}
24682473

24692474
// If the client didn't request a specific port, the chosen port needs to
24702475
// be reported back.
2471-
srcHost, _, err := sshutils.SplitHostPort(scx.SrcAddr)
2476+
srcHost, srcPort, err := sshutils.SplitHostPort(scx.SrcAddr)
24722477
if err != nil {
2478+
if lerr := listener.Close(); lerr != nil {
2479+
s.logger.DebugContext(ctx, "Failed while cleaning up request",
2480+
"request_type", teleport.TCPIPForwardRequest,
2481+
"listener_close_error", lerr,
2482+
"error", err)
2483+
}
2484+
if serr := scx.Close(); serr != nil {
2485+
s.logger.DebugContext(ctx, "Failed while cleaning up request",
2486+
"request_type", teleport.TCPIPForwardRequest,
2487+
"server_context_close_error", serr,
2488+
"error", err)
2489+
}
24732490
return trace.Wrap(err)
24742491
}
24752492
_, listenPort, err := sshutils.SplitHostPort(listener.Addr().String())
24762493
if err != nil {
2494+
if lerr := listener.Close(); lerr != nil {
2495+
s.logger.DebugContext(ctx, "Failed while cleaning up request",
2496+
"request_type", teleport.TCPIPForwardRequest,
2497+
"listener_close_error", lerr,
2498+
"error", err)
2499+
}
2500+
if serr := scx.Close(); serr != nil {
2501+
s.logger.DebugContext(ctx, "Failed while cleaning up request",
2502+
"request_type", teleport.TCPIPForwardRequest,
2503+
"server_context_close_error", serr,
2504+
"error", err)
2505+
}
24772506
return trace.Wrap(err)
24782507
}
24792508
scx.SrcAddr = sshutils.JoinHostPort(srcHost, listenPort)
@@ -2483,6 +2512,7 @@ func (s *Server) handleTCPIPForwardRequest(ctx context.Context, ccx *sshutils.Co
24832512

24842513
// spawn remote forwarding handler to multiplex connections to the forwarded port
24852514
go func() {
2515+
defer scx.Close()
24862516
stopEvent := scx.GetPortForwardEvent(events.PortForwardRemoteEvent, events.PortForwardStopCode, scx.SrcAddr)
24872517
defer s.emitAuditEventWithLog(ctx, &stopEvent)
24882518

@@ -2525,6 +2555,7 @@ func (s *Server) handleTCPIPForwardRequest(ctx context.Context, ccx *sshutils.Co
25252555
logger.WarnContext(ctx, "failed to open channel", "error", err)
25262556
continue
25272557
}
2558+
ch = scx.TrackActivity(ch)
25282559
go ssh.DiscardRequests(rch)
25292560
go io.Copy(io.Discard, ch.Stderr())
25302561
go func() {
@@ -2548,11 +2579,7 @@ func (s *Server) handleTCPIPForwardRequest(ctx context.Context, ccx *sshutils.Co
25482579
// Report addr back to the client.
25492580
if r.WantReply {
25502581
var payload []byte
2551-
req, err := sshutils.ParseTCPIPForwardReq(r.Payload)
2552-
if err != nil {
2553-
return trace.Wrap(err)
2554-
}
2555-
if req.Port == 0 {
2582+
if srcPort == 0 {
25562583
payload = ssh.Marshal(struct {
25572584
Port uint32
25582585
}{Port: uint32(listener.Addr().(*net.TCPAddr).Port)})

0 commit comments

Comments
 (0)