Re: [PATCH] Issue separate DNS queries for ipv4 and ipv6

[Gustavo Luiz Duarte <gustavold@linux.vnet.ibm.com>]

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
>