Draft: Prevent generating broken glueless referrals
I think this MR really needs a high-level description, so here I go. (As usually, I tried hard to be very thorough both in both commit log messages and code comments, but still.)
#1967 turned out to be surprisingly frustrating to fix across all supported versions of BIND as:
~"v9.11" uses non-refactored query handling code (
bin/named/query.c) and also has an optional additional cache, but no glue cache (which is 9.12+),
v9.16+ has refactored query handling code (
lib/ns/query.c) and does not support the additional cache feature, but does have an optional glue cache (different feature).
All in all, fixing this in all maintained branches would require preparing four different variants of a non-trivial fix:
bin/named/query.cwith additional cache enabled,
bin/named/query.cwith additional cache disabled,
lib/ns/query.cwith glue cache enabled,
lib/ns/query.cwith glue cache disabled.
This looks like overkill to me for something that is not a critical issue.
This MR only contains the last two variants of the fix from the list above, i.e. it addresses the problem in v9.16 and ~"v9.17" both with and without the glue cache.
But honestly, I would like to go a step further. My suggestion is to:
only fix the "glue cache enabled" variant,
glue-cacheoption in ~"v9.17" and leave it always on (in a separate MR, of course).
The rationale for introducing the
glue-cache option was:
The rationale for this option is that we don't have any feedback on how the glue cache is going to behave in the real world just yet. There should be an option to bypass it.
To the best of my knowledge, we have received zero problem reports which
mention this feature in the past 3 years (since glue cache was
introduced). I also have not seen the
glue-cache no; setting in
configuration files people use these days.
Here is a brief overview of the branch (doc updates excluded):
gluesystem test cleanup; nothing fancy and there are just two checks in that test right now,
0cd1dbe5: "glue cache on" fix, part 1,
dddbe70d: "glue cache on" fix, part 2,
7684320f: "glue cache off" fix.
The only reason I split the "glue cache on" fix into two parts is to
make the fix easier to follow as each of these parts touches a different
I would be more than happy to drop the "glue cache off" part of this MR, please do not treat the amount of comments I put in there as a factor :)
Finally, both "glue cache on" and "glue cache off" variants could
probably be replaced with a fix that affects a common code path,
similarly to e.g.
the code implementing such a fix would be more complicated than the changes suggested in this MR,
I expect such a fix to perform worse than the changes suggested in this MR (by putting the fix in glue cache code, each referral is only "analyzed" once - a "common path" fix would need to do that for every response sent).
Once the path forward with this MR is agreed upon, I would like to run it through Perflab before merging it.