-
Notifications
You must be signed in to change notification settings - Fork 915
dtls 1.3: allow rtx interval to be less than a second #9623
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
base: master
Are you sure you want to change the base?
Conversation
|
🛟 Devin Lifeguard found 1 likely issues in this PR
@julek-wolfssl |
d8b2992 to
8c416f2
Compare
|
Retest this please RequestAbortedException |
| /* Save the message count to make sure no new messages are sent */ | ||
| ExpectIntGE(c_msg_count = test_ctx.c_msg_count, 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: two instruction to make it more readable
| test_memio_clear_buffer(&test_ctx, 1); | ||
|
|
||
| /* First timeout. This one should trigger a retransmission */ | ||
| if (wolfSSL_dtls13_use_quick_timeout(ssl_s)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wrap the result of the wolfSSL_dtls13_use_quick_timeout in an Expect assert
| #define DTLS13_MIN_CIPHERTEXT 16 | ||
| #define DTLS13_MIN_RTX_INTERVAL 1 | ||
| #ifndef DTLS13_MIN_RTX_INTERVAL | ||
| #define DTLS13_MIN_RTX_INTERVAL 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think of using DTLS_TIMEOUT_INIT as default here?
| * wolfSSL_dtls_got_timeout() to simulate timeouts and verify that | ||
| * retransmissions are spaced at least DTLS13_MIN_RTX_INTERVAL apart. | ||
| */ | ||
| int test_dtls13_min_rtx_interval(void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test relies on timing and may introduce flakiness. I don't see an easy way to test without timing dependency. If we want to keep it as it is, we need at least a clear comment about the risk imo.
No description provided.