From patchwork Thu Jun 29 18:35:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 782408 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wz7d41bqlz9s72 for ; Fri, 30 Jun 2017 04:35:20 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="OQH4WR3s"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:from:subject:message-id:date :mime-version:content-type; q=dns; s=default; b=d0J6e4Gy7VY2Ekly 2ygy70s5EMvIupOQi6wVwqezI14cFu9OzkQbpsBNqZW4wHcEuOYUSp/fDxo5PZaX 2ChlEhQpn0IcDzyd4euWPgSlsv5Y+HzGFJNx+0cuN1/meXijCewqiH2Kj9Tq5sI8 vBU6qpu4ZGuZYHcz529uIvnsTec= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:to:cc:from:subject:message-id:date :mime-version:content-type; s=default; bh=KTMAtUHiIgKptgmJwLahYo MoJRw=; b=OQH4WR3srlpTsE1MxUWY6Ij7RMDdB2nwoiont0ake2bpAxMrI5wgJF 6KCsgy+r6tcunNFA6WKBK1XuMbKGvZ6jIyXEnUGV5wkwW7LgiXCRpEx11JA6ACXz 4NE+/pewcvnXFWrXhWd3reepZnhS+gyM97OzpSD9uDamBAJfJ6xRk= Received: (qmail 80642 invoked by alias); 29 Jun 2017 18:35:13 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 80604 invoked by uid 89); 29 Jun 2017 18:35:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=sockets, nameserver, Hx-languages-length:5290 X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2607031E7DE Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 2607031E7DE To: GNU C Library Cc: Andreas Schwab , Carlos O'Donell From: Florian Weimer Subject: [PATCH] resolv: Do not copy IPv4 addresses to IPv6 address array Message-ID: <8113b7c9-b9a8-7157-f957-53853dc4e232@redhat.com> Date: Thu, 29 Jun 2017 20:35:00 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 While working on the resolver code, I discovered some address copying which appears to be unnecessary. The attached patch removes that. The comments refer to timing data, but we currently do not have that. Andreas, if I recall correctly, you wrote get_nsaddr. What do you think about this change? Thanks, Florian resolv: Do not copy IPv4 addresses to IPv6 address array get_nsaddr already prefers IPv4 addresses if they are present (af_family != 0). If we ensure that nssocks is initialized to -1 and sockets are closed even without an allocated server address, there is no need to make these copies anymore. 2017-06-29 Florian Weimer * resolv/bits/types/res_state.h (struct __res_state): Rename _u._ext.nscount to _u._ext.__glibc_unused_nscount. * resolv/res_init.c (res_vinit): Adjust. Initialize nssocks member. * resolv/res_send.c (__libc_res_nsend): Do not copy IPv4 nameserver addresses. * resolv/res-close.c (__res_iclose): Close sockets even if the name server address has not been allocated. * support/resolv_test.c (resolv_test_start): Do not clear _res._u._ext.nscount because it is unused. diff --git a/resolv/bits/types/res_state.h b/resolv/bits/types/res_state.h index cee4b6d..d274cf3 100644 --- a/resolv/bits/types/res_state.h +++ b/resolv/bits/types/res_state.h @@ -40,7 +40,7 @@ struct __res_state { union { char pad[52]; /* On an i386 this means 512b total. */ struct { - uint16_t nscount; + uint16_t __glibc_unused_nscount; uint16_t nsmap[MAXNS]; int nssocks[MAXNS]; uint16_t nscount6; diff --git a/resolv/res-close.c b/resolv/res-close.c index 73f18d1..3195298 100644 --- a/resolv/res-close.c +++ b/resolv/res-close.c @@ -97,19 +97,18 @@ __res_iclose (res_state statp, bool free_addr) statp->_flags &= ~(RES_F_VC | RES_F_CONN); } for (int ns = 0; ns < statp->nscount; ns++) - if (statp->_u._ext.nsaddrs[ns] != NULL) - { - if (statp->_u._ext.nssocks[ns] != -1) - { - close_not_cancel_no_status (statp->_u._ext.nssocks[ns]); - statp->_u._ext.nssocks[ns] = -1; - } - if (free_addr) - { - free (statp->_u._ext.nsaddrs[ns]); - statp->_u._ext.nsaddrs[ns] = NULL; - } - } + { + if (statp->_u._ext.nssocks[ns] != -1) + { + close_not_cancel_no_status (statp->_u._ext.nssocks[ns]); + statp->_u._ext.nssocks[ns] = -1; + } + if (free_addr) + { + free (statp->_u._ext.nsaddrs[ns]); + statp->_u._ext.nsaddrs[ns] = NULL; + } + } } libc_hidden_def (__res_iclose) diff --git a/resolv/res_init.c b/resolv/res_init.c index 821f060..cd42ac6 100644 --- a/resolv/res_init.c +++ b/resolv/res_init.c @@ -155,9 +155,12 @@ res_vinit_1 (res_state statp, bool preinit, FILE *fp, char **buffer) statp->_flags = 0; statp->__glibc_unused_qhook = NULL; statp->__glibc_unused_rhook = NULL; - statp->_u._ext.nscount = 0; + statp->_u._ext.__glibc_unused_nscount = 0; for (int n = 0; n < MAXNS; n++) - statp->_u._ext.nsaddrs[n] = NULL; + { + statp->_u._ext.nsaddrs[n] = NULL; + statp->_u._ext.nssocks[n] = -1; + } /* Allow user to override the local domain definition. */ if ((cp = getenv ("LOCALDOMAIN")) != NULL) @@ -328,7 +331,6 @@ res_vinit_1 (res_state statp, bool preinit, FILE *fp, char **buffer) statp->nsaddr_list[nserv].sin_family = 0; statp->_u._ext.nsaddrs[nserv] = sa6; - statp->_u._ext.nssocks[nserv] = -1; have_serv6 = true; nserv++; } diff --git a/resolv/res_send.c b/resolv/res_send.c index b7b8ecd..ef91d0f 100644 --- a/resolv/res_send.c +++ b/resolv/res_send.c @@ -373,54 +373,6 @@ __libc_res_nsend(res_state statp, const u_char *buf, int buflen, terrno = ETIMEDOUT; /* - * If the ns_addr_list in the resolver context has changed, then - * invalidate our cached copy and the associated timing data. - */ - if (EXT(statp).nscount != 0) { - int needclose = 0; - - if (EXT(statp).nscount != statp->nscount) - needclose++; - else - for (ns = 0; ns < statp->nscount; ns++) { - if (statp->nsaddr_list[ns].sin_family != 0 - && !sock_eq((struct sockaddr_in6 *) - &statp->nsaddr_list[ns], - EXT(statp).nsaddrs[ns])) - { - needclose++; - break; - } - } - if (needclose) { - __res_iclose(statp, false); - EXT(statp).nscount = 0; - } - } - - /* - * Maybe initialize our private copy of the ns_addr_list. - */ - if (EXT(statp).nscount == 0) { - for (ns = 0; ns < statp->nscount; ns++) { - EXT(statp).nssocks[ns] = -1; - if (statp->nsaddr_list[ns].sin_family == 0) - continue; - if (EXT(statp).nsaddrs[ns] == NULL) - EXT(statp).nsaddrs[ns] = - malloc(sizeof (struct sockaddr_in6)); - if (EXT(statp).nsaddrs[ns] != NULL) - memset (mempcpy(EXT(statp).nsaddrs[ns], - &statp->nsaddr_list[ns], - sizeof (struct sockaddr_in)), - '\0', - sizeof (struct sockaddr_in6) - - sizeof (struct sockaddr_in)); - } - EXT(statp).nscount = statp->nscount; - } - - /* * Some resolvers want to even out the load on their nameservers. * Note that RES_BLAST overrides RES_ROTATE. */ diff --git a/support/resolv_test.c b/support/resolv_test.c index 050cd71..b5df5ff 100644 --- a/support/resolv_test.c +++ b/support/resolv_test.c @@ -1103,7 +1103,6 @@ resolv_test_start (struct resolv_redirect_config config) /* Disable IPv6 name server addresses. The code below only overrides the IPv4 addresses. */ __res_iclose (&_res, true); - _res._u._ext.nscount = 0; /* Redirect queries to the server socket. */ if (test_verbose)