Replies: 19 comments 40 replies
-
|
I'll answer some of the questions leaving the rest for @hlandau. Re (1): This is definitely something that is missing in our API and should be added. We could either add SSL_write_ex2() with a flag parameter or we could add a flag to be used with SSL_stream_conclude() such as SSL_STREAM_CONCLUDE_FLAG_ON_NEXT_WRITE. This should be a fairly simple API addition. Re (5): Polling will be added with the server support at the latest. Please see this #21795 PR for the design document. |
Beta Was this translation helpful? Give feedback.
-
|
BTW thank you very much for this feedback, it is much appreciated. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks @t8m. Re (1): seems a good way forward. Personally, I'd prefer the Re (5): curl needs and does its own polling. It may be even done by the |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the feedback. It is greatly appreciated and invaluable to our future efforts. Answering your questions out of order:
Thanks for your feedback and your work on integrating this into curl. This is invaluable for us and will definitely inform our future development efforts. Don't hesitate to drop us a line if you have other questions. |
Beta Was this translation helpful? Give feedback.
-
|
@icing So, to reiterate the current status of things:
With regards to (2), it seems to me like there's a bit more going on here than I thought. You say that the CPU is getting pegged because SSL_want_write() is returning false too early. But of course, the CPU needs to be executing code to call SSL_want_write() in the first place; so I guess my question is how you're avoiding pegging the CPU in other cases where SSL_want_write() is true. The basic idea here for an asynchronous application is to use SSL_net_write_desired(), SSL_net_read_desired() and SSL_get_event_timeout() to determine when the QUIC engine wants to do internal processing:
When any of these things happen, there is useful work for the QUIC engine to do and that internal processing might cause a situation where a stream which blocked on SSL_write() can now accept more data. I assume you're already plugging whatever asynchronous I/O event handling system you have into SSL_net_read_desired(), SSL_net_write_desired() and SSL_get_event_timeout(), which is the idea. So the simple thing to do here is, when one of those events occurs, is call SSL_write() and see if it works, and As an aside - the 'want write' API is a little bit gnarly and this is a reflection of the existing libssl API here. SSL_want_write() is just a front for SSL_get_error(). SSL_get_error() is supposed to be updated with the result of the last API call, if
In other words, the current value of SSL_want_write() gets clobbered by making e.g. a SSL_read() call and vice versa. But other calls like SSL_get_stream_read_state() are "non-I/O" and only clobber the value of SSL_get_error()/SSL_want_read()/SSL_want_write() if they fail. Basically these APIs are only really intended to be used for retrieving information about the last API call immediately after it is made. Admittedly this subtlety might be quite confusing, especially the fact that some API calls modify SSL_get_error() and some don't. This is part of our existing TLS API, so we've tried to preserve the existing semantics here. As always looking forward to your input. |
Beta Was this translation helpful? Give feedback.
-
|
Another detail: |
Beta Was this translation helpful? Give feedback.
-
Statement of the problem (write blocking)This is a summary of the problem as I understand it, which is a real issue:
Proposed solutionThere are two solutions I propose to provide various options here:
The design of potential additional APIs which offer callbacks as an alternative to polling is also still under investigation and I will keep you informed. But would these two things be a start to giving you some viable options here? Let me know what you think — all of this is based on the requirements you've articulated. As mentioned, this input has been really valuable — thanks again. |
Beta Was this translation helpful? Give feedback.
-
|
#23360 has merged and will be in OpenSSL 3.3. |
Beta Was this translation helpful? Give feedback.
-
|
PRs coming out of this discussion which have merged and will ship in 3.3:
PRs undergoing review and expected to ship in 3.3:
Current status:
I've done some investigation into the feasibility of delivering the registered polling API for 3.3, and come to the conclusion that it's not really plausible. There is relatively little opportunity for scope reduction in terms of the work required that would still deliver something useful while also offering greater performance than SSL_poll. Of course this API is guaranteed to happen, but will need to be scheduled post-3.3. (As you may appreciate from e.g. some of the design mistakes in epoll that we are stuck with, poller APIs are a complex and subtle matter and it is important to get them right, so it's not something to be rushed. Plus, since the objective of such an API is to offer greater performance than SSL_poll(), I don't think it makes sense to ship it without having the opportunity to test and validate that the design meets the performance objectives.) The prospect of providing some kind of state transition callbacks is interesting, and I'm happy to have something like that if it makes things easier for specific applications, but there are a fair number of design considerations around the API and some of the related complications it creates. Realistically, that's not something to be rushed, and isn't going to be deliverable for 3.3. I think that this basically concludes the business that comes out of this discussion which can make 3.3.
Are you looking to determine if a connection has been terminated due to the QUIC idle timeout? You can identify this condition by calling Let me know if there's anything else I can do to help. I'll try to make some time to look at your branch some point, since it will definitely be useful input. |
Beta Was this translation helpful? Give feedback.
-
|
Any plans to have this discussion for openssl 3.4.0? |
Beta Was this translation helpful? Give feedback.
-
|
@levitte This discussion is sufficient, or a new discussion will be more helpful? |
Beta Was this translation helpful? Give feedback.
-
|
For the If I use the Could there an API like |
Beta Was this translation helpful? Give feedback.
-
@icing Can you confirm that 1,3,4 are fully addressed, and if not, what missing on them? Thank you all for your work. |
Beta Was this translation helpful? Give feedback.
-
|
1 and 3 have been addressed. About 4 I am not sure how it addresses the situation. If QUIC packet processing/generating needs to be done "explicit", it is not clear to me what this means. Is there sample code somewhere that shows how a client is supposed to use that? The proposed 5 seems to imply an iteration over all ongoing streams whenever the state of the connection needs to be assessed. If the current "partially addressed" proposal "will not scale to large numbers of objects" I am hesitant in investing time to evaluate it. As I described before, we need to be able to quickly asses the POLL status of a QUIC connection. Is a stream blocked on writing or not? This is not to be confused with the total connection state. Stream A and B are sending. A is blocked, B may write. We have data for A, but no data for B right now. So we should not POLLOUT the socket. These kind of decisions we currently are unable to make in a cost efficient manner. The current state of our QUIC implementation can be seen here: https://github.com/icing/blog/blob/main/curl-h3-performance.md. As you see in the second chart, OpenSSL QUIC is at 50% of the peformance of the other implementations. We'd appreciate any advice on how that can be improved. |
Beta Was this translation helpful? Give feedback.
-
|
@icing For (4), does |
Beta Was this translation helpful? Give feedback.
-
|
Regarding item 5 above, does SSL_poll not provide the data needed here? That is to say, SSL_poll will tell you which SSL stream objects need reading via the SSL_EVENT_READ revent flag. Or is there something more you are looking for here? |
Beta Was this translation helpful? Give feedback.
-
@icing Is 4 is address by that? Thank you for your time and your hard work on curl! 🙏🏻 |
Beta Was this translation helpful? Give feedback.
-
|
@icing regarding item 2 above, why does SSL_poll on a stream object waiting for a SSL_POLL_EVENT_W event not meet your needs here? Just trying to get my head around what you need here. |
Beta Was this translation helpful? Give feedback.
-
|
@nhorman |
Beta Was this translation helpful? Give feedback.
-
With curl/curl#12734 we have an experimental PR that adds HTTP/3 support to curl using the OpenSSL's own QUIC implementation. It has shortcomings which may be the result of us missing features in the API or them being absent. I open this discussion to find out which is which and how we can improve our use of the API or where the API needs improvement.
1. Requests without BODY
Update: this is resolved with OpenSSL 3.3.x and curl 8.7.1.
When sending a HTTP request without BODY (commonly a GET), nghttp3 gives us the data to be written to QUIC, plus the indication of FIN. This we translate into (roughly):
This seems to result in QUIC STREAM packets with the data and a final empty STREAM with the FIN set. This is sub-optimal, as server implementations may start processing the request before seeing the last STREAM+FIN, thus needing to check for a request body. HTTP/3 gateways to a HTTP/1.1 may have to add
Transfer-Encoding: chunkedto such a request. This is sub-optimal for GET requests that make a majority of the traffic.Question: is there a way to write data to the SSL* together with the FIN?
2. Send Blocking
When doing large uploads, the stream/connection window will get exhausted, and QUIC waits for server ACKs to make progress. This is detected by writes to a stream SSL* returning SSL_ERROR_WANT_WRITE. We then block the nghttp3 stream for futher processing.
What we lack in curl is the indication when to correctly unblock that stream processing again. For that we currently use
SSL_want_write(stream->ssl). When that returns FALSE, we unblock. However, this returns FALSE much too early, unblocking the stream only to immediately giving SSL_ERROR_WANT_WRITE again. This leads to curl using 100% cpu on a single large upload that is merely under flow control.Question: how can we manage this situation better?
Update: we use
SSL_net_write_desired(connssl)to decide the POLLOUT status. A single, blocked stream seems to result in us keep on triggering writes.3. Idle Timeout
Update: this is resolved with OpenSSL 3.3.x and curl 8.7.1.
In curl, we have a configurable idle time for a connection until it is considered stale and will be discarded. In other QUIC stacks we set this in the transport parameters so that the peer learns about it. We also inspect the remote transport parameter to learn what the peer's idle time is. The minimum of both then is the effective time we use in our connection pools.
Question: is there a way to set the local and query the remote idle time?
4. Left Bidi streams
Update: this is resolved with OpenSSL 3.3.x and curl 8.7.1.
QUIC negotiates and updates the max number of bidi streams a peer may open. This is important to know for curl in order to decide when to use an existing connection, open a new one, or delay starting the transfer. Since the number of streams currently open is an internal state of the QUIC stack and depends on various things, curl needs an up-to-date indication for this. For example, the quiche implementation has a
quiche_conn_peer_streams_left_bidi(conn)that gives us this.Question: is there a facility in OpenSSL's QUIC to get that information?
5. Efficient Stream Receives
When curl polls the QUIC socket for POLLIN and then invokes receiving and processing the data, we have found no indications or callbacks that inform us which QUIC streams need action afterwards. Our current implementation therefore needs to
SSL_read_ex(stream->ssl)on all open streams of a connection, every time we suspect that QUIC packets from the server have arrived. We have curl applications that do hundreds of transfers in parallel, depending on the peer's capabilities many on the same QUIC connection.Question: can curl somehow learn if QUIC stream need reads and, ideally, which streams need read? Or install a callback that is invoked with the stream id and either the data or an event identifier?
Thanks for your support.
Beta Was this translation helpful? Give feedback.
All reactions