Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
184 changes: 184 additions & 0 deletions .github/workflows/sftp-benchmark.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
name: Benchmark Test

on:
push:
branches: [ '*' ]
pull_request:
branches: [ '*' ]

# give permissions to write a comment on the pull request
permissions:
pull-requests: write
actions: read
contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
build_wolfssl:
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]
wolfssl: [ master ]
name: Build wolfssl
runs-on: ${{ matrix.os }}
timeout-minutes: 4
steps:
- name: Checking cache for wolfssl
uses: actions/cache@v4
id: cache-wolfssl
with:
path: build-dir/
key: wolfssh-benchmark-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }}
lookup-only: true

- name: Checkout, build, and install wolfssl
if: steps.cache-wolfssl.outputs.cache-hit != 'true'
uses: wolfSSL/actions-build-autotools-project@v1
with:
repository: wolfssl/wolfssl
ref: ${{ matrix.wolfssl }}
path: wolfssl
configure: --enable-all --enable-intelasm --enable-sp --enable-sp-asm
check: false
install: true

build_wolfssh:
needs:
- build_wolfssl
strategy:
fail-fast: false
matrix:
os: [ ubuntu-latest ]
wolfssl: [ master ]
name: Collect SFTP performance
runs-on: ${{ matrix.os }}
timeout-minutes: 15
steps:
- name: Checking cache for wolfssl
uses: actions/cache@v4
with:
path: build-dir/
key: wolfssh-benchmark-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }}
fail-on-cache-miss: true

- uses: actions/checkout@v4
with:
path: wolfssh/

- name: autogen
working-directory: ./wolfssh/
run: ./autogen.sh

- name: configure
working-directory: ./wolfssh/
run : |
./configure --enable-all LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DWOLFSSH_NO_FPKI -DWOLFSSH_NO_SFTP_TIMEOUT -DWOLFSSH_MAX_SFTP_RW=4000000 -DMAX_PATH_SZ=120 -DEXAMPLE_SFTP_BENCHMARK"

- name: make
working-directory: ./wolfssh/
run: make

- name: Get Saved OpenSSH Upload Results
id: cache-upload
uses: actions/cache/restore@v4
with:
path: wolfssh/openssh-average-upload.csv
key: openssh-average-upload.csv

- name: Get Saved OpenSSH Download Results
id: cache-download
uses: actions/cache/restore@v4
with:
path: wolfssh/openssh-average-download.csv
key: openssh-average-download.csv

- name: Install gnuplot
run: sudo apt-get install gnuplot

- name: Setup OpenSSH Test Server
working-directory: ./wolfssh/
run: |
sudo apt-get install openssh-server
mkdir ~/.ssh
chmod 700 ~/.ssh
echo "AuthorizedKeysFile $PWD/keys/hansel-key-ecc.pub" >> sshd-config-test.txt
echo "PubkeyAuthentication yes" >> sshd-config-test.txt
echo "Subsystem sftp internal-sftp" >> sshd-config-test.txt
echo "KbdInteractiveAuthentication no" >> sshd-config-test.txt
echo "PasswordAuthentication no" >> sshd-config-test.txt
echo "StrictModes no" >> sshd-config-test.txt
sed -i.bak "s/hansel/$USER/" ./keys/hansel-key-ecc.pub
chmod 600 ./keys/hansel-key-*.pem
chmod 600 ./keys/hansel-key-*.pub
sudo mkdir -p /run/sshd
sudo chmod 755 /run/sshd
sudo /usr/sbin/sshd -p 22222 -f sshd-config-test.txt -E $PWD/sshd-log.txt
cat sshd-config-test.txt
ps -e | grep sshd

- name: Run SFTP client benchmark
working-directory: ./wolfssh/
timeout-minutes: 8
run: |
./scripts/get-sftp-benchmark.sh 22222

- name: Save OpenSSH Upload Results
if: always() && steps.cache-upload.outputs.cache-hit != 'true' && hashFiles('wolfssh/openssh-average-upload.csv') != ''
uses: actions/cache/save@v4
with:
path: wolfssh/openssh-average-upload.csv
key: openssh-average-upload.csv

- name: Save OpenSSH Download Results
if: always() && steps.cache-download.outputs.cache-hit != 'true' && hashFiles('wolfssh/openssh-average-download.csv') != ''
uses: actions/cache/save@v4
with:
path: wolfssh/openssh-average-download.csv
key: openssh-average-download.csv

- name: Store Upload Speed PNG
uses: actions/upload-artifact@v4
with:
name: upload-results-pr${{ github.event.pull_request.number }}.png
path: wolfssh/upload-results.png
retention-days: 2

- name: Store Download Speed PNG
uses: actions/upload-artifact@v4
with:
name: download-results-pr${{ github.event.pull_request.number }}.png
path: wolfssh/download-results.png
retention-days: 2

# Currently the comment in PR does not work correctly
# - name: Comment on PR about performance
# env:
# GITHUB_URL: ${{ github.event.pull_request.comments_url }}
# GH_TOKEN: ${{ github.token }}
# PR_NUMBER: ${{ github.event.pull_request.number }}
# RUN_ID: ${{ github.run_id }}
# run: |
# # Get both artifact IDs
# DOWNLOAD_ARTIFACT=$(gh api repos/${{ github.repository }}/actions/artifacts \
# --jq '.artifacts[] | select(.name | contains("download-results-pr")) | .id')
# UPLOAD_ARTIFACT=$(gh api repos/${{ github.repository }}/actions/artifacts \
# --jq '.artifacts[] | select(.name | contains("upload-results-pr")) | .id')
#
# # Create the comment with direct link to download
# curl -X POST \
# $GITHUB_URL \
# -H "Content-Type: application/json" \
# -H "Authorization: token $GH_TOKEN" \
# -d "{\"body\":\"Performance test results:\n\n- [Download Results](https://github.com/${{ github.repository }}/actions/runs/$RUN_ID/artifacts/$DOWNLOAD_ARTIFACT)\n- [Upload Results](https://github.com/${{ github.repository }}/actions/runs/$RUN_ID/artifacts/$UPLOAD_ARTIFACT)\"}"

