* [PATCH] bootp: use the ipv* version given by the dhcp server @ 2016-02-24 19:10 Josef Bacik 2016-02-27 18:05 ` Andrei Borzenkov 0 siblings, 1 reply; 4+ messages in thread From: Josef Bacik @ 2016-02-24 19:10 UTC (permalink / raw) To: kernel-team, grub-devel 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; -- 2.5.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] bootp: use the ipv* version given by the dhcp server 2016-02-24 19:10 [PATCH] bootp: use the ipv* version given by the dhcp server Josef Bacik @ 2016-02-27 18:05 ` Andrei Borzenkov 2016-03-01 16:34 ` Josef Bacik 0 siblings, 1 reply; 4+ messages in thread From: Andrei Borzenkov @ 2016-02-27 18:05 UTC (permalink / raw) To: The development of GNU GRUB, kernel-team 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. 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". ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bootp: use the ipv* version given by the dhcp server 2016-02-27 18:05 ` Andrei Borzenkov @ 2016-03-01 16:34 ` Josef Bacik 2016-03-01 19:35 ` Andrei Borzenkov 0 siblings, 1 reply; 4+ messages in thread From: Josef Bacik @ 2016-03-01 16:34 UTC (permalink / raw) To: Andrei Borzenkov, The development of GNU GRUB, kernel-team 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. > 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, 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, it just removes unexpected behavior in mixed and ipv6 only environments. Thanks, Josef ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] bootp: use the ipv* version given by the dhcp server 2016-03-01 16:34 ` Josef Bacik @ 2016-03-01 19:35 ` Andrei Borzenkov 0 siblings, 0 replies; 4+ messages in thread From: Andrei Borzenkov @ 2016-03-01 19:35 UTC (permalink / raw) To: Josef Bacik, The development of GNU GRUB, kernel-team 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-01 19:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-24 19:10 [PATCH] bootp: use the ipv* version given by the dhcp server Josef Bacik 2016-02-27 18:05 ` Andrei Borzenkov 2016-03-01 16:34 ` Josef Bacik 2016-03-01 19:35 ` Andrei Borzenkov
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).