resolve/mdns: split out mdns_make_dummy_packet()
Then, this fixes the following issues: - if dns_packet_append_zone() for other transaction is failed with EMSGSIZE, the previously added key was not removed, - if dns_transaction_prepare() for other transaction returns 0, then we restated the loop without dropping previously appended keys, which might not be necessary any more.
This commit is contained in:
Yu Watanabe
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user