Skip to content

Enable specifying the tls protocol version to use via --tls-protocols argument#234

Merged
YaacovHazan merged 4 commits intomasterfrom
tls-protocols
Sep 14, 2023
Merged

Enable specifying the tls protocol version to use via --tls-protocols argument#234
YaacovHazan merged 4 commits intomasterfrom
tls-protocols

Conversation

@filipecosta90
Copy link
Collaborator

@filipecosta90 filipecosta90 commented Sep 12, 2023 •
edited
Loading

By default it will always try to use v1.2 and v1.3.
It allows specifying multiple versions via comma delimiter.

Examples:

Using either v1.1 or v1.2

./memtier_benchmark   --cert ~/redislabs/redis/tests/tls/redis.crt     --key ~/redislabs/redis/tests/tls/redis.key --cacert ~/redislabs/redis/tests/tls/ca.crt  -c 1 -t 1 --tls --hide-histogram --tls-protocols tlsv1.1,tlsv1.2

Confirmation of data capture that indeed we're using v1.2:
image

Using only v1.3

./memtier_benchmark   --cert ~/redislabs/redis/tests/tls/redis.crt     --key ~/redislabs/redis/tests/tls/redis.key --cacert ~/redislabs/redis/tests/tls/ca.crt  -c 1 -t 1 --tls --hide-histogram --tls-protocols tlsv1.3

Confirmation of data capture that indeed we're using v1.3:
image

Trying to use an non allowed version by the server (v1.1) -- check error

$ ./memtier_benchmark   --cert ~/redislabs/redis/tests/tls/redis.crt     --key ~/redislabs/redis/tests/tls/redis.key --cacert ~/redislabs/redis/tests/tls/ca.crt  -c 1 -t 1 --tls --hide-histogram --tls-protocols tlsv1.1 -n 2
Writing results to stdout
[RUN #1] Preparing benchmark client...
[RUN #1] Launching threads now...
TLS connection error: no protocols available
TLS connection error: (null)
[RUN #1 0%,   0 secs]  0 threads:           0 ops,       0 (avg:       0) ops/sec, 0.00KB/sec (avg: 0.00KB/sec),  0.00 (avg:  0.00) msec latency

1         Threads
1         Connections per thread
2         Requests per client


ALL STATS
============================================================================================================================
Type         Ops/sec     Hits/sec   Misses/sec    Avg. Latency     p50 Latency     p99 Latency   p99.9 Latency       KB/sec 
----------------------------------------------------------------------------------------------------------------------------
Sets            0.00          ---          ---             ---             ---             ---             ---         0.00 
Gets            0.00         0.00         0.00             ---             ---             ---             ---         0.00 
Waits           0.00          ---          ---             ---             ---             ---             ---          --- 
Totals          0.00         0.00         0.00            -nan         0.00000         0.00000         0.00000         0.00

@codecov
Copy link

codecov bot commented Sep 12, 2023 •
edited
Loading

Codecov Report

Merging #234 (ced480f) into master (4203084) will decrease coverage by 0.19%.
The diff coverage is 29.62%.

❗ Current head ced480f differs from pull request most recent head 1a179c8. Consider uploading reports for the commit 1a179c8 to get more accurate results

@@            Coverage Diff             @@
##           master     #234      +/-   ##
==========================================
- Coverage   56.23%   56.04%   -0.19%     
==========================================
  Files          21       21              
  Lines        4332     4359      +27     
==========================================
+ Hits         2436     2443       +7     
- Misses       1896     1916      +20
Files Changed Coverage Δ
config_types.cpp 42.51% <ø> (-0.40%) ⬇️
memtier_benchmark.h 100.00% <ø> (ø)
memtier_benchmark.cpp 52.10% <29.62%> (-0.72%) ⬇️

YaacovHazan
YaacovHazan requested changes Sep 14, 2023
View reviewed changes
@YaacovHazan YaacovHazan merged commit bec3471 into master Sep 14, 2023
@filipecosta90 filipecosta90 deleted the tls-protocols branch September 14, 2023 10:09
@Penguin-zlh Penguin-zlh mentioned this pull request Sep 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@YaacovHazan YaacovHazan YaacovHazan approved these changes

@yossigo yossigo Awaiting requested review from yossigo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@filipecosta90 @YaacovHazan