Skip to content

Commit f7edb96

Browse files
Yanmei-LiuYanmei-Liu
committed
[~] #722 add regression test for PADDING-only bytes_in_flight accounting
Drive xqc_send_ctl_increase_inflight and decrease_inflight with stack-allocated xqc_packet_out_t objects covering the relevant frame-bit combinations: pure PADDING -> bytes_in_flight bumped, ack_eliciting untouched pure ACK -> neither counter changes (excluded by CAN_IN_FLIGHT) pure CONNECTION_CLOSE -> same as ACK STREAM -> both counters bumped PADDING + ACK -> only bytes_in_flight bumped (still in flight, not ack-eliciting) repeat increase -> guarded by XQC_POF_IN_FLIGHT, no double count decrease symmetry -> bytes_in_flight goes back down, ack_eliciting only when the packet was ack-eliciting repeat decrease -> guarded by XQC_POF_IN_FLIGHT, no underflow Mutation-tested by reverting src/transport/xqc_send_ctl.c to the buggy version: the PADDING-only, PADDING+ACK and repeat cases all fail, confirming the test actually exercises the new branching.
1 parent ddee702 commit f7edb96

3 files changed

Lines changed: 142 additions & 1 deletion

File tree

‎tests/unittest/main.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,9 @@ main()
126126
xqc_test_pto_uses_remote_max_ack_delay)
127127
|| !CU_add_test(pSuite, "xqc_test_pto_remote_default_when_unset",
128128
xqc_test_pto_remote_default_when_unset)
129-
/* ADD TESTS HERE */)
129+
|| !CU_add_test(pSuite, "xqc_test_send_ctl_inflight_padding",
130+
xqc_test_send_ctl_inflight_padding)
131+
/* ADD TESTS HERE */)
130132
{
131133
CU_cleanup_registry();
132134
return (int)CU_get_error();

‎tests/unittest/xqc_send_ctl_test.c‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
#include "src/transport/xqc_conn.h"
1111
#include "src/transport/xqc_multipath.h"
1212
#include "src/transport/xqc_transport_params.h"
13+
#include "src/transport/xqc_packet_out.h"
14+
#include "src/transport/xqc_frame.h"
1315

1416

1517
/*
@@ -99,3 +101,133 @@ xqc_test_pto_remote_default_when_unset(void)
99101

100102
xqc_engine_destroy(conn->engine);
101103
}
104+
105+
106+
/*
107+
* Issue #722 regression test.
108+
*
109+
* RFC 9002 defines an in-flight packet as one that carries any frame
110+
* besides ACK or CONNECTION_CLOSE. Before the fix,
111+
* xqc_send_ctl_increase_inflight gated the bytes_in_flight accounting
112+
* on XQC_IS_ACK_ELICITING, which excludes PADDING, so a packet that
113+
* carried nothing but PADDING (PMTUD probes, Initial padding, anti-
114+
* amplification fill) never reached the counter. The decrement path
115+
* was symmetrically wrong, so the per-packet XQC_POF_IN_FLIGHT flag
116+
* never flipped and the counters merely stayed balanced -- always
117+
* understated. After the fix bytes_in_flight follows
118+
* XQC_CAN_IN_FLIGHT, while bytes_ack_eliciting_inflight remains
119+
* gated on XQC_IS_ACK_ELICITING.
120+
*
121+
* We construct minimal xqc_packet_out_t objects on the stack and
122+
* drive the two counters directly; the goal is to lock the
123+
* branching logic, not to exercise the surrounding scheduler.
124+
*/
125+
static void
126+
xqc_test_inflight_init_packet(xqc_packet_out_t *po, uint64_t types,
127+
uint64_t path_id)
128+
{
129+
memset(po, 0, sizeof(*po));
130+
po->po_frame_types = types;
131+
po->po_used_size = 1200;
132+
po->po_path_id = path_id;
133+
po->po_pkt.pkt_pns = XQC_PNS_APP_DATA;
134+
}
135+
136+
void
137+
xqc_test_send_ctl_inflight_padding(void)
138+
{
139+
xqc_connection_t *conn = test_engine_connect();
140+
CU_ASSERT_FATAL(conn != NULL);
141+
CU_ASSERT_FATAL(conn->conn_initial_path != NULL);
142+
143+
xqc_send_ctl_t *send_ctl = conn->conn_initial_path->path_send_ctl;
144+
CU_ASSERT_FATAL(send_ctl != NULL);
145+
146+
uint64_t path_id = conn->conn_initial_path->path_id;
147+
148+
/* Baseline */
149+
send_ctl->ctl_bytes_in_flight = 0;
150+
send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA] = 0;
151+
152+
/* Case 1: pure PADDING packet must enter bytes_in_flight but
153+
* leave bytes_ack_eliciting_inflight untouched. */
154+
xqc_packet_out_t padding_only;
155+
xqc_test_inflight_init_packet(&padding_only,
156+
XQC_FRAME_BIT_PADDING, path_id);
157+
xqc_send_ctl_increase_inflight(conn, &padding_only);
158+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
159+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
160+
CU_ASSERT((padding_only.po_flag & XQC_POF_IN_FLIGHT) != 0);
161+
162+
/* Case 2: pure ACK packet must NOT be counted (XQC_CAN_IN_FLIGHT
163+
* excludes ACK). */
164+
xqc_packet_out_t ack_only;
165+
xqc_test_inflight_init_packet(&ack_only,
166+
XQC_FRAME_BIT_ACK, path_id);
167+
xqc_send_ctl_increase_inflight(conn, &ack_only);
168+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
169+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
170+
CU_ASSERT((ack_only.po_flag & XQC_POF_IN_FLIGHT) == 0);
171+
172+
/* Case 3: pure CONNECTION_CLOSE must NOT be counted. */
173+
xqc_packet_out_t cc_only;
174+
xqc_test_inflight_init_packet(&cc_only,
175+
XQC_FRAME_BIT_CONNECTION_CLOSE, path_id);
176+
xqc_send_ctl_increase_inflight(conn, &cc_only);
177+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
178+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
179+
180+
/* Case 4: STREAM packet bumps both counters (ack-eliciting). */
181+
xqc_packet_out_t stream_pkt;
182+
xqc_test_inflight_init_packet(&stream_pkt,
183+
XQC_FRAME_BIT_STREAM, path_id);
184+
xqc_send_ctl_increase_inflight(conn, &stream_pkt);
185+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 2400);
186+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
187+
188+
/* Case 5: PADDING + ACK in the same packet -- PADDING keeps
189+
* XQC_CAN_IN_FLIGHT true so bytes_in_flight increases, but the
190+
* packet is not ack-eliciting (PADDING and ACK are both in the
191+
* XQC_IS_ACK_ELICITING exclusion list). */
192+
xqc_packet_out_t padding_plus_ack;
193+
xqc_test_inflight_init_packet(&padding_plus_ack,
194+
XQC_FRAME_BIT_PADDING | XQC_FRAME_BIT_ACK,
195+
path_id);
196+
xqc_send_ctl_increase_inflight(conn, &padding_plus_ack);
197+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 3600);
198+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
199+
200+
/* Case 6: a second increase_inflight on the same packet is a
201+
* no-op -- the XQC_POF_IN_FLIGHT flag must guard against double
202+
* counting. */
203+
xqc_send_ctl_increase_inflight(conn, &padding_only);
204+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 3600);
205+
206+
/* Case 7: decrease_inflight on the PADDING-only packet must
207+
* subtract from bytes_in_flight without touching the
208+
* ack_eliciting counter -- symmetric with case 1. */
209+
xqc_send_ctl_decrease_inflight(conn, &padding_only);
210+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 2400);
211+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
212+
CU_ASSERT((padding_only.po_flag & XQC_POF_IN_FLIGHT) == 0);
213+
214+
/* Case 8: decrease the PADDING+ACK packet -- bytes_in_flight only. */
215+
xqc_send_ctl_decrease_inflight(conn, &padding_plus_ack);
216+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
217+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
218+
219+
/* Case 9: decrease the STREAM packet -- both counters go back to zero. */
220+
xqc_send_ctl_decrease_inflight(conn, &stream_pkt);
221+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
222+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
223+
224+
/* Case 10: a second decrease on the same packet must not
225+
* underflow either counter. xqc_uint32_bounded_subtract clamps
226+
* at zero but the XQC_POF_IN_FLIGHT guard should short-circuit
227+
* the call first. */
228+
xqc_send_ctl_decrease_inflight(conn, &stream_pkt);
229+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
230+
CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
231+
232+
xqc_engine_destroy(conn->engine);
233+
}

‎tests/unittest/xqc_send_ctl_test.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,11 @@
1313
void xqc_test_pto_uses_remote_max_ack_delay(void);
1414
void xqc_test_pto_remote_default_when_unset(void);
1515

16+
/*
17+
* Regression test for issue #722 (RFC 9002 in-flight definition):
18+
* pure PADDING packets must count toward bytes_in_flight even though
19+
* they are not ack-eliciting.
20+
*/
21+
void xqc_test_send_ctl_inflight_padding(void);
22+
1623
#endif

0 commit comments

Comments
 (0)