- name: Print logs if failed
working-directory: ./wolfssh/
if: failure()
run: |
sudo cat sshd-log.txt
cat log.csv

45 changes: 45 additions & 0 deletions examples/sftpclient/sftpclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,21 @@ static void err_msg(const char* s)
gettimeofday(&tv, 0);
return (word32)tv.tv_sec;
}

#ifdef EXAMPLE_SFTP_BENCHMARK
double current_time_ms(int);

/* return number of micro seconds */
double current_time_ms(int reset)
{
struct timeval tv;

(void)reset;

gettimeofday(&tv, 0);
return (word64)(tv.tv_sec*1000000) + tv.tv_usec;
}
#endif
#endif /* USE_WINDOWS_API */
#endif /* !WOLFSSH_NO_TIMESTAMP */

Expand Down Expand Up @@ -1505,6 +1520,13 @@ static int doAutopilot(int cmd, char* local, char* remote)
WS_SFTPNAME* name = NULL;
word32 remoteSz;
byte remoteAbsPath = 0;
#if !defined(WOLFSSH_NO_TIMESTAMP) && !defined(USE_WINDOWS_API) &&\
defined(EXAMPLE_SFTP_BENCHMARK)
double currentTime;
double longBytes = 0;
FILE* f;
#endif


/* check if is absolute path before making it one */
if (remote != NULL && WSTRLEN(remote) > 2 && remote[1] == ':' &&
Expand Down Expand Up @@ -1533,6 +1555,19 @@ static int doAutopilot(int cmd, char* local, char* remote)
return WS_FATAL_ERROR;
}

#if !defined(WOLFSSH_NO_TIMESTAMP) && !defined(USE_WINDOWS_API) &&\
defined(EXAMPLE_SFTP_BENCHMARK)
ret = WFOPEN(NULL, &f, fullpath, "rb");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Benchmark sizes the remote path via local fopen instead of the transferred source file · Logic errors

fullpath is the remote SFTP path, but it is opened on the local filesystem. For PUT the size comes from the remote destination and not from local. Any remote path that does not also exist locally returns WS_BAD_FILE_E and aborts the transfer. The benchmark only works on loopback with a pre-copied file.

Suggested fix: For PUT, take the size from the local source file local. For GET, take it after the transfer from local or from the remote file attributes. Never fail the transfer because the size lookup failed.

if (ret != 0 || f == WBADFILE) return WS_BAD_FILE_E;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Early returns in benchmark block leak the SFTP name list from doBuildRemotePath · Resource leaks on error paths

When doBuildRemotePath has allocated name, the new return WS_BAD_FILE_E paths (open failure and seek failure) skip wolfSSH_SFTPNAME_list_free(name), which leaks the list.

Suggested fix: Free name with wolfSSH_SFTPNAME_list_free before each new early return, or send these failures to a common cleanup path.

Related known findings (similar but distinct; listed for context, not part of this finding)

  • F-10551 (open): File/function: candidate is in sftpclient.c/doAutopilot vs. F-10551 in wolfsftp.c/wolfSSH_SFTP_RecvOpenDir — different files and functions. Faulting operation: both are early error-return paths that skip freeing/unlinking an already-allocated list-type resource (SFTP name list vs. directory-list node) before returning an error code, so the general pattern is closely related. Root cause: 'new/existing error return bypasses required cleanup for a previously allocated resource' is shared, though the

if (WFSEEK(NULL, f, 0, WSEEK_END) != 0) {
WFCLOSE(NULL, f);
return WS_BAD_FILE_E;
}
longBytes = (word32)WFTELL(NULL, f);
WREWIND(NULL, f);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FILE handle opened for size measurement is never closed on the success path · Resource leaks

After WFTELL/WREWIND, f is never closed with WFCLOSE. Each benchmarked autopilot transfer leaks one open FILE stream and its file descriptor.

Suggested fix: Call WFCLOSE(NULL, f) right after the size is read, since the stream is not used again.
Basis: C11 7.21.5.1: fclose flushes and closes the stream and releases its resources; an unclosed stream is only released at program exit.

currentTime = current_time_ms(0);
#endif

do {
if (err == WS_REKEYING || err == WS_WINDOW_FULL) { /* handle rekeying and window-full state */
do {
Expand Down Expand Up @@ -1579,6 +1614,16 @@ static int doAutopilot(int cmd, char* local, char* remote)
fullpath, local);
}
}
#if !defined(WOLFSSH_NO_TIMESTAMP) && !defined(USE_WINDOWS_API) &&\
defined(EXAMPLE_SFTP_BENCHMARK)
else {
currentTime = current_time_ms(0) - currentTime;
double result;
result = (double)longBytes / 1000000;
result = result / ((double)currentTime / 1000000);
printf("Transferred %s at %.2fMB/s\n", fullpath, result);
}
#endif

wolfSSH_SFTPNAME_list_free(name);
return ret;
Expand Down
Loading
Loading