Redesign dnssec sign statistics

The first attempt to add DNSSEC sign statistics was naive: for each
zone we allocated 64K counters, twice.  In reality each zone has at
most four keys, so the new approach only has room for four keys per
zone. If after a rollover more keys have signed the zone, existing
keys are rotated out.

The DNSSEC sign statistics has three counters per key, so twelve
counters per zone. First counter is actually a key id, so it is
clear what key contributed to the metrics.  The second counter
tracks the number of generated signatures, and the third tracks
how many of those are refreshes.

This means that in the zone structure we no longer need two separate
references to DNSSEC sign metrics: both the resign and refresh stats
are kept in a single dns_stats structure.

Incrementing dnssecsignstats:

Whenever a dnssecsignstat is incremented, we look up the key id
to see if we already are counting metrics for this key.  If so,
we update the corresponding operation counter (resign or
refresh).

If the key is new, store the value in a new counter and increment
corresponding counter.

If all slots are full, we rotate the keys and overwrite the last
slot with the new key.

Dumping dnssecsignstats:

Dumping dnssecsignstats is no longer a simple wrapper around
isc_stats_dump, but uses the same principle.  The difference is that
rather than dumping the index (key tag) and counter, we have to look
up the corresponding counter.

(cherry picked from commit 705810d577)
This commit is contained in:
Matthijs Mekking 2020-03-26 16:02:36 +01:00
parent 86933f4a27
commit f59f446122
9 changed files with 142 additions and 91 deletions

View file

@ -1,3 +1,10 @@
5373. [bug] Collecting DNSSEC signing operations introduced by
GL #513 (change 5254) allocated counters for every
possible key id per zone which results in a lot of
wasted memory. Fix by tracking up to four keys
per zone, rotate counters when keys are replaced.
[GL #1179]
5372. [bug] Fix migration from existing DNSSEC key files using
auto-dnssec maintain to dnssec-policy. [GL #1706]

View file

@ -1814,7 +1814,6 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
isc_stats_t *gluecachestats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
uint64_t nsstat_values[ns_statscounter_max];
uint64_t gluecachestats_values[dns_gluecachestatscounter_max];
@ -1880,6 +1879,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
if (dnssecsignstats != NULL) {
/* counters type="dnssec-sign"*/
TRY0(xmlTextWriterStartElement(writer,
ISC_XMLCHAR "counters"));
TRY0(xmlTextWriterWriteAttribute(
@ -1887,7 +1887,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
ISC_XMLCHAR "dnssec-sign"));
dumparg.result = ISC_R_SUCCESS;
dns_dnssecsignstats_dump(dnssecsignstats,
dns_dnssecsignstats_dump(dnssecsignstats, false,
dnssecsignstat_dump, &dumparg,
0);
if (dumparg.result != ISC_R_SUCCESS) {
@ -1896,10 +1896,8 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
/* counters type="dnssec-sign"*/
TRY0(xmlTextWriterEndElement(writer));
}
dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone);
if (dnssecrefreshstats != NULL) {
/* counters type="dnssec-refresh"*/
TRY0(xmlTextWriterStartElement(writer,
ISC_XMLCHAR "counters"));
TRY0(xmlTextWriterWriteAttribute(
@ -1907,7 +1905,7 @@ zone_xmlrender(dns_zone_t *zone, void *arg) {
ISC_XMLCHAR "dnssec-refresh"));
dumparg.result = ISC_R_SUCCESS;
dns_dnssecsignstats_dump(dnssecrefreshstats,
dns_dnssecsignstats_dump(dnssecsignstats, true,
dnssecsignstat_dump, &dumparg,
0);
if (dumparg.result != ISC_R_SUCCESS) {
@ -2626,7 +2624,6 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
isc_stats_t *gluecachestats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
uint64_t nsstat_values[ns_statscounter_max];
uint64_t gluecachestats_values[dns_gluecachestatscounter_max];
@ -2714,7 +2711,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
dumparg.type = isc_statsformat_json;
dumparg.arg = counters;
dumparg.result = ISC_R_SUCCESS;
dns_dnssecsignstats_dump(dnssecsignstats,
dns_dnssecsignstats_dump(dnssecsignstats, false,
dnssecsignstat_dump, &dumparg,
0);
if (dumparg.result != ISC_R_SUCCESS) {
@ -2730,8 +2727,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
}
}
dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone);
if (dnssecrefreshstats != NULL) {
if (dnssecsignstats != NULL) {
stats_dumparg_t dumparg;
json_object *counters = json_object_new_object();
CHECKMEM(counters);
@ -2739,7 +2735,7 @@ zone_jsonrender(dns_zone_t *zone, void *arg) {
dumparg.type = isc_statsformat_json;
dumparg.arg = counters;
dumparg.result = ISC_R_SUCCESS;
dns_dnssecsignstats_dump(dnssecrefreshstats,
dns_dnssecsignstats_dump(dnssecsignstats, true,
dnssecsignstat_dump, &dumparg,
0);
if (dumparg.result != ISC_R_SUCCESS) {

View file

@ -890,7 +890,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
isc_stats_t *zoneqrystats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
dns_zonestat_level_t statlevel = dns_zonestat_none;
int seconds;
dns_zone_t *mayberaw = (raw != NULL) ? raw : zone;
@ -1187,18 +1186,15 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
zoneqrystats = NULL;
rcvquerystats = NULL;
dnssecsignstats = NULL;
dnssecrefreshstats = NULL;
if (statlevel == dns_zonestat_full) {
RETERR(isc_stats_create(mctx, &zoneqrystats,
ns_statscounter_max));
RETERR(dns_rdatatypestats_create(mctx, &rcvquerystats));
RETERR(dns_dnssecsignstats_create(mctx, &dnssecsignstats));
RETERR(dns_dnssecsignstats_create(mctx, &dnssecrefreshstats));
}
dns_zone_setrequeststats(zone, zoneqrystats);
dns_zone_setrcvquerystats(zone, rcvquerystats);
dns_zone_setdnssecsignstats(zone, dnssecsignstats);
dns_zone_setdnssecrefreshstats(zone, dnssecrefreshstats);
if (zoneqrystats != NULL) {
isc_stats_detach(&zoneqrystats);
@ -1212,10 +1208,6 @@ named_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig,
dns_stats_detach(&dnssecsignstats);
}
if (dnssecrefreshstats != NULL) {
dns_stats_detach(&dnssecrefreshstats);
}
/*
* Configure master functionality. This applies
* to primary masters (type "master") and slaves

View file

@ -684,9 +684,11 @@ dns_rcodestats_increment(dns_stats_t *stats, dns_opcode_t code);
*/
void
dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id);
dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id,
bool refresh);
/*%<
* Increment the statistics counter for the DNSKEY 'id'.
* Increment the statistics counter for the DNSKEY 'id'. If 'refresh' is set
* to true, update the refresh counter, otherwise update the sign counter.
*
* Requires:
*\li 'stats' is a valid dns_stats_t created by dns_dnssecsignstats_create().
@ -737,7 +739,7 @@ dns_rdatasetstats_dump(dns_stats_t *stats, dns_rdatatypestats_dumper_t dump_fn,
*/
void
dns_dnssecsignstats_dump(dns_stats_t * stats,
dns_dnssecsignstats_dump(dns_stats_t *stats, bool refresh,
dns_dnssecsignstats_dumper_t dump_fn, void *arg,
unsigned int options);
/*%<

View file

@ -1940,9 +1940,6 @@ dns_zone_setrcvquerystats(dns_zone_t *zone, dns_stats_t *stats);
void
dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats);
void
dns_zone_setdnssecrefreshstats(dns_zone_t *zone, dns_stats_t *stats);
/*%<
* Set additional statistics sets to zone. These are attached to the zone
* but are not counted in the zone module; only the caller updates the
@ -1962,9 +1959,6 @@ dns_zone_getrcvquerystats(dns_zone_t *zone);
dns_stats_t *
dns_zone_getdnssecsignstats(dns_zone_t *zone);
dns_stats_t *
dns_zone_getdnssecrefreshstats(dns_zone_t *zone);
/*%<
* Get the additional statistics for zone, if one is installed.
*
@ -1976,17 +1970,6 @@ dns_zone_getdnssecrefreshstats(dns_zone_t *zone);
* otherwise NULL.
*/
/*%<
* Set additional statistics sets to zone. These are attached to the zone
* but are not counted in the zone module; only the caller updates the
* counters.
*
* Requires:
* \li 'zone' to be a valid zone.
*
*\li stats is a valid statistics.
*/
void
dns_zone_dialup(dns_zone_t *zone);
/*%<

View file

@ -20,6 +20,7 @@
#include <isc/stats.h>
#include <isc/util.h>
#include <dns/log.h>
#include <dns/opcode.h>
#include <dns/rdatatype.h>
#include <dns/stats.h>
@ -93,8 +94,18 @@ typedef enum {
*/
#define RDTYPECOUNTER_MAXVAL 0x0602
/* dnssec maximum key id */
static int dnssec_keyid_max = 65535;
/*
* DNSSEC sign statistics.
*
* Per key we maintain 3 counters. The first is actually no counter but
* a key id reference. The second is the number of signatures the key created.
* The third is the number of signatures refreshed by the key.
*/
/* Maximum number of keys to keep track of for DNSSEC signing statistics. */
static int dnssec_max_keys = 4;
/* Attribute to signal whether a counter is actually a key id. */
#define DNSSECSIGNSTATS_IS_KEY 0x10000
struct dns_stats {
unsigned int magic;
@ -228,7 +239,11 @@ isc_result_t
dns_dnssecsignstats_create(isc_mem_t *mctx, dns_stats_t **statsp) {
REQUIRE(statsp != NULL && *statsp == NULL);
return (create_stats(mctx, dns_statstype_dnssec, dnssec_keyid_max,
/*
* Create two counters per key, one is the key id, the other two are
* the actual counters for creating and refreshing signatures.
*/
return (create_stats(mctx, dns_statstype_dnssec, dnssec_max_keys * 3,
statsp));
}
@ -342,10 +357,64 @@ dns_rcodestats_increment(dns_stats_t *stats, dns_rcode_t code) {
}
void
dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id) {
dns_dnssecsignstats_increment(dns_stats_t *stats, dns_keytag_t id,
bool refresh) {
isc_statscounter_t operation;
uint32_t kval;
REQUIRE(DNS_STATS_VALID(stats) && stats->type == dns_statstype_dnssec);
isc_stats_increment(stats->counters, (isc_statscounter_t)id);
kval = (uint32_t)id;
kval |= DNSSECSIGNSTATS_IS_KEY;
/* What operation are we counting? */
if (refresh) {
operation = (isc_statscounter_t)dnssec_max_keys * 2;
} else {
operation = (isc_statscounter_t)dnssec_max_keys;
}
/* Look up correct counter. */
for (int i = 0; i < dnssec_max_keys; i++) {
uint32_t counter = isc_stats_get_counter(stats->counters, i);
if (counter == kval) {
/* Match */
isc_stats_increment(stats->counters, operation + i);
return;
}
}
/* No match found. Store key in unused slot. */
for (int i = 0; i < dnssec_max_keys; i++) {
uint32_t counter = isc_stats_get_counter(stats->counters, i);
if (counter == 0) {
isc_stats_set(stats->counters, kval, i);
isc_stats_increment(stats->counters, operation + i);
return;
}
}
/* No room, rotate keys. */
for (int i = 1; i < dnssec_max_keys; i++) {
uint32_t keyv = isc_stats_get_counter(stats->counters, i);
uint32_t sign = isc_stats_get_counter(stats->counters,
(dnssec_max_keys + i));
uint32_t refr = isc_stats_get_counter(
stats->counters, (dnssec_max_keys * 2 + i));
isc_stats_set(stats->counters, keyv, i - 1);
isc_stats_set(stats->counters, sign, dnssec_max_keys + i - 1);
isc_stats_set(stats->counters, refr,
dnssec_max_keys * 2 + i - 1);
}
/* Reset counters for new key. */
isc_stats_set(stats->counters, kval, dnssec_max_keys - 1);
isc_stats_set(stats->counters, 0, 2 * dnssec_max_keys - 1);
isc_stats_set(stats->counters, 0, 3 * dnssec_max_keys - 1);
/* And increment the counter for the given operation. */
isc_stats_increment(stats->counters, operation + dnssec_max_keys - 1);
}
/*%
@ -452,8 +521,41 @@ dnssec_dumpcb(isc_statscounter_t counter, uint64_t value, void *arg) {
dnssecarg->fn((dns_keytag_t)counter, value, dnssecarg->arg);
}
static void
dnssec_statsdump(isc_stats_t *stats, bool refresh, isc_stats_dumper_t dump_fn,
void *arg, unsigned int options) {
int i;
isc_statscounter_t operation;
if (refresh) {
operation = (isc_statscounter_t)dnssec_max_keys * 2;
} else {
operation = (isc_statscounter_t)dnssec_max_keys;
}
for (i = 0; i < dnssec_max_keys; i++) {
uint32_t kval, val;
dns_keytag_t id;
kval = isc_stats_get_counter(stats, i);
if (kval == 0) {
continue;
}
val = isc_stats_get_counter(stats, (operation + i));
if ((options & ISC_STATSDUMP_VERBOSE) == 0 && val == 0) {
continue;
}
id = (dns_keytag_t)kval;
id &= ~DNSSECSIGNSTATS_IS_KEY;
dump_fn((isc_statscounter_t)id, val, arg);
}
}
void
dns_dnssecsignstats_dump(dns_stats_t *stats,
dns_dnssecsignstats_dump(dns_stats_t *stats, bool refresh,
dns_dnssecsignstats_dumper_t dump_fn, void *arg0,
unsigned int options) {
dnssecsigndumparg_t arg;
@ -462,7 +564,9 @@ dns_dnssecsignstats_dump(dns_stats_t *stats,
arg.fn = dump_fn;
arg.arg = arg0;
isc_stats_dump(stats->counters, dnssec_dumpcb, &arg, options);
dnssec_statsdump(stats->counters, refresh, dnssec_dumpcb, &arg,
options);
}
static void

View file

@ -1259,8 +1259,8 @@ add_sigs(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
added_sig = true;
/* Update DNSSEC sign statistics. */
if (dnssecsignstats != NULL) {
dns_dnssecsignstats_increment(dnssecsignstats,
dst_key_id(keys[i]));
dns_dnssecsignstats_increment(
dnssecsignstats, dst_key_id(keys[i]), false);
}
}
if (!added_sig) {

View file

@ -1195,7 +1195,6 @@ dns_zone_getchecknames
dns_zone_getclass
dns_zone_getdb
dns_zone_getdbtype
dns_zone_getdnssecrefreshstats
dns_zone_getdnssecsignstats
dns_zone_getexpiretime
dns_zone_getfile
@ -1298,7 +1297,6 @@ dns_zone_setclass
dns_zone_setdb
dns_zone_setdbtype
dns_zone_setdialup
dns_zone_setdnssecrefreshstats
dns_zone_setdnssecsignstats
dns_zone_setfile
dns_zone_setflag

View file

@ -337,7 +337,6 @@ struct dns_zone {
isc_stats_t *requeststats;
dns_stats_t *rcvquerystats;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
uint32_t notifydelay;
dns_isselffunc_t isself;
void *isselfarg;
@ -1088,7 +1087,6 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) {
zone->requeststats = NULL;
zone->rcvquerystats = NULL;
zone->dnssecsignstats = NULL;
zone->dnssecrefreshstats = NULL;
zone->notifydelay = 5;
zone->isself = NULL;
zone->isselfarg = NULL;
@ -1266,9 +1264,6 @@ zone_free(dns_zone_t *zone) {
if (zone->dnssecsignstats != NULL) {
dns_stats_detach(&zone->dnssecsignstats);
}
if (zone->dnssecrefreshstats != NULL) {
dns_stats_detach(&zone->dnssecrefreshstats);
}
if (zone->db != NULL) {
zone_detachdb(zone);
}
@ -6714,7 +6709,6 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
dns_dbnode_t *node = NULL;
dns_kasp_t *kasp = dns_zone_getkasp(zone);
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
dns_rdataset_t rdataset;
dns_rdata_t sig_rdata = DNS_RDATA_INIT;
unsigned char data[1024]; /* XXX */
@ -6888,16 +6882,13 @@ add_sigs(dns_db_t *db, dns_dbversion_t *ver, dns_name_t *name, dns_zone_t *zone,
/* Update DNSSEC sign statistics. */
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone);
if (dnssecsignstats != NULL) {
/* Generated a new signature. */
dns_dnssecsignstats_increment(
dns_zone_getdnssecsignstats(zone),
dst_key_id(keys[i]));
}
if (dnssecrefreshstats != NULL) {
dnssecsignstats, dst_key_id(keys[i]), false);
/* This is a refresh. */
dns_dnssecsignstats_increment(
dns_zone_getdnssecrefreshstats(zone),
dst_key_id(keys[i]));
dnssecsignstats, dst_key_id(keys[i]), true);
}
}
@ -7358,7 +7349,6 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
dns_rdataset_t rdataset;
dns_rdata_t rdata = DNS_RDATA_INIT;
dns_stats_t *dnssecsignstats;
dns_stats_t *dnssecrefreshstats;
isc_buffer_t buffer;
unsigned char data[1024];
@ -7477,16 +7467,13 @@ sign_a_node(dns_db_t *db, dns_zone_t *zone, dns_name_t *name,
/* Update DNSSEC sign statistics. */
dnssecsignstats = dns_zone_getdnssecsignstats(zone);
dnssecrefreshstats = dns_zone_getdnssecrefreshstats(zone);
if (dnssecsignstats != NULL) {
dns_dnssecsignstats_increment(
dns_zone_getdnssecsignstats(zone),
dst_key_id(key));
}
if (dnssecrefreshstats != NULL) {
dns_dnssecsignstats_increment(
dns_zone_getdnssecrefreshstats(zone),
dst_key_id(key));
/* Generated a new signature. */
dns_dnssecsignstats_increment(dnssecsignstats,
dst_key_id(key), false);
/* This is a refresh. */
dns_dnssecsignstats_increment(dnssecsignstats,
dst_key_id(key), true);
}
(*signatures)--;
@ -18420,17 +18407,6 @@ dns_zone_setdnssecsignstats(dns_zone_t *zone, dns_stats_t *stats) {
UNLOCK_ZONE(zone);
}
void
dns_zone_setdnssecrefreshstats(dns_zone_t *zone, dns_stats_t *stats) {
REQUIRE(DNS_ZONE_VALID(zone));
LOCK_ZONE(zone);
if (stats != NULL && zone->dnssecrefreshstats == NULL) {
dns_stats_attach(stats, &zone->dnssecrefreshstats);
}
UNLOCK_ZONE(zone);
}
dns_stats_t *
dns_zone_getdnssecsignstats(dns_zone_t *zone) {
REQUIRE(DNS_ZONE_VALID(zone));
@ -18438,13 +18414,6 @@ dns_zone_getdnssecsignstats(dns_zone_t *zone) {
return (zone->dnssecsignstats);
}
dns_stats_t *
dns_zone_getdnssecrefreshstats(dns_zone_t *zone) {
REQUIRE(DNS_ZONE_VALID(zone));
return (zone->dnssecrefreshstats);
}
isc_stats_t *
dns_zone_getrequeststats(dns_zone_t *zone) {
/*