fix(connlib): don't buffer when recreating DNS resource NAT - #8935
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
6fa0d51 to
b5270f1
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR improves DNS resource NAT handling by allowing DNS handshakes to occur in parallel with packet flow when recreating a DNS resource NAT, thereby reducing packet buffering delays. Key changes include:
- Adding telemetry for tracking dropped packets due to full buffers.
- Introducing a new DNS NAT state "Recreating" and a new is_recreating flag.
- Refactoring DNS resource NAT recreation logic in the client.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| rust/connlib/tunnel/src/unique_packet_buffer.rs | Enhanced logging and telemetry tracking when buffering packets. |
| rust/connlib/tunnel/src/client.rs | Added new state and flag for DNS NAT recreation as well as updating DNS NAT handling methods. |
b5270f1 to
6563c80
Compare
|
@jamilbk Let me know what you think of the changelog entry, I figured it is important enough to include but wasn't quite sure how to best word this. |
jamilbk
left a comment
jamilbk
left a comment
There was a problem hiding this comment.
Interesting find. I'm sure this fixes an edge case or two.
| /// | ||
| /// We model the [`DnsResourceNatState::Recreating`] state differently from just removing the entry to allow packets | ||
| /// to continue flowing to the Gateway while the DNS resource NAT is being recreated. | ||
| /// In most cases, the DNS records will not change and as such, performing this will not interrupt the flow of packets. |
There was a problem hiding this comment.
Just to confirm, does musl's dns on the gateway always return the same IP if there are multiple A or AAAA records for a domain? If not would that cause anything to break here?
There was a problem hiding this comment.
These codepaths don't affect Mac/Windows?
In order to detect changes to DNS records of DNS resources,
connlibwill recreate the DNS resource NAT whenever it receives a query for a DNS resource. The way we implemented this was by clearing the local state of the DNS resource NAT, which triggered us to perform the handshake with the Gateway again upon the next packet for this resource. The Gateway would then perform the DNS query and respond back when this was finished.In order to not drop any packets,
connlibhas a buffer where it keeps the packets that are arriving in the meantime. This works reasonably well when the connection is first set up because we are only buffering a TCP SYN or equivalent handshake packet. Yet, when the connection is full use, and the application just so happens to make another DNS query, we halt the entire flow of packets until this is confirmed again. To prevent high memory use, the buffer for this packets is constrained to 32 packets which is nowhere near enough when a connection is actively transferring data (like a file upload).In most cases, the DNS query on the Gateway will yield the exact same results as because the records haven't changed. Thus, there is no reason for us to actually halt the flow of these packets when we are recreating the DNS resource NAT. That way, this handshake happens in parallel to the actual packet flow and does not interrupt anything in the happy path case.