CORE-6: Pre-fetching algorithms #17003

182: single run iterations:    0
182: single run duration:      1.000s
182: number of runs:           5
182: number of cores:          1
182: random seed:              2394275564
182:
182: test                         iterations      median         mad         min         max      allocs       tasks        inst
182: openssl_perf.md5_1k           958464000     0.974ns     0.000ns     0.972ns     0.974ns       0.003       0.000         0.0
182: openssl_perf.sha256_1k       2172928000     0.461ns     0.000ns     0.461ns     0.461ns       0.004       0.000         0.0
182: openssl_perf.sha512_1k       1044480000     0.959ns     0.001ns     0.958ns     0.964ns       0.004       0.000         0.0
182: gnutls.md5_1k                 946176000     1.056ns     0.000ns     1.056ns     1.057ns       0.002       0.000         0.0
182: gnutls.sha256_1k             2406400000     0.416ns     0.000ns     0.415ns     0.419ns       0.002       0.000         0.0
182: gnutls.sha512_1k             1029120000     0.972ns     0.000ns     0.971ns     0.972ns       0.002       0.000         0.0
182: openssl_perf.hmac_sha256_1k  1004544000     0.997ns     0.000ns     0.997ns     1.000ns       0.015       0.000         0.0
182: openssl_perf.hmac_sha512_1k   562176000     1.768ns     0.001ns     1.767ns     1.770ns       0.015       0.000         0.0
182: gnutls.hmac_sha256_1k        1613824000     0.619ns     0.000ns     0.619ns     0.620ns       0.003       0.000         0.0
182: gnutls.hmac_sha512_1k         668672000     1.500ns     0.000ns     1.499ns     1.500ns       0.003       0.000         0.0

FIPS:

183: single run iterations:    0
183: single run duration:      1.000s
183: number of runs:           5
183: number of cores:          1
183: random seed:              3628882004
183:
183: test                         iterations      median         mad         min         max      allocs       tasks        inst
183: openssl_perf.sha256_1k       2163712000     0.463ns     0.000ns     0.463ns     0.464ns       0.004       0.000         0.0
183: openssl_perf.sha512_1k       1041408000     0.961ns     0.001ns     0.957ns     0.961ns       0.004       0.000         0.0
183: gnutls.md5_1k                 943104000     1.059ns     0.000ns     1.059ns     1.060ns       0.002       0.000         0.0
183: gnutls.sha256_1k             2404352000     0.416ns     0.000ns     0.416ns     0.417ns       0.002       0.000         0.0
183: gnutls.sha512_1k             1033216000     0.971ns     0.000ns     0.967ns     0.971ns       0.002       0.000         0.0
183: openssl_perf.hmac_sha256_1k   998400000     1.010ns     0.001ns     1.009ns     1.011ns       0.015       0.000         0.0
183: openssl_perf.hmac_sha512_1k   564224000     1.778ns     0.000ns     1.777ns     1.779ns       0.015       0.000         0.0
183: gnutls.hmac_sha256_1k        1624064000     0.617ns     0.000ns     0.616ns     0.617ns       0.003       0.000         0.0
183: gnutls.hmac_sha512_1k         670720000     1.495ns     0.001ns     1.492ns     1.496ns       0.003       0.000         0.0

michael-redpanda · 2024-03-13T13:25:36Z

[CORE-6]

michael-redpanda · 2024-03-13T13:32:28Z

Force push f8af602:

Seeing if JIRA integration works with changing initial commit to reference CORE-6

dotnwat · 2024-03-14T04:37:04Z

src/v/crypto/crypto.cc

