Skip to content
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

kTLS: implement sendmsg #4147

Merged
merged 8 commits into from
Aug 21, 2023
Merged

kTLS: implement sendmsg #4147

merged 8 commits into from
Aug 21, 2023

Conversation

toidiu
Copy link
Contributor

@toidiu toidiu commented Aug 15, 2023

Description of changes:

This PR adds an implementation for s2n_ktls_sendmsg. This essentially takes some data and some record_type and calls the sendmsg syscall. It creates a msghdr and configures the control data on it. Finally it is also doing some error handling (mainly IO blocked error).

Call-outs:

  • In the next PR we will add a s2n_ktls_recvmsg.
  • We will also handle the IO errors (should result in closing the connection) at higher layers.

Testing:

Unit tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Aug 15, 2023
@toidiu toidiu changed the title Ak ktls1 sendmsg kTLS: implement sendmsg Aug 15, 2023
tls/s2n_ktls_io.c Outdated Show resolved Hide resolved
tls/s2n_ktls_io.c Outdated Show resolved Hide resolved
tls/s2n_ktls_io.c Outdated Show resolved Hide resolved
tests/unit/s2n_ktls_io_test.c Outdated Show resolved Hide resolved
@toidiu toidiu force-pushed the ak-ktls1_sendmsg branch 4 times, most recently from 49f2867 to ab61b19 Compare August 17, 2023 19:43
@toidiu toidiu requested a review from lrstewart August 17, 2023 19:43
@toidiu toidiu marked this pull request as ready for review August 17, 2023 20:26
@toidiu toidiu force-pushed the ak-ktls1_sendmsg branch 2 times, most recently from 23c3f3a to 60e6dd2 Compare August 17, 2023 22:57
tests/testlib/s2n_ktls_test_utils.c Outdated Show resolved Hide resolved
tests/unit/s2n_ktls_io_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_ktls_io_test.c Outdated Show resolved Hide resolved
tests/unit/s2n_ktls_io_test.c Show resolved Hide resolved
tests/unit/s2n_ktls_io_test.c Outdated Show resolved Hide resolved
Comment on lines 186 to 189

RESULT_ENSURE(msg_iov->iov_base, S2N_ERR_INVALID_ARGUMENT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are creating a msghdr using iovec and there is an assumption that iovec is not malformed. This check confirms that assumption. If iovec is NULL then msghdr is also malformed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the iovec be considered malformed if iov_base is NULL and iov_len is 0?

Copy link
Contributor Author

@toidiu toidiu Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ill check and ban both NULL and 0.. that seems safer for now and we can support sending 0 later if we need
changed my mind. I'll allow both since we should be able to send 0 data and thats valid.

@toidiu toidiu force-pushed the ak-ktls1_sendmsg branch 2 times, most recently from 1f3fa42 to 5dab077 Compare August 18, 2023 18:14
@toidiu toidiu requested a review from lrstewart August 19, 2023 00:00
@toidiu toidiu enabled auto-merge (squash) August 21, 2023 22:19
@toidiu toidiu merged commit 346bd1a into aws:main Aug 21, 2023
23 checks passed
@toidiu toidiu deleted the ak-ktls1_sendmsg branch August 21, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants