Re: [PATCH] bootp: use the ipv* version given by the dhcp server

[Andrei Borzenkov <arvidjaar@gmail.com>]

01.03.2016 19:34, Josef Bacik пишет:
> On 02/27/2016 01:05 PM, Andrei Borzenkov wrote:
>> 24.02.2016 22:10, Josef Bacik пишет:
>>> We were setting DNS_OPTION_PREVER_IPV* based on which version of the
>>> dhcp
>>> request we go back.  However this would mean that we would mistakenly
>>> fall back
>>> on ipv4 in an ipv6 only system, or vice versa.  For the sake of
>>> simplicity lets
>>> only use the ip version for the dns server we got in our dhcp
>>> response.  Thanks,
>>>
>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>> ---
>>>   grub-core/net/bootp.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
>>> index 345ad7a..cde818c 100644
>>> --- a/grub-core/net/bootp.c
>>> +++ b/grub-core/net/bootp.c
>>> @@ -95,7 +95,7 @@ parse_dhcp_vendor (const char *name, const void
>>> *vend, int limit, int *mask)
>>>           struct grub_net_network_level_address s;
>>>           s.type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV4;
>>>           s.ipv4 = grub_get_unaligned32 (ptr);
>>> -        s.option = DNS_OPTION_PREFER_IPV4;
>>> +        s.option = DNS_OPTION_IPV4;
>>>           grub_net_add_dns_server (&s);
>>>           ptr += 4;
>>>             }
>>> @@ -492,7 +492,7 @@ get_dhcpv6_dns_address (const struct
>>> grub_net_dhcpv6_packet *packet,
>>>         (la + i)->type = GRUB_NET_NETWORK_LEVEL_PROTOCOL_IPV6;
>>>         (la + i)->ipv6[0] = grub_get_unaligned64 (pa);
>>>         (la + i)->ipv6[1] = grub_get_unaligned64 (pa + 8);
>>> -      (la + i)->option = DNS_OPTION_PREFER_IPV6;
>>> +      (la + i)->option = DNS_OPTION_IPV6;
>>>       }
>>>
>>>     *addr = la;
>>>
>>
>> that's pretty radical behavior change. It may break existing
>> configuration. As the only justification is side stepping issues in
>> current DNS implementation I think we should rather fix implementation.
>>
> 
> No, the justification is that if you have ipv6 only systems
> "PREFER_IPV6" isn't the appropriate behavior.  The fact is you are only
> going to get ipv6 dns servers from dhcpv6 responses and same for ipv4,
> you don't want to "prefer", you want to bind it to the response you get
> to avoid trying an incompatible ip version that you can't talk to.
> 

How do you know it is ipv6 only system? People are quite creative in
grub.cfg and the fact that firmware boots over IPv6 does not mean
grub.cfg cannot call net_bootp.

>> And even if it does not break things knowing that host has "other" IP
>> and additional interface may be needed is IMHO more useful than getting
>> confusing "host does not exist".
>>
> 
> The end result is the same, you can't talk to it.  This is the same
> behavior you get when you try to ping ipv6.google.com from a non-ipv6
> system, it'll say it can't find the host, which is true,

bor@bor-Latitude-E5450:~/obs/home:arvidjaar:grub2-next/grub2$ ping6
ipv6.google.com
connect: Network is unreachable

ping is IPv4 only program so it asks for A only.

> there is no way
> we can talk to it so it can't see it.  This isn't going to break
> existing setups at all,

The question is not, whether it does not break something, but what does
it fix? The bug is that we get IPv4 address when we asked for IPv6 if
available. *That* is real bug and *that* should be fixed. This patch
just hides it, sorry.

> it just removes unexpected behavior in mixed and
> ipv6 only environments.

Well ... filtering DNS replies based on which interfaces are configured
looks sensible; but that is topic for post-2.02.