aws
Add a buffered send to s2n.
Resolved issues:
N/A
Description of changes:
Currently s2n-tls will flush everytime a TLS record is created. This results in a large amount of IO writes. This change introduces a way to buffer multiple records into a single IO write to reduce the amount of writes / kernel context switches.
This feature can be useful for applications that need a higher throughput.
Call-outs:
Should we introduce an integration tests for this feature?
Testing:
How is this change tested (unit tests, fuzz tests, etc.)? Are there any testing steps to be verified by the reviewer? Added unit tests and manually did a self-talk tests with s2nc and s2nd.
Is this a refactor change? If so, how have you proved that the intended behavior hasn't changed? I've added unit tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
One thing we need to call out is after enabling this, there will be data stored in the connection's buffer and may need to be flushed, even though we indicated that all bytes were accepted from the s2n_send method.
We may want to introduce s2n_flush as a public function so applications can ensure all of their buffered records are actually written to the socket before doing something else.
One thing we need to call out is after enabling this, there will be data stored in the connection's buffer and may need to be flushed, even though we indicated that all bytes were accepted from the
s2n_sendmethod.
I think this is already how s2n_send is operating right? We do subtract the bytes that were not written here
Currently there is a scenario where the flush might fail here and the flush here will drain what the previous send failed to process.
IMO the bytes written on line 99 should be added to conn->current_user_data_consumed but I wasn't entirely sure, and figured I could do a follow up PR.
We may want to introduce
s2n_flushas a public function so applications can ensure all of their buffered records are actually written to the socket before doing something else.
I agree it should be exposed, it would give applications a lot more control
CI is failing for an unrelated reason. I'll merge in the fix when it's ready (May be having to point to a different OpenSSL version).
I still don't see any test cases for post-handshake messages interrupting the buffered write-- am I missing them?
I need to determine what's the right behavior in https://github.com/aws/s2n-tls/pull/3364