Say when a blocked UDP port is why loss and jitter are missing

An iperf3 server that allows the port for TCP but not UDP measures throughput
perfectly and never records packet loss or jitter. It is the most common way
that happens, and until now nothing said so.

The warning already tried to name this case, but it could only catch a server
that answers and reports no packets. A firewall that drops rather than refuses
gives the client nothing to answer with, so the pass waits out its window and
the failure arrives looking like a busy server instead.

The two facts that identify it were already in hand and never put together: TCP
and UDP use the same host and same port, so TCP moving data while the UDP pass
stalls leaves very little besides a filter treating the two differently. When
that pair shows up, the log now names the likely cause and the rule to add.

The README also described the UDP pass twice without ever saying its port has to
be open for UDP. It says so now, including that runs taken while the port was
closed have no loss or jitter to recover later.

This came from a real install: 212 runs across five days recorded no loss and no
jitter, warning every single time, with one missing firewall rule behind it. The
website's iperf3 setup guide had the rule right all along.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: cansofgrease

README.md

@@ -680,6 +680,17 @@ stream. Set Ookla's retries to `0` and that fallback cannot run, so on a link th
 slow the upload - and with it the whole run - fails. The error says so and names the
 setting.
 
+That UDP pass needs the iperf3 port open for **UDP as well as TCP** - the same
+port, both protocols (`ufw allow 5201/tcp` and `ufw allow 5201/udp`, or the
+equivalent security-group rules). Allowing only TCP is the usual reason a server
+reports throughput perfectly while loss and jitter stay blank forever: the
+control connection and both transfers are TCP and connect fine, and the UDP
+datagrams are dropped without a refusal, so the pass waits out its window and
+gives up. The daemon logs `iperf3 udp pass failed, loss and jitter unrecorded`
+each time, and when the run's TCP transfers succeeded it names the firewall as
+the likely cause. Nothing is retried later, so runs taken while the port was
+closed have no loss or jitter to recover.
+
 For iperf3, the separate UDP loss/jitter pass probes the same direction you
 test: downstream normally, upstream for an upload-only run - so a one-direction
 test on an asymmetric line reports loss for the direction you asked about. That

internal/speedtest/iperf.go

@@ -767,12 +767,36 @@ func (i *Iperf) Run(ctx context.Context) (Result, error) {
 			}
 		} else if udpErr != nil && i.Log != nil {
 			// The run still succeeds without loss/jitter, so this failure is
-			// invisible everywhere else. The message distinguishes the three cases
-			// that actually happen: UDP blocked upstream ("no datagrams"), the
-			// server rejecting the UDP pass ("authorization failed"), and a busy
-			// server - which need different answers from support.
-			i.Log.Warn("iperf3 udp pass failed, loss and jitter unrecorded",
-				"err", i.withEnvHint(udpErr), "downstream", downstream, "rate_mbps", rate)
+			// invisible everywhere else. The message distinguishes the cases that
+			// actually happen: UDP blocked upstream ("no datagrams"), the server
+			// rejecting the UDP pass ("authorization failed"), and a busy server -
+			// which need different answers from support.
+			//
+			// "no datagrams" only catches a server that ANSWERS and reports zero
+			// packets - iperf3 has to finish and print its JSON to be counted that
+			// way. A firewall that DROPs rather than REJECTs (the default for ufw
+			// and most cloud security groups) sends the datagrams into silence, so
+			// iperf3 never returns, the per-run deadline fires, and the failure
+			// arrives here as a stall instead. That is why the stall is worth a
+			// hint of its own: on its own it reads like a busy server.
+			//
+			// What makes it diagnosable is the pair. TCP and UDP use the same host
+			// and port here, so TCP moving data while UDP stalls leaves almost
+			// nothing but a filter that treats the two protocols differently. Both
+			// figures are the run's own, measured moments earlier.
+			hint := ""
+			if errors.Is(udpErr, errStalled) && (res.DownloadMbps > 0 || res.UploadMbps > 0) {
+				hint = "TCP moved data on this host and port while the UDP pass stalled - " +
+					"the far end is most likely dropping UDP (open the same port for UDP, e.g. ufw allow 5201/udp)"
+			}
+			if hint != "" {
+				i.Log.Warn("iperf3 udp pass failed, loss and jitter unrecorded",
+					"err", i.withEnvHint(udpErr), "downstream", downstream, "rate_mbps", rate,
+					"likely_cause", hint)
+			} else {
+				i.Log.Warn("iperf3 udp pass failed, loss and jitter unrecorded",
+					"err", i.withEnvHint(udpErr), "downstream", downstream, "rate_mbps", rate)
+			}
 		}
 	}
 	// Anything spent beyond what got MEASURED - a retried direction's earlier
@@ -1291,9 +1315,16 @@ func iperfBindArgs(bind string) []string {
 // keeps its own error. The message deliberately avoids the transient-error keywords: the
 // stall already ate the full window, so a retry costs another one and rarely helps (same
 // policy as "no data transferred").
+//
+// errStalled carries that verdict for callers who can say something useful about
+// WHY it stalled. The wording is unchanged - the sentinel is the whole message
+// text, so "%w (killed after 8s)" still reads "transfer stalled (killed after
+// 8s)" - it is just matchable now instead of only printable.
+var errStalled = errors.New("transfer stalled")
+
 func stalledErr(err error, rctx, ctx context.Context, budget time.Duration) error {
 	if err != nil && errors.Is(rctx.Err(), context.DeadlineExceeded) && ctx.Err() == nil {
-		return fmt.Errorf("transfer stalled (killed after %v)", budget)
+		return fmt.Errorf("%w (killed after %v)", errStalled, budget)
 	}
 	return err
 }

internal/speedtest/iperf_test.go

@@ -881,3 +881,38 @@ func TestAvailableCongestionControlFreeBSD(t *testing.T) {
 		t.Errorf("darwin: got %v, want nil", got)
 	}
 }
+
+// A firewall that DROPs UDP is the single most common reason loss and jitter go
+// missing, and it is the case the "no datagrams" branch cannot see: that branch
+// needs iperf3 to finish and report zero packets, but a drop gives it nothing to
+// finish on, so the per-run deadline fires and it arrives as a stall instead.
+// Real instance: 212 consecutive iperf3 runs recorded no loss and no jitter over
+// five days, every one of them warning "transfer stalled (killed after 8s)"
+// while the same run's TCP passes moved ~495/480 Mbps. The cause was one missing
+// ufw rule for 5201/udp; nothing in the warning pointed there.
+//
+// The message text is unchanged - the sentinel IS the text - so this also guards
+// that errStalled stays wrapped rather than being rebuilt as a plain string, which
+// is what would silently switch the hint back off.
+func TestStalledErrIsMatchableAndKeepsItsWording(t *testing.T) {
+	expired, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second))
+	defer cancel()
+	<-expired.Done()
+
+	err := stalledErr(errors.New("signal: killed"), expired, context.Background(), 8*time.Second)
+	if err == nil {
+		t.Fatal("stalledErr returned nil for a deadline kill")
+	}
+	if got, want := err.Error(), "transfer stalled (killed after 8s)"; got != want {
+		t.Errorf("message = %q, want %q - callers and operators both read this string", got, want)
+	}
+	if !errors.Is(err, errStalled) {
+		t.Error("a stall is not errors.Is(errStalled): the UDP pass cannot tell a firewall " +
+			"drop from a busy server without it, so the firewall hint goes silent")
+	}
+	// A non-stall failure must not borrow the verdict: "no datagrams" means the
+	// server answered and reported nothing, which is a different fix.
+	if errors.Is(errors.New("no datagrams"), errStalled) {
+		t.Error("an unrelated error matched errStalled")
+	}
+}