Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for parallel read and write #188

Merged
merged 18 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
68882c0
Remove bullseye platform from CI
kp-mariappan-ramasamy Nov 26, 2024
d6074d9
Pin ceedling to 0.31.1 since 1.0.0 has breaking changes
kp-mariappan-ramasamy Jan 2, 2025
ec9cdb7
Disable single threaded optimisations for wolfssl
kp-mariappan-ramasamy Nov 27, 2024
6fae65b
Enable threaded crypt in wolfssl
kp-mariappan-ramasamy Nov 27, 2024
aabb2ab
Make frag id as atomic variable
kp-mariappan-ramasamy Nov 26, 2024
c7f4d45
Make session id atomic for he_internal_update_session_incoming
kp-mariappan-ramasamy Nov 26, 2024
bc843ad
Make nudge threadsafe
kp-mariappan-ramasamy Nov 27, 2024
7c081d0
Make first message received thread safe
kp-mariappan-ramasamy Nov 26, 2024
a16e914
Handle wolf error inside conn
kp-mariappan-ramasamy Nov 26, 2024
8481da6
Make write buffer local and threadsafe
kp-mariappan-ramasamy Nov 26, 2024
cd72bf7
Remove read_packet from he_conn to make it threadsafe
kp-mariappan-ramasamy Jan 2, 2025
b36de33
Reorg variables for clarity
kp-mariappan-ramasamy Jan 2, 2025
6391165
Refactor pmtud variables in struct
kp-mariappan-ramasamy Jan 3, 2025
f95e824
Remove pmtud struct access from outside pmtud module
kp-mariappan-ramasamy Jan 3, 2025
4dfb75c
Instead of checking pmtud state everywhere, use default max value
kp-mariappan-ramasamy Jan 3, 2025
76f1a0a
Use thread local for read buffer
kp-mariappan-ramasamy Nov 26, 2024
fc6b87f
Enable multi threaded test in CI
kp-mariappan-ramasamy Nov 26, 2024
e25cf1a
Windows CI reusing is messing up compilation
kp-mariappan-ramasamy Jan 7, 2025
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
43 changes: 29 additions & 14 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-22.04
strategy:
matrix:
distro: [bullseye, bookworm]
distro: [bookworm]
env:
FORCE_COLOR: 1
steps:
Expand All @@ -37,7 +37,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Build dependencies
run: ceedling project:linux verbosity[4] clobber dependencies:make
- name: Run wolfSSL Tests
Expand All @@ -46,12 +46,26 @@ jobs:
./wolfcrypt/test/testwolfcrypt
- name: Run build and test
run: ceedling project:linux verbosity[4] test:all
linux-multithread:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Build dependencies
run: ceedling project:linux_multithread verbosity[4] clobber dependencies:make
- name: Run wolfSSL Tests
run: |
cd third_party/wolfssl
./wolfcrypt/test/testwolfcrypt
- name: Run build and test
run: ceedling project:linux_multithread verbosity[4] test:all
linux-386:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install gcc multi lib
run: |
sudo apt update
Expand All @@ -69,7 +83,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install ARM Tools
run: |
sudo apt update
Expand All @@ -87,7 +101,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install ARM Tools
run: |
sudo apt update
Expand All @@ -105,7 +119,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install ARM Tools
run: |
sudo apt update
Expand All @@ -123,7 +137,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install RISC-V Toolchain
run: |
sudo apt update
Expand All @@ -141,7 +155,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install automake
run: HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake libtool
- name: Build dependencies
Expand All @@ -153,7 +167,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install automake
run: HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake libtool
- name: Build dependencies
Expand All @@ -168,20 +182,21 @@ jobs:
config:
[
{ project: windows_64, arch: x64 },
{ project: windows_64_multithread, arch: x64 },
{ project: windows_32, arch: x86 },
{ project: windows_arm64, arch: amd64_arm64 },
]
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: gem install ceedling --no-user-install
run: gem install ceedling -v 0.31.1 --no-user-install
- name: Check Ceedling version
run: ceedling version
- name: Set up Visual Studio shell
uses: TheMrMilchmann/setup-msvc-dev@v3
with:
arch: ${{ matrix.config.arch }}
- if: ${{ matrix.config.project != 'windows_arm64' }}
- if: ${{ matrix.config.project != 'windows_arm64' }}
# Skip making dependencies for ARM64 as we can only apply git patch once for WolfSSL
name: Build dependencies
run: ceedling project:${{ matrix.config.project }} verbosity[4] clobber dependencies:make
Expand All @@ -196,7 +211,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install automake
run: HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake libtool
- name: Run build
Expand All @@ -212,7 +227,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Install automake
run: HOMEBREW_NO_AUTO_UPDATE=1 brew install autoconf automake libtool
- name: Run build
Expand All @@ -226,7 +241,7 @@ jobs:
steps:
- uses: actions/checkout@v4
- name: Install Ceedling
run: sudo gem install ceedling --no-user-install
run: sudo gem install ceedling -v 0.31.1 --no-user-install
- name: Run build
run: |
source android/android_env.sh ${{ matrix.arch }}
Expand Down
2 changes: 1 addition & 1 deletion 3rd_party_deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
--enable-dtls-frag-ch
--enable-dtls-mtu
--enable-secure-renegotiation
--enable-singlethreaded
--disable-singlethreaded
--enable-sni
--enable-sp=yes,4096
--enable-static
Expand Down
2 changes: 1 addition & 1 deletion Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ debian-deps:
RUN apt-get -y install --no-install-recommends build-essential git automake m4 libtool-bin cmake ruby-full python3-pip
# Not including colrm seems to give an error when configuring wolfssl
RUN apt-get -y install --no-install-recommends bsdmainutils
RUN gem install ceedling --no-user-install
RUN gem install ceedling -v 0.31.1 --no-user-install
RUN apt-get -y install --no-install-recommends gcovr

