diff options
author | Thomas Deutschmann <whissi@gentoo.org> | 2020-04-07 20:09:06 +0200 |
---|---|---|
committer | Thomas Deutschmann <whissi@gentoo.org> | 2020-04-07 20:09:24 +0200 |
commit | 865c1a561c7eaabd160cd1d7b0aef09bfbcdb327 (patch) | |
tree | eaeb6e3cbfdfe2831e34512539bd99a12d68effb | |
parent | net-misc/iputils: fix arping (diff) | |
download | gentoo-865c1a561c7eaabd160cd1d7b0aef09bfbcdb327.tar.gz gentoo-865c1a561c7eaabd160cd1d7b0aef09bfbcdb327.tar.bz2 gentoo-865c1a561c7eaabd160cd1d7b0aef09bfbcdb327.zip |
net-misc/iputils: add some more patches for ping
- ping: try next addrinfo on connect failure
- ping: fix main loop over multiple addrinfo results
Package-Manager: Portage-2.3.96, Repoman-2.3.22
Signed-off-by: Thomas Deutschmann <whissi@gentoo.org>
3 files changed, 278 insertions, 0 deletions
diff --git a/net-misc/iputils/files/iputils-20190709-ping-fix-main-loop-over-multiple-addrinfo-results.patch b/net-misc/iputils/files/iputils-20190709-ping-fix-main-loop-over-multiple-addrinfo-results.patch new file mode 100644 index 000000000000..9814707d7239 --- /dev/null +++ b/net-misc/iputils/files/iputils-20190709-ping-fix-main-loop-over-multiple-addrinfo-results.patch @@ -0,0 +1,86 @@ +From: Benjamin Poirier <benjamin.poirier@gmail.com> +Date: Thu, 26 Dec 2019 10:44:03 +0900 +Subject: ping: fix main loop over multiple addrinfo results + +Despite what the log of commit f68eec0eafad ("ping: perform dual-stack ping +by default") says, main() was not designed to loop over multiple addresses +returned by getaddrinfo(). This is apparent because until commit +db11bc96a68c ("ping: make command to return from main()"), ping{4,6}_run() +never returned (they always exited). After commit db11bc96a68c, we +encounter unexpected situations if getaddrinfo returns multiple results and +ping{4,6}_run() return != 0. + +For example (notice echo reply is not received): + + root@vsid:/src/iputils# ./builddir/ping/ping -w1 google.com + PING google.com(nrt12s22-in-x0e.1e100.net (2404:6800:4004:80c::200e)) 56 data bytes + + --- google.com ping statistics --- + 1 packets transmitted, 0 received, 100% packet loss, time 0ms + + PING (216.58.197.142) 56(84) bytes of data. + + --- ping statistics --- + 1 packets transmitted, 0 received, 100% packet loss, time -1002ms + + root@vsid:/src/iputils# + +Establish the following convention: + +* return value >= 0 -> exit with this code (same behavior as before commit + db11bc96a68c) + +* return value < 0 -> go on to next addrinfo result + +The second case will be used in the following patch. + +Fixes: db11bc96a68c ("ping: make command to return from main()") +Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com> +Origin: https://github.com/iputils/iputils/commit/c249e48bb865e731896b7d8ceaf4bca7d28768b6 +Bug-Debian: https://bugs.debian.org/947921 +See-also: https://github.com/iputils/iputils/pull/244 +--- + ping.c | 6 +++++- + ping6_common.c | 1 + + 2 files changed, 6 insertions(+), 1 deletion(-) + +diff --git a/ping.c b/ping.c +index 34653be..ae687b4 100644 +--- a/ping.c ++++ b/ping.c +@@ -495,8 +495,11 @@ main(int argc, char **argv) + error(2, 0, _("unknown protocol family: %d"), ai->ai_family); + } + +- if (ret_val == 0) ++ if (ret_val >= 0) + break; ++ /* ret_val < 0 means to go on to next addrinfo result, there ++ * better be one. */ ++ assert(ai->ai_next); + } + + freeaddrinfo(result); +@@ -504,6 +507,7 @@ main(int argc, char **argv) + return ret_val; + } + ++/* return >= 0: exit with this code, < 0: go on to next addrinfo result */ + int ping4_run(int argc, char **argv, struct addrinfo *ai, socket_st *sock) + { + static const struct addrinfo hints = { +diff --git a/ping6_common.c b/ping6_common.c +index 6cc5404..731dc6d 100644 +--- a/ping6_common.c ++++ b/ping6_common.c +@@ -551,6 +551,7 @@ int niquery_option_handler(const char *opt_arg) + return ret; + } + ++/* return >= 0: exit with this code, < 0: go on to next addrinfo result */ + int ping6_run(int argc, char **argv, struct addrinfo *ai, struct socket_st *sock) + { + static const struct addrinfo hints = { +-- +2.25.0.rc2 + diff --git a/net-misc/iputils/files/iputils-20190709-ping-try-next-addrinfo-on-connect-failure.patch b/net-misc/iputils/files/iputils-20190709-ping-try-next-addrinfo-on-connect-failure.patch new file mode 100644 index 000000000000..a308fb7c9f63 --- /dev/null +++ b/net-misc/iputils/files/iputils-20190709-ping-try-next-addrinfo-on-connect-failure.patch @@ -0,0 +1,190 @@ +From: Benjamin Poirier <benjamin.poirier@gmail.com> +Date: Wed, 25 Dec 2019 13:33:12 +0900 +Subject: ping: try next addrinfo on connect failure + +On hosts that have routing rules matching on the outgoing interface [1], +getaddrinfo() may return results sorted in a suboptimal order because it is +not aware of the network interface passed to ping via the "-I" option. In +particular, address reachability detection may fail and getaddrinfo() will +return ipv6 results first, even though the only routes available are ipv4. + +Improve user experience by trying next addrinfo entry if we encounter a +failure at connect() time because of missing or unreachable routes. + +[1] For example, on switches running Cumulus Linux, the default VRF is used +for front ports and a "mgmt" VRF is used for the management interface, which +also handles all DNS traffic. (VRFs apply different routing rules based on +the iif/oif, ie. influenced by SO_BINDTODEVICE.) In the default vrf, it's +possible to ping an ipv4 address via the mgmt vrf by specifying "-I mgmt". +However, that will fail if the target host is specified by name, has a AAAA +record and there is no ipv6 route to it. + +Since libc commit 5ddb5bf5fb, getaddrinfo() does a udp connect to result +addresses to check if there is a route to them. This is to implement +RFC3484 §6 Rule 1 ("Avoid unusable destinations") which is part of the +algorithm to order results. getaddrinfo() is unaware of ping's "-I" option +and tries to connect its socket via the default vrf, which has no ipv6 route +to the target host (and, in fact, no ipv4 route either). Following this +failure, getaddrinfo() returns results ordered according to +/etc/gai.conf (Rule 6) - by default, ipv6 first. + +ping tries only the first entry returned by getaddrinfo() and fails to +connect to it because there is no ipv6 route to the host, even in the mgmt +vrf. However, if getaddrinfo() had ordered the ipv4 result first or ping +had tried the next addrinfo entry (the ipv4 one), ping could connect a udp +socket to it and later successfully exchange icmp messages with it. + +Example: + + cumulus@act-5812-10:~$ ip vrf list + Name Table + ----------------------- + mgmt 1001 + cumulus@act-5812-10:~$ ip vrf identify + cumulus@act-5812-10:~$ # --> default vrf + cumulus@act-5812-10:~$ + cumulus@act-5812-10:~$ ip rule + 99: from all to 10.230.0.53 ipproto udp dport 53 lookup mgmt + 99: from all to 10.20.249.1 ipproto udp dport 53 lookup mgmt + 1000: from all lookup [l3mdev-table] + 32765: from all lookup local + 32766: from all lookup main + 32767: from all lookup default + + cumulus@act-5812-10:~$ ip route + + cumulus@act-5812-10:~$ ip -6 route + ::1 dev lo proto kernel metric 256 pref medium + + cumulus@act-5812-10:~$ ip route show vrf mgmt + default via 10.230.130.1 dev eth0 + unreachable default metric 4278198272 + 10.230.130.0/24 dev eth0 proto kernel scope link src 10.230.130.211 + 127.0.0.0/8 dev mgmt proto kernel scope link src 127.0.0.1 + + cumulus@act-5812-10:~$ ip -6 route show vrf mgmt + ::1 dev mgmt proto kernel metric 256 pref medium + anycast fe80:: dev eth0 proto kernel metric 0 pref medium + fe80::/64 dev eth0 proto kernel metric 256 pref medium + ff00::/8 dev eth0 metric 256 pref medium + unreachable default dev lo metric 4278198272 pref medium + + cumulus@act-5812-10:~$ host google.com + google.com has address 172.217.0.46 + google.com has IPv6 address 2607:f8b0:4005:802::200e + google.com mail is handled by 30 alt2.aspmx.l.google.com. + google.com mail is handled by 40 alt3.aspmx.l.google.com. + google.com mail is handled by 20 alt1.aspmx.l.google.com. + google.com mail is handled by 10 aspmx.l.google.com. + google.com mail is handled by 50 alt4.aspmx.l.google.com. + +Success with numeric address + + cumulus@act-5812-10:~$ ping -n -c1 -I mgmt 172.217.0.46 + ping: Warning: source address might be selected on device other than mgmt. + PING 172.217.0.46 (172.217.0.46) from 10.230.130.211 mgmt: 56(84) bytes of data. + 64 bytes from 172.217.0.46: icmp_seq=1 ttl=51 time=4.68 ms + + --- 172.217.0.46 ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 4.675/4.675/4.675/0.000 ms + +Failure with host by name + + cumulus@act-5812-10:~$ ping -n -c1 -I mgmt google.com + connect: No route to host + +Success when running in the mgmt vrf because getaddrinfo()'s address +reachability test is effective and ipv4 result(s) are ordered first. + + cumulus@act-5812-10:~$ ip vrf exec mgmt ping -n -c1 google.com + PING google.com (172.217.0.46) 56(84) bytes of data. + 64 bytes from 172.217.0.46: icmp_seq=1 ttl=51 time=4.65 ms + + --- google.com ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 4.650/4.650/4.650/0.000 ms + +For demonstration purposes, the following configuration allows one to +reproduce a similar problem. Starting from a host with a vanilla +configuration, default ipv4 route using eth0, no ipv6 global routes: + + root@vsid:~# ip route + default via 192.168.15.1 dev eth0 + 192.168.15.0/24 dev eth0 proto kernel scope link src 192.168.15.100 + + root@vsid:~# ip -6 route + ::1 dev lo proto kernel metric 256 pref medium + fe80::/64 dev eth0 proto kernel metric 256 pref medium + + root@vsid:~# ip rou flush table main + + root@vsid:~# ip rou add table 1 192.168.15.0/24 dev eth0 + + root@vsid:~# ip rou add table 1 default via 192.168.15.1 + + root@vsid:~# ip rule + 0: from all lookup local + 32766: from all lookup main + 32767: from all lookup default + root@vsid:~# ip rule add pref 1 to 192.168.15.1 ipproto udp dport 53 lookup 1 + root@vsid:~# ip rule add pref 2 oif eth0 lookup 1 + root@vsid:~# ping -c1 -I eth0 google.com + + ping: connect: Network is unreachable + +With the current patch + + root@vsid:~# /src/iputils/builddir/ping/ping -c1 -I eth0 google.com + PING (172.217.174.110) from 192.168.15.100 eth0: 56(84) bytes of data. + 64 bytes from nrt12s28-in-f14.1e100.net (172.217.174.110): icmp_seq=1 ttl=53 time=11.3 ms + + --- ping statistics --- + 1 packets transmitted, 1 received, 0% packet loss, time 0ms + rtt min/avg/max/mdev = 11.313/11.313/11.313/0.000 ms + +Signed-off-by: Benjamin Poirier <bpoirier@cumulusnetworks.com> +Origin: https://github.com/iputils/iputils/commit/2705c8248281fbb8efaa5326ab1d0ed0a670bd3d +Bug-Debian: https://bugs.debian.org/947921 +See-also: https://github.com/iputils/iputils/pull/244 +--- + ping.c | 3 +++ + ping6_common.c | 7 ++++++- + 2 files changed, 9 insertions(+), 1 deletion(-) + +diff --git a/ping.c b/ping.c +index 34653be..013c4e6 100644 +--- a/ping.c ++++ b/ping.c +@@ -628,6 +628,9 @@ int ping4_run(int argc, char **argv, struct addrinfo *ai, socket_st *sock) + error(2, errno, _("cannot set broadcasting")); + if (connect(probe_fd, (struct sockaddr *)&dst, sizeof(dst)) == -1) + error(2, errno, "connect"); ++ } else if ((errno == EHOSTUNREACH || errno == ENETUNREACH) && ai->ai_next) { ++ close(probe_fd); ++ return -1; + } else + error(2, errno, "connect"); + } +diff --git a/ping6_common.c b/ping6_common.c +index 6cc5404..bc1030b 100644 +--- a/ping6_common.c ++++ b/ping6_common.c +@@ -651,8 +651,13 @@ int ping6_run(int argc, char **argv, struct addrinfo *ai, struct socket_st *sock + firsthop.sin6_family = AF_INET6; + + firsthop.sin6_port = htons(1025); +- if (connect(probe_fd, (struct sockaddr *)&firsthop, sizeof(firsthop)) == -1) ++ if (connect(probe_fd, (struct sockaddr *)&firsthop, sizeof(firsthop)) == -1) { ++ if ((errno == EHOSTUNREACH || errno == ENETUNREACH) && ai->ai_next) { ++ close(probe_fd); ++ return -1; ++ } + error(2, errno, "connect"); ++ } + alen = sizeof source6; + if (getsockname(probe_fd, (struct sockaddr *)&source6, &alen) == -1) + error(2, errno, "getsockname"); +-- +2.25.0.rc2 + diff --git a/net-misc/iputils/iputils-20190709-r1.ebuild b/net-misc/iputils/iputils-20190709-r1.ebuild index 52bdec97980e..de8b4a40ad6e 100644 --- a/net-misc/iputils/iputils-20190709-r1.ebuild +++ b/net-misc/iputils/iputils-20190709-r1.ebuild @@ -76,6 +76,8 @@ fi PATCHES=( "${FILESDIR}"/${P}-arping-revert-partially-fix-sent-vs-received-package.patch + "${FILESDIR}"/${P}-ping-try-next-addrinfo-on-connect-failure.patch + "${FILESDIR}"/${P}-ping-fix-main-loop-over-multiple-addrinfo-results.patch ) src_prepare() { |