+
+namespace internal {
+EVP_MD* get_md(digest_type type) {
+    // Map of pre-fetched MD pointers.  This replaces the older way of getting


older way

this being "implicit fetch"?

dotnwat · 2024-03-14T04:48:33Z

src/v/crypto/crypto.cc

+        if (!md_ptr) {
+            throw ossl_error(fmt::format("Failed to fetch algorithm {}", alg));
+        }
+        md_map.insert_or_assign(type, EVP_MD_ptr(md_ptr));


[[maybe_unused]] auto res = md_map.insert_or_assign(type, EVP_MD_ptr(md_ptr)); assert(res.second); // otherwise we'd leak the fetched algo ptr

or just a comment, tho assert won't have any performance impact in release build.

dotnwat · 2024-03-14T04:58:31Z

src/v/crypto/hmac.cc

+    case digest_type::SHA512:
+        return sha512_params;
+    default:
+        vassert(false, "NOPE");


Since this isn't a covering set, the assertion is nice. But maybe print out what it is, like MD5 looks like it could be the problem.

ha, completely forgot to change that message... meant to go back and fix that

michael-redpanda · 2024-03-14T12:53:37Z

Force push e6bb97f:

Updated per PR comments

dotnwat

lgtm

dotnwat · 2024-03-14T14:43:40Z

src/v/crypto/crypto.cc

+        [[maybe_unused]] auto res = md_map.insert_or_assign(
+          type, EVP_MD_ptr(md_ptr));
+        vassert(
+          res.second, "Failed to insert/create the fetched algorithm {}", alg);


this is fine. just fyi, you don't need [[maybe_unused]] if you use vassert because vassert is always compiled in so res is always used. but with vanilla assert it is compiled out in Release builds so res is not used and you need the annotation.

CORE-6: Fixes this issue Signed-off-by: Michael Boquard <michael@redpanda.com>

Similar to pre-fetching the MD, this will pre-fetch the HMAC provider. Signed-off-by: Michael Boquard <michael@redpanda.com>

Signed-off-by: Michael Boquard <michael@redpanda.com>

Added perf tests to compare GnuTLS vs OpenSSL both in and out of FIPS mode. Signed-off-by: Michael Boquard <michael@redpanda.com>

michael-redpanda · 2024-03-14T18:39:16Z

Force push 7425d18:

Removed unncessary [[maybe_unused]]

oleiman

lgtm

src/v/crypto/crypto.cc

oleiman · 2024-03-14T21:02:55Z

src/v/crypto/test/crypto_bench.cc

+static size_t test_body(size_t msg_len, F n) {
+    auto buffer = random_generators::gen_alphanum_string(msg_len);
+    for (auto i = inner_iters; i--;) {
+        auto s = n(buffer);
+        perf_tests::do_not_optimize(s);
+    }
+    perf_tests::stop_measuring_time();


q: does the time measurement include the string generation?

I believe it does

oleiman

lgtm

graphcareful · 2024-03-15T14:01:10Z

src/v/crypto/test/crypto_bench.cc

+    auto buffer = random_generators::gen_alphanum_string(msg_len);
+    for (auto i = inner_iters; i--;) {
+        auto s = n(buffer);
+        perf_tests::do_not_optimize(s);


should the entire call of n(buffer) be passed to do_not_optimize? Here it seems just its return value is

michael-redpanda self-assigned this Mar 11, 2024

michael-redpanda requested review from BenPope and graphcareful March 11, 2024 19:21

github-actions bot added the area/redpanda label Mar 11, 2024

michael-redpanda force-pushed the issues/ci-1152 branch from 1205e21 to 213c15e Compare March 13, 2024 00:36

michael-redpanda force-pushed the issues/ci-1152 branch from 213c15e to f8af602 Compare March 13, 2024 13:30

michael-redpanda changed the title ~~Pre-fetching algorithms~~ CORE-6: Pre-fetching algorithms Mar 13, 2024

mergify bot mentioned this pull request Mar 13, 2024

CORE-6 test jira #17030

Closed

6 tasks

dotnwat reviewed Mar 14, 2024

View reviewed changes

michael-redpanda force-pushed the issues/ci-1152 branch from f8af602 to e6bb97f Compare March 14, 2024 12:53

michael-redpanda requested a review from dotnwat March 14, 2024 12:53

dotnwat reviewed Mar 14, 2024

View reviewed changes

michael-redpanda added 5 commits March 14, 2024 14:38

crypto: Added ability to pre-fetch MD pointer

977fee4

CORE-6: Fixes this issue Signed-off-by: Michael Boquard <michael@redpanda.com>

crypto: Pre-fetching HMAC pointer

f35ebfb

Similar to pre-fetching the MD, this will pre-fetch the HMAC provider. Signed-off-by: Michael Boquard <michael@redpanda.com>

crypto: Clean up using appropriate OpenSSL macro

045279d

Signed-off-by: Michael Boquard <michael@redpanda.com>

crypto: Staging OSSL param structure for HMAC

ab929cf

Signed-off-by: Michael Boquard <michael@redpanda.com>

crypto/tests: Added perf tests

7425d18

Added perf tests to compare GnuTLS vs OpenSSL both in and out of FIPS mode. Signed-off-by: Michael Boquard <michael@redpanda.com>

michael-redpanda force-pushed the issues/ci-1152 branch from e6bb97f to 7425d18 Compare March 14, 2024 18:38

michael-redpanda requested a review from dotnwat March 14, 2024 18:39

oleiman reviewed Mar 14, 2024

View reviewed changes

oleiman approved these changes Mar 15, 2024

View reviewed changes

graphcareful approved these changes Mar 15, 2024

View reviewed changes

michael-redpanda merged commit f7c03fd into redpanda-data:dev Mar 15, 2024
18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CORE-6: Pre-fetching algorithms #17003

CORE-6: Pre-fetching algorithms #17003

michael-redpanda commented Mar 11, 2024 •

edited by jira bot

vbotbuildovich commented Mar 11, 2024 •

edited

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

dotnwat Mar 14, 2024

michael-redpanda Mar 14, 2024

dotnwat Mar 14, 2024

dotnwat Mar 14, 2024

michael-redpanda Mar 14, 2024 •

edited

michael-redpanda commented Mar 14, 2024

dotnwat left a comment

dotnwat Mar 14, 2024

michael-redpanda Mar 14, 2024

michael-redpanda commented Mar 14, 2024

oleiman left a comment

oleiman Mar 14, 2024

michael-redpanda Mar 15, 2024

oleiman left a comment

graphcareful Mar 15, 2024

CORE-6: Pre-fetching algorithms #17003

CORE-6: Pre-fetching algorithms #17003

Conversation

michael-redpanda commented Mar 11, 2024 • edited by jira bot

Backports Required

Release Notes

vbotbuildovich commented Mar 11, 2024 • edited

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

michael-redpanda commented Mar 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-redpanda Mar 14, 2024 • edited

Choose a reason for hiding this comment

michael-redpanda commented Mar 14, 2024

dotnwat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-redpanda commented Mar 14, 2024

oleiman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleiman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michael-redpanda commented Mar 11, 2024 •

edited by jira bot

vbotbuildovich commented Mar 11, 2024 •

edited

michael-redpanda Mar 14, 2024 •

edited