Skip to content

rrset - refactoring, memory optimizations

What

There are two problems with knot_rrset_t - the code has repetitive nature and is not very clear and rrsets take up space.

Summary of ideas from the discussion

  • This is not a priority for the upcoming release, but the work should slowly start.
  • Ordering is also a matter of discussion and is up to implementor with a reasonable explanation.
  • Possibly duplicated RR types: NS, CNAME, MX make sense for compression.
  • Pros/Cons of indices or inline lengths are in discussion.
  • Make branch for each partial change to make reviews easier.

Steps

✅ Storing DNAMEs directly in RDATA. (#109 (closed)) 🕓 Decide on whether to keep indices, make it an array of sizes or put sizes directly to RDATA (see discussion for rationale). (No issue yet) 🕒 Clean it up first, depends on the previous step. Since dnames are stored directly in the RDATA a lot of changes can be now done in bulk. (No issue yet)

  • All the iterations are very similar, only specialty is compressing/deflating names
  • Also split the long functions
  • Compression is wonky with 2-3 different structures, only {wire, pos, table} and dname is required for compression.
  • Writing to journal (aka binary) and to wire should be the same, writing to wire is hardly a drag and would provide less code and less potential bugs.
  • Reordering should be done depending on how the RDATA is stored, either indices only or RDATA.
  • Also replace xmalloc like functions when you see them, the rrset should be a part of the core library.
  • This is more a question: replace direct accessors with to extra value with direct access, like knot_rrset_set_ttl(rrset, 1) has no extra value over rrset->ttl = 1 and is only longer to write. This is not a C++ and the structures are not opaque. Great way to fix this is http://coccinelle.lip6.fr/sp.php When the function has any extra benefit, than by all means provide a function.
  • Very important - clear questions and code review remarks from comments.
  • Even static functions which are not oneliners deserve a short comment of what they do. e.g. knot_rrset_add_rr_sort_n
  • Split somehow functions that manipulate with rrset and functions that interpret/process the rdata.
  • Very important - no 1/0 parameters like ddns_check in knot_rrset_remove_rr_using_rrset. I have 99 problems with it and here are two good - caller has no immediate idea of what the value does, handling special cases like that almost always means wrong design.
  • Very important - ordering of the functions should be the same as in header and should be sensible, functions that are related somehow should be grouped together.
  • Change descriptors so that uncompressed DNAMEs no longer exist.

🕒 Remove owner and RRSIGs from the RRSet (#142 (closed)) 🕒 RRSet reference counting. (No issue yet) 🕒 If possible, merge zone file loading and AXFR code. (No issue yet)