Happy Eyeballs, resolution time delay#21354
Conversation
|
The PR's "resolve delay" follows the HEv3 spec. But should it and how much? Waiting on the AAAA result to arrive (if it is queried) makes sense as it keeps the ipv6 preference alive. Only when AAAA takes longer than 50ms will an earlier A response trigger the first connect attempt. I think that is fine. Including HTTPS resolution in the wait has the benefit that the HTTPS information is more likely to be applied to the connect. The counter argument is that there are still few sites with HTTPS records and (Waiting for HTTPS when needed for ECH is always done in either case. This wait is done in the TLS/QUIC filter and has nothing to do with the "resolve delay".) Update: I changed the implementation to only wait for HTTPS-RR when the connection is |
HEv3 describes conditions on when first connect attempts shall be started. https://www.ietf.org/archive/id/draft-ietf-happy-happyeyeballs-v3-01.html Chapter 4.2 libcurl now waits 50ms for AAAA and HTTPS results (when requested) to return before continuing with the connect. Added HTTPS-RR to the "was resolved" information info message. Changed logging of HTTPS-RR to a one-liner with RFC 9460 like formatting. This way the user can see if/what was resolved and used in connecting.
CF_TYPE_HTTPSRR to indicate a filter that makes use of HTTPS-RR information. Let cf-dns delay connecting the sub-filter chain until all wanted DNS resolve answer are available or the "DNS resolution delay" of 50ms was passed. De facto this applies HTTPS-RR delays only for https: connections. All other connection will only delay for AAAA.
icing
force-pushed
the
HEv3-resolution-delay
branch
from
67ff515 to
3a746c8
Compare
There was a problem hiding this comment.
Pull request overview
Implements Happy Eyeballs v3–style DNS resolution delay handling in the cf-dns connection filter, adds a cfilter capability flag for HTTPS-RR consumers, and improves user-visible/trace reporting of HTTPS-RR resolution details.
Changes:
- Add
CF_TYPE_HTTPSRRand a helper to detect whether a filter chain wants HTTPS-RR, socf-dnscan decide whether to wait for HTTPS RR answers. - Delay connecting the sub-filter chain until required DNS answers are available (or a 50ms cap is reached), aligning with HEv3 resolution delay behavior.
- Improve verbose/trace output: include HTTPS-RR in “was resolved” info and format HTTPS-RR as a single RFC 9460–like line.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/httpsrr.h | Adds forward decls and new APIs for printing and applicability checks. |
| lib/httpsrr.c | Implements HTTPS-RR one-line printing and applicability helper; refactors trace output. |
| lib/hostip.h | Adds async-resolve helper APIs for elapsed time and “has answers” checks. |
| lib/hostip.c | Implements new async-resolve helper APIs; adjusts async resolver logging paths. |
| lib/cfilters.h | Adds CF_TYPE_HTTPSRR and declares Curl_conn_cf_wants_httpsrr(). |
| lib/cfilters.c | Implements Curl_conn_cf_wants_httpsrr() flag scan through filter chain. |
| lib/cf-ip-happy.c | Minor connect-state handling tweak for CURLE_AGAIN. |
| lib/cf-https-connect.c | Uses Curl_httpsrr_applicable() and marks filter as HTTPS-RR consumer. |
| lib/cf-dns.h | Removes Curl_conn_dns_has_any_ai() API. |
| lib/cf-dns.c | Adds connect-delay logic and HTTPS-RR reporting in verbose resolve output. |
| lib/asyn.h | Adds stubs for HTTPS-RR async accessors when async resolver is not built. |
| lib/asyn-thrdd.c | Adjusts DNS reporting and fixes async result propagation. |
| lib/asyn-ares.c | Updates async completion tracking (async->done) for A/AAAA/HTTPS callbacks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* We want AAAA answer as we prefer ipv6. If a sub-filter desires | ||
| * HTTPS-RR, we check for that query as well. */ | ||
| uint8_t wanted_answers = CURL_DNSQ_AAAA; | ||
| if(Curl_conn_cf_wants_httpsrr(cf, data)) | ||
| wanted_answers |= CURL_DNSQ_HTTPS; | ||
|
|
||
| /* Note: if a query was never started, it is considered to have | ||
| * an answer (e.g. a negative one). */ | ||
| if(Curl_resolv_has_answers(data, ctx->resolv_id, wanted_answers)) | ||
| return TRUE; |
There was a problem hiding this comment.
cf_dns_ready_to_connect() always waits for an AAAA answer (and optionally HTTPS), even when this resolve was started with only an A query (e.g. IPv6 disabled / ipv6works()==FALSE). In that case Curl_resolv_has_answers() will treat AAAA as "answered" and allow the sub-filter chain to start connecting immediately, potentially before any IP address exists. Consider deriving the "wanted" IP answer from ctx->dns_queries (wait for AAAA only if it was actually requested; otherwise wait for A) so the chain never starts before at least one requested IP-family response is available (or the 50ms delay expires).
There was a problem hiding this comment.
I don't understand this comment.
- httpsrr: drop redundant checks. Follow-up to 809dda3 #21354 - httpsrr.h: drop obsolete comment. Follow-up to 2b3dfb4 #21175 - ws: drop redundant check in `curl_ws_start_frame()`. Follow-up to 37cecfc #17683 - ws: fix typo in comment. - tool_operate: fix VMS build. (broken since 2019-07-20, v7.66.0) Follow-up to b889408 #3804 Pointed out by Copilot Code Quality Closes #21393
HEv3 describes conditions on when first connect attempts shall be started. https://www.ietf.org/archive/id/draft-ietf-happy-happyeyeballs-v3-01.html Chapter 4.2
DNS resolution delay happens inside cf-dns. Add cfilter type flag
CF_TYPE_HTTPSRRto indicate a filter that makes use of HTTPS-RR information.Let cf-dns delay connecting the sub-filter chain until all wanted DNS resolve answer are available or the "DNS resolution delay" of 50ms was passed. De facto this applies HTTPS-RR delays only for https: connections. All other connection will only delay for AAAA.
Added HTTPS-RR to the "was resolved" information info message. Changed logging of HTTPS-RR to a one-liner with RFC 9460 like formatting. This way the user can see if/what was resolved and used in connecting.
Example: