1. 21 Jul, 2020 1 commit
    • Ondřej Surý's avatar
      Fix the rbt hashtable and grow it when setting max-cache-size · e24bc324
      Ondřej Surý authored
      There were several problems with rbt hashtable implementation:
      
      1. Our internal hashing function returns uint64_t value, but it was
         silently truncated to unsigned int in dns_name_hash() and
         dns_name_fullhash() functions.  As the SipHash 2-4 higher bits are
         more random, we need to use the upper half of the return value.
      
      2. The hashtable implementation in rbt.c was using modulo to pick the
         slot number for the hash table.  This has several problems because
         modulo is: a) slow, b) oblivious to patterns in the input data.  This
         could lead to very uneven distribution of the hashed data in the
         hashtable.  Combined with the single-linked lists we use, it could
         really hog-down the lookup and removal of the nodes from the rbt
         tree[a].  The Fibonacci Hashing is much better fit for the hashtable
         function here.  For longer description, read "Fibonacci Hashing: The
         Optimization that the World Forgot"[b] or just look at the Linux
         kernel.  Also this will make Diego very happy :).
      
      3. The hashtable would rehash every time the number of nodes in the rbt
         tree would exceed 3 * (hashtable size).  The overcommit will make the
         uneven distribution in the hashtable even worse, but the main problem
         lies in the rehashing - every time the database grows beyond the
         limit, each subsequent rehashing will be much slower.  The mitigation
         here is letting the rbt know how big the cache can grown and
         pre-allocate the hashtable to be big enough to actually never need to
         rehash.  This will consume more memory at the start, but since the
         size of the hashtable is capped to `1 << 32` (e.g. 4 mio entries), it
         will only consume maximum of 32GB of memory for hashtable in the
         worst case (and max-cache-size would need to be set to more than
         4TB).  Calling the dns_db_adjusthashsize() will also cap the maximum
         size of the hashtable to the pre-computed number of bits, so it won't
         try to consume more gigabytes of memory than available for the
         database.
      
         FIXME: What is the average size of the rbt node that gets hashed?  I
         chose the pagesize (4k) as initial value to precompute the size of
         the hashtable, but the value is based on feeling and not any real
         data.
      
      For future work, there are more places where we use result of the hash
      value modulo some small number and that would benefit from Fibonacci
      Hashing to get better distribution.
      
      Notes:
      a. A doubly linked list should be used here to speedup the removal of
         the entries from the hashtable.
      b. https://probablydance.com/2018/06/16/fibonacci-hashing-the-optimization-that-the-world-forgot-or-a-better-alternative-to-integer-modulo/
      e24bc324
  2. 08 Jul, 2020 1 commit
    • Ondřej Surý's avatar
      Update STALE and ANCIENT header attributes atomically · 81d4230e
      Ondřej Surý authored
      The ThreadSanitizer found a data race when updating the stale header.
      Instead of trying to acquire the write lock and failing occasionally
      which would skew the statistics, the dns_rdatasetheader_t.attributes
      field has been promoted to use stdatomics.  Updating the attributes in
      the mark_header_ancient() and mark_header_stale() now uses the cmpxchg
      to update the attributes forfeiting the need to hold the write lock on
      the tree.  Please note that mark_header_ancient() still needs to hold
      the lock because .dirty is being updated in the same go.
      81d4230e
  3. 01 Jul, 2020 1 commit
  4. 18 Jun, 2020 1 commit
    • Mark Andrews's avatar
      Remove INSIST from from new_reference · 0854f631
      Mark Andrews authored
      RBTDB node can now appear on the deadnodes lists following the changes
      to decrement_reference in 176b23b6 to
      defer checking of node->down when the tree write lock is not held.  The
      node should be unlinked instead.
      0854f631
  5. 29 May, 2020 2 commits
  6. 12 May, 2020 1 commit
  7. 21 Apr, 2020 1 commit
    • Ondřej Surý's avatar
      Complete rewrite the BIND 9 build system · 978c7b2e
      Ondřej Surý authored
      The rewrite of BIND 9 build system is a large work and cannot be reasonable
      split into separate merge requests.  Addition of the automake has a positive
      effect on the readability and maintainability of the build system as it is more
      declarative, it allows conditional and we are able to drop all of the custom
      make code that BIND 9 developed over the years to overcome the deficiencies of
      autoconf + custom Makefile.in files.
      
      This squashed commit contains following changes:
      
      - conversion (or rather fresh rewrite) of all Makefile.in files to Makefile.am
        by using automake
      
      - the libtool is now properly integrated with automake (the way we used it
        was rather hackish as the only official way how to use libtool is via
        automake
      
      - the dynamic module loading was rewritten from a custom patchwork to libtool's
        libltdl (which includes the patchwork to support module loading on different
        systems internally)
      
      - conversion of the unit test executor from kyua to automake parallel driver
      
      - conversion of the system test executor from custom make/shell to automake
        parallel driver
      
      - The GSSAPI has been refactored, the custom SPNEGO on the basis that
        all major KRB5/GSSAPI (mit-krb5, heimdal and Windows) implementations
        support SPNEGO mechanism.
      
      - The various defunct tests from bin/tests have been removed:
        bin/tests/optional and bin/tests/pkcs11
      
      - The text files generated from the MD files have been removed, the
        MarkDown has been designed to be readable by both humans and computers
      
      - The xsl header is now generated by a simple sed command instead of
        perl helper
      
      - The <irs/platform.h> header has been removed
      
      - cleanups of configure.ac script to make it more simpler, addition of multiple
        macros (there's still work to be done though)
      
      - the tarball can now be prepared with `make dist`
      
      - the system tests are partially able to run in oot build
      
      Here's a list of unfinished work that needs to be completed in subsequent merge
      requests:
      
      - `make distcheck` doesn't yet work (because of system tests oot run is not yet
        finished)
      
      - documentation is not yet built, there's a different merge request with docbook
        to sphinx-build rst conversion that needs to be rebased and adapted on top of
        the automake
      
      - msvc build is non functional yet and we need to decide whether we will just
        cross-compile bind9 using mingw-w64 or fix the msvc build
      
      - contributed dlz modules are not included neither in the autoconf nor automake
      978c7b2e
  8. 13 Mar, 2020 1 commit
    • Mark Andrews's avatar
      Silence missing unlock from Coverity. · 8dd8d48c
      Mark Andrews authored
      Save 'i' to 'locknum' and use that rather than using
      'header->node->locknum' when performing the deferred
      unlock as 'header->node->locknum' can theoretically be
      different to 'i'.
      8dd8d48c
  9. 06 Mar, 2020 1 commit
    • Evan Hunt's avatar
      improve calculation of database transfer size · 98b55eb4
      Evan Hunt authored
      - change name of 'bytes' to 'xfrsize' in dns_db_getsize() parameter list
        and related variables; this is a more accurate representation of what
        the function is doing
      - change the size calculations in dns_db_getsize() to more accurately
        represent the space needed for a *XFR message or journal file to contain
        the data in the database. previously we returned the sizes of all
        rdataslabs, including header overhead and offset tables, which
        resulted in the database size being reported as much larger than the
        equivalent *XFR or journal.
      - map files caused a particular problem here: the fullname can't be
        determined from the node while a file is being deserialized, because
        the uppernode pointers aren't set yet. so we store "full name length"
        in the dns_rbtnode structure while serializing, and clear it after
        deserialization is complete.
      98b55eb4
  10. 28 Feb, 2020 2 commits
  11. 27 Feb, 2020 1 commit
  12. 21 Feb, 2020 1 commit
  13. 14 Feb, 2020 1 commit
  14. 13 Feb, 2020 3 commits
    • Evan Hunt's avatar
      apply the modified style · e851ed0b
      Evan Hunt authored
      e851ed0b
    • Ondřej Surý's avatar
      Use clang-tidy to add curly braces around one-line statements · 056e133c
      Ondřej Surý authored
      The command used to reformat the files in this commit was:
      
      ./util/run-clang-tidy \
      	-clang-tidy-binary clang-tidy-11
      	-clang-apply-replacements-binary clang-apply-replacements-11 \
      	-checks=-*,readability-braces-around-statements \
      	-j 9 \
      	-fix \
      	-format \
      	-style=file \
      	-quiet
      clang-format -i --style=format $(git ls-files '*.c' '*.h')
      uncrustify -c .uncrustify.cfg --replace --no-backup $(git ls-files '*.c' '*.h')
      clang-format -i --style=format $(git ls-files '*.c' '*.h')
      056e133c
    • Ondřej Surý's avatar
      Use coccinelle to add braces to nested single line statement · 36c6105e
      Ondřej Surý authored
      Both clang-tidy and uncrustify chokes on statement like this:
      
      for (...)
      	if (...)
      		break;
      
      This commit uses a very simple semantic patch (below) to add braces around such
      statements.
      
      Semantic patch used:
      
      @@
      statement S;
      expression E;
      @@
      
      while (...)
      - if (E) S
      + { if (E) { S } }
      
      @@
      statement S;
      expression E;
      @@
      
      for (...;...;...)
      - if (E) S
      + { if (E) { S } }
      
      @@
      statement S;
      expression E;
      @@
      
      if (...)
      - if (E) S
      + { if (E) { S } }
      36c6105e
  15. 12 Feb, 2020 1 commit
  16. 05 Feb, 2020 2 commits
    • Mark Andrews's avatar
      'closest' must be non NULL, remove test. · 0312e73e
      Mark Andrews authored
      6412 cleanup:
      6413        dns_rdataset_disassociate(&neg);
      6414        dns_rdataset_disassociate(&negsig);
      
      	CID 1452700 (#1 of 1): Dereference before null check (REVERSE_INULL)
      	check_after_deref: Null-checking closest suggests that it
      	may be null, but it has already been dereferenced on all
      	paths leading to the check.
      
      6415        if (closest != NULL)
      6416                free_noqname(mctx, &closest);
      0312e73e
    • Mark Andrews's avatar
      'noqname' must be non NULL, remove test. · 1b1a94ea
      Mark Andrews authored
      6367cleanup:
      6368        dns_rdataset_disassociate(&neg);
      6369        dns_rdataset_disassociate(&negsig);
      
      	CID 1452704 (#1 of 1): Dereference before null check
      	(REVERSE_INULL) check_after_deref: Null-checking noqname
      	suggests that it may be null, but it has already been
      	dereferenced on all paths leading to the check.
      
      6370        if (noqname != NULL)
      6371                free_noqname(mctx, &noqname);
      1b1a94ea
  17. 04 Feb, 2020 1 commit
    • Matthijs Mekking's avatar
      Simplify cachedb rrset statistic counters · 37b41ff6
      Matthijs Mekking authored
      This commit simplifies the cachedb rrset statistics in two ways:
      - Introduce new rdtypecounter arithmetics, allowing bitwise
        operations.
      - Remove the special DLV statistic counter.
      
      New rdtypecounter arithmetics
      -----------------------------
      "The rdtypecounter arithmetics is a brain twister".  Replace the
      enum counters with some defines.  A rdtypecounter is now 8 bits for
      RRtypes and 3 bits for flags:
      
            0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15
          +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
          |  |  |  |  |  |  S  |NX|         RRType        |
          +--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+--+
      
      If the 8 bits for RRtype are all zero, this is an Other RRtype.
      
      Bit 7 is the NXRRSET (NX) flag and indicates whether this is a
      positive (0) or a negative (1) RRset.
      
      Then bit 5 and 6 mostly tell you if this counter is for an active,
      stale, or ancient RRtype:
      
          S = 0x00 means Active
          S = 0x01 means Stale
          S = 0x10 means Ancient
      
      Since a counter cannot be stale and ancient at the same time, we
      treat S = 0x11 as a special case to deal with NXDOMAIN counters.
      
      S = 0x11 indicates an NXDOMAIN counter and in this case the RRtype
      field signals the expiry of this cached item:
      
          RRType = 0 means Active
          RRType = 1 means Stale
          RRType = 2 means Ancient
      37b41ff6
  18. 03 Feb, 2020 1 commit
  19. 14 Jan, 2020 2 commits
    • Ondřej Surý's avatar
      Remove duplicate INSIST checks for isc_refcount API · 6afa9936
      Ondřej Surý authored
      This commits removes superfluous checks when using the isc_refcount API.
      
      Examples of superfluous checks:
      
      1. The isc_refcount_decrement function ensures there was not underflow,
         so this check is superfluous:
      
          INSIST(isc_refcount_decrement(&r) > 0);
      
      2 .The isc_refcount_destroy() includes check whether the counter
         is zero, therefore this is superfluous:
      
          INSIST(isc_refcount_decrement(&r) == 1 && isc_refcount_destroy(&r));
      6afa9936
    • Ondřej Surý's avatar
      7c3e3429
  20. 10 Dec, 2019 1 commit
  21. 09 Dec, 2019 2 commits
    • Mark Andrews's avatar
      Add is_leaf and send_to_prune_tree. · c6efc0e5
      Mark Andrews authored
      Add is_leaf and send_to_prune_tree to make the logic easier
      to understand in cleanup_dead_nodes and decrement_reference.
      c6efc0e5
    • Mark Andrews's avatar
      Testing node->down requires the tree lock to be held. · 176b23b6
      Mark Andrews authored
      In decrement_reference only test node->down if the tree lock
      is held.  As node->down is not always tested in
      decrement_reference we need to test that it is non NULL in
      cleanup_dead_nodes prior to removing the node from the rbt
      tree.  Additionally it is not always possible to aquire the
      node lock and reactivate a node when adding parent nodes.
      Reactivate such nodes in cleanup_dead_nodes if required.
      176b23b6
  22. 02 Dec, 2019 1 commit
  23. 29 Nov, 2019 1 commit
  24. 28 Nov, 2019 1 commit
  25. 27 Nov, 2019 2 commits
  26. 19 Nov, 2019 3 commits
  27. 03 Oct, 2019 1 commit
  28. 01 Oct, 2019 3 commits