Remove the random ordering of resource records in RRset

The rrset-order random doesn't offer uniform distribution of all
permutations and it isn't superior to cyclic order in any way.  Make the
random ordering an alias to the cyclic ordering.

diff --git a/bin/named/config.c b/bin/named/config.c
index b6718a4cef2f5dc05162c69204607b4afea5bb2e..6e7d481aad66c65f642b475003f9ab2a9a77dd74 100644 (file)
--- a/bin/named/config.c
+++ b/bin/named/config.c
@@ -105,7 +105,7 @@ options {\n\
        request-zoneversion false;\n\
        resolver-query-timeout 10;\n\
 #      responselog <boolean>;\n\
-       rrset-order { order random; };\n\
+       rrset-order { order cyclic; };\n\
        secroots-file \"named.secroots\";\n\
        send-cookie true;\n\
        serial-query-rate 20;\n\

diff --git a/bin/named/server.c b/bin/named/server.c
index e93d252c1d0502850f55c4289db1669dbba5e7d4..cce0be6d7bf840930f0fc27556e11527260512ce 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -1360,7 +1360,10 @@ configure_order(dns_order_t *order, const cfg_obj_t *ent) {
        INSIST(cfg_obj_isstring(obj));
        str = cfg_obj_asstring(obj);
        if (!strcasecmp(str, "random")) {
-               mode = dns_order_randomize;
+               cfg_obj_log(obj, ISC_LOG_WARNING,
+                           "random ordering is obsolete; "
+                           "cyclic ordering is being used instead");
+               mode = dns_order_cyclic;
        } else if (!strcasecmp(str, "cyclic")) {
                mode = dns_order_cyclic;
        } else if (!strcasecmp(str, "none")) {

diff --git a/bin/tests/system/rrsetorder/ns1/named.conf.j2 b/bin/tests/system/rrsetorder/ns1/named.conf.j2
index 213a006545bd734a2039b29dd5774752b3c5254f..21c94f0197a7b31ddf558767f953c0fd82b54239 100644 (file)
--- a/bin/tests/system/rrsetorder/ns1/named.conf.j2
+++ b/bin/tests/system/rrsetorder/ns1/named.conf.j2
@@ -24,7 +24,6 @@ options {
        dnssec-validation no;
        notify no;
        rrset-order {
-               name "random.example" order random;
                name "cyclic.example" order cyclic;
                name "none.example" order none;
                type NS order random;

diff --git a/bin/tests/system/rrsetorder/tests.sh b/bin/tests/system/rrsetorder/tests.sh
index 0358601d1d3f39443d921e1fcdb7f3f7665ca0c7..5064d7ebaf145e358fcfe51ca2ec3a68f3cca67b 100644 (file)
--- a/bin/tests/system/rrsetorder/tests.sh
+++ b/bin/tests/system/rrsetorder/tests.sh
@@ -76,28 +76,6 @@ diff dig.out.2 dig.out.3 >/dev/null && ret=1
 if [ $matches -ne 16 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
-echo_i "Checking order random (primary)"
-ret=0
-for i in $GOOD_RANDOM; do
-  eval match$i=0
-done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9; do
-  dig_cmd @10.53.0.1 random.example >dig.out.random || ret=1
-  match=0
-  for j in $GOOD_RANDOM; do
-    eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true"
-    if [ $match -eq 1 ]; then break; fi
-  done
-  if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM; do
-  eval "match=\$((match + match$i))"
-done
-echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi
-if [ $ret != 0 ]; then echo_i "failed"; fi
-status=$((status + ret))
 
 echo_i "Checking order none (primary)"
 ret=0
@@ -164,29 +142,6 @@ if [ $matches -ne 16 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
-echo_i "Checking order random (secondary)"
-ret=0
-for i in $GOOD_RANDOM; do
-  eval match$i=0
-done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9; do
-  dig_cmd @10.53.0.2 random.example >dig.out.random || ret=1
-  match=0
-  for j in $GOOD_RANDOM; do
-    eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true"
-    if [ $match -eq 1 ]; then break; fi
-  done
-  if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM; do
-  eval "match=\$((match + match$i))"
-done
-echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi
-if [ $ret != 0 ]; then echo_i "failed"; fi
-status=$((status + ret))
-
 echo_i "Checking order none (secondary)"
 ret=0
 # Fetch the "reference" response and ensure it contains the expected records.
@@ -267,29 +222,6 @@ if [ $matches -ne 16 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
-echo_i "Checking order random (secondary loaded from disk)"
-ret=0
-for i in $GOOD_RANDOM; do
-  eval match$i=0
-done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9; do
-  dig_cmd @10.53.0.2 random.example >dig.out.random || ret=1
-  match=0
-  for j in $GOOD_RANDOM; do
-    eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true"
-    if [ $match -eq 1 ]; then break; fi
-  done
-  if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM; do
-  eval "match=\$((match + match$i))"
-done
-echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi
-if [ $ret != 0 ]; then echo_i "failed"; fi
-status=$((status + ret))
-
 echo_i "Checking order none (secondary loaded from disk)"
 ret=0
 # Fetch the "reference" response and ensure it contains the expected records.
@@ -359,29 +291,6 @@ if [ $matches -ne 16 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
-echo_i "Checking order random (cache)"
-ret=0
-for i in $GOOD_RANDOM; do
-  eval match$i=0
-done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9; do
-  dig_cmd @10.53.0.3 random.example >dig.out.random || ret=1
-  match=0
-  for j in $GOOD_RANDOM; do
-    eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true"
-    if [ $match -eq 1 ]; then break; fi
-  done
-  if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM; do
-  eval "match=\$((match + match$i))"
-done
-echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi
-if [ $ret != 0 ]; then echo_i "failed"; fi
-status=$((status + ret))
-
 echo_i "Checking order none (cache)"
 ret=0
 # Fetch the "reference" response and ensure it contains the expected records.
@@ -397,29 +306,6 @@ done
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
-echo_i "Checking default order (cache)"
-ret=0
-for i in $GOOD_RANDOM; do
-  eval match$i=0
-done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9; do
-  dig_cmd @10.53.0.5 random.example >dig.out.random || ret=1
-  match=0
-  for j in $GOOD_RANDOM; do
-    eval "diff dig.out.random reference.dig.out.random.good$j >/dev/null && match$j=1 match=1 || true"
-    if [ $match -eq 1 ]; then break; fi
-  done
-  if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM; do
-  eval "match=\$((match + match$i))"
-done
-echo_i "Default selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -lt $((GOOD_RANDOM_NO / 3)) ]; then ret=1; fi
-if [ $ret != 0 ]; then echo_i "failed"; fi
-status=$((status + ret))
-
 echo_i "Checking default order no match in rrset-order (cache)"
 ret=0
 # Fetch the "reference" response and ensure it contains the expected records.

diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst
index 6f64db05e6d3b64b9ced17ff92a752d7ca3513cf..5467a099affcaf43c14e9859f24caac26592cfbb 100644 (file)
--- a/doc/arm/reference.rst
+++ b/doc/arm/reference.rst
@@ -4050,8 +4050,8 @@ RRset Ordering
    The legal values for ``<ordering>`` are:
 
    ``random``
-       Records are returned in a non-deterministic order.  The random ordering
-       doesn't guarantee uniform distribution of all permutations.
+       This value has been deprecated and using ``random`` behaves
+       exactly like ``cyclic``.
 
    ``cyclic``
        Records are returned in a cyclic round-robin order, rotating by one
@@ -4086,9 +4086,9 @@ RRset Ordering
    ::
 
        rrset-order {
-           type A name "foo.isc.org" order random;
+           type A name "foo.isc.org" order none;
            type AAAA name "foo.isc.org" order cyclic;
-           name "*.bar.isc.org" order random;
+           name "*.bar.isc.org" order none;
            name "*.baz.isc.org" order cyclic;
        };
 
@@ -4097,11 +4097,11 @@ RRset Ordering
    ===================    ========    ===========
    QNAME                  QTYPE       RRset Order
    ===================    ========    ===========
-   ``foo.isc.org``        ``A``       ``random``
+   ``foo.isc.org``        ``A``       ``none``
    ``foo.isc.org``        ``AAAA``    ``cyclic``
    ``foo.isc.org``        ``TXT``     ``none``
    ``sub.foo.isc.org``    all         ``none``
-   ``sub.bar.isc.org``    all         ``random``
+   ``sub.bar.isc.org``    all         ``none``
    ``baz.isc.org``        all         ``none``
    ``sub.baz.isc.org``    all         ``cyclic``
    ===================    ========    ===========

diff --git a/lib/dns/include/dns/types.h b/lib/dns/include/dns/types.h
index 11eab4baf73bc08de937367703e2a0c4f9d8b162..d45a877bf8470eaba264c61ac8b723ce9100857c 100644 (file)
--- a/lib/dns/include/dns/types.h
+++ b/lib/dns/include/dns/types.h
@@ -238,7 +238,6 @@ typedef enum {
 typedef enum {
        dns_order_none,
        dns_order_cyclic,
-       dns_order_randomize
 } dns_orderopt_t;
 
 typedef enum {

diff --git a/lib/dns/rdataset.c b/lib/dns/rdataset.c
index fb7a919459d400d69b11ba0ea32ab8bccc79a21d..db5d8c9322eec44af10966b4603fe267e8a39f28 100644 (file)
--- a/lib/dns/rdataset.c
+++ b/lib/dns/rdataset.c
@@ -209,21 +209,8 @@ dns_rdataset_current(dns_rdataset_t *rdataset, dns_rdata_t *rdata) {
 }
 
 #define MAX_SHUFFLE    32
-#define WANT_RANDOM(r) (((r)->attributes.order == dns_order_randomize))
 #define WANT_CYCLIC(r) (((r)->attributes.order == dns_order_cyclic))
 
-struct towire_sort {
-       int key;
-       dns_rdata_t *rdata;
-};
-
-static void
-swap_rdata(dns_rdata_t *in, unsigned int a, unsigned int b) {
-       dns_rdata_t rdata = in[a];
-       in[a] = in[b];
-       in[b] = rdata;
-}
-
 static isc_result_t
 towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
        dns_compress_t *cctx, isc_buffer_t *target, bool partial,
@@ -236,11 +223,11 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
        unsigned int headlen;
        bool question = false;
        bool shuffle = false;
-       bool want_random, want_cyclic;
+       bool want_cyclic;
        dns_rdata_t in_fixed[MAX_SHUFFLE];
        dns_rdata_t *in = in_fixed;
-       struct towire_sort out_fixed[MAX_SHUFFLE];
-       struct towire_sort *out = out_fixed;
+       dns_rdata_t *out_fixed[MAX_SHUFFLE];
+       dns_rdata_t **out = out_fixed;
        dns_fixedname_t fixed;
        dns_name_t *name = NULL;
 
@@ -254,7 +241,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
        REQUIRE(countp != NULL);
        REQUIRE(cctx != NULL && cctx->mctx != NULL);
 
-       want_random = WANT_RANDOM(rdataset);
        want_cyclic = WANT_CYCLIC(rdataset);
 
        if (rdataset->attributes.question) {
@@ -287,7 +273,7 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
         * Do we want to shuffle this answer?
         */
        if (!question && count > 1 && rdataset->type != dns_rdatatype_rrsig) {
-               if (want_random || want_cyclic) {
+               if (want_cyclic) {
                        shuffle = true;
                }
        }
@@ -303,7 +289,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
        }
 
        if (shuffle) {
-               uint32_t seed = 0;
                unsigned int j = 0;
 
                /*
@@ -322,10 +307,6 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
                }
                INSIST(i == count);
 
-               if (want_random) {
-                       seed = isc_random32();
-               }
-
                if (want_cyclic &&
                    (rdataset->count != DNS_RDATASET_COUNT_UNDEFINED))
                {
@@ -333,12 +314,7 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
                }
 
                for (i = 0; i < count; i++) {
-                       if (want_random) {
-                               swap_rdata(in, j, j + seed % (count - j));
-                       }
-
-                       out[i].key = 0;
-                       out[i].rdata = &in[j];
+                       out[i] = &in[j];
                        if (++j == count) {
                                j = 0;
                        }
@@ -379,7 +355,8 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
                isc_buffer_putuint16(target, rdataset->type);
                isc_buffer_putuint16(target, rdataset->rdclass);
                if (!question) {
-                       dns_rdata_t rdata = DNS_RDATA_INIT;
+                       dns_rdata_t rdata_s = DNS_RDATA_INIT;
+                       dns_rdata_t *rdata = &rdata_s;
 
                        isc_buffer_putuint32(target, rdataset->ttl);
 
@@ -393,12 +370,12 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
                         * Copy out the rdata
                         */
                        if (shuffle) {
-                               rdata = *(out[i].rdata);
+                               rdata = out[i];
                        } else {
-                               dns_rdata_reset(&rdata);
-                               dns_rdataset_current(rdataset, &rdata);
+                               dns_rdata_reset(&rdata_s);
+                               dns_rdataset_current(rdataset, &rdata_s);
                        }
-                       result = dns_rdata_towire(&rdata, cctx, target);
+                       result = dns_rdata_towire(rdata, cctx, target);
                        if (result != ISC_R_SUCCESS) {
                                goto rollback;
                        }