From e3e64a1abd2c333bb658470c0681a7a32f5702b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 09:04:47 +0200 Subject: [PATCH 01/11] resolved: indent tables for readability Some of the tables in resolved were already doing using this convention. This makes the rest of them do so too. --- src/resolve/resolved-dns-dnssec.c | 26 ++++++++--------- src/resolve/resolved-dns-packet.c | 40 +++++++++++++------------- src/resolve/resolved-dns-rr.c | 6 ++-- src/resolve/resolved-dns-server.c | 16 +++++------ src/resolve/resolved-dns-transaction.c | 40 +++++++++++++------------- 5 files changed, 64 insertions(+), 64 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index a107769130..e191badad5 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -2234,24 +2234,24 @@ int dnssec_test_positive_wildcard( #endif static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = { - [DNSSEC_VALIDATED] = "validated", - [DNSSEC_VALIDATED_WILDCARD] = "validated-wildcard", - [DNSSEC_INVALID] = "invalid", - [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired", + [DNSSEC_VALIDATED] = "validated", + [DNSSEC_VALIDATED_WILDCARD] = "validated-wildcard", + [DNSSEC_INVALID] = "invalid", + [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired", [DNSSEC_UNSUPPORTED_ALGORITHM] = "unsupported-algorithm", - [DNSSEC_NO_SIGNATURE] = "no-signature", - [DNSSEC_MISSING_KEY] = "missing-key", - [DNSSEC_UNSIGNED] = "unsigned", - [DNSSEC_FAILED_AUXILIARY] = "failed-auxiliary", - [DNSSEC_NSEC_MISMATCH] = "nsec-mismatch", - [DNSSEC_INCOMPATIBLE_SERVER] = "incompatible-server", + [DNSSEC_NO_SIGNATURE] = "no-signature", + [DNSSEC_MISSING_KEY] = "missing-key", + [DNSSEC_UNSIGNED] = "unsigned", + [DNSSEC_FAILED_AUXILIARY] = "failed-auxiliary", + [DNSSEC_NSEC_MISMATCH] = "nsec-mismatch", + [DNSSEC_INCOMPATIBLE_SERVER] = "incompatible-server", }; DEFINE_STRING_TABLE_LOOKUP(dnssec_result, DnssecResult); static const char* const dnssec_verdict_table[_DNSSEC_VERDICT_MAX] = { - [DNSSEC_SECURE] = "secure", - [DNSSEC_INSECURE] = "insecure", - [DNSSEC_BOGUS] = "bogus", + [DNSSEC_SECURE] = "secure", + [DNSSEC_INSECURE] = "insecure", + [DNSSEC_BOGUS] = "bogus", [DNSSEC_INDETERMINATE] = "indeterminate", }; DEFINE_STRING_TABLE_LOOKUP(dnssec_verdict, DnssecVerdict); diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index a197f94bcf..c93b7c1dde 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2635,31 +2635,31 @@ size_t dns_packet_size_unfragmented(DnsPacket *p) { } static const char* const dns_rcode_table[_DNS_RCODE_MAX_DEFINED] = { - [DNS_RCODE_SUCCESS] = "SUCCESS", - [DNS_RCODE_FORMERR] = "FORMERR", - [DNS_RCODE_SERVFAIL] = "SERVFAIL", - [DNS_RCODE_NXDOMAIN] = "NXDOMAIN", - [DNS_RCODE_NOTIMP] = "NOTIMP", - [DNS_RCODE_REFUSED] = "REFUSED", - [DNS_RCODE_YXDOMAIN] = "YXDOMAIN", - [DNS_RCODE_YXRRSET] = "YRRSET", - [DNS_RCODE_NXRRSET] = "NXRRSET", - [DNS_RCODE_NOTAUTH] = "NOTAUTH", - [DNS_RCODE_NOTZONE] = "NOTZONE", - [DNS_RCODE_BADVERS] = "BADVERS", - [DNS_RCODE_BADKEY] = "BADKEY", - [DNS_RCODE_BADTIME] = "BADTIME", - [DNS_RCODE_BADMODE] = "BADMODE", - [DNS_RCODE_BADNAME] = "BADNAME", - [DNS_RCODE_BADALG] = "BADALG", - [DNS_RCODE_BADTRUNC] = "BADTRUNC", + [DNS_RCODE_SUCCESS] = "SUCCESS", + [DNS_RCODE_FORMERR] = "FORMERR", + [DNS_RCODE_SERVFAIL] = "SERVFAIL", + [DNS_RCODE_NXDOMAIN] = "NXDOMAIN", + [DNS_RCODE_NOTIMP] = "NOTIMP", + [DNS_RCODE_REFUSED] = "REFUSED", + [DNS_RCODE_YXDOMAIN] = "YXDOMAIN", + [DNS_RCODE_YXRRSET] = "YRRSET", + [DNS_RCODE_NXRRSET] = "NXRRSET", + [DNS_RCODE_NOTAUTH] = "NOTAUTH", + [DNS_RCODE_NOTZONE] = "NOTZONE", + [DNS_RCODE_BADVERS] = "BADVERS", + [DNS_RCODE_BADKEY] = "BADKEY", + [DNS_RCODE_BADTIME] = "BADTIME", + [DNS_RCODE_BADMODE] = "BADMODE", + [DNS_RCODE_BADNAME] = "BADNAME", + [DNS_RCODE_BADALG] = "BADALG", + [DNS_RCODE_BADTRUNC] = "BADTRUNC", [DNS_RCODE_BADCOOKIE] = "BADCOOKIE", }; DEFINE_STRING_TABLE_LOOKUP(dns_rcode, int); static const char* const dns_protocol_table[_DNS_PROTOCOL_MAX] = { - [DNS_PROTOCOL_DNS] = "dns", - [DNS_PROTOCOL_MDNS] = "mdns", + [DNS_PROTOCOL_DNS] = "dns", + [DNS_PROTOCOL_MDNS] = "mdns", [DNS_PROTOCOL_LLMNR] = "llmnr", }; DEFINE_STRING_TABLE_LOOKUP(dns_protocol, DnsProtocol); diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 5b0e601e97..0493faee6b 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1868,9 +1868,9 @@ DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(dnssec_algorithm, int, 255); static const char* const dnssec_digest_table[_DNSSEC_DIGEST_MAX_DEFINED] = { /* Names as listed on https://www.iana.org/assignments/ds-rr-types/ds-rr-types.xhtml */ - [DNSSEC_DIGEST_SHA1] = "SHA-1", - [DNSSEC_DIGEST_SHA256] = "SHA-256", + [DNSSEC_DIGEST_SHA1] = "SHA-1", + [DNSSEC_DIGEST_SHA256] = "SHA-256", [DNSSEC_DIGEST_GOST_R_34_11_94] = "GOST_R_34.11-94", - [DNSSEC_DIGEST_SHA384] = "SHA-384", + [DNSSEC_DIGEST_SHA384] = "SHA-384", }; DEFINE_STRING_TABLE_LOOKUP_WITH_FALLBACK(dnssec_digest, int, 255); diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 073489acae..a094d472c0 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -1086,19 +1086,19 @@ DnsScope *dns_server_scope(DnsServer *s) { } static const char* const dns_server_type_table[_DNS_SERVER_TYPE_MAX] = { - [DNS_SERVER_SYSTEM] = "system", + [DNS_SERVER_SYSTEM] = "system", [DNS_SERVER_FALLBACK] = "fallback", - [DNS_SERVER_LINK] = "link", + [DNS_SERVER_LINK] = "link", }; DEFINE_STRING_TABLE_LOOKUP(dns_server_type, DnsServerType); static const char* const dns_server_feature_level_table[_DNS_SERVER_FEATURE_LEVEL_MAX] = { - [DNS_SERVER_FEATURE_LEVEL_TCP] = "TCP", - [DNS_SERVER_FEATURE_LEVEL_UDP] = "UDP", - [DNS_SERVER_FEATURE_LEVEL_EDNS0] = "UDP+EDNS0", + [DNS_SERVER_FEATURE_LEVEL_TCP] = "TCP", + [DNS_SERVER_FEATURE_LEVEL_UDP] = "UDP", + [DNS_SERVER_FEATURE_LEVEL_EDNS0] = "UDP+EDNS0", [DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN] = "TLS+EDNS0", - [DNS_SERVER_FEATURE_LEVEL_DO] = "UDP+EDNS0+DO", - [DNS_SERVER_FEATURE_LEVEL_LARGE] = "UDP+EDNS0+DO+LARGE", - [DNS_SERVER_FEATURE_LEVEL_TLS_DO] = "TLS+EDNS0+D0", + [DNS_SERVER_FEATURE_LEVEL_DO] = "UDP+EDNS0+DO", + [DNS_SERVER_FEATURE_LEVEL_LARGE] = "UDP+EDNS0+DO+LARGE", + [DNS_SERVER_FEATURE_LEVEL_TLS_DO] = "TLS+EDNS0+D0", }; DEFINE_STRING_TABLE_LOOKUP(dns_server_feature_level, DnsServerFeatureLevel); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 89a8e415e8..9535a7ba4c 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -3538,31 +3538,31 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { } static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX] = { - [DNS_TRANSACTION_NULL] = "null", - [DNS_TRANSACTION_PENDING] = "pending", - [DNS_TRANSACTION_VALIDATING] = "validating", - [DNS_TRANSACTION_RCODE_FAILURE] = "rcode-failure", - [DNS_TRANSACTION_SUCCESS] = "success", - [DNS_TRANSACTION_NO_SERVERS] = "no-servers", - [DNS_TRANSACTION_TIMEOUT] = "timeout", + [DNS_TRANSACTION_NULL] = "null", + [DNS_TRANSACTION_PENDING] = "pending", + [DNS_TRANSACTION_VALIDATING] = "validating", + [DNS_TRANSACTION_RCODE_FAILURE] = "rcode-failure", + [DNS_TRANSACTION_SUCCESS] = "success", + [DNS_TRANSACTION_NO_SERVERS] = "no-servers", + [DNS_TRANSACTION_TIMEOUT] = "timeout", [DNS_TRANSACTION_ATTEMPTS_MAX_REACHED] = "attempts-max-reached", - [DNS_TRANSACTION_INVALID_REPLY] = "invalid-reply", - [DNS_TRANSACTION_ERRNO] = "errno", - [DNS_TRANSACTION_ABORTED] = "aborted", - [DNS_TRANSACTION_DNSSEC_FAILED] = "dnssec-failed", - [DNS_TRANSACTION_NO_TRUST_ANCHOR] = "no-trust-anchor", - [DNS_TRANSACTION_RR_TYPE_UNSUPPORTED] = "rr-type-unsupported", - [DNS_TRANSACTION_NETWORK_DOWN] = "network-down", - [DNS_TRANSACTION_NOT_FOUND] = "not-found", - [DNS_TRANSACTION_NO_SOURCE] = "no-source", - [DNS_TRANSACTION_STUB_LOOP] = "stub-loop", + [DNS_TRANSACTION_INVALID_REPLY] = "invalid-reply", + [DNS_TRANSACTION_ERRNO] = "errno", + [DNS_TRANSACTION_ABORTED] = "aborted", + [DNS_TRANSACTION_DNSSEC_FAILED] = "dnssec-failed", + [DNS_TRANSACTION_NO_TRUST_ANCHOR] = "no-trust-anchor", + [DNS_TRANSACTION_RR_TYPE_UNSUPPORTED] = "rr-type-unsupported", + [DNS_TRANSACTION_NETWORK_DOWN] = "network-down", + [DNS_TRANSACTION_NOT_FOUND] = "not-found", + [DNS_TRANSACTION_NO_SOURCE] = "no-source", + [DNS_TRANSACTION_STUB_LOOP] = "stub-loop", }; DEFINE_STRING_TABLE_LOOKUP(dns_transaction_state, DnsTransactionState); static const char* const dns_transaction_source_table[_DNS_TRANSACTION_SOURCE_MAX] = { - [DNS_TRANSACTION_NETWORK] = "network", - [DNS_TRANSACTION_CACHE] = "cache", - [DNS_TRANSACTION_ZONE] = "zone", + [DNS_TRANSACTION_NETWORK] = "network", + [DNS_TRANSACTION_CACHE] = "cache", + [DNS_TRANSACTION_ZONE] = "zone", [DNS_TRANSACTION_TRUST_ANCHOR] = "trust-anchor", }; DEFINE_STRING_TABLE_LOOKUP(dns_transaction_source, DnsTransactionSource); From e1004d0aba336d9ce2e7d1e12780530894ae5c14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 11:12:12 +0200 Subject: [PATCH 02/11] resolved: make counter for n_auxiliary_queries smaller It can never go above 64, so let's use a narrower type. Order will be adjusted later to remove holes. --- src/resolve/resolved-dns-query.c | 2 ++ src/resolve/resolved-dns-query.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 5517db149d..beb20592bd 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -13,6 +13,8 @@ #define QUERIES_MAX 2048 #define AUXILIARY_QUERIES_MAX 64 +assert_cc(AUXILIARY_QUERIES_MAX < UINT8_MAX); + static int dns_query_candidate_new(DnsQueryCandidate **ret, DnsQuery *q, DnsScope *s) { DnsQueryCandidate *c; diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index fa584fe3de..be9b5fa548 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -38,7 +38,7 @@ struct DnsQuery { * TXT+SRV query. */ DnsQuery *auxiliary_for; LIST_HEAD(DnsQuery, auxiliary_queries); - unsigned n_auxiliary_queries; + uint8_t n_auxiliary_queries; int auxiliary_result; /* The question, formatted in IDNA for use on classic DNS, and as UTF8 for use in LLMNR or mDNS. Note From b4d1227858fb507b3bc8dc0a5ba2e4fdc0d2606f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 11:20:18 +0200 Subject: [PATCH 03/11] resolved: make counter for redirects smaller It can never go above 16, so let's use a narrower type. Order will be adjusted later to remove holes. --- src/resolve/resolved-dns-query.c | 6 ++++-- src/resolve/resolved-dns-query.h | 4 +--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index beb20592bd..d1bb686254 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -12,8 +12,10 @@ #define QUERIES_MAX 2048 #define AUXILIARY_QUERIES_MAX 64 +#define CNAME_REDIRECTS_MAX 16 assert_cc(AUXILIARY_QUERIES_MAX < UINT8_MAX); +assert_cc(CNAME_REDIRECTS_MAX < UINT8_MAX); static int dns_query_candidate_new(DnsQueryCandidate **ret, DnsQuery *q, DnsScope *s) { DnsQueryCandidate *c; @@ -984,9 +986,9 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) assert(q); - q->n_cname_redirects++; - if (q->n_cname_redirects > CNAME_REDIRECT_MAX) + if (q->n_cname_redirects >= CNAME_REDIRECTS_MAX) return -ELOOP; + q->n_cname_redirects++; r = dns_question_cname_redirect(q->question_idna, cname, &nq_idna); if (r < 0) diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index be9b5fa548..fcc57a0e0d 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -64,7 +64,7 @@ struct DnsQuery { int ifindex; DnsTransactionState state; - unsigned n_cname_redirects; + uint8_t n_cname_redirects; LIST_HEAD(DnsQueryCandidate, candidates); sd_event_source *timeout_event_source; @@ -154,5 +154,3 @@ static inline uint64_t dns_query_reply_flags_make(DnsQuery *q) { dns_query_fully_confidential(q)) | (q->answer_query_flags & (SD_RESOLVED_FROM_MASK|SD_RESOLVED_SYNTHETIC)); } - -#define CNAME_REDIRECT_MAX 16 From 9079bd32ce72a7e2e131e214d9b64b464f67fb2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 11:28:17 +0200 Subject: [PATCH 04/11] resolved: pahole optimization for DnsQuery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change from the last three commits: /* size: 312, cachelines: 5, members: 46 */ /* sum members: 296, holes: 5, sum holes: 16 */ ↓ /* size: 288, cachelines: 5, members: 46 */ /* sum members: 286, holes: 1, sum holes: 1 */ It's not a big difference, but we might have quite a few queries in flight, so let' make this a bit more efficient. --- src/resolve/resolved-dns-query.h | 39 ++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index fcc57a0e0d..43a833a08a 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -33,14 +33,6 @@ struct DnsQueryCandidate { struct DnsQuery { Manager *manager; - /* When resolving a service, we first create a TXT+SRV query, and then for the hostnames we discover - * auxiliary A+AAAA queries. This pointer always points from the auxiliary queries back to the - * TXT+SRV query. */ - DnsQuery *auxiliary_for; - LIST_HEAD(DnsQuery, auxiliary_queries); - uint8_t n_auxiliary_queries; - int auxiliary_result; - /* The question, formatted in IDNA for use on classic DNS, and as UTF8 for use in LLMNR or mDNS. Note * that even on classic DNS some labels might use UTF8 encoding. Specifically, DNS-SD service names * (in contrast to their domain suffixes) use UTF-8 encoding even on DNS. Thus, the difference @@ -63,8 +55,12 @@ struct DnsQuery { uint64_t flags; int ifindex; - DnsTransactionState state; - uint8_t n_cname_redirects; + /* When resolving a service, we first create a TXT+SRV query, and then for the hostnames we discover + * auxiliary A+AAAA queries. This pointer always points from the auxiliary queries back to the + * TXT+SRV query. */ + int auxiliary_result; + DnsQuery *auxiliary_for; + LIST_HEAD(DnsQuery, auxiliary_queries); LIST_HEAD(DnsQueryCandidate, candidates); sd_event_source *timeout_event_source; @@ -76,18 +72,26 @@ struct DnsQuery { uint64_t answer_query_flags; DnsProtocol answer_protocol; int answer_family; - DnsSearchDomain *answer_search_domain; - int answer_errno; /* if state is DNS_TRANSACTION_ERRNO */ - bool previous_redirect_unauthenticated; - bool previous_redirect_non_confidential; - bool previous_redirect_non_synthetic; DnsPacket *answer_full_packet; + DnsSearchDomain *answer_search_domain; + + DnsTransactionState state; + int answer_errno; /* if state is DNS_TRANSACTION_ERRNO */ + + unsigned block_ready; + + uint8_t n_auxiliary_queries; + uint8_t n_cname_redirects; + + bool previous_redirect_unauthenticated:1; + bool previous_redirect_non_confidential:1; + bool previous_redirect_non_synthetic:1; + bool request_address_valid:1; /* Bus + Varlink client information */ sd_bus_message *bus_request; Varlink *varlink_request; int request_family; - bool request_address_valid; union in_addr_union request_address; unsigned block_all_complete; char *request_address_string; @@ -102,12 +106,13 @@ struct DnsQuery { /* Completion callback */ void (*complete)(DnsQuery* q); - unsigned block_ready; sd_bus_track *bus_track; LIST_FIELDS(DnsQuery, queries); LIST_FIELDS(DnsQuery, auxiliary_queries); + + /* Note: fields should be ordered to minimize alignment gaps. Use pahole! */ }; enum { From 35aa30697a3f747d183fe134d55d1b177736ebd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 16:47:21 +0200 Subject: [PATCH 05/11] resolved: shorten dns_name_count_labels() --- src/shared/dns-domain.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index ec3021d602..716c010076 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1191,13 +1191,11 @@ int dns_name_skip(const char *a, unsigned n_labels, const char **ret) { int dns_name_count_labels(const char *name) { unsigned n = 0; - const char *p; int r; assert(name); - p = name; - for (;;) { + for (const char *p = name;;) { r = dns_name_parent(&p); if (r < 0) return r; @@ -1210,7 +1208,7 @@ int dns_name_count_labels(const char *name) { n++; } - return (int) n; + return n; } int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b) { From 98e80bf9d2f069e58283a74a6cfd8bf69578af31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:03:30 +0200 Subject: [PATCH 06/11] resolved: use narrower types for label counts in rr Order will be adjusted later to remove holes. --- src/resolve/resolved-dns-dnssec.c | 11 ++++++----- src/resolve/resolved-dns-rr.c | 10 +++++----- src/resolve/resolved-dns-rr.h | 8 ++++++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index e191badad5..70d7837a3a 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -483,14 +483,14 @@ static int dnssec_rrsig_prepare(DnsResourceRecord *rrsig) { const char *name; int r; - /* Checks whether the specified RRSIG RR is somewhat valid, and initializes the .n_skip_labels_source and - * .n_skip_labels_signer fields so that we can use them later on. */ + /* Checks whether the specified RRSIG RR is somewhat valid, and initializes the .n_skip_labels_source + * and .n_skip_labels_signer fields so that we can use them later on. */ assert(rrsig); assert(rrsig->key->type == DNS_TYPE_RRSIG); /* Check if this RRSIG RR is already prepared */ - if (rrsig->n_skip_labels_source != UINT_MAX) + if (rrsig->n_skip_labels_source != UINT8_MAX) return 0; if (rrsig->rrsig.inception > rrsig->rrsig.expiration) @@ -523,6 +523,7 @@ static int dnssec_rrsig_prepare(DnsResourceRecord *rrsig) { if (r == 0) return -EINVAL; + assert(n_key_labels < UINT8_MAX); /* UINT8_MAX/-1 means unsigned. */ rrsig->n_skip_labels_source = n_key_labels - rrsig->rrsig.labels; rrsig->n_skip_labels_signer = n_key_labels - n_signer_labels; @@ -1291,10 +1292,10 @@ static int nsec3_is_good(DnsResourceRecord *rr, DnsResourceRecord *nsec3) { /* Ignore NSEC3 RRs generated from wildcards. If these NSEC3 RRs weren't correctly signed we can't make this * check (since rr->n_skip_labels_source is -1), but that's OK, as we won't trust them anyway in that case. */ - if (!IN_SET(rr->n_skip_labels_source, 0, UINT_MAX)) + if (!IN_SET(rr->n_skip_labels_source, 0, UINT8_MAX)) return 0; /* Ignore NSEC3 RRs that are located anywhere else than one label below the zone */ - if (!IN_SET(rr->n_skip_labels_signer, 1, UINT_MAX)) + if (!IN_SET(rr->n_skip_labels_signer, 1, UINT8_MAX)) return 0; if (!nsec3) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 0493faee6b..e493c32158 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -381,8 +381,8 @@ DnsResourceRecord* dns_resource_record_new(DnsResourceKey *key) { .n_ref = 1, .key = dns_resource_key_ref(key), .expiry = USEC_INFINITY, - .n_skip_labels_signer = UINT_MAX, - .n_skip_labels_source = UINT_MAX, + .n_skip_labels_signer = UINT8_MAX, + .n_skip_labels_source = UINT8_MAX, }; return rr; @@ -1258,7 +1258,7 @@ int dns_resource_record_signer(DnsResourceRecord *rr, const char **ret) { /* Returns the RRset's signer, if it is known. */ - if (rr->n_skip_labels_signer == UINT_MAX) + if (rr->n_skip_labels_signer == UINT8_MAX) return -ENODATA; n = dns_resource_key_name(rr->key); @@ -1281,7 +1281,7 @@ int dns_resource_record_source(DnsResourceRecord *rr, const char **ret) { /* Returns the RRset's synthesizing source, if it is known. */ - if (rr->n_skip_labels_source == UINT_MAX) + if (rr->n_skip_labels_source == UINT8_MAX) return -ENODATA; n = dns_resource_key_name(rr->key); @@ -1315,7 +1315,7 @@ int dns_resource_record_is_synthetic(DnsResourceRecord *rr) { /* Returns > 0 if the RR is generated from a wildcard, and is not the asterisk name itself */ - if (rr->n_skip_labels_source == UINT_MAX) + if (rr->n_skip_labels_source == UINT8_MAX) return -ENODATA; if (rr->n_skip_labels_source == 0) diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 43bbcb3073..10c05dd9c3 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -4,6 +4,7 @@ #include #include "bitmap.h" +#include "dns-def.h" #include "dns-type.h" #include "hashmap.h" #include "in-addr-util.h" @@ -98,9 +99,9 @@ struct DnsResourceRecord { usec_t expiry; /* RRSIG signature expiry */ /* How many labels to strip to determine "signer" of the RRSIG (aka, the zone). -1 if not signed. */ - unsigned n_skip_labels_signer; + uint8_t n_skip_labels_signer; /* How many labels to strip to determine "synthesizing source" of this RR, i.e. the wildcard's immediate parent. -1 if not signed. */ - unsigned n_skip_labels_source; + uint8_t n_skip_labels_source; bool unparsable:1; @@ -245,6 +246,9 @@ struct DnsResourceRecord { }; }; +/* We use uint8_t for label counts above, and UINT8_MAX/-1 has special meaning. */ +assert_cc(DNS_N_LABELS_MAX < UINT8_MAX); + static inline const void* DNS_RESOURCE_RECORD_RDATA(const DnsResourceRecord *rr) { if (!rr) return NULL; From a6e890d4935b27f35c2171c9e2998a73606fa7bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:15:53 +0200 Subject: [PATCH 07/11] resolved: rearrange DnsResourceRecord for better layout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The structure is rearranged to have less holes. Also fields in the union are rearranged not to have holes (though most variants of the union still have some padding at the end). The full size does not decrease a lot, but the compiler should be able to copy less bytes when it knows the specific type of the union. Bitfields are dropped because they don't give any memory savings due to alignment requirements (but would still require more complex to access). The change from the this and previous commit: /* size: 128, cachelines: 2, members: 13 */ /* sum members: 112, holes: 3, sum holes: 15 */ /* sum bitfield members: 2 bits, bit holes: 1, sum bit holes: 6 bits */ ↓ /* size: 112, cachelines: 2, members: 13 */ /* sum members: 108, holes: 1, sum holes: 4 */ --- src/resolve/resolved-dns-rr.h | 55 ++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 10c05dd9c3..66aa10b82c 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -91,21 +91,21 @@ struct DnsTxtItem { struct DnsResourceRecord { unsigned n_ref; + uint32_t ttl; + usec_t expiry; /* RRSIG signature expiry */ + DnsResourceKey *key; char *to_string; - uint32_t ttl; - usec_t expiry; /* RRSIG signature expiry */ - /* How many labels to strip to determine "signer" of the RRSIG (aka, the zone). -1 if not signed. */ uint8_t n_skip_labels_signer; /* How many labels to strip to determine "synthesizing source" of this RR, i.e. the wildcard's immediate parent. -1 if not signed. */ uint8_t n_skip_labels_source; - bool unparsable:1; + bool unparsable; + bool wire_format_canonical; - bool wire_format_canonical:1; void *wire_format; size_t wire_format_size; size_t wire_format_rdata_offset; @@ -117,10 +117,10 @@ struct DnsResourceRecord { } generic, opt; struct { + char *name; uint16_t priority; uint16_t weight; uint16_t port; - char *name; } srv; struct { @@ -155,8 +155,8 @@ struct DnsResourceRecord { } soa; struct { - uint16_t priority; char *exchange; + uint16_t priority; } mx; /* https://tools.ietf.org/html/rfc1876 */ @@ -172,23 +172,29 @@ struct DnsResourceRecord { /* https://tools.ietf.org/html/rfc4255#section-3.1 */ struct { - uint8_t algorithm; - uint8_t fptype; void *fingerprint; size_t fingerprint_size; + + uint8_t algorithm; + uint8_t fptype; } sshfp; /* http://tools.ietf.org/html/rfc4034#section-2.1 */ struct { + void* key; + size_t key_size; + uint16_t flags; uint8_t protocol; uint8_t algorithm; - void* key; - size_t key_size; } dnskey; /* http://tools.ietf.org/html/rfc4034#section-3.1 */ struct { + char *signer; + void *signature; + size_t signature_size; + uint16_t type_covered; uint8_t algorithm; uint8_t labels; @@ -196,9 +202,6 @@ struct DnsResourceRecord { uint32_t expiration; uint32_t inception; uint16_t key_tag; - char *signer; - void *signature; - size_t signature_size; } rrsig; /* https://tools.ietf.org/html/rfc4034#section-4.1 */ @@ -209,41 +212,47 @@ struct DnsResourceRecord { /* https://tools.ietf.org/html/rfc4034#section-5.1 */ struct { + void *digest; + size_t digest_size; + uint16_t key_tag; uint8_t algorithm; uint8_t digest_type; - void *digest; - size_t digest_size; } ds; struct { - uint8_t algorithm; - uint8_t flags; - uint16_t iterations; + Bitmap *types; void *salt; size_t salt_size; void *next_hashed_name; size_t next_hashed_name_size; - Bitmap *types; + + uint8_t algorithm; + uint8_t flags; + uint16_t iterations; } nsec3; /* https://tools.ietf.org/html/draft-ietf-dane-protocol-23 */ struct { + void *data; + size_t data_size; + uint8_t cert_usage; uint8_t selector; uint8_t matching_type; - void *data; - size_t data_size; } tlsa; /* https://tools.ietf.org/html/rfc6844 */ struct { - uint8_t flags; char *tag; void *value; size_t value_size; + + uint8_t flags; } caa; }; + + /* Note: fields should be ordered to minimize alignment gaps. Use pahole! */ }; /* We use uint8_t for label counts above, and UINT8_MAX/-1 has special meaning. */ From 9aeb1a541bbb08616af1b4e5495d4bd4bfed6490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:28:48 +0200 Subject: [PATCH 08/11] resolved: optimize layout of DnsTransaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /* size: 296, cachelines: 5, members: 46 */ /* sum members: 278, holes: 5, sum holes: 17 */ /* sum bitfield members: 4 bits, bit holes: 1, sum bit holes: 4 bits */ ↓ /* size: 280, cachelines: 5, members: 46 */ /* sum members: 278, holes: 1, sum holes: 1 */ /* sum bitfield members: 4 bits, bit holes: 1, sum bit holes: 4 bits */ --- src/resolve/resolved-dns-transaction.h | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index c2d73cbedc..498cabb7e5 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -57,17 +57,6 @@ struct DnsTransaction { uint64_t query_flags; - DnsTransactionState state; - - uint16_t id; - - bool tried_stream:1; - - bool initial_jitter_scheduled:1; - bool initial_jitter_elapsed:1; - - bool probing:1; - DnsPacket *sent, *received; DnsAnswer *answer; @@ -77,6 +66,8 @@ struct DnsTransaction { uint32_t answer_nsec_ttl; int answer_errno; /* if state is DNS_TRANSACTION_ERRNO */ + DnsTransactionState state; + /* SD_RESOLVED_AUTHENTICATED here indicates whether the primary answer is authenticated, i.e. whether * the RRs from answer which directly match the question are authenticated, or, if there are none, * whether the NODATA or NXDOMAIN case is. It says nothing about additional RRs listed in the answer, @@ -93,8 +84,6 @@ struct DnsTransaction { sd_event_source *timeout_event_source; unsigned n_attempts; - unsigned n_picked_servers; - /* UDP connection logic, if we need it */ int dns_udp_fd; sd_event_source *dns_udp_event_source; @@ -114,6 +103,15 @@ struct DnsTransaction { DnsServerFeatureLevel clamp_feature_level_servfail; DnsServerFeatureLevel clamp_feature_level_nxdomain; + uint16_t id; + + bool tried_stream:1; + + bool initial_jitter_scheduled:1; + bool initial_jitter_elapsed:1; + + bool probing:1; + /* Query candidates this transaction is referenced by and that * shall be notified about this specific transaction * completing. */ @@ -133,11 +131,15 @@ struct DnsTransaction { * created in order to request DNSKEY or DS RRs. */ Set *dnssec_transactions; + unsigned n_picked_servers; + unsigned block_gc; LIST_FIELDS(DnsTransaction, transactions_by_scope); LIST_FIELDS(DnsTransaction, transactions_by_stream); LIST_FIELDS(DnsTransaction, transactions_by_key); + + /* Note: fields should be ordered to minimize alignment gaps. Use pahole! */ }; int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key, DnsPacket *bypass, uint64_t flags); From 421b1f0a4126f4bc4dace3a892c9e59d6c54e590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:31:26 +0200 Subject: [PATCH 09/11] resolved: optimize layout of DnsPacket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We obviously have lots of those, so even small savings add up. Bitfields are dropped because they don't give any memory savings due to alignment requirements (but would still require more complex to access). /* size: 184, cachelines: 3, members: 28 */ /* sum members: 172, holes: 1, sum holes: 4 */ /* sum bitfield members: 4 bits (0 bytes) */ /* padding: 7 */ /* bit_padding: 4 bits */ ↓ /* size: 176, cachelines: 3, members: 28 */ --- src/resolve/resolved-dns-packet.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index e9820795c0..6e7175c754 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -71,21 +71,23 @@ struct DnsPacket { DnsAnswer *answer; DnsResourceRecord *opt; + /* For support of truncated packets */ + DnsPacket *more; + /* Packet reception metadata */ + usec_t timestamp; /* CLOCK_BOOTTIME (or CLOCK_MONOTONIC if the former doesn't exist) */ int ifindex; int family, ipproto; union in_addr_union sender, destination; uint16_t sender_port, destination_port; uint32_t ttl; - usec_t timestamp; /* CLOCK_BOOTTIME (or CLOCK_MONOTONIC if the former doesn't exist) */ - /* For support of truncated packets */ - DnsPacket *more; + bool on_stack; + bool extracted; + bool refuse_compression; + bool canonical_form; - bool on_stack:1; - bool extracted:1; - bool refuse_compression:1; - bool canonical_form:1; + /* Note: fields should be ordered to minimize alignment gaps. Use pahole! */ }; static inline uint8_t* DNS_PACKET_DATA(const DnsPacket *p) { From ebfdf45a20fcf614f8b094655ab8f67c51ca125a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:32:50 +0200 Subject: [PATCH 10/11] resolved: drop pointless bitfield on a lone bool --- src/resolve/resolved-dns-scope.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h index f63452330c..a2b9546b38 100644 --- a/src/resolve/resolved-dns-scope.h +++ b/src/resolve/resolved-dns-scope.h @@ -43,7 +43,6 @@ struct DnsScope { OrderedHashmap *conflict_queue; sd_event_source *conflict_event_source; - bool announced:1; sd_event_source *announce_event_source; RateLimit ratelimit; @@ -63,6 +62,8 @@ struct DnsScope { LIST_HEAD(DnsTransaction, transactions); LIST_FIELDS(DnsScope, scopes); + + bool announced; }; int dns_scope_new(Manager *m, DnsScope **ret, Link *l, DnsProtocol p, int family); From 6f1d18ae9319bad3d13b90caa6dc0b34ea0a1b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 May 2021 17:33:54 +0200 Subject: [PATCH 11/11] resolved: inline iterator variable declarations --- src/resolve/resolved-dns-dnssec.c | 14 +++++--------- src/resolve/resolved-dns-rr.c | 4 +--- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 70d7837a3a..4fa3c1d995 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -35,7 +35,6 @@ uint16_t dnssec_keytag(DnsResourceRecord *dnskey, bool mask_revoke) { const uint8_t *p; uint32_t sum, f; - size_t i; /* The algorithm from RFC 4034, Appendix B. */ @@ -51,7 +50,7 @@ uint16_t dnssec_keytag(DnsResourceRecord *dnskey, bool mask_revoke) { p = dnskey->dnskey.key; - for (i = 0; i < dnskey->dnskey.key_size; i++) + for (size_t i = 0; i < dnskey->dnskey.key_size; i++) sum += (i & 1) == 0 ? (uint32_t) p[i] << 8 : (uint32_t) p[i]; sum += (sum >> 16) & UINT32_C(0xFFFF); @@ -605,13 +604,11 @@ static void dnssec_fix_rrset_ttl( DnsResourceRecord *rrsig, usec_t realtime) { - unsigned k; - assert(list); assert(n > 0); assert(rrsig); - for (k = 0; k < n; k++) { + for (unsigned k = 0; k < n; k++) { DnsResourceRecord *rr = list[k]; /* Pick the TTL as the minimum of the RR's TTL, the @@ -641,7 +638,7 @@ int dnssec_verify_rrset( const char *source, *name; _cleanup_(gcry_md_closep) gcry_md_hd_t md = NULL; int r, md_algorithm; - size_t k, n = 0; + size_t n = 0; size_t sig_size = 0; _cleanup_free_ char *sig_data = NULL; _cleanup_fclose_ FILE *f = NULL; @@ -771,7 +768,7 @@ int dnssec_verify_rrset( if (r < 0) return r; - for (k = 0; k < n; k++) { + for (size_t k = 0; k < n; k++) { size_t l; rr = list[k]; @@ -1211,7 +1208,6 @@ int dnssec_nsec3_hash(DnsResourceRecord *nsec3, const char *name, void *ret) { size_t hash_size; int algorithm; void *result; - unsigned k; int r; assert(nsec3); @@ -1253,7 +1249,7 @@ int dnssec_nsec3_hash(DnsResourceRecord *nsec3, const char *name, void *ret) { if (!result) return -EIO; - for (k = 0; k < nsec3->nsec3.iterations; k++) { + for (unsigned k = 0; k < nsec3->nsec3.iterations; k++) { uint8_t tmp[hash_size]; memcpy(tmp, result, hash_size); diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index e493c32158..5d59a90a8e 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -795,14 +795,12 @@ static char *format_txt(DnsTxtItem *first) { return NULL; LIST_FOREACH(items, i, first) { - size_t j; - if (i != first) *(p++) = ' '; *(p++) = '"'; - for (j = 0; j < i->length; j++) { + for (size_t j = 0; j < i->length; j++) { if (i->data[j] < ' ' || i->data[j] == '"' || i->data[j] >= 127) { *(p++) = '\\'; *(p++) = '0' + (i->data[j] / 100);