libhelium-deps:
Expand Down
12 changes: 12 additions & 0 deletions linux_multithread.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
--- # ceedling project file for Linux
:import:
- linux.yml

:defines:
:test:
- HE_ENABLE_MULTITHREADED

:tools_test_linker:
:arguments:
- -latomic

31 changes: 25 additions & 6 deletions src/he/conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ he_return_code_t he_internal_send_message(he_conn_t *conn, uint8_t *message, uin
if(res == 0) {
return HE_ERR_CONNECTION_WAS_CLOSED;
} else {
conn->wolf_error = error;
he_conn_set_ssl_error(conn, error);
return HE_ERR_SSL_ERROR;
}
}
Expand Down Expand Up @@ -536,7 +536,7 @@ bool he_internal_is_valid_state_for_server_config(he_conn_t *conn) {
}

// The server config message is only valid after the TLS link is established
switch(conn->state) {
switch((int)conn->state) {
case HE_STATE_LINK_UP:
case HE_STATE_AUTHENTICATING:
case HE_STATE_CONFIGURING:
Expand Down Expand Up @@ -746,7 +746,7 @@ he_return_code_t he_internal_renegotiate_ssl(he_conn_t *conn) {
case SECURE_RENEGOTIATION_E:
return HE_ERR_SECURE_RENEGOTIATION_ERROR;
default:
conn->wolf_error = error;
he_conn_set_ssl_error(conn, error);
return HE_ERR_SSL_ERROR;
}
}
Expand Down Expand Up @@ -1213,7 +1213,7 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
if(conn->pmtud_state_change_cb == NULL || conn->pmtud_time_cb == NULL) {
return HE_ERR_PMTUD_CALLBACKS_NOT_SET;
}
if(conn->pmtud_state != HE_PMTUD_STATE_DISABLED) {
if(conn->pmtud.state != HE_PMTUD_STATE_DISABLED) {
// PMTUD is already started
return HE_SUCCESS;
}
Expand All @@ -1223,10 +1223,10 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
}

uint16_t he_conn_get_effective_pmtu(he_conn_t *conn) {
if(!conn || conn->effective_pmtu == 0) {
if(!conn || conn->pmtud.effective_pmtu == 0 || conn->pmtud.state != HE_PMTUD_STATE_SEARCH_COMPLETE) {
return HE_MAX_MTU;
}
return conn->effective_pmtu;
return conn->pmtud.effective_pmtu;
}

he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn) {
Expand All @@ -1236,6 +1236,25 @@ he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn) {
return he_internal_pmtud_handle_probe_timeout(conn);
}

#ifdef HE_ENABLE_MULTITHREADED
// wolf_error as thread local variable, so it can be used
// from multiple threads without collision
HE_THREAD_LOCAL int wolf_error = 0;
#endif

void he_conn_set_ssl_error(he_conn_t *conn, int error) {
#ifdef HE_ENABLE_MULTITHREADED
(void) conn;
wolf_error = error;
#else
conn->wolf_error = error;
#endif
}

int he_conn_get_ssl_error(he_conn_t *conn) {
#ifdef HE_ENABLE_MULTITHREADED
kp-mariappan-ramasamy marked this conversation as resolved.
Show resolved Hide resolved
return wolf_error;
#else
return conn->wolf_error;
#endif
}
3 changes: 3 additions & 0 deletions src/he/conn.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,4 +517,7 @@ he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn);
*/
int he_conn_get_ssl_error(he_conn_t* conn);


void he_conn_set_ssl_error(he_conn_t *conn, int error);

#endif // CONN_H
3 changes: 2 additions & 1 deletion src/he/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@
*/

#include "core.h"
#include "conn.h"

he_return_code_t he_internal_setup_stream_state(he_conn_t *conn, uint8_t *data, size_t length) {
if(conn->incoming_data_left_to_read != 0) {
// Somehow this function was called without reading all data from a previous buffer
// This is bad
conn->wolf_error = 0;
he_conn_set_ssl_error(conn, 0);
return HE_ERR_SSL_ERROR;
}
// Set up the location of the buffer and its length
Expand Down
Loading
Loading