Merge pull request #6214 from keszybz/resolved-packet-size

Resolved packet size

6 files changed, 81 insertions, 10 deletions

diff --git a/.gitignore b/.gitignore
index 60eda2b8ce..bc47db6481 100644
--- a/.gitignore
+++ b/.gitignore
@@ -271,6 +271,7 @@
 /test-replace-var
 /test-resolve
 /test-resolve-tables
+/test-resolved-packet
 /test-ring
 /test-rlimit-util
 /test-sched-prio
diff --git a/Makefile.am b/Makefile.am
index 9fc2faf211..d42a70a4b8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5759,6 +5759,7 @@ dist_zshcompletion_data += \
 tests += \
 	test-dns-packet \
 	test-resolve-tables \
+	test-resolved-packet \
 	test-dnssec
 
 manual_tests += \
@@ -5780,6 +5781,19 @@ test_resolve_tables_LDADD = \
 	$(GCRYPT_LIBS) \
 	-lm
 
+test_resolved_packet_SOURCES = \
+	src/resolve/test-resolved-packet.c \
+	$(basic_dns_sources)
+
+test_resolved_packet_CFLAGS = \
+	$(AM_CFLAGS) \
+	$(GCRYPT_CFLAGS)
+
+test_resolved_packet_LDADD = \
+	libsystemd-shared.la \
+	$(GCRYPT_LIBS) \
+	-lm
+
 test_dns_packet_SOURCES = \
 	src/resolve/test-dns-packet.c \
 	$(basic_dns_sources)
diff --git a/src/resolve/meson.build b/src/resolve/meson.build
index f3c411ffee..fe228784fa 100644
--- a/src/resolve/meson.build
+++ b/src/resolve/meson.build
@@ -160,6 +160,15 @@ tests += [
           libm],
          'ENABLE_RESOLVED'],
 
+        [['src/resolve/test-resolved-packet.c',
+          basic_dns_sources,
+          dns_type_headers],
+         [],
+         [libgcrypt,
+          libgpg_error,
+          libm],
+         'ENABLE_RESOLVED'],
+
         [['src/resolve/test-dnssec.c',
           basic_dns_sources,
           dns_type_headers],
diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index 240ee448f4..a486216d68 100644
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -28,6 +28,9 @@
 
 #define EDNS0_OPT_DO (1<<15)
 
+#define DNS_PACKET_SIZE_START 512u
+assert_cc(DNS_PACKET_SIZE_START > UDP_PACKET_HEADER_SIZE)
+
 typedef struct DnsPacketRewinder {
         DnsPacket *packet;
         size_t saved_rindex;
@@ -47,13 +50,14 @@ int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t mtu) {
 
         assert(ret);
 
-        if (mtu <= UDP_PACKET_HEADER_SIZE)
+        /* When dns_packet_new() is called with mtu == 0, allocate more than the
+         * absolute minimum (which is the dns packet header size), to avoid
+         * resizing immediately again after appending the first data to the packet.
+         */
+        if (mtu < UDP_PACKET_HEADER_SIZE)
                 a = DNS_PACKET_SIZE_START;
         else
-                a = mtu - UDP_PACKET_HEADER_SIZE;
-
-        if (a < DNS_PACKET_HEADER_SIZE)
-                a = DNS_PACKET_HEADER_SIZE;
+                a = MAX(mtu, DNS_PACKET_HEADER_SIZE);
 
         /* round up to next page size */
         a = PAGE_ALIGN(ALIGN(sizeof(DnsPacket)) + a) - ALIGN(sizeof(DnsPacket));
diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h
index 2c92392e4d..5dff272fd9 100644
--- a/src/resolve/resolved-dns-packet.h
+++ b/src/resolve/resolved-dns-packet.h
@@ -58,15 +58,13 @@ struct DnsPacketHeader {
 /* The various DNS protocols deviate in how large a packet can grow,
    but the TCP transport has a 16bit size field, hence that appears to
    be the absolute maximum. */
-#define DNS_PACKET_SIZE_MAX 0xFFFF
+#define DNS_PACKET_SIZE_MAX 0xFFFFu
 
 /* RFC 1035 say 512 is the maximum, for classic unicast DNS */
-#define DNS_PACKET_UNICAST_SIZE_MAX 512
+#define DNS_PACKET_UNICAST_SIZE_MAX 512u
 
 /* With EDNS0 we can use larger packets, default to 4096, which is what is commonly used */
-#define DNS_PACKET_UNICAST_SIZE_LARGE_MAX 4096
-
-#define DNS_PACKET_SIZE_START 512
+#define DNS_PACKET_UNICAST_SIZE_LARGE_MAX 4096u
 
 struct DnsPacket {
         int n_ref;
diff --git a/src/resolve/test-resolved-packet.c b/src/resolve/test-resolved-packet.c
new file mode 100644
index 0000000000..8b7da1408d
--- /dev/null
+++ b/src/resolve/test-resolved-packet.c
@@ -0,0 +1,45 @@
+/***
+  This file is part of systemd
+
+  Copyright 2017 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "log.h"
+#include "resolved-dns-packet.h"
+
+static void test_dns_packet_new(void) {
+        size_t i;
+
+        for (i = 0; i < DNS_PACKET_SIZE_MAX + 2; i++) {
+                _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
+
+                assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, i) == 0);
+
+                log_debug("dns_packet_new: %zu → %zu", i, p->allocated);
+                assert_se(p->allocated >= MIN(DNS_PACKET_SIZE_MAX, i));
+        }
+}
+
+int main(int argc, char **argv) {
+
+        log_set_max_level(LOG_DEBUG);
+        log_parse_environment();
+        log_open();
+
+        test_dns_packet_new();
+
+        return 0;
+}