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
Rec: fix setting of policy tags #13021
Merged
Merged
Conversation
This file contains 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
omoerbeek
force-pushed
the
rec-pb-cache-tags
branch
2 times, most recently
from
July 14, 2023 10:53
207389b
to
161d83a
Compare
rgacogne
approved these changes
Jul 17, 2023
rgacogne
approved these changes
Jul 17, 2023
…arate. We do pass them to the other Lua functions, but take care to erase them aagin before creating the partial PB message stored into the cache.
Co-authored-by: Remi Gacogne <github@coredump.fr>
omoerbeek
force-pushed
the
rec-pb-cache-tags
branch
from
July 20, 2023 08:30
1dbbc46
to
a89c325
Compare
wojas
approved these changes
Jul 20, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Also tested and confirmed that this resolves the issue.
omoerbeek
added a commit
to omoerbeek/pdns
that referenced
this pull request
Jul 20, 2023
omoerbeek
added a commit
to omoerbeek/pdns
that referenced
this pull request
Jul 20, 2023
omoerbeek
added a commit
to omoerbeek/pdns
that referenced
this pull request
Jul 20, 2023
This was referenced Jul 20, 2023
omoerbeek
added a commit
to omoerbeek/pdns
that referenced
this pull request
Jul 20, 2023
8 tasks
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.
Short description
As
gettag
can add protobuf policy tags, we do not want to cache the protobuf policy tags stored (and therefore fixed) in the packet cache.Also, set
SocketFamily
always (was missing on a cache hit).This is a Draft PR, as the current state of this PR changes behaviour if policy tags are set in an interception function (e.g.
postResolve
) later in the resolving process. In that case the policy tags should en up in the packet cache. I have to think a bit how to arrange that, and what to do ifgettag
also add tags for a case that does have policy tags stored already in the packet cache.Reported by @wojas
Follow-up: we now keep track of the tags set by
gettag(_ffi)
separately. This allows for them to not end up in the PC.Do pass the tags set by
gettag(_ffi)
to other other Lua functions, they might expect to see them and use them for conditional processing.Before constructing the partial protobuf message that gets stored into the PC, we strip the tags set by
gettag(_ffi)
from the set to be stored into the PC.Checklist
I have: