]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Implement Fisher-Yates shuffle for nameserver selection
authorOndřej Surý <ondrej@sury.org>
Wed, 25 Feb 2026 15:46:40 +0000 (16:46 +0100)
committerOndřej Surý <ondrej@isc.org>
Thu, 26 Feb 2026 07:17:23 +0000 (08:17 +0100)
Replace the two-pass "random start index and wrap around" logic in
fctx_getaddresses_nameservers() with a statistically sound Fisher-Yates
shuffle.

The previous implementation picked a random starting node and did two
passes over the linked list to find query candidates.  The new logic
extracts the available nameservers into a bounded, stack-allocated array
of dns_rdata_t structures.

This array is then randomized in-place using a Fisher-Yates shuffle.
Finally, the shuffled array is traversed sequentially to launch fetches
until the dynamic quota (fctx->pending_running >= fetches_allowed) is
reached.

This guarantees a fair random distribution for outbound queries while
properly respecting dynamic query limits, entirely within O(1) memory
and without the overhead of linked-list pointer shuffling or multiple
dataset traversals.

(cherry picked from commit 3c33e7d9370006b1599e3d99c0d5fa6a6dad7979)

lib/dns/resolver.c

index 1557aef2e8a06323c2eb10ed0a5ac0bf6b23e554..1a3bfb054f1ba679b6120ca2d77c0d5a12d0c788 100644 (file)
@@ -3474,6 +3474,8 @@ fctx_getaddresses(fetchctx_t *fctx) {
        bool have_address = false;
        unsigned int ns_processed = 0;
        size_t fetches_allowed = 0;
+       dns_rdata_t nameservers_s[NS_PROCESSING_LIMIT];
+       dns_rdata_t *nameservers[NS_PROCESSING_LIMIT];
 
        FCTXTRACE5("getaddresses", "fctx->depth=", fctx->depth);
 
@@ -3657,73 +3659,74 @@ normal_nses:
                break;
        }
 
-       for (;;) {
-               size_t nscount = dns_rdataset_count(&fctx->nameservers);
-               size_t maxstartns = nscount > NS_PROCESSING_LIMIT
-                                           ? NS_PROCESSING_LIMIT
-                                           : nscount;
-               size_t startns = isc_random_uniform(maxstartns);
+       for (result = dns_rdataset_first(&fctx->nameservers);
+            result == ISC_R_SUCCESS;
+            result = dns_rdataset_next(&fctx->nameservers))
+       {
+               dns_rdata_t *rdata = nameservers[ns_processed] =
+                       &nameservers_s[ns_processed];
 
-               for (size_t pass = 0; pass < 2; pass++) {
-                       size_t curns = 0;
+               dns_rdata_init(rdata);
 
-                       for (result = dns_rdataset_first(&fctx->nameservers);
-                            result == ISC_R_SUCCESS;
-                            result = dns_rdataset_next(&fctx->nameservers))
-                       {
-                               dns_rdata_t rdata = DNS_RDATA_INIT;
-                               bool overquota = false;
-                               unsigned int static_stub = 0;
-                               unsigned int no_fetch = 0;
+               dns_rdataset_current(&fctx->nameservers, rdata);
 
-                               if (pass == 0 && curns++ < startns) {
-                                       continue;
-                               }
-                               if (pass == 1 && curns++ >= startns) {
-                                       break;
-                               }
+               if (++ns_processed >= NS_PROCESSING_LIMIT) {
+                       break;
+               }
+       }
 
-                               dns_rdataset_current(&fctx->nameservers,
-                                                    &rdata);
-                               /*
-                                * Extract the name from the NS record.
-                                */
-                               result = dns_rdata_tostruct(&rdata, &ns, NULL);
-                               if (result != ISC_R_SUCCESS) {
-                                       continue;
-                               }
+       if (ns_processed > 1 && ns_processed > fetches_allowed) {
+               /*
+                * Skip the shuffle if:
+                * - there's nothing to shuffle (no or one nameserver)
+                * - there are less nameserver than allowed fetches as
+                *   we are going to start fetches for all of them.
+                */
+               for (size_t i = 0; i < ns_processed - 1; i++) {
+                       size_t j = i + isc_random_uniform(ns_processed - i);
 
-                               if (STATICSTUB(&fctx->nameservers) &&
-                                   dns_name_equal(&ns.name, fctx->domain))
-                               {
-                                       static_stub = DNS_ADBFIND_STATICSTUB;
-                               }
+                       ISC_SWAP(nameservers[i], nameservers[j]);
+               }
+       }
 
-                               /*
-                                * Make sure we only launch a limited number of
-                                * outgoing fetches.
-                                */
-                               if (fctx->pending_running >= fetches_allowed) {
-                                       no_fetch = DNS_ADBFIND_NOFETCH;
-                               }
+       for (;;) {
+               for (size_t i = 0; i < ns_processed; i++) {
+                       bool overquota = false;
+                       unsigned int static_stub = 0;
+                       unsigned int no_fetch = 0;
+                       dns_rdata_t *rdata = nameservers[i];
 
-                               findname(fctx, &ns.name, 0,
-                                        stdoptions | static_stub | no_fetch, 0,
-                                        now, &overquota, &need_alternate,
-                                        &have_address);
+                       /*
+                        * Extract the name from the NS record.
+                        */
+                       result = dns_rdata_tostruct(rdata, &ns, NULL);
+                       if (result != ISC_R_SUCCESS) {
+                               continue;
+                       }
 
-                               if (!overquota) {
-                                       all_spilled = false;
-                               }
+                       if (STATICSTUB(&fctx->nameservers) &&
+                           dns_name_equal(&ns.name, fctx->domain))
+                       {
+                               static_stub = DNS_ADBFIND_STATICSTUB;
+                       }
 
-                               dns_rdata_reset(&rdata);
-                               dns_rdata_freestruct(&ns);
+                       /*
+                        * Make sure we only launch a limited number of
+                        * outgoing fetches.
+                        */
+                       if (fctx->pending_running >= fetches_allowed) {
+                               no_fetch = DNS_ADBFIND_NOFETCH;
+                       }
 
-                               if (++ns_processed >= NS_PROCESSING_LIMIT) {
-                                       result = ISC_R_NOMORE;
-                                       break;
-                               }
+                       findname(fctx, &ns.name, 0,
+                                stdoptions | static_stub | no_fetch, 0, now,
+                                &overquota, &need_alternate, &have_address);
+
+                       if (!overquota) {
+                               all_spilled = false;
                        }
+
+                       dns_rdata_freestruct(&ns);
                }
 
                /*