From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1VdlzP-0004ck-99 for mharc-grub-devel@gnu.org; Tue, 05 Nov 2013 14:08:35 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59222) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdlzG-0004bR-4X for grub-devel@gnu.org; Tue, 05 Nov 2013 14:08:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vdlz8-0008Hm-GR for grub-devel@gnu.org; Tue, 05 Nov 2013 14:08:26 -0500 Received: from e24smtp05.br.ibm.com ([32.104.18.26]:60061) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vdlz8-0008A9-3s for grub-devel@gnu.org; Tue, 05 Nov 2013 14:08:18 -0500 Received: from /spool/local by e24smtp05.br.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 5 Nov 2013 17:07:50 -0200 Received: from d24dlp02.br.ibm.com (9.18.248.206) by e24smtp05.br.ibm.com (10.172.0.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 5 Nov 2013 17:07:48 -0200 Received: from d24relay03.br.ibm.com (d24relay03.br.ibm.com [9.13.184.25]) by d24dlp02.br.ibm.com (Postfix) with ESMTP id 4A1C91DC0072 for ; Tue, 5 Nov 2013 14:07:47 -0500 (EST) Received: from d24av01.br.ibm.com (d24av01.br.ibm.com [9.8.31.91]) by d24relay03.br.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id rA5J7fKk31654062 for ; Tue, 5 Nov 2013 17:07:41 -0200 Received: from d24av01.br.ibm.com (localhost [127.0.0.1]) by d24av01.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id rA5J7kU5009470 for ; Tue, 5 Nov 2013 17:07:46 -0200 Received: from [9.8.9.187] ([9.8.9.187]) by d24av01.br.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id rA5J7hbu009300; Tue, 5 Nov 2013 17:07:43 -0200 Message-ID: <527941FE.4090508@linux.vnet.ibm.com> Date: Tue, 05 Nov 2013 17:07:42 -0200 From: Gustavo Luiz Duarte User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6 References: <507E01FB.4050303@linux.vnet.ibm.com> <5275298D.1050604@gmail.com> In-Reply-To: <5275298D.1050604@gmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-TM-AS-MML: No X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13110519-2362-0000-0000-00000C422B13 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 32.104.18.26 Cc: phcoder@gmail.com, Paulo Flabiano Smorigo X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Nov 2013 19:08:33 -0000 Vladimir, Thanks for your reply. Unfortunately, I wrote this patch more than a year ago and I've not been working much on grub2 code since then. I know Paulo Smorigo (CC) is working to address your concerns and he is much more familiar with grub2 code than I am. Paulo, thanks for your help. []'s Gustavo On 11/02/2013 02:34 PM, Vladimir 'φ-coder/phcoder' Serbinenko wrote: > On 17.10.2012 02:55, Gustavo Luiz Duarte wrote: >> >> Adding multiple questions on a single DNS query is not supportted by >> most DNS servers. This patch issues two separate DNS queries >> sequentially for ipv4 and then for ipv6. >> >> There are 4 possible config options: >> DNS_OPTION_IPV4: issue only one ipv4 query >> DNS_OPTION_IPV6: issue only one ipv6 query >> DNS_OPTION_PREFER_IPV4: issue the ipv4 query first and fallback to ipv6 >> DNS_OPTION_PREFER_IPV6: issue the ipv6 query first and fallback to ipv4 >> However, there is no code yet to set such config option. The default is >> DNS_OPTION_PREFER_IPV4. >> >> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=860829 >> --- >> grub-core/net/dns.c | 99 ++++++++++++++++++++++++++++++++++++----------------- >> include/grub/net.h | 9 +++++ >> 2 files changed, 76 insertions(+), 32 deletions(-) >> >> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c >> index 3381ea7..725725c 100644 >> --- a/grub-core/net/dns.c >> +++ b/grub-core/net/dns.c >> @@ -34,6 +34,14 @@ struct dns_cache_element >> #define DNS_CACHE_SIZE 1021 >> #define DNS_HASH_BASE 423 >> >> +typedef enum grub_dns_qtype_id >> + { >> + GRUB_DNS_QTYPE_A = 1, >> + GRUB_DNS_QTYPE_AAAA = 28 >> + } grub_dns_qtype_id_t; >> + >> +static grub_dns_option_t dns_type_option = DNS_OPTION_PREFER_IPV4; >> + >> static struct dns_cache_element dns_cache[DNS_CACHE_SIZE]; >> static struct grub_net_network_level_address *dns_servers; >> static grub_size_t dns_nservers, dns_servers_alloc; >> @@ -410,13 +418,13 @@ recv_hook (grub_net_udp_socket_t sock __attribute__ ((unused)), >> return GRUB_ERR_NONE; >> } >> >> -grub_err_t >> -grub_net_dns_lookup (const char *name, >> +static grub_err_t >> +grub_net_dns_lookup_qtype (const char *name, >> const struct grub_net_network_level_address *servers, >> grub_size_t n_servers, >> grub_size_t *naddresses, >> struct grub_net_network_level_address **addresses, >> - int cache) >> + int cache, grub_dns_qtype_id_t qtype) >> { >> grub_size_t send_servers = 0; >> grub_size_t i, j; >> @@ -471,8 +479,7 @@ grub_net_dns_lookup (const char *name, >> + GRUB_NET_MAX_LINK_HEADER_SIZE >> + GRUB_NET_UDP_HEADER_SIZE >> + sizeof (struct dns_header) >> - + grub_strlen (name) + 2 + 4 >> - + 2 + 4); >> + + grub_strlen (name) + 2 + 4); >> if (!nb) >> { >> grub_free (data.name); >> @@ -482,7 +489,7 @@ grub_net_dns_lookup (const char *name, >> + GRUB_NET_MAX_LINK_HEADER_SIZE >> + GRUB_NET_UDP_HEADER_SIZE); >> grub_netbuff_put (nb, sizeof (struct dns_header) >> - + grub_strlen (name) + 2 + 4 + 2 + 4); >> + + grub_strlen (name) + 2 + 4); >> head = (struct dns_header *) nb->data; >> optr = (grub_uint8_t *) (head + 1); >> for (iptr = name; *iptr; ) >> @@ -509,18 +516,7 @@ grub_net_dns_lookup (const char *name, >> >> /* Type: A. */ >> *optr++ = 0; >> - *optr++ = 1; >> - >> - /* Class. */ >> - *optr++ = 0; >> - *optr++ = 1; >> - >> - /* Compressed name. */ >> - *optr++ = 0xc0; >> - *optr++ = 0x0c; >> - /* Type: AAAA. */ >> - *optr++ = 0; >> - *optr++ = 28; >> + *optr++ = qtype; >> >> /* Class. */ >> *optr++ = 0; >> @@ -529,7 +525,7 @@ grub_net_dns_lookup (const char *name, >> head->id = data.id; >> head->flags = FLAGS_RD; >> head->ra_z_r_code = 0; >> - head->qdcount = grub_cpu_to_be16_compile_time (2); >> + head->qdcount = grub_cpu_to_be16_compile_time (1); >> head->ancount = grub_cpu_to_be16_compile_time (0); >> head->nscount = grub_cpu_to_be16_compile_time (0); >> head->arcount = grub_cpu_to_be16_compile_time (0); >> @@ -587,16 +583,47 @@ grub_net_dns_lookup (const char *name, >> if (*data.naddresses) >> return GRUB_ERR_NONE; >> if (data.dns_err) >> - return grub_error (GRUB_ERR_NET_NO_DOMAIN, >> - N_("no DNS record found")); >> - >> + { >> + grub_dprintf ("dns", "%s. QTYPE: %u QNAME: %s\n", >> + N_("no DNS record found"), qtype, name); >> + return GRUB_ERR_NET_NO_DOMAIN; >> + } >> if (err) >> { >> grub_errno = err; >> return err; >> } >> - return grub_error (GRUB_ERR_TIMEOUT, >> - N_("no DNS reply received")); >> + grub_dprintf ("dns", "%s. QTYPE: %u QNAME: %s\n", >> + N_("no DNS reply received"), qtype, name); >> + return GRUB_ERR_TIMEOUT; >> +} >> + >> +grub_err_t >> +grub_net_dns_lookup (const char *name, >> + const struct grub_net_network_level_address *servers, >> + grub_size_t n_servers, >> + grub_size_t *naddresses, >> + struct grub_net_network_level_address **addresses, >> + int cache) >> +{ >> + if (dns_type_option == DNS_OPTION_IPV6 || dns_type_option == DNS_OPTION_PREFER_IPV6) >> + grub_net_dns_lookup_qtype (name, servers, n_servers, naddresses, >> + addresses, cache, GRUB_DNS_QTYPE_AAAA); >> + else >> + grub_net_dns_lookup_qtype (name, servers, n_servers, naddresses, >> + addresses, cache, GRUB_DNS_QTYPE_A); > Here you don't handle error conditions in called functions. >> + if (!*naddresses) >> + { >> + if (dns_type_option == DNS_OPTION_PREFER_IPV4) >> + grub_net_dns_lookup_qtype (name, servers, n_servers, naddresses, >> + addresses, cache, GRUB_DNS_QTYPE_AAAA); >> + else if (dns_type_option == DNS_OPTION_PREFER_IPV6) >> + grub_net_dns_lookup_qtype (name, servers, n_servers, naddresses, >> + addresses, cache, GRUB_DNS_QTYPE_A); >> + } >> + if (!*naddresses) >> + return GRUB_ERR_NET_NO_DOMAIN; > In this and many other places in this patch you return error value > without using grub_error. >> + return GRUB_ERR_NONE; >> } >> >> static grub_err_t >> @@ -604,22 +631,28 @@ grub_cmd_nslookup (struct grub_command *cmd __attribute__ ((unused)), >> int argc, char **args) >> { >> grub_err_t err; >> - grub_size_t naddresses, i; >> + struct grub_net_network_level_address cmd_server; >> + struct grub_net_network_level_address *servers; >> + grub_size_t nservers, i, naddresses = 0; >> struct grub_net_network_level_address *addresses = 0; >> if (argc != 2 && argc != 1) >> return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected")); >> if (argc == 2) >> { >> - struct grub_net_network_level_address server; >> - err = grub_net_resolve_address (args[1], &server); >> + err = grub_net_resolve_address (args[1], &cmd_server); >> if (err) >> return err; >> - err = grub_net_dns_lookup (args[0], &server, 1, &naddresses, >> - &addresses, 0); >> + servers = &cmd_server; >> + nservers = 1; >> } >> else >> - err = grub_net_dns_lookup (args[0], dns_servers, dns_nservers, &naddresses, >> - &addresses, 0); >> + { >> + servers = dns_servers; >> + nservers = dns_nservers; >> + } >> + >> + grub_net_dns_lookup (args[0], servers, nservers, &naddresses, >> + &addresses, 0); >> >> for (i = 0; i < naddresses; i++) >> { >> @@ -628,7 +661,9 @@ grub_cmd_nslookup (struct grub_command *cmd __attribute__ ((unused)), >> grub_printf ("%s\n", buf); >> } >> grub_free (addresses); >> - return GRUB_ERR_NONE; >> + if (naddresses) >> + return GRUB_ERR_NONE; >> + return grub_error (GRUB_ERR_NET_NO_DOMAIN, N_("no DNS record found")); >> } >> >> static grub_err_t >> diff --git a/include/grub/net.h b/include/grub/net.h >> index 3877451..a7e5b2c 100644 >> --- a/include/grub/net.h >> +++ b/include/grub/net.h >> @@ -505,6 +505,15 @@ grub_err_t >> grub_net_link_layer_resolve (struct grub_net_network_level_interface *inf, >> const grub_net_network_level_address_t *proto_addr, >> grub_net_link_level_address_t *hw_addr); >> + >> +typedef enum >> + { >> + DNS_OPTION_IPV4, >> + DNS_OPTION_IPV6, >> + DNS_OPTION_PREFER_IPV4, >> + DNS_OPTION_PREFER_IPV6 >> + } grub_dns_option_t; >> + >> grub_err_t >> grub_net_dns_lookup (const char *name, >> const struct grub_net_network_level_address *servers, >> > > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >