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

Forward vectored writes #25

Closed

Conversation

thomaseizinger
Copy link

@thomaseizinger thomaseizinger commented Oct 29, 2023

Early draft implementation to collect feedback. Also missing tests! Would appreciate some guidance on how to best test this (and perhaps avoid some code duplication?).

Resolves: #26.

src/client.rs Outdated
@@ -141,6 +142,18 @@ impl<IO> AsyncWrite for TlsStream<IO>
where
IO: AsyncRead + AsyncWrite + Unpin,
{
fn poll_write_vectored(self: Pin<&mut Self>, cx: &mut Context<'_>, bufs: &[IoSlice<'_>]) -> Poll<Result<usize, Error>> {
Copy link
Member

Choose a reason for hiding this comment

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

Also process early_data and provide tests.

Copy link
Author

Choose a reason for hiding this comment

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

Can you give me some guidance on how to best write tests for this? There are quite a few edge cases that would need testing and I am unsure how to do that.

Copy link
Member

Choose a reason for hiding this comment

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

You can refer to https://github.com/rustls/tokio-rustls/blob/main/tests/early-data.rs .
because early rustls does not support server-side early_data, only limited testing can be done with openssl. now we should be able to use rustls for testing.

src/common/mod.rs Outdated Show resolved Hide resolved
Comment on lines +168 to +171
// If we didn't write the entire buffer, return early.
if len != buf.len() {
return Poll::Ready(Ok(written));
}
Copy link
Author

Choose a reason for hiding this comment

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

I am not sure this is entirely correct. We will be called again because we returned Poll::Ready, ending up in an endless loop because len will probably be 0 and we thus won't ever get out of this.

Perhaps we shouldn't bother with looping over bufs and just only write the first non-empty buffer? As long as we return Poll::Ready, any caller should call us again until we return Poll::Pending.

Copy link
Member

Choose a reason for hiding this comment

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

If early_data write returns 0, it means the early_data buffer is full and we should break out of loop and try handshake.

I agree that it is safe to just write to first non-empty buf and have no preference for this.


while self.session.wants_write() {
match self.write_io(cx) {
Poll::Ready(Ok(0)) | Poll::Pending => return Poll::Pending,
Copy link
Member

Choose a reason for hiding this comment

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

here need check written != 0

@djc
Copy link
Member

djc commented Mar 5, 2024

Superseded by #45.

@djc djc closed this Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Override poll_write_vectored?
3 participants