Add feature to return the original instead of a decrementing TTL - #275
#275Merged
gthess merged 8 commits intoJan 26, 2021
Merged
Conversation
added 6 commits
July 15, 2020 15:15
ralphdolmans
approved these changes
Jul 30, 2020
ralphdolmans
left a comment
Contributor
There was a problem hiding this comment.
Looks good! I have added some comments.
It would be nice to also have a testbound test that makes use of this option.
| d->ttl = SERVE_EXPIRED?SERVE_EXPIRED_REPLY_TTL:0; | ||
| else d->ttl -= now; | ||
| else d->ttl -= SERVE_ORIGINAL_TTL ? data->ttl_add : now; | ||
| d->ttl_add = 0; /* TTLs have been made relative */ |
Contributor
There was a problem hiding this comment.
For my understanding, is the d->ttl_add = 0 here "just to be safe", or really needed? Guess it is fine to have it either way.
Contributor
Author
There was a problem hiding this comment.
Since I don't know what will happen to the data in struct packed_rrset_data I added this just to be safe.
wcawijngaards
approved these changes
Jan 22, 2021
wcawijngaards
left a comment
Member
There was a problem hiding this comment.
Looks good, with comment on comparison in packed_rrset.c improvement.
Contributor
Author
|
Yup @gthess go ahead, thanks! |
gthess
added a commit
that referenced
this pull request
Jan 26, 2021
gthess
added a commit
that referenced
this pull request
Jan 26, 2021
- Merge PR #275 by Roland van Rijswijk-Deij, Add feature to return the original instead of a decrementing TTL ('serve-original-ttl').
jedisct1
added a commit
to jedisct1/unbound
that referenced
this pull request
Jan 30, 2021
* nlnet/master: (34 commits) - Fix for doxygen 1.8.20 compatibility. - Fix fwd ancil test post script when not supported. - Fix empty clause warning in edns pass for padding. - Fix to use correct type for label count in ipdnametoaddr rpz routine. - Fix empty clause warning in config_file nsid parse. - Fix to use correct type for label count in rpz routine. - Annotate that we ignore the return value of if_indextoname. - Fix compile of unbound-dnstap-socket without dnstap installed. - Ignore cache blacklisting when trying to reply with expired data from cache. (NLnetLabs#394) Changelog entry for: - Merge PR NLnetLabs#355 from noloader: Make ICANN Update CA and DS Trust Anchor static data. Changelog entry for: - Merge PR NLnetLabs#275 by Roland van Rijswijk-Deij, Add feature to return the original instead of a decrementing TTL ('serve-original-ttl'). Changelog entry for: - Merge PR NLnetLabs#408 from fobser: Prevent a few more yacc clashes. - Update example.con.in and add a testcase for PR NLnetLabs#275. Some review nits from George Test some different padding sizes padding.tdir text in single TXT RR tdir test for padding option Addressed review comment from @wcawijngaards Changelog entry for padding option Move NSID Changelog entry to day of merge ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a feature called serve-original-ttl which, if enabled, will make Unbound return the original TTL for records as received from upstream. This feature may be useful if Unbound serves as a front-end to, e.g., a hidden authoritative name server.
Notes on the PR:
packed_rrset_datastructures are treated the same as before. Instead, the new code stores an additional value in this structure callttl_addwhich records how much has been added to the TTLs in the structure, and can be subtracted from these absolute TTLs instead of the current time when encoding responses.packed_rrset_encodeinmsgencode.cwas changed slightly to make the time adjustment explicit. The original code sometimes modified the value of the function's input parametertimenow, setting it to 0. With the introduction of the new feature, this would have led to messy conditionals when adjusting the TTLs during encoding.cachedb/cachedb.cwas also changed such that the semantics of the decrement should be preserved while the original TTL will still be returned if this feature is enabled.struct packed_rrset_datathe code was changed in several places to explicitly initialise the newly allocated memory to zero, using eitherregional_alloc_zeroorcalloc(1, <size>).