fix: wait for at least one ipv6 and ipv4 qad report (#3413)

## Description

If an endpoint only has a single relay URL configured in its relay map,
we currently abort the QAD net reports after a single successful report.
However, we also decide whether an endpoint supports IPv6 on the fact if
a QAD IPv6 report completed successfully or not. Those are *two* reports
though, and if we abort after the first one, we falsely assume that the
endpoint has no IPv6 connectivity.

This PR changes this in a rather simple way: If we did start both IPv4
and IPv6 reports, we always wait at least until one of each completed.

Unrelated to this change, the PR also adds logging when the best address
changes.

## Breaking Changes

<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist
<!-- Remove any that are not relevant. -->
- [x] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [ ] Tests if relevant.
- [ ] All breaking changes documented.
- [ ] List all breaking changes in the above "Breaking Changes" section.
- [ ] Open an issue or PR on any number0 repos that are affected by this
breaking change. Give guidance on how the updates should be handled or
do the actual updates themselves. The major ones are:
    - [ ] [`quic-rpc`](https://github.com/n0-computer/quic-rpc)
    - [ ] [`iroh-gossip`](https://github.com/n0-computer/iroh-gossip)
    - [ ] [`iroh-blobs`](https://github.com/n0-computer/iroh-blobs)
    - [ ] [`dumbpipe`](https://github.com/n0-computer/dumbpipe)
    - [ ] [`sendme`](https://github.com/n0-computer/sendme)

---------

Co-authored-by: Philipp Krüger <[email protected]>

Author: Frando

iroh/src/magicsock/node_map/udp_paths.rs

@@ -8,6 +8,7 @@
 use std::{collections::BTreeMap, net::SocketAddr};
 
 use n0_future::time::Instant;
+use tracing::debug;
 
 use super::{IpPort, path_state::PathState};
 
@@ -24,7 +25,7 @@ use super::{IpPort, path_state::PathState};
 ///
 /// [`MagicSock`]: crate::magicsock::MagicSock
 /// [`NodeState`]: super::node_state::NodeState
-#[derive(Debug, Default, Clone, Copy)]
+#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)]
 pub(super) enum UdpSendAddr {
     /// The UDP address can be relied on to deliver data to the remote node.
     ///
@@ -117,8 +118,16 @@ impl NodeUdpPaths {
     ///
     /// This should be called any time that `paths` is modified.
     pub(super) fn update_to_best_addr(&mut self, now: Instant) {
-        self.best_ipv4 = self.best_addr(false, now);
-        self.best = self.best_addr(true, now);
+        let best_ipv4 = self.best_addr(false, now);
+        let best = self.best_addr(true, now);
+        if best_ipv4 != self.best_ipv4 {
+            debug!(?best_ipv4, "update best addr (IPv4)");
+        }
+        if best != self.best {
+            debug!(?best, "update best addr (IPv4 or IPv6)");
+        }
+        self.best_ipv4 = best_ipv4;
+        self.best = best;
     }
 
     /// Returns the current best address of all available paths, ignoring

iroh/src/net_report.rs

@@ -475,8 +475,17 @@ impl Client {
 
         let mut reports = Vec::new();
 
+        // We set _pending to true if at least one report was started for each category.
+        // If we did not start any report for either category, _pending is set to false right away
+        // (it "completed" in the sense that nothing will ever run). If we did start at least one report,
+        // _pending is set to true, and will be set to false further down once the first task
+        // completed.
+        let mut ipv4_pending = !v4_buf.is_empty();
+        let mut ipv6_pending = !v6_buf.is_empty();
         loop {
-            if reports.len() >= enough_relays {
+            // We early-abort the tasks once we have at least `enough_relays` reports,
+            // and at least one ipv4 and one ipv6 report completed (if they were started, see comment above).
+            if reports.len() >= enough_relays && !ipv4_pending && !ipv6_pending {
                 debug!("enough probes: {}", reports.len());
                 cancel_v4.cancel();
                 cancel_v6.cancel();
@@ -487,6 +496,7 @@ impl Client {
                 biased;
 
                 val = v4_buf.join_next(), if !v4_buf.is_empty() => {
+                    ipv4_pending = false;
                     match val {
                         Some(Ok(Some(Ok(res)))) => {
                             match res {
@@ -521,6 +531,7 @@ impl Client {
                     }
                 }
                 val = v6_buf.join_next(), if !v6_buf.is_empty() => {
+                    ipv6_pending = false;
                     match val {
                         Some(Ok(Some(Ok(res)))) => {
                             match res {