diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 7a1a1620af..78665bc93b 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -1308,6 +1308,19 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, if (usec_sub_unsigned(j->until, ts) < j->rr->ttl * USEC_PER_SEC / 2) continue; + if (max_rr > 0 && ancount >= max_rr) { + DNS_PACKET_HEADER(p)->ancount = htobe16(ancount); + ancount = 0; + + r = dns_packet_new_query(&p->more, p->protocol, 0, true); + if (r < 0) + return r; + + p = p->more; + + max_rr = UINT_MAX; + } + r = dns_packet_append_rr(p, j->rr, 0, NULL, NULL); if (r == -EMSGSIZE) { if (max_rr == 0) @@ -1333,18 +1346,6 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p, usec_t ts, return r; ancount++; - if (max_rr > 0 && ancount >= max_rr) { - DNS_PACKET_HEADER(p)->ancount = htobe16(ancount); - ancount = 0; - - r = dns_packet_new_query(&p->more, p->protocol, 0, true); - if (r < 0) - return r; - - p = p->more; - - max_rr = UINT_MAX; - } } finalize: diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index ecc75c6590..ca1eea4346 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -307,13 +307,13 @@ int dns_packet_validate_query(DnsPacket *p) { if (DNS_PACKET_OPCODE(p) != 0) return -EBADMSG; - if (DNS_PACKET_TC(p)) - return -EBADMSG; - switch (p->protocol) { case DNS_PROTOCOL_LLMNR: case DNS_PROTOCOL_DNS: + if (DNS_PACKET_TC(p)) /* mDNS query may have truncation flag. */ + return -EBADMSG; + /* RFC 4795, Section 2.1.1. says to discard all queries with QDCOUNT != 1 */ if (DNS_PACKET_QDCOUNT(p) != 1) return -EBADMSG; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index c5c0a2312f..696fce532a 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1786,21 +1786,20 @@ static int dns_packet_append_zone(DnsPacket *p, DnsTransaction *t, DnsResourceKe return dns_packet_append_answer(p, answer, nscount); } -static int dns_transaction_make_packet_mdns(DnsTransaction *t) { +static int mdns_make_dummy_packet(DnsTransaction *t, DnsPacket **ret_packet, Set **ret_keys) { _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; _cleanup_set_free_ Set *keys = NULL; - unsigned qdcount, ancount = 0 /* avoid false maybe-uninitialized warning */, nscount; bool add_known_answers = false; + unsigned qdcount; usec_t ts; int r; assert(t); + assert(t->scope); assert(t->scope->protocol == DNS_PROTOCOL_MDNS); + assert(ret_packet); + assert(ret_keys); - /* Discard any previously prepared packet, so we can start over and coalesce again */ - t->sent = dns_packet_unref(t->sent); - - /* First, create a dummy packet to calculate packet size. */ r = dns_packet_new_query(&p, t->scope->protocol, 0, false); if (r < 0) return r; @@ -1823,120 +1822,139 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) { if (r < 0) return r; - /* - * For mDNS, we want to coalesce as many open queries in pending transactions into one single - * query packet on the wire as possible. To achieve that, we iterate through all pending transactions - * in our current scope, and see whether their timing constraints allow them to be sent. - */ - assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &ts) >= 0); - for (bool restart = true; restart;) { - restart = false; - LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) { + LIST_FOREACH(transactions_by_scope, other, t->scope->transactions) { + + /* Skip ourselves */ + if (other == t) + continue; + + if (other->state != DNS_TRANSACTION_PENDING) + continue; + + if (other->next_attempt_after > ts) + continue; + + if (!set_contains(keys, dns_transaction_key(other))) { size_t saved_packet_size; - bool append = false; - /* Skip ourselves */ - if (other == t) - continue; - - if (other->state != DNS_TRANSACTION_PENDING) - continue; - - if (other->next_attempt_after > ts) - continue; - - if (!set_contains(keys, dns_transaction_key(other))) { - r = dns_packet_append_key(p, dns_transaction_key(other), 0, &saved_packet_size); - /* If we can't stuff more questions into the packet, just give up. - * One of the 'other' transactions will fire later and take care of the rest. */ - if (r == -EMSGSIZE) - break; - if (r < 0) - return r; - - r = dns_packet_append_zone(p, t, dns_transaction_key(other), NULL); - if (r == -EMSGSIZE) - break; - if (r < 0) - return r; - - append = true; - } - - r = dns_transaction_prepare(other, ts); - if (r < 0) - return r; - if (r == 0) { - if (append) - dns_packet_truncate(p, saved_packet_size); - - /* In this case, not only this transaction, but multiple transactions may be - * freed. Hence, we need to restart the loop. */ - restart = true; + r = dns_packet_append_key(p, dns_transaction_key(other), 0, &saved_packet_size); + /* If we can't stuff more questions into the packet, just give up. + * One of the 'other' transactions will fire later and take care of the rest. */ + if (r == -EMSGSIZE) break; - } - - usec_t timeout = transaction_get_resend_timeout(other); - r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout)); if (r < 0) return r; - if (dns_key_is_shared(dns_transaction_key(other))) - add_known_answers = true; - - if (append) { - r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other)); - if (r < 0) - return r; - } - - qdcount++; - if (qdcount >= UINT16_MAX) + r = dns_packet_append_zone(p, t, dns_transaction_key(other), NULL); + if (r == -EMSGSIZE) { + dns_packet_truncate(p, saved_packet_size); break; + } + if (r < 0) + return r; + + r = set_ensure_put(&keys, &dns_resource_key_hash_ops, dns_transaction_key(other)); + if (r < 0) + return r; } + + r = dns_transaction_prepare(other, ts); + if (r < 0) + return r; + if (r == 0) + /* In this case, not only this transaction, but multiple transactions may be + * freed. Hence, we need to restart the loop. */ + return -EAGAIN; + + usec_t timeout = transaction_get_resend_timeout(other); + r = dns_transaction_setup_timeout(other, timeout, usec_add(ts, timeout)); + if (r < 0) + return r; + + if (dns_key_is_shared(dns_transaction_key(other))) + add_known_answers = true; + + qdcount++; + if (qdcount >= UINT16_MAX) + break; } - /* Append known answer section if we're asking for any shared record */ + DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount); + + /* Append known answers section if we're asking for any shared record */ if (add_known_answers) { r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, 0); if (r < 0) return r; + } - ancount = be16toh(DNS_PACKET_HEADER(p)->ancount); + *ret_packet = TAKE_PTR(p); + *ret_keys = TAKE_PTR(keys); + return add_known_answers; +} + +static int dns_transaction_make_packet_mdns(DnsTransaction *t) { + _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL, *dummy = NULL; + _cleanup_set_free_ Set *keys = NULL; + bool add_known_answers; + DnsResourceKey *k; + unsigned c; + int r; + + assert(t); + assert(t->scope->protocol == DNS_PROTOCOL_MDNS); + + /* Discard any previously prepared packet, so we can start over and coalesce again */ + t->sent = dns_packet_unref(t->sent); + + /* First, create a dummy packet to calculate the number of known answers to be appended in the first packet. */ + for (;;) { + r = mdns_make_dummy_packet(t, &dummy, &keys); + if (r == -EAGAIN) + continue; + if (r < 0) + return r; + + add_known_answers = r; + break; } /* Then, create actual packet. */ - p = dns_packet_unref(p); r = dns_packet_new_query(&p, t->scope->protocol, 0, false); if (r < 0) return r; /* Questions */ - DnsResourceKey *k; + c = 0; SET_FOREACH(k, keys) { r = dns_packet_append_key(p, k, 0, NULL); if (r < 0) return r; + c++; } - DNS_PACKET_HEADER(p)->qdcount = htobe16(qdcount); + DNS_PACKET_HEADER(p)->qdcount = htobe16(c); /* Known answers */ if (add_known_answers) { - r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, ancount); + usec_t ts; + + assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &ts) >= 0); + + r = dns_cache_export_shared_to_packet(&t->scope->cache, p, ts, be16toh(DNS_PACKET_HEADER(dummy)->ancount)); if (r < 0) return r; } /* Authorities */ - nscount = 0; + c = 0; SET_FOREACH(k, keys) { - r = dns_packet_append_zone(p, t, k, &nscount); + r = dns_packet_append_zone(p, t, k, &c); if (r < 0) return r; } - DNS_PACKET_HEADER(p)->nscount = htobe16(nscount); + DNS_PACKET_HEADER(p)->nscount = htobe16(c); t->sent = TAKE_PTR(p); return 0; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index fc8d7412fb..b52619e287 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1198,8 +1198,11 @@ int manager_send( assert(port > 0); assert(p); + /* For mDNS, it is natural that the packet have truncated flag when we have many known answers. */ + bool truncated = DNS_PACKET_TC(p) && (p->protocol != DNS_PROTOCOL_MDNS || !p->more); + log_debug("Sending %s%s packet with id %" PRIu16 " on interface %i/%s of size %zu.", - DNS_PACKET_TC(p) ? "truncated (!) " : "", + truncated ? "truncated (!) " : "", DNS_PACKET_QR(p) ? "response" : "query", DNS_PACKET_ID(p), ifindex, af_to_name(family), diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index 11e3b14125..b63073af7f 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -254,8 +254,9 @@ static int mdns_scope_process_query(DnsScope *s, DnsPacket *p) { if (r < 0) return log_debug_errno(r, "Failed to extract resource records from incoming packet: %m"); + /* TODO: Support Known-Answers only packets gracefully. */ if (dns_question_size(p->question) <= 0) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Received mDNS query without question, ignoring."); + return 0; unicast_reply = mdns_should_reply_using_unicast(p); if (unicast_reply && !sender_on_local_subnet(s, p)) {