-
Notifications
You must be signed in to change notification settings - Fork 151
feat(s2n-quic-core): add null TLS provider for testing #1834
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
Conversation
f1c699c to
4fbd614
Compare
4fbd614 to
9a27e81
Compare
9a27e81 to
2eece61
Compare
WesleyRosenblum
left a comment
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.
Opened #1857 to add to the CI, as discussed offline
| fn new_server_session<Params: s2n_codec::EncoderValue>( | ||
| &mut self, | ||
| transport_parameters: &Params, | ||
| ) -> Self::Session { |
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.
do you think its worth a println here and in new_client_session just warning that a null TLS session has been initiated that will not perform any encryption?
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.
my worry is if we're testing the connection creation latency outside of crypto it would also affect the measurement since we're printing to the console
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.
Actually thinking about it a bit, I'll put it in the Endpoint::default() method, since that's only created at start up.
2eece61 to
274b7f7
Compare
274b7f7 to
e41b09a
Compare
| eprintln!(" An s2n-quic endpoint has configured without cryptographic protections."); | ||
| eprintln!(" This should ONLY be used for testing purposed only."); |
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.
| eprintln!(" An s2n-quic endpoint has configured without cryptographic protections."); | |
| eprintln!(" This should ONLY be used for testing purposed only."); | |
| eprintln!(" An s2n-quic endpoint has been configured without cryptographic protections."); | |
| eprintln!(" This should ONLY be used for testing purposes."); |
Description of changes:
When doing performance testing, it can be helpful to disable crypto entirely in order to isolate and identify bottlenecks.
This change adds a private TLS provider that simply exchanges transport parameters over cleartext and doesn't encrypt anything. This has somewhat similar goals as the quic-disable-encryption draft, but is quite a bit simpler since it doesn't do any negotiation or handshaking.
Testing:
I added an integration test to make sure the null TLS provider negotiated with itself. I also added support for using it in the qns application to make it easier to benchmark.
Send
Send (XDP)
Receive
Receive (XDP)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.