grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Issue separate DNS queries for ipv4 and ipv6
@ 2012-10-17  0:55 Gustavo Luiz Duarte
  2012-10-17  1:06 ` Richard Laager
  2013-11-02 16:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 2 replies; 6+ messages in thread
From: Gustavo Luiz Duarte @ 2012-10-17  0:55 UTC (permalink / raw)
  To: grub-devel


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);
+  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;
+  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,
-- 
1.7.11.4



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6
  2012-10-17  0:55 [PATCH] Issue separate DNS queries for ipv4 and ipv6 Gustavo Luiz Duarte
@ 2012-10-17  1:06 ` Richard Laager
  2012-10-18 21:21   ` Gustavo Luiz Duarte
  2013-11-02 16:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Laager @ 2012-10-17  1:06 UTC (permalink / raw)
  To: Gustavo Luiz Duarte; +Cc: grub-devel

[-- Attachment #1: Type: text/plain, Size: 170 bytes --]

On Tue, 2012-10-16 at 21:55 -0300, Gustavo Luiz Duarte wrote:
> The default is DNS_OPTION_PREFER_IPV4.

Why prefer IPv4 when most things prefer IPv6?

-- 
Richard

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6
  2012-10-17  1:06 ` Richard Laager
@ 2012-10-18 21:21   ` Gustavo Luiz Duarte
  2012-10-18 21:50     ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 1 reply; 6+ messages in thread
From: Gustavo Luiz Duarte @ 2012-10-18 21:21 UTC (permalink / raw)
  To: grub-devel

Because IPv4 is much more prevalent at present, what means less 
unnecessary traffic and a slightly better performance on most of the 
cases. We can always change the default in the future (when IPv6 is more 
prevalent) as such "preference" is just an optimization.
Also, I should soon have a patch to allow the user to configure this 
lookup behavior if he/she wants it.



On 10/16/2012 10:06 PM, Richard Laager wrote:
> On Tue, 2012-10-16 at 21:55 -0300, Gustavo Luiz Duarte wrote:
>> The default is DNS_OPTION_PREFER_IPV4.
>
> Why prefer IPv4 when most things prefer IPv6?
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6
  2012-10-18 21:21   ` Gustavo Luiz Duarte
@ 2012-10-18 21:50     ` Vladimir 'φ-coder/phcoder' Serbinenko
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2012-10-18 21:50 UTC (permalink / raw)
  To: The development of GNU GRUB

[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]

On 18.10.2012 23:21, Gustavo Luiz Duarte wrote:

> Because IPv4 is much more prevalent at present, what means less
> unnecessary traffic and a slightly better performance on most of the
> cases.

Also way too often IPv6 is misconfigured and detecting this condition
may be slow if the packets simply get dropped.

> We can always change the default in the future (when IPv6 is more
> prevalent) as such "preference" is just an optimization.
> Also, I should soon have a patch to allow the user to configure this
> lookup behavior if he/she wants it.
> 
> 
> 
> On 10/16/2012 10:06 PM, Richard Laager wrote:
>> On Tue, 2012-10-16 at 21:55 -0300, Gustavo Luiz Duarte wrote:
>>> The default is DNS_OPTION_PREFER_IPV4.
>>
>> Why prefer IPv4 when most things prefer IPv6?
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6
  2012-10-17  0:55 [PATCH] Issue separate DNS queries for ipv4 and ipv6 Gustavo Luiz Duarte
  2012-10-17  1:06 ` Richard Laager
@ 2013-11-02 16:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
  2013-11-05 19:07   ` Gustavo Luiz Duarte
  1 sibling, 1 reply; 6+ messages in thread
From: Vladimir 'φ-coder/phcoder' Serbinenko @ 2013-11-02 16:34 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Gustavo Luiz Duarte

[-- Attachment #1: Type: text/plain, Size: 8266 bytes --]

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,
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6
  2013-11-02 16:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
@ 2013-11-05 19:07   ` Gustavo Luiz Duarte
  0 siblings, 0 replies; 6+ messages in thread
From: Gustavo Luiz Duarte @ 2013-11-05 19:07 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: phcoder, Paulo Flabiano Smorigo

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
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-11-05 19:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-17  0:55 [PATCH] Issue separate DNS queries for ipv4 and ipv6 Gustavo Luiz Duarte
2012-10-17  1:06 ` Richard Laager
2012-10-18 21:21   ` Gustavo Luiz Duarte
2012-10-18 21:50     ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-02 16:34 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-05 19:07   ` Gustavo Luiz Duarte

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).