--- /dev/null
+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
+
--- /dev/null
+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
+
projects / gentoo.git / commitdiff