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

Do not force SSL_MODE_AUTO_RETRY #122

Open
craff opened this issue May 31, 2023 · 4 comments
Open

Do not force SSL_MODE_AUTO_RETRY #122

craff opened this issue May 31, 2023 · 4 comments

Comments

@craff
Copy link
Contributor

craff commented May 31, 2023

Currently all context have this flag, which is bad in non blocking context with eio or simple_httpd as the scheduler will loose opportunities to switch task.

@craff
Copy link
Contributor Author

craff commented Jun 2, 2023

PR #134 should solve this

@anmonteiro
Copy link
Collaborator

I don't know if we need to solve this at all. Here's what the documentation says:

SSL_MODE_AUTO_RETRY
Never bother the application with retries if the transport is blocking. If a renegotiation take place during normal operation, a SSL_read(3) or SSL_write(3) would return with -1 and indicate the need to retry with SSL_ERROR_WANT_READ. In a non-blocking environment applications must be prepared to handle incomplete read/write operations. In a blocking environment, applications are not always prepared to deal with read/write operations returning without success report. The flag SSL_MODE_AUTO_RETRY will cause read/write operations to only return after the handshake and successful completion.

My reading is that this flag only has effect on blocking sockets. So your non-blocking use case shouldn't be impacted. And I've definitely verified myself in eio-ssl that non-blocking sockets get retry exceptions.

@craff
Copy link
Contributor Author

craff commented Jun 3, 2023

Yes, I saw that also. But I think with non blocking we want allow_partial_write in most application ?
and we probably want async (but the documentation is really very short for this).

So in the PR #134, I put as default

  • auto_retry in the general case
    and
  • async + allow_partial_write
    in Runtime_lock

I want to find more information on async ...

For auto_retry in non blocking setting, the documentation states clearly that we must be prepared to retry anyway, but I don't known for sure if the number of retries is affected by this option. There could be other reason for retry than renegociation ? We could try and count the retry on an example with and without the option. Now that the PR contains an example.

Anyway the PR give the oportunuty to set all mode (except "accept moving buffer", which is mandatory in OCaml if we allocate the buffer in the heap)

@craff
Copy link
Contributor Author

craff commented Jun 3, 2023

I forgot to say: I think we want the mode with as much as possible retry to switch context as much as possible.

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

No branches or pull requests

2 participants