Speedup DTLS max fragment size calculation for MTU limits by julek-wolfssl · Pull Request #10323 · wolfSSL/wolfssl

julek-wolfssl · 2026-04-27T16:34:03Z

Refactor wolfssl_local_GetRecordSize to compute the record size directly
from cipher specs instead of calling BuildMessage. Add a unit test that
compares the new calculation to BuildMessage's size-only output across
every registered cipher suite and supported (D)TLS version.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors wolfSSL’s record-size computation to avoid calling BuildMessage and adds a unit test to ensure the new direct calculation matches BuildMessage’s size-only behavior across cipher suites and protocol versions.

Changes:

Reimplemented wolfssl_local_GetRecordSize() to compute record size directly from negotiated cipher specs (including TLS 1.3 and DTLS(+CID) handling).
Exposed a few internal helpers (BuildMessage, GetCipherNames*, wolfssl_local_GetRecordSize) with test visibility for unit testing.
Added an API test that cross-checks the new calculation vs BuildMessage(sizeOnly) across versions/ciphers/payload sizes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
`src/internal.c`	Replaces `BuildMessage` call with direct record-size calculation logic.
`wolfssl/internal.h`	Marks internal functions as test-visible so the new unit test can call them.
`tests/api/test_tls.c`	Adds exhaustive cross-check test for record-size calculations across ciphers/versions.
`tests/api/test_tls.h`	Registers the new test in the TLS API test group list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-27T16:43:00Z

+    /* Sanity: at least one cipher/version combination must have been
+     * exercised per supported version, otherwise the test is silently a
+     * no-op. */
+    ExpectIntGE(tested, (int)XELEM_CNT(versions));


The sanity check can produce false failures on legitimate builds where none of the “default memio” credential types are available (e.g., PSK-only builds, ECDSA-only builds without certs configured by test_memio_setup, RSA disabled, etc.). In those cases, handshakes are expected to fail/skip for all ciphers and tested remains 0, making the test fail even though the environment cannot exercise any negotiated cipher. Consider tracking “selectable ciphers for this version” vs “successfully negotiated”, and only enforcing a per-version minimum when at least one cipher is selectable and not excluded by the allow-list (or downgrade to an explicit SKIP outcome when nothing can be negotiated in the current build config).

Suggested change

/* Sanity: at least one cipher/version combination must have been

* exercised per supported version, otherwise the test is silently a

* no-op. */

ExpectIntGE(tested, (int)XELEM_CNT(versions));

/* Sanity: when at least one cipher/version combination could be

* exercised, require broad enough coverage to ensure the test is not

* silently a partial no-op. Some legitimate build configurations do

* not provide any default-memio-negotiable credentials/ciphers for the

* versions under test; in that case, do not fail the test solely

* because nothing could be negotiated. */

if (tested > 0) {

ExpectIntGE(tested, (int)XELEM_CNT(versions));

}

else {

fprintf(stderr,

" [SKIP no negotiable default-memio cipher/version "

"combination in current build]\n");

}

Copilot · 2026-04-27T16:43:00Z

+    for (v = 0; v < XELEM_CNT(versions) && EXPECT_SUCCESS(); v++) {
+        for (i = 0; i < numCiphers && EXPECT_SUCCESS(); i++) {
+            WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;


This test performs a full handshake for every (version × cipher-suite) combination and then loops over many payload sizes. On configurations with a large cipher list, this can be very slow and can dominate CI time. Consider gating the exhaustive sweep behind an “extended/long tests” knob (build flag or runtime option), and running a smaller representative subset by default (while keeping the exhaustive mode available for CI/nightly).

Copilot · 2026-04-27T16:43:00Z

+                fprintf(stderr,
+                        "  [SKIP %-12s %-40s] cipher not selectable\n",
+                        versions[v].label, name);


Printing a line for every skipped combination can generate extremely large stderr output (especially when many ciphers are unavailable for a given version/build), which can make CI logs noisy and harder to use. Consider reducing verbosity (e.g., only log unexpected failures/mismatches, or aggregate skip counts per reason/version and print a short summary).

Copilot · 2026-04-27T16:43:00Z

+            WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
+            struct test_memio_ctx test_ctx;
+            const char* name = allCiphers[i].name;
+            int isTls13Cipher = (XSTRSTR(name, "TLS13-") != NULL);


Using a name substring to determine whether a cipher is TLS 1.3 is brittle (it couples test correctness to naming conventions rather than cipher properties). Since CipherSuiteInfo includes structured fields (e.g., suite bytes and flags), prefer using those to classify TLS 1.3 vs <=1.2 ciphers. This makes the allow-list logic and “version mismatch” reasoning much more robust to future naming changes.

Suggested change

int isTls13Cipher = (XSTRSTR(name, "TLS13-") != NULL);

int isTls13Cipher = (allCiphers[i].cipherSuite0 == 0x13);

Copilot · 2026-04-27T16:43:01Z

+WOLFSSL_TEST_VIS int wolfssl_local_GetRecordSize(WOLFSSL *ssl, int payloadSz,
        int isEncrypted);


Marking these internal symbols as WOLFSSL_TEST_VIS in a shared internal header can inadvertently widen the exported surface in any build where WOLFSSL_TEST_VIS maps to default visibility, potentially affecting ABI/symbol export policies. If the intent is strictly “unit tests only”, consider additionally guarding these declarations with a dedicated test-only macro (or moving them into a test-only header) so production builds cannot accidentally expose or rely on them.

Copilot · 2026-04-27T16:43:01Z

+WOLFSSL_TEST_VIS int BuildMessage(WOLFSSL* ssl, byte* output, int outSz,
                        const byte* input, int inSz, int type, int hashOutput,
                        int sizeOnly, int asyncOkay, int epochOrder);


Marking these internal symbols as WOLFSSL_TEST_VIS in a shared internal header can inadvertently widen the exported surface in any build where WOLFSSL_TEST_VIS maps to default visibility, potentially affecting ABI/symbol export policies. If the intent is strictly “unit tests only”, consider additionally guarding these declarations with a dedicated test-only macro (or moving them into a test-only header) so production builds cannot accidentally expose or rely on them.

github-actions · 2026-04-27T18:23:48Z

MemBrowse Memory Report

gcc-arm-cortex-m4

FLASH: .text +64 B (+0.0%, 196,923 B / 262,144 B, total: 75% used)
No memory changes detected for:
gcc-arm-cortex-m4-baremetal
gcc-arm-cortex-m4-min-ecc
gcc-arm-cortex-m4-tls12

Refactor wolfssl_local_GetRecordSize to compute the record size directly from cipher specs instead of calling BuildMessage. Add a unit test that compares the new calculation to BuildMessage's size-only output across every registered cipher suite and supported (D)TLS version.

RFC 8998 registers TLS_SM4_GCM_SM3 / TLS_SM4_CCM_SM3 (and the TLS 1.2 SM4 suites) with DTLS-OK: No and defines no record-number-mask construction for SM4. RFC 9147 Section 4.2.3 requires any non-AES / non-ChaCha20 cipher to define its own record sequence number encryption to be usable with DTLS. Drop SM cipher suites from the negotiable list in InitSuites when DTLS, and reject them defensively in VerifyServerSuite for the case where the user pins a cipher list explicitly. Update test_record_size_matches_build_message to set up an SM2 cert chain and SM2 key share for SM ciphers (so they actually handshake over TLS 1.2 / TLS 1.3) and to skip the DTLS variants with the RFC reason.

Copilot AI review requested due to automatic review settings April 27, 2026 16:34

julek-wolfssl self-assigned this Apr 27, 2026

julek-wolfssl requested a review from rizlik April 27, 2026 16:34

Copilot AI reviewed Apr 27, 2026

View reviewed changes

Copilot started reviewing on behalf of julek-wolfssl April 27, 2026 16:45 View session

rizlik requested changes Apr 27, 2026

View reviewed changes

Comment thread src/internal.c

julek-wolfssl force-pushed the wolfssl_local_GetRecordSize-refactor branch from a26a840 to 7a83f0a Compare April 28, 2026 11:50

julek-wolfssl added 2 commits April 28, 2026 15:01

julek-wolfssl force-pushed the wolfssl_local_GetRecordSize-refactor branch from 7a83f0a to 6fc100b Compare April 28, 2026 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speedup DTLS max fragment size calculation for MTU limits#10323

Speedup DTLS max fragment size calculation for MTU limits#10323
julek-wolfssl wants to merge 2 commits intowolfSSL:masterfrom
julek-wolfssl:wolfssl_local_GetRecordSize-refactor

julek-wolfssl commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Uh oh!

github-actions Bot commented Apr 27, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    /* Sanity: at least one cipher/version combination must have been
-     * exercised per supported version, otherwise the test is silently a
-     * no-op. */
-    ExpectIntGE(tested, (int)XELEM_CNT(versions));
+    /* Sanity: when at least one cipher/version combination could be
+     * exercised, require broad enough coverage to ensure the test is not
+     * silently a partial no-op. Some legitimate build configurations do
+     * not provide any default-memio-negotiable credentials/ciphers for the
+     * versions under test; in that case, do not fail the test solely
+     * because nothing could be negotiated. */
+    if (tested > 0) {
+        ExpectIntGE(tested, (int)XELEM_CNT(versions));
+    }
+    else {
+        fprintf(stderr,
+                "  [SKIP no negotiable default-memio cipher/version "
+                "combination in current build]\n");
+    }

	int isTls13Cipher = (XSTRSTR(name, "TLS13-") != NULL);
	int isTls13Cipher = (allCiphers[i].cipherSuite0 == 0x13);

		WOLFSSL_TEST_VIS int wolfssl_local_GetRecordSize(WOLFSSL *ssl, int payloadSz,
		int isEncrypted);

Conversation

julek-wolfssl commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

gcc-arm-cortex-m4

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Apr 27, 2026 •

edited

Loading