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

dnsdist: Add support for range-based lookups into a Key-Value store #10525

Merged
merged 2 commits into from
Aug 1, 2021

Conversation

rgacogne
Copy link
Member

Short description

This feature allows doing a range-based lookup (mostly useful for IP addresses), assuming that:

  • there is a key for the last element of the range (2001:0db8:ffff:ffff:ffff:ffff:ffff:ffff for 2001:db8::/32)
    which contains the first element of the range (2001:0db8:0000:0000:0000:0000:0000:0000) followed by any data in the value
  • AND there is no overlapping ranges in the database !!

This requires that the underlying store supports ordered keys, which is true for LMDB but not for CDB, for example.

PS: this PR is based on the work done in #10520 and might require a rebase once it is merged. So you can skip the first two commits on that branch when reviewing :)

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@github-actions
Copy link

Misspellings found, please review:

  • exampe
To accept these changes, run the following commands from this repository on this branch
pushd $(git rev-parse --show-toplevel)
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"autohint dnsparser fastopen fff githubusercontent htm issuecomment proxyprotocol smb "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect.txt";
use File::Path qw(make_path);
make_path ".github/actions/spell-check";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"exampe "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
popd

@rgacogne rgacogne marked this pull request as ready for review June 25, 2021 07:00
@rgacogne rgacogne requested a review from omoerbeek July 7, 2021 10:10
pdns/dnsdist-lua-actions.cc Show resolved Hide resolved
pdns/dnsdistdist/dnsdist-kvs.cc Show resolved Hide resolved
Copy link
Member

@omoerbeek omoerbeek left a comment

Choose a reason for hiding this comment

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

This looks all good to me now. TSAN failure should be investigated.

@rgacogne
Copy link
Member Author

rgacogne commented Jul 9, 2021

This looks all good to me now.

Thanks, Otto!

TSAN failure should be investigated.

I looked into it, and it seems to be a timing issue (we have a QPS limit that might not match if the test takes too long). It's not related to this PR anyway so I'm going to merge it.

@rgacogne
Copy link
Member Author

rgacogne commented Jul 9, 2021

Ah, I need to merge 10520 first so I'll wait a bit :)

This feature allows doing a range-based lookup (mostly useful for IP addresses), assuming that:
- there is a key for the last element of the range (2001:0db8:ffff:ffff:ffff:ffff:ffff:ffff for 2001:db8::/32)
which contains the first element of the range (2001:0db8:0000:0000:0000:0000:0000:0000) followed by any data in the value
- AND there is no overlapping ranges in the database !!

This requires that the underlying store supports ordered keys, which is true for LMDB but not for CDB, for example.
…k byte order

Also document that tags are always created on a lookup, even when the
key does not exist. It's a bit weird but we should probably not change
that right now.
@rgacogne rgacogne merged commit 64f9fba into PowerDNS:master Aug 1, 2021
@rgacogne rgacogne deleted the ddist-lmdb-range branch August 1, 2021 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants