Skip to content

Commit 231772c

Browse files
ejohnstownphilljj
authored andcommitted
wolfsshd: stop polling when the SFTP channel has nothing buffered
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.
1 parent ad059d7 commit 231772c

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

apps/wolfsshd/test/run_all_sshd_tests.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ test_cases=(
88
"sshd_term_size_test.sh"
99
"sshd_large_sftp_test.sh"
1010
"sshd_bad_sftp_test.sh"
11+
"sshd_sftp_idle_cpu_test.sh"
1112
"sshd_scp_fail.sh"
1213
"sshd_term_close_test.sh"
1314
"ssh_kex_algos.sh"
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/bin/sh
2+
3+
# An idle SFTP session must not keep the server busy. wolfSSHd's SFTP loop
4+
# polls at the select() floor whenever wolfSSH_stream_peek() reports an empty
5+
# channel, so before the timeout was raised on that path a connected client
6+
# that simply sat there cost most of a core for as long as it stayed open.
7+
8+
ROOT_PWD=$(pwd)
9+
. ./wolfssh_options.sh
10+
cd ../../..
11+
12+
TEST_SFTP_CLIENT="./examples/sftpclient/wolfsftp"
13+
PRIVATE_KEY="./keys/hansel-key-ecc.der"
14+
PUBLIC_KEY="./keys/hansel-key-ecc.pub"
15+
16+
SAMPLE_SECONDS=10
17+
# Ticks of CPU the connection process may use while idle.
18+
# The poll this guards against measured 16 per 10 seconds; a server that
19+
# waits properly measures 0.
20+
MAX_TICKS=5
21+
22+
FIFO="/tmp/wolfssh_idle_stdin_$$"
23+
HOLDER=""
24+
CLIENT=""
25+
26+
cleanup() {
27+
[ -n "$CLIENT" ] && kill "$CLIENT" 2>/dev/null
28+
[ -n "$HOLDER" ] && kill "$HOLDER" 2>/dev/null
29+
rm -f "$FIFO"
30+
}
31+
32+
# tear down on every exit path, including an interrupt during the ten second
33+
# measurement, so no FIFO or client is left behind
34+
trap cleanup EXIT
35+
36+
if [ -z "$1" ] || [ -z "$2" ] || [ -z "$3" ]; then
37+
echo "expecting host, port and user as arguments"
38+
echo "$0 127.0.0.1 22222 $USER"
39+
exit 1
40+
fi
41+
42+
if ! wolfssh_has SFTP || [ ! -x "$TEST_SFTP_CLIENT" ]; then
43+
echo "SFTP client not available in this build, skipping"
44+
exit 77
45+
fi
46+
47+
# The measurement reads the server's CPU time out of /proc, so it only works
48+
# against a local daemon on a system that has one.
49+
if [ ! -r /proc/self/stat ]; then
50+
echo "no /proc on this system, skipping"
51+
exit 77
52+
fi
53+
54+
PIDS_BEFORE=$(pgrep wolfsshd | sort)
55+
if [ -z "$PIDS_BEFORE" ]; then
56+
echo "no local wolfsshd to measure, skipping"
57+
exit 77
58+
fi
59+
60+
# Hold a session open without sending a single request. The client takes its
61+
# commands from this pipe, and nothing ever writes one; the sleep bounds how
62+
# long the pipe stays open so no part of this outlives the test.
63+
mkfifo "$FIFO" || exit 1
64+
sleep $((SAMPLE_SECONDS + 30)) > "$FIFO" &
65+
HOLDER=$!
66+
"$TEST_SFTP_CLIENT" -u "$3" -i "$PRIVATE_KEY" -j "$PUBLIC_KEY" \
67+
-h "$1" -p "$2" < "$FIFO" > /dev/null 2>&1 &
68+
CLIENT=$!
69+
sleep 5
70+
71+
PIDS_AFTER=$(pgrep wolfsshd | sort)
72+
CHILD=$(printf '%s\n%s\n' "$PIDS_BEFORE" "$PIDS_AFTER" | sort | uniq -u | head -1)
73+
if [ -z "$CHILD" ] || [ ! -r "/proc/$CHILD/stat" ]; then
74+
echo "Expecting another wolfSSHd pid after connection"
75+
exit 1
76+
fi
77+
78+
# Fields 14 and 15 of /proc/<pid>/stat are utime and stime, in clock ticks.
79+
BEFORE=$(awk '{print $14+$15}' "/proc/$CHILD/stat")
80+
sleep "$SAMPLE_SECONDS"
81+
if [ ! -r "/proc/$CHILD/stat" ]; then
82+
echo "Connection process $CHILD exited while idle"
83+
exit 1
84+
fi
85+
AFTER=$(awk '{print $14+$15}' "/proc/$CHILD/stat")
86+
USED=$((AFTER-BEFORE))
87+
88+
echo "idle connection used $USED ticks over $SAMPLE_SECONDS seconds"
89+
if [ "$USED" -ge "$MAX_TICKS" ]; then
90+
echo "Expecting an idle SFTP session to cost under $MAX_TICKS ticks"
91+
exit 1
92+
fi
93+
94+
cd "$ROOT_PWD"
95+
exit 0

apps/wolfsshd/wolfsshd.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -967,6 +967,12 @@ static int SFTP_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
967967
if (error == WS_EOF)
968968
break;
969969
}
970+
else {
971+
/* Channel is live with nothing buffered. Let the next select
972+
* block instead of polling at its 100 us floor. */
973+
timeout = TEST_SFTP_TIMEOUT;
974+
continue;
975+
}
970976

971977
if (ret == WS_FATAL_ERROR && error == 0) {
972978
WOLFSSH_CHANNEL* channel =

0 commit comments

Comments
 (0)