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.
This commit is contained in:
Ondřej Surý 2025-08-28 12:24:17 +02:00 committed by Ondřej Surý
parent b449fa9500
commit 7dc6048f93
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
7 changed files with 22 additions and 158 deletions

View file

@ -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\

View file

@ -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")) {

View file

@ -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;

View file

@ -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.

View file

@ -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``
=================== ======== ===========

View file

@ -238,7 +238,6 @@ typedef enum {
typedef enum {
dns_order_none,
dns_order_cyclic,
dns_order_randomize
} dns_orderopt_t;
typedef enum {

View file

@ -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;
}