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

Add max-query-restarts option #461

Merged
merged 5 commits into from Dec 13, 2022
Merged

Conversation

cgallred
Copy link
Contributor

@cgallred cgallred commented Apr 6, 2021

Resolves #438

This is a proposed new option to allow a user to override the hardcoded MAX_RESTART_COUNT limit.

I looked at other PRs that added config options, please let me know if I'm missing anything. I've verified that with a test config file test.conf containing max-query-restarts: 9, I'm able to successfully resolve logincdn.msauth.net with unbound-host -C test.conf.

@steve-boyle
Copy link

I keep running into the bug in #438 on the latest version of pfSense (CE 2.6.0 as of this writing). This pull request would let me resolve the issue myself. My only choice now is to use a different DNS resolver - other resolvers do not have this issue. I'm happy to replace unbound but I prefer to just tweak it to work with Microsoft logins.

Unbound users cannot control what Microsoft and CDNs do with CNAMEs. You've left us out in the cold with no solution. You need to allow this or create a real solution for #438. As it stands I cannot use Unbound with Microsoft services.

@johnnyxwan
Copy link

johnnyxwan commented Apr 18, 2022

I have been running into this MAX_RESTART_COUNT issue for long. Silly enough, I have to create forward-zones for domains affected by this issue and forward the queries to external DNS resolver. This new option is very important and essential in my opinion, since I encounter this issue more and more frequently. Although not a scientific research, it seems a lot of CDNs are using longer and longer cname chain.

In the related issue #438, contributor of this project showed their reluctant to this change in Aug 2021. However, I would encourage the team to review this decision and hopefully merge this pull one day.

gthess added a commit that referenced this pull request Dec 13, 2022
@gthess gthess merged commit 71db243 into NLnetLabs:master Dec 13, 2022
@gthess
Copy link
Member

gthess commented Dec 13, 2022

In the end we merged this. We have been seeing more demand for a configurable option both here and from other sources.
Thanks for this :)

jedisct1 added a commit to jedisct1/unbound that referenced this pull request Dec 13, 2022
* nlnet/master:
  - Updates for NLnetLabs#461 (Add max-query-restarts option).
  - Expose 'max-sent-count' as a configuration option; the   default value retains Unbound's behavior.
  - Expose 'statistics-inhibit-zero' as a configuration option; the   default value retains Unbound's behavior.
  - Fix to wrap Makefile scripts directory in quotes for uninstall.
  Changelog note for NLnetLabs#808 - Merge NLnetLabs#808: Wrap Makefile script's directory variables in quotes.
  wrap directory variables in quotes
  Fix date.
  - Fix NLnetLabs#773: When used with systemd-networkd, unbound does not start   until systemd-networkd-wait-online.service times out.
  - Clear documentation for interactivity between the subnet module and   the serve-expired and prefetch configuration options.
  - Add SVCB and HTTPS to the types removed by 'unbound-control flush'.
  - Fix NLnetLabs#782: Segmentation fault in stats.c:404.
  Changelog entry for NLnetLabs#720
  Document max-query-restarts option
  Use max-query-restarts in iterative resolver
  Add max-query-restarts to grammar and lexer
  Add max-query-restarts config parameter
@cgallred cgallred deleted the restart_conf branch January 4, 2023 18:48
@cgallred
Copy link
Contributor Author

cgallred commented Jan 4, 2023

Just catching up from vacation. Thanks for accepting the change!

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.

Resolving records through more than 8 CNAME fails due to hardcoded MAX_RESTART_COUNT
4 participants