Skip to content

LLT-7052: DNS packet codec#1688

Merged
tomasz-grz merged 1 commit intomainfrom
packet_codec
Mar 24, 2026
Merged

LLT-7052: DNS packet codec#1688
tomasz-grz merged 1 commit intomainfrom
packet_codec

Conversation

@tomasz-grz
Copy link
Copy Markdown
Contributor

@tomasz-grz tomasz-grz commented Feb 20, 2026

Problem

We want to get rid of all hickory_server dependencies, because it set's up a full blown DNS server, we maintain our own (outdated) fork of, and it was source of a lot of bugs.

As a first step we need to decide if DNS requests are related to .nord zone.
If they are, they should handled by our local resolver, and we need to serve the response packet back.
If not, the request is forwarded and resolved by the upstream nameservers.

Solution

This module incorporates:

  • a lightweight decoder, that converts DNS request bytes into a DnsPacket<'_> struct, and exposes a helper functions like parse_dns_packet(), find_nord_query() and normalize_qname() to simplify with the correct request handling.
  • A packet builder specific for the local resolver use case, that converts a ResponseKind enum into appropriate DNS response bytes.

☑️ Definition of Done checklist

  • Commit history is clean (requirements)
  • README.md is updated
  • Functionality is covered by unit or integration tests

Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Copy link
Copy Markdown
Contributor

@stalowyjez stalowyjez left a comment

Choose a reason for hiding this comment

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

I have mixed feeling for this PR, reducing dependencies is always nice, but it feels a bit like reinventing the wheel. We also have some code in libfirewall which does some DNS parsing, maybe we should create some public DNS helper crate for pnet which would address our needs?

Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs
Copy link
Copy Markdown
Contributor

@stalowyjez stalowyjez left a comment

Choose a reason for hiding this comment

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

Looks good enough, +1

Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
@tomaszklak
Copy link
Copy Markdown
Contributor

I have mixed feeling for this PR, reducing dependencies is always nice, but it feels a bit like reinventing the wheel.

I have the same feeling, and I'm also missing explanation why we want to "get rid of all hickory_server dependencies."

@tomaszklak
Copy link
Copy Markdown
Contributor

If they are, they should handled by our local resolver, and we need to serve the response packet back.

And what if they are not?

Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
@tomaszklak
Copy link
Copy Markdown
Contributor

Please, remember to cleanup (squash?) commits.

@tomaszklak
Copy link
Copy Markdown
Contributor

+1

@Jauler
Copy link
Copy Markdown
Contributor

Jauler commented Mar 18, 2026

Next time: It would be really nice to include ticket number in the branch name 🙏 🙏 🙏

Comment thread crates/telio-dns/src/packet_decoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs Outdated
Comment thread crates/telio-dns/src/packet_encoder.rs
Comment thread crates/telio-dns/tests/nameserver.rs
Copy link
Copy Markdown
Contributor

@Jauler Jauler left a comment

Choose a reason for hiding this comment

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

+1.0

Copy link
Copy Markdown
Contributor

@mathiaspeters mathiaspeters left a comment

Choose a reason for hiding this comment

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

+1

This module incorporates:
- a lightweight decoder, that converts DNS request bytes into a `DnsPacket<'_>`, and exposes a helper functions like
`parse_dns_query_packet()`, `find_nord_query()` and `normalize_qname()` to simplify with the correct request handling.
- A packet builder specific for the local resolver use case, that converts a `ResponseKind` enum into appropriate DNS response bytes.
@tomasz-grz tomasz-grz added this pull request to the merge queue Mar 24, 2026
Merged via the queue into main with commit 3a88026 Mar 24, 2026
58 checks passed
@tomasz-grz tomasz-grz deleted the packet_codec branch March 24, 2026 17:17
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.

5 participants