Skip to content

wolfsshd: stop polling when the SFTP channel has nothing buffered - #1207

Merged
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:ccb-phase1-3
Aug 29, 2026
Merged

wolfsshd: stop polling when the SFTP channel has nothing buffered#1207
philljj merged 1 commit into
wolfSSL:masterfrom
ejohnstown:ccb-phase1-3

Conversation

@ejohnstown

@ejohnstown ejohnstown commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

wolfSSH_stream_peek() returns 0 for a live channel with an empty buffer, which is the ordinary idle case. None of the arms after the peek match that, so the loop falls through with the timeout still at TEST_SFTP_TIMEOUT_NONE and tcp_select() returns on its 100 us floor. An idle SFTP session keeps a core busy for as long as it stays connected.

  • take the peek's zero return as "nothing to do" and let the next select wait a second, the same value the want-read paths already use
  • sshd_sftp_idle_cpu_test.sh parks an idle SFTP session on the daemon and reads the connection process's CPU time out of /proc, failing if it spends 5 ticks or more over ten seconds

The measurement the test automates: 21 ticks per 10 seconds before, 0 after. It skips where there is no /proc or no local daemon to measure,so it is a no-op outside Linux.

Copilot AI lite review requested due to automatic review settings August 27, 2026 23:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a busy-polling condition in wolfsshd’s SFTP subsystem loop by treating a wolfSSH_stream_peek() return of 0 as an idle/no-buffered-data state, allowing select() to block for a normal timeout instead of repeatedly hitting its 100µs floor. It also adds a regression test that verifies an idle SFTP session does not consume significant CPU time on Linux.

Changes:

  • Update the SFTP subsystem loop to set a 1-second timeout and continue when wolfSSH_stream_peek() returns 0 (idle channel with empty buffer).
  • Add a Linux-only test script that measures the connection process CPU ticks via /proc/<pid>/stat during an idle SFTP session.
  • Register the new test in run_all_sshd_tests.sh.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
apps/wolfsshd/wolfsshd.c Avoids tight polling by treating stream_peek()==0 as idle and increasing the next select() timeout.
apps/wolfsshd/test/sshd_sftp_idle_cpu_test.sh Adds a regression test to detect excessive CPU usage during idle SFTP sessions.
apps/wolfsshd/test/run_all_sshd_tests.sh Includes the new idle-CPU regression test in the sshd test suite.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/wolfsshd/test/sshd_sftp_idle_cpu_test.sh
Comment thread apps/wolfsshd/test/sshd_sftp_idle_cpu_test.sh
wolfSSH_stream_peek() returns 0 for a live channel with an empty buffer,
which is the ordinary idle case. None of the arms after the peek match
that, so the loop falls through with the timeout still at
TEST_SFTP_TIMEOUT_NONE and tcp_select() returns on its 100 us floor. An
idle SFTP session keeps a core busy for as long as it stays connected.

- take the peek's zero return as "nothing to do" and let the next select
  wait a second, the same value the want-read paths already use
- sshd_sftp_idle_cpu_test.sh parks an idle SFTP session on the daemon and
  reads the connection process's CPU time out of /proc, failing if it
  spends 5 ticks or more over ten seconds

The measurement the test automates: 21 ticks per 10 seconds before, 0
after. It skips where there is no /proc or no local daemon to measure.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1207

Scan targets checked: wolfssh-bugs, wolfssh-src

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

@ejohnstown
ejohnstown requested a review from philljj August 28, 2026 22:31
@philljj philljj self-assigned this Aug 28, 2026
@philljj
philljj merged commit 231772c into wolfSSL:master Aug 29, 2026
166 checks passed
@ejohnstown
ejohnstown deleted the ccb-phase1-3 branch August 29, 2026 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants