20230517-linuxkm-benchmarks by douzzer · Pull Request #6418 · wolfSSL/wolfssl

douzzer · 2023-05-17T07:03:28Z

refactor benchmark.c for linux kernel compatibility -- WOLFSSL_SMALL_STACK and WOLFSSL_NO_FLOAT_FMT codepaths, SAVE/RESTORE_VECTOR_REGISTERS, refactor of several stack array initializations that broke in the kernel, and replacement of an fputs() call with printf();

configure.ac: add --enable-linuxkm-benchmarks; add check for async.{c,h} when --enable-asynccrypt; update failure message for the opensslextra AC_CHECK_HEADER() test;

linuxkm/linuxkm_memory.c: refactor SAVE/RESTORE_VECTOR_REGISTERS() to be per-process rather than per-CPU, and add migrate_disable/enable() to kernel_fpu_begin/end() because preempt_disable() is just a barrier on _PREEMPT_VOLUNTARY kernels;

linuxkm/linuxkm_wc_port.h: activate SAVE/RESTORE_VECTOR_REGISTERS() whenever defined(WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS) for benchmark.c support, independent of vector crypto features;

linuxkm/linuxkm_wc_port.h: fix and optimize various alignment issues with stack and heap allocations;

fix macro definitions for XMALLOC/XREALLOC/XFREE to correctly use kvmalloc and friends when defined(HAVE_KVMALLOC), and to use wolfSSL_Malloc() and friends when defined(WOLFSSL_TRACK_MEMORY);

purge stale LINUXKM_SIMD_IRQ code;

wolfssl/wolfcrypt/mem_track.h: refactor for linuxkm compatibility, mainly by supporting NO_STDIO_FILESYSTEM;

wolfcrypt/src/aes.c: in wc_AesSetKeyLocal(), add an alignment check in the haveAESNI path for WOLFSSL_LINUXKM, because the failure mode is module crash.

also various unrelated point fixes for analyzer carps, one commit per fix.

tested with wolfssl-multi-test.sh ... super-quick-check '.*linuxkm.*' benchmark-wolfcrypt-intelasm-all benchmark-wolfcrypt-noasm-all

…ble-asynccrypt; update failure message for the opensslextra AC_CHECK_HEADER() test.

…STACK and WOLFSSL_NO_FLOAT_FMT codepaths, SAVE/RESTORE_VECTOR_REGISTERS, refactor of several stack array initializations that broke in the kernel, and replacement of an fputs() call with printf().

…lo().

… memset, not the zero initializer, for C++ compatibility.

…n the haveAESNI path for WOLFSSL_LINUXKM, because the failure mode is module crash.

…FREE_VAR() and WC_FREE_ARRAY() for pedantic semicolon swallowing.

…inly by supporting NO_STDIO_FILESYSTEM.

… be per-process rather than per-CPU, and add migrate_disable/enable() to kernel_fpu_begin/end() because preempt_disable() is just a barrier on _PREEMPT_VOLUNTARY kernels; linuxkm/linuxkm_wc_port.h: activate SAVE/RESTORE_VECTOR_REGISTERS() whenever defined(WOLFSSL_LINUXKM_USE_SAVE_VECTOR_REGISTERS) for benchmark.c support, independent of vector crypto features; fix and optimize various alignment issues with stack and heap allocations; fix macro definitions for XMALLOC/XREALLOC/XFREE to correctly use kvmalloc and friends when defined(HAVE_KVMALLOC), and to use wolfSSL_Malloc() and friends when defined(WOLFSSL_TRACK_MEMORY); purge stale LINUXKM_SIMD_IRQ code.

dgarske · 2023-05-17T15:56:56Z

        if (haveAESNI) {
+            #ifdef WOLFSSL_LINUXKM
+            /* runtime alignment check */
+            if ((unsigned long)&aes->key & 0xf) {


For portability seems like size_t would be better? Also using 0xFULL?

Only the bottom 4 bits matter here, so I figured casting to unsigned long was fine and keeps it maximally efficient.

But we do actually have a right way to do this -- wc_ptr_t, set up in types.h. Will use that.

Btw, size_t is not reliably the same size as a pointer -- ran into this recently in-real-life. That can produce inefficiencies or truncation warnings depending on which way it goes.

dgarske · 2023-05-17T15:57:36Z

            ret = DoClientHelloStateless(ssl, input, inOutIdx, helloSz);
            if (ret != 0 || !ssl->options.dtlsStateful) {
                int alertType = TranslateErrorToAlert(ret);
-                if (alertType != invalid_alert)


Wow, good find

all credit to cppcheck -- it's a fidgity tool but it's worth the trouble to figure it out.

dgarske · 2023-05-17T15:58:08Z

+#define NO_MAIN_FUNCTION
+#define current_time benchmark_current_time
+#define WOLFSSL_NO_FLOAT_FMT
+#include "/home/douzzer/com/wolfssl/src/wolfssl/wolfcrypt/benchmark/benchmark.c"


Hard coded douzzer?? Is it possible to have a relative path or to get the path a different way?

LOL!!!

thank you @dgarske 😁

dgarske · 2023-05-17T15:59:14Z

+    #define FLT_FMT "%0ld,%09lu"
+    #define FLT_FMT_PREC "%0ld.%0*lu"
+    #define FLT_FMT_PREC2 FLT_FMT_PREC
+    #define FLT_FMT_ARGS(x) (long)(x), ((x) < 0) ? (unsigned long)(-(((x) - (double)(long)(x)) * 1000000000.0)) : (unsigned long)(((x) - (double)(long)(x)) * 1000000000.0)


Line length?

yeah sure I'll fix that -- but note there's a ton of overlong line in benchmark.c. it's a bit of a shambles...

dgarske · 2023-05-17T15:59:39Z

 void bench_hmac_sha(int useDeviceID)
 {
-    byte key[] = { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,
+    static const byte key[] = { 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b,


Would prefer just const. These shouldn't need to be static right?

crashes without static. the problem is stack initialization. exact root cause unknown, but suspect alignment weirdness.

(am changing all of those to WOLFSSL_SMALL_STACK_STATIC -- good enough for test.c, good enough for benchmark.c)

dgarske · 2023-05-17T16:00:15Z

 {
-    ecc_key userA, userB;
+#define BENCH_ECCENCRYPT_MSG_SIZE 48
+#define BENCH_ECCENCRYPT_OUT_SIZE (BENCH_ECCENCRYPT_MSG_SIZE + WC_SHA256_DIGEST_SIZE + (MAX_ECC_BITS+3)/4 + 2)


Line length

…GISTERS() failure trap in benchmark.c, proper path to benchmark.c in linuxkm/module_hooks.c, and proper casting in aes.c. also harmonized semantics and prototype of bench_ripemd().

douzzer · 2023-05-17T19:37:39Z

Jenkins, retest this please.

douzzer · 2023-05-17T22:05:21Z

note for posterity: the CI failure on the final commit in this PR is a Jenkins/github state model conflict. the single subtest at issue, PRB-fips-repo-and-harness-test-v2, was hit by a github network glitch on 1st try, and succeeded (https://cloud.wolfssl-test.com/jenkins/job/PRB-fips-repo-and-harness-test-v2/5248/) on 2nd try.

douzzer added 9 commits May 17, 2023 01:03

add --enable-linuxkm-benchmarks; add check for async.{c,h} when --ena…

42cce9d

…ble-asynccrypt; update failure message for the opensslextra AC_CHECK_HEADER() test.

refactor benchmark.c for linux kernel compatibility -- WOLFSSL_SMALL_…

3ad7e39

…STACK and WOLFSSL_NO_FLOAT_FMT codepaths, SAVE/RESTORE_VECTOR_REGISTERS, refactor of several stack array initializations that broke in the kernel, and replacement of an fputs() call with printf().

src/internal.c: fix for typo (identicalInnerCondition) in DoClientHel…

5aceae1

…lo().

fix for benign sign clash in wc_RNG_GenerateBlock().

85e0bf3

src/ssl_asn1.c: in wolfssl_asn1_time_to_tm(), initialize localTm with…

40b5982

… memset, not the zero initializer, for C++ compatibility.

wolfcrypt/src/aes.c: in wc_AesSetKeyLocal(), add an alignment check i…

a18dc7f

…n the haveAESNI path for WOLFSSL_LINUXKM, because the failure mode is module crash.

wolfssl/wolfcrypt/types.h: add missing do-while-0 nonstatments to WC_…

cc1ec8a

…FREE_VAR() and WC_FREE_ARRAY() for pedantic semicolon swallowing.

wolfssl/wolfcrypt/mem_track.h: refactor for linuxkm compatibility, ma…

fc589d6

…inly by supporting NO_STDIO_FILESYSTEM.

douzzer requested review from danielinux and dgarske May 17, 2023 07:03

douzzer assigned dgarske and douzzer May 17, 2023

wolfcrypt/benchmark/benchmark.c: fix typo in bench_stats_start().

37b7161

dgarske requested changes May 17, 2023

View reviewed changes

dgarske removed their assignment May 17, 2023

cleanups: line length, WOLFSSL_SMALL_STACK_STATIC, and SAVE_VECTOR_RE…

fffff65

…GISTERS() failure trap in benchmark.c, proper path to benchmark.c in linuxkm/module_hooks.c, and proper casting in aes.c. also harmonized semantics and prototype of bench_ripemd().

douzzer requested a review from dgarske May 17, 2023 18:01

douzzer assigned dgarske May 17, 2023

dgarske approved these changes May 17, 2023

View reviewed changes

dgarske merged commit 0530ee7 into wolfSSL:master May 17, 2023

Conversation

douzzer commented May 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

douzzer commented May 17, 2023

Uh oh!

douzzer commented May 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants