ikcp_flush: pacing budget is checked after a segment's retransmit state is mutated, which wedges paced streams

[dangowrt]

Summary

With a pluggable congestion controller installed (ikcp_setcc) that
implements pacing_rate, ikcp_flush() mutates a segment's retransmit
state before it checks the pacing budget and breaks out of the send
loop. A segment held back by pacing is therefore punished as if it had
timed out: its rto doubles, its resendts moves into the future, and
its xmit counter climbs -- all without a single byte reaching the wire.

Observed on 2.1.1 and current master (b1a7a21).

Details

In the "flush data segments" loop, all three eligibility branches mutate
the segment as a side effect of deciding needsend:

if (segment->xmit == 0) {
    needsend = 1;
    segment->xmit++;
    segment->rto = kcp->rx_rto;
    segment->resendts = current + segment->rto + rtomin;
}
else if (_itimediff(current, segment->resendts) >= 0) {
    needsend = 1;
    segment->xmit++;
    kcp->xmit++;
    ... rto backoff ...
    segment->resendts = current + segment->rto;
    lost = 1;
}
else if (segment->fastack >= resent) { ... segment->xmit++; ... }

only for the budget check to arrive later, inside if (needsend):

if (pacing_budget >= 0 && pacing_budget < (IINT32)segment->len) {
    break;
}

Consequences for the first eligible segment whenever the remaining
budget is smaller than its length:

1. Head-of-queue wedge. If the controller paces below one MSS per
   flush (perfectly reasonable on slow links: 1 MSS per 10 ms interval
   is already ~117 kB/s), the head segment is "retransmitted" on paper
   every flush, its rto doubling each time (nodelay=1 path:
   rto += rto/2), while never actually being sent. The stream stalls
   almost completely.
2. False loss signals. lost = 1 / change++ fire, so
   on_timeout / on_fast_retransmit report loss events that never
   happened on the wire; a controller that reacts to them is being fed
   noise generated by its own pacing.
3. Spurious dead link. segment->xmit grows without transmissions,
   so a paced connection can reach dead_link and be declared dead
   without the peer ever having dropped anything.

Reproduction

Two ikcpcb instances connected in-process, a congestion controller
whose pacing_rate returns a constant 500 (less than one MSS), sender
saturated: nothing is ever delivered, and the head segment's rto
grows without bound. In a shaped-link simulation (50 kB/s bottleneck,
300 ms RTT, token-bucket pacing at the delivery rate), goodput is
0.02 Mbit/s before the fix and 0.32 Mbit/s -- 80% of the link -- after
it, with no other change.

Fix

Decide eligibility first without side effects, check the budget, then
mutate and send (patch attached, against master b1a7a21):

• needsend becomes a small enum of the three cases (1 = first send,
  2 = timeout, 3 = fast retransmit);
• the pacing_budget check moves to the top of if (needsend);
• the state mutations follow the check, unchanged.

Verified behaviour-neutral where the bug does not bite: without ccops
(or without pacing_rate) the budget is -1, the break never triggers,
and the reordered loop is semantically identical -- a shaped-link
simulation produces bit-identical traces. With pacing at >= 1 MSS per
flush, results are unchanged within noise (the marginal segment merely
stops receiving a spurious RTO push).

--- ikcp.c.orig	2026-08-23 03:09:36.087959900 +0100
+++ ikcp.c	2026-08-23 03:09:54.479554427 +0100
@@ -1119,45 +1119,57 @@
 		int needsend = 0;
 		if (segment->xmit == 0) {
 			needsend = 1;
-			segment->xmit++;
-			segment->rto = kcp->rx_rto;
-			segment->resendts = current + segment->rto + rtomin;
 		}
 		else if (_itimediff(current, segment->resendts) >= 0) {
-			needsend = 1;
-			segment->xmit++;
-			kcp->xmit++;
-			if (kcp->nodelay == 0) {
-				segment->rto += _imax_(segment->rto, (IUINT32)kcp->rx_rto);
-			}	else {
-				IINT32 step = (kcp->nodelay < 2)? 
-					((IINT32)(segment->rto)) : kcp->rx_rto;
-				segment->rto += step / 2;
-			}
-			segment->resendts = current + segment->rto;
-			lost = 1;
+			needsend = 2;
 		}
 		else if (segment->fastack >= resent) {
-			if ((int)segment->xmit <= kcp->fastlimit || 
+			if ((int)segment->xmit <= kcp->fastlimit ||
 				kcp->fastlimit <= 0) {
-				needsend = 1;
-				segment->xmit++;
-				segment->fastack = 0;
-				segment->resendts = current + segment->rto;
-				change++;
+				needsend = 3;
 			}
 		}
 
 		if (needsend) {
 			int need;
-			segment->ts = current;
-			segment->wnd = seg.wnd;
-			segment->una = kcp->rcv_nxt;
 
+			// check the pacing budget before touching the
+			// segment: a paced-out segment must keep its rto
+			// and resendts, or being paced out is punished
+			// like a timeout
 			if (pacing_budget >= 0 && pacing_budget < (IINT32)segment->len) {
 				break;
 			}
 
+			if (needsend == 1) {
+				segment->xmit++;
+				segment->rto = kcp->rx_rto;
+				segment->resendts = current + segment->rto + rtomin;
+			}
+			else if (needsend == 2) {
+				segment->xmit++;
+				kcp->xmit++;
+				if (kcp->nodelay == 0) {
+					segment->rto += _imax_(segment->rto, (IUINT32)kcp->rx_rto);
+				}	else {
+					IINT32 step = (kcp->nodelay < 2)?
+						((IINT32)(segment->rto)) : kcp->rx_rto;
+					segment->rto += step / 2;
+				}
+				segment->resendts = current + segment->rto;
+				lost = 1;
+			}
+			else {
+				segment->xmit++;
+				segment->fastack = 0;
+				segment->resendts = current + segment->rto;
+				change++;
+			}
+
+			segment->ts = current;
+			segment->wnd = seg.wnd;
+			segment->una = kcp->rcv_nxt;
+
 			if (kcp->ccops && kcp->ccops->on_pkt_sent) {
 				kcp->ccops->on_pkt_sent(kcp, segment->sn, current,
 						segment->len, kcp->nsnd_buf, segment->xmit);