Refactor the cyclic ordering to be more efficient

With random ordering removed, the cyclic ordering can be rewritten in a
that it uses thread_local static array to keep the cyclic order.

This could be further improved by keeping the current position inside
the slabheader and adding a function to start directly there instead at
dns_rdataset_first().

diff --git a/lib/dns/rdataset.c b/lib/dns/rdataset.c
index db5d8c9322eec44af10966b4603fe267e8a39f28..bb0a2d6edc6c0b0e156f745477db094d133d4c41 100644 (file)
--- a/lib/dns/rdataset.c
+++ b/lib/dns/rdataset.c
@@ -20,6 +20,7 @@
 #include <isc/buffer.h>
 #include <isc/mem.h>
 #include <isc/random.h>
+#include <isc/result.h>
 #include <isc/serial.h>
 #include <isc/util.h>
 
@@ -31,6 +32,9 @@
 #include <dns/rdataset.h>
 #include <dns/types.h>
 
+#define MAX_SHUFFLE 100
+thread_local dns_rdata_t dns__rdataset_rdatas[MAX_SHUFFLE];
+
 static const char *trustnames[] = {
        "none",           "pending-additional",
        "pending-answer", "additional",
@@ -208,26 +212,182 @@ dns_rdataset_current(dns_rdataset_t *rdataset, dns_rdata_t *rdata) {
        (rdataset->methods->current)(rdataset, rdata);
 }
 
-#define MAX_SHUFFLE    32
 #define WANT_CYCLIC(r) (((r)->attributes.order == dns_order_cyclic))
 
+static isc_result_t
+towire_addtypeclass(dns_rdataset_t *rdataset, const dns_name_t *name,
+                   dns_compress_t *cctx, isc_buffer_t *target,
+                   isc_buffer_t *rrbuffer, size_t extralen) {
+       isc_region_t r;
+       isc_result_t result;
+       size_t headlen;
+
+       *rrbuffer = *target;
+       dns_compress_setpermitted(cctx, true);
+       result = dns_name_towire(name, cctx, target);
+       if (result != ISC_R_SUCCESS) {
+               return result;
+       }
+       headlen = sizeof(dns_rdataclass_t) + sizeof(dns_rdatatype_t) + extralen;
+       isc_buffer_availableregion(target, &r);
+       if (r.length < headlen) {
+               return ISC_R_NOSPACE;
+       }
+       isc_buffer_putuint16(target, rdataset->type);
+       isc_buffer_putuint16(target, rdataset->rdclass);
+       return ISC_R_SUCCESS;
+}
+
+static void
+towire_addttl(dns_rdataset_t *rdataset, isc_buffer_t *target,
+             isc_buffer_t *rdlen) {
+       isc_buffer_putuint32(target, rdataset->ttl);
+
+       /* Save space for rdlen. */
+       *rdlen = *target;
+       isc_buffer_add(target, 2);
+}
+
+static isc_result_t
+towire_addrdata(dns_rdata_t *rdata, dns_compress_t *cctx, isc_buffer_t *target,
+               isc_buffer_t *rdlen) {
+       isc_result_t result = dns_rdata_towire(rdata, cctx, target);
+       if (result != ISC_R_SUCCESS) {
+               return result;
+       }
+       INSIST((target->used >= rdlen->used + 2) &&
+              (target->used - rdlen->used - 2 < 65536));
+       isc_buffer_putuint16(rdlen, (uint16_t)(target->used - rdlen->used - 2));
+       return ISC_R_SUCCESS;
+}
+
+static isc_result_t
+towire_question(dns_rdataset_t *rdataset, const dns_name_t *name,
+               dns_compress_t *cctx, isc_buffer_t *target,
+               isc_buffer_t *rrbuffer, unsigned int options ISC_ATTR_UNUSED,
+               unsigned int *countp) {
+       isc_result_t result;
+
+       result = dns_rdataset_first(rdataset);
+       INSIST(result == ISC_R_NOMORE);
+
+       result = towire_addtypeclass(rdataset, name, cctx, target, rrbuffer, 0);
+       if (result != ISC_R_SUCCESS) {
+               return ISC_R_SUCCESS;
+       }
+
+       *countp += 1;
+
+       return ISC_R_SUCCESS;
+}
+
+static isc_result_t
+towire_answer(dns_rdataset_t *rdataset, const dns_name_t *name,
+             dns_compress_t *cctx, isc_buffer_t *target,
+             isc_buffer_t *rrbuffer, unsigned int options ISC_ATTR_UNUSED,
+             unsigned int *countp) {
+       isc_result_t result;
+       size_t start = 0, count = 0, added = 0;
+       isc_buffer_t rdlen;
+       dns_rdata_t *rdatas = dns__rdataset_rdatas;
+
+       count = dns_rdataset_count(rdataset);
+       result = dns_rdataset_first(rdataset);
+       if (result == ISC_R_NOMORE) {
+               return ISC_R_SUCCESS;
+       } else if (result != ISC_R_SUCCESS) {
+               return result;
+       }
+
+       if (WANT_CYCLIC(rdataset) && rdataset->type != dns_rdatatype_rrsig &&
+           rdataset->count != DNS_RDATASET_COUNT_UNDEFINED)
+       {
+               start = rdataset->count % count;
+
+               /* Do we need larger buffer? */
+               if (start > ARRAY_SIZE(dns__rdataset_rdatas)) {
+                       rdatas = isc_mem_cget(cctx->mctx, start,
+                                             sizeof(rdatas[0]));
+               }
+       }
+
+       /*
+        * Save the rdata up until the start.  If we are not
+        * doing cyclic, the start == 0, so this is no-op.
+        */
+       for (size_t i = 0; i < start; i++) {
+               dns_rdata_init(&rdatas[i]);
+               dns_rdataset_current(rdataset, &rdatas[i]);
+
+               result = dns_rdataset_next(rdataset);
+               if (result == ISC_R_NOMORE) {
+                       result = ISC_R_SUCCESS;
+                       break;
+               } else if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+       }
+
+       for (size_t i = start; i < count; i++) {
+               dns_rdata_t rdata = DNS_RDATA_INIT;
+
+               result = towire_addtypeclass(rdataset, name, cctx, target,
+                                            rrbuffer, sizeof(dns_ttl_t) + 2);
+               if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+               towire_addttl(rdataset, target, &rdlen);
+
+               dns_rdataset_current(rdataset, &rdata);
+               result = towire_addrdata(&rdata, cctx, target, &rdlen);
+               if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+               added++;
+
+               result = dns_rdataset_next(rdataset);
+               if (result == ISC_R_NOMORE) {
+                       result = ISC_R_SUCCESS;
+                       break;
+               } else if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+       }
+
+       for (size_t i = 0; i < start; i++) {
+               result = towire_addtypeclass(rdataset, name, cctx, target,
+                                            rrbuffer, sizeof(dns_ttl_t) + 2);
+               if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+               towire_addttl(rdataset, target, &rdlen);
+
+               result = towire_addrdata(&rdatas[i], cctx, target, &rdlen);
+               if (result != ISC_R_SUCCESS) {
+                       goto cleanup;
+               }
+               added++;
+       }
+
+       INSIST(added == count);
+
+cleanup:
+       *countp += added;
+       if (rdatas != dns__rdataset_rdatas) {
+               isc_mem_cput(cctx->mctx, rdatas, start, sizeof(rdatas[0]));
+       }
+
+       return result;
+}
+
 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,
        unsigned int options, unsigned int *countp,
        void **state ISC_ATTR_UNUSED) {
-       isc_region_t r;
        isc_result_t result;
-       unsigned int i, count = 0, added;
-       isc_buffer_t savedbuffer, rdlen, rrbuffer;
-       unsigned int headlen;
-       bool question = false;
-       bool shuffle = false;
-       bool want_cyclic;
-       dns_rdata_t in_fixed[MAX_SHUFFLE];
-       dns_rdata_t *in = in_fixed;
-       dns_rdata_t *out_fixed[MAX_SHUFFLE];
-       dns_rdata_t **out = out_fixed;
+       isc_buffer_t savedbuffer = *target;
+       isc_buffer_t rrbuffer;
        dns_fixedname_t fixed;
        dns_name_t *name = NULL;
 
@@ -241,14 +401,7 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
        REQUIRE(countp != NULL);
        REQUIRE(cctx != NULL && cctx->mctx != NULL);
 
-       want_cyclic = WANT_CYCLIC(rdataset);
-
-       if (rdataset->attributes.question) {
-               question = true;
-               count = 1;
-               result = dns_rdataset_first(rdataset);
-               INSIST(result == ISC_R_NOMORE);
-       } else if (rdataset->attributes.negative) {
+       if (rdataset->attributes.negative) {
                /*
                 * This is a negative caching rdataset.
                 */
@@ -258,73 +411,8 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
                }
                return dns_ncache_towire(rdataset, cctx, target, ncache_opts,
                                         countp);
-       } else {
-               count = dns_rdataset_count(rdataset);
-               result = dns_rdataset_first(rdataset);
-               if (result == ISC_R_NOMORE) {
-                       return ISC_R_SUCCESS;
-               }
-               if (result != ISC_R_SUCCESS) {
-                       return result;
-               }
-       }
-
-       /*
-        * Do we want to shuffle this answer?
-        */
-       if (!question && count > 1 && rdataset->type != dns_rdatatype_rrsig) {
-               if (want_cyclic) {
-                       shuffle = true;
-               }
        }
 
-       if (shuffle) {
-               if (count > MAX_SHUFFLE) {
-                       in = isc_mem_cget(cctx->mctx, count, sizeof(*in));
-                       out = isc_mem_cget(cctx->mctx, count, sizeof(*out));
-                       if (in == NULL || out == NULL) {
-                               shuffle = false;
-                       }
-               }
-       }
-
-       if (shuffle) {
-               unsigned int j = 0;
-
-               /*
-                * First we get handles to all of the rdata.
-                */
-               i = 0;
-               do {
-                       INSIST(i < count);
-                       dns_rdata_init(&in[i]);
-                       dns_rdataset_current(rdataset, &in[i]);
-                       i++;
-                       result = dns_rdataset_next(rdataset);
-               } while (result == ISC_R_SUCCESS);
-               if (result != ISC_R_NOMORE) {
-                       goto cleanup;
-               }
-               INSIST(i == count);
-
-               if (want_cyclic &&
-                   (rdataset->count != DNS_RDATASET_COUNT_UNDEFINED))
-               {
-                       j = rdataset->count % count;
-               }
-
-               for (i = 0; i < count; i++) {
-                       out[i] = &in[j];
-                       if (++j == count) {
-                               j = 0;
-                       }
-               }
-       }
-
-       savedbuffer = *target;
-       i = 0;
-       added = 0;
-
        name = dns_fixedname_initname(&fixed);
        dns_name_copy(owner_name, name);
        dns_rdataset_getownercase(rdataset, name);
@@ -332,100 +420,32 @@ towire(dns_rdataset_t *rdataset, const dns_name_t *owner_name,
 
        name->attributes.nocompress |= owner_name->attributes.nocompress;
 
-       do {
-               /*
-                * Copy out the name, type, class, ttl.
-                */
-
-               rrbuffer = *target;
-               dns_compress_setpermitted(cctx, true);
-               result = dns_name_towire(name, cctx, target);
+       if (rdataset->attributes.question) {
+               result = towire_question(rdataset, name, cctx, target,
+                                        &rrbuffer, options, countp);
                if (result != ISC_R_SUCCESS) {
                        goto rollback;
                }
-               headlen = sizeof(dns_rdataclass_t) + sizeof(dns_rdatatype_t);
-               if (!question) {
-                       headlen += sizeof(dns_ttl_t) + 2;
-               } /* XXX 2 for rdata len */
-               isc_buffer_availableregion(target, &r);
-               if (r.length < headlen) {
-                       result = ISC_R_NOSPACE;
+       } else {
+               result = towire_answer(rdataset, name, cctx, target, &rrbuffer,
+                                      options, countp);
+               if (result != ISC_R_SUCCESS) {
                        goto rollback;
                }
-               isc_buffer_putuint16(target, rdataset->type);
-               isc_buffer_putuint16(target, rdataset->rdclass);
-               if (!question) {
-                       dns_rdata_t rdata_s = DNS_RDATA_INIT;
-                       dns_rdata_t *rdata = &rdata_s;
-
-                       isc_buffer_putuint32(target, rdataset->ttl);
-
-                       /*
-                        * Save space for rdlen.
-                        */
-                       rdlen = *target;
-                       isc_buffer_add(target, 2);
-
-                       /*
-                        * Copy out the rdata
-                        */
-                       if (shuffle) {
-                               rdata = out[i];
-                       } else {
-                               dns_rdata_reset(&rdata_s);
-                               dns_rdataset_current(rdataset, &rdata_s);
-                       }
-                       result = dns_rdata_towire(rdata, cctx, target);
-                       if (result != ISC_R_SUCCESS) {
-                               goto rollback;
-                       }
-                       INSIST((target->used >= rdlen.used + 2) &&
-                              (target->used - rdlen.used - 2 < 65536));
-                       isc_buffer_putuint16(
-                               &rdlen,
-                               (uint16_t)(target->used - rdlen.used - 2));
-                       added++;
-               }
-
-               if (shuffle) {
-                       i++;
-                       if (i == count) {
-                               result = ISC_R_NOMORE;
-                       } else {
-                               result = ISC_R_SUCCESS;
-                       }
-               } else {
-                       result = dns_rdataset_next(rdataset);
-               }
-       } while (result == ISC_R_SUCCESS);
-
-       if (result != ISC_R_NOMORE) {
-               goto rollback;
        }
 
-       *countp += count;
-
-       result = ISC_R_SUCCESS;
-       goto cleanup;
+       return ISC_R_SUCCESS;
 
 rollback:
        if (partial && result == ISC_R_NOSPACE) {
                dns_compress_rollback(cctx, rrbuffer.used);
-               *countp += added;
                *target = rrbuffer;
-               goto cleanup;
+               return result;
        }
        dns_compress_rollback(cctx, savedbuffer.used);
        *countp = 0;
        *target = savedbuffer;
 
-cleanup:
-       if (out != NULL && out != out_fixed) {
-               isc_mem_cput(cctx->mctx, out, count, sizeof(*out));
-       }
-       if (in != NULL && in != in_fixed) {
-               isc_mem_cput(cctx->mctx, in, count, sizeof(*in));
-       }
        return result;
 }