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

Integration of DNS resolver functions #827

Open
nabetti1720 opened this issue Feb 10, 2025 · 1 comment · May be fixed by #825
Open

Integration of DNS resolver functions #827

nabetti1720 opened this issue Feb 10, 2025 · 1 comment · May be fixed by #825

Comments

@nabetti1720
Copy link
Contributor

nabetti1720 commented Feb 10, 2025

While working on a simple implementation of the dns module in #825, I received an interesting comment.

This should reuse the DNS resolver we added for llrt_http. Probably need to split a lib crate.
Originally posted by @Sytten in #825 (comment)

I am struggling with how to proceed with the integration, so I would like to discuss this here.

  1. Should the results obtained from either fetch() or dns.lookup() be stored in the CachedDnsResolver?
  2. fetch() already reuses CachedDnsResolver results, but should dns.lookup() also reuse CachedDnsResolver results?
  3. For actual DNS name resolution, should I use toklo::net::lookup_host() or dns_lookup:lookup_host()? At least, dns_lookup:lookup_host() seems to behave the same as OS name resolution and also refers to /etc/hosts. I don't know about tokio's.

If my thinking is fundamentally flawed, please feel free to point it out. :)

@richarddavison
Copy link
Collaborator

toklo::net::lookup_host() should be used. AFAIK they both lower level syscalls/libc to perform the lookups. We can then remove the dns_lookup dependency

@nabetti1720 nabetti1720 linked a pull request Feb 21, 2025 that will close this issue
5 tasks
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 a pull request may close this issue.

2 participants