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

Configure default DNS resolver in 6lbr example #21117

Merged

Conversation

OlegHahm
Copy link
Member

@OlegHahm OlegHahm commented Jan 2, 2025

Contribution description

For convenience, it would be nice to have DNS resolution enabled by default in the examples. Hence, this patch configures a default DNS resolver on the 6lbr example and propagates its via RDNSS option in RAs to the nodes.

Once again the tradeoff is some increased ROM and RAM usage.

Testing procedure

  1. Setup a border router (for instance on the nrf52840dongle), for instance, via BOARD=nrf52840dongle make -C examples/gnrc_border_router clean all flash ULINK=cdc-ecm.
  2. Setup a second 6lowpan capable device with, for instance, gnrc_networking while enabling USEMODULE += sock_dns and USEMODULE += gnrc_ipv6_nib_dns.
  3. Check if pinging a hostname works, e.g., ping riot-os.org.

Issues/PRs references

Can be tested with #21116.

For convenience, it would be nice to have DNS resolution enabled by
default in the examples. Hence, this patch configures a default DNS
resolver on the 6lbr example and propagates its via RDNSS option in RAs
to the nodes.
@github-actions github-actions bot added the Area: examples Area: Example Applications label Jan 2, 2025
@benpicco benpicco requested a review from miri64 January 2, 2025 13:57
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 2, 2025
@riot-ci
Copy link

riot-ci commented Jan 2, 2025

Murdock results

✔️ PASSED

115b8d9 examples/gnrc_border_router: add DNS option

Success Failures Total Runtime
19 0 19 01m:54s

Artifacts

@@ -39,8 +39,9 @@ USEMODULE += ps
# Optionally include DNS support. This includes resolution of names at an
# upstream DNS server and the handling of RDNSS options in Router Advertisements
# to auto-configure that upstream DNS server.
#USEMODULE += sock_dns # include DNS client
#USEMODULE += gnrc_ipv6_nib_dns # include RDNSS option handling
USEMODULE += sock_dns # include DNS client
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
USEMODULE += sock_dns # include DNS client
USEMODULE += dns_cache # cache DNS responses
USEMODULE += sock_dns # include DNS client

Copy link
Member

Choose a reason for hiding this comment

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

Also, I think here too some ENABLE_DNS ?= 1 switch or something of the like would be helpful, in case e.g. you don't want the border router include RDNSS options in its RAs (e.g. to prevent RA fragmentation).

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to address both comments in my latest commit.

Introduces two Makefile switches for enabling DNS resolution and
caching.
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Untested ACK, but this just adds modules so any issue should be picked up by the CI.

@miri64 miri64 added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Jan 28, 2025
@miri64 miri64 enabled auto-merge January 28, 2025 20:06
@miri64 miri64 added this pull request to the merge queue Jan 28, 2025
Merged via the queue into RIOT-OS:master with commit 2e40d92 Jan 28, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants