fix issue 722: count PADDING-only packets toward bytes_in_flight

scripts/case_test.sh

@@ -5002,4 +5002,31 @@ else
     case_print_result "ack_timestamp_frame_case_6" "fail"
 fi
 
+
+# issue #722 regression: PADDING-bearing packets (PMTUD probes, Initial
+# coalesce padding) must register against bytes_in_flight. Enable PMTUD on
+# both ends, push a sizeable transfer, and assert that:
+#   1. PMTUD was actually negotiated (enable_pmtud != 0 in conn_destroy),
+#      so PADDING-bearing probes were scheduled,
+#   2. inflight accounting was non-zero during the run (counter exercised),
+#   3. the transfer completed end-to-end with no error log.
+killall test_server 2> /dev/null
+clear_log
+echo -e "inflight padding accounting (issue #722) ...\c"
+${SERVER_BIN} -l d -e -x 48 > /dev/null &
+sleep 1
+${CLIENT_BIN} -s 1024000 -l d -t 2 -E -x 48 > stdlog
+transfer_ok=`grep ">>>>>>>> pass" stdlog`
+inflight_nonzero=`grep "|inflight:" clog | grep -v "|inflight:0|" | wc -l`
+pmtud_negotiated=`grep "enable_pmtud:" clog | grep -v "enable_pmtud:0|" | wc -l`
+errlog=`grep_err_log`
+if [ -z "$errlog" ] && [ -n "$transfer_ok" ] && [ "$inflight_nonzero" -gt 0 ] && [ "$pmtud_negotiated" -gt 0 ]; then
+    echo ">>>>>>>> pass:1"
+    case_print_result "inflight_padding_accounting_issue_722" "pass"
+else
+    echo ">>>>>>>> pass:0"
+    case_print_result "inflight_padding_accounting_issue_722" "fail"
+fi
+grep_err_log
+
 cd -

src/transport/xqc_send_ctl.c

@@ -552,12 +552,22 @@ xqc_send_ctl_increase_inflight(xqc_connection_t *conn, xqc_packet_out_t *packet_
     }
 
     xqc_send_ctl_t *send_ctl = path->path_send_ctl;
-    if (!(packet_out->po_flag & XQC_POF_IN_FLIGHT) && XQC_CAN_IN_FLIGHT(packet_out->po_frame_types)) {
+    /*
+     * RFC 9002 in-flight definition: a packet is in flight when it
+     * carries any frame other than ACK or CONNECTION_CLOSE -- that
+     * includes PADDING-only packets such as PMTUD probes and Initial
+     * packets padded to the minimum size. The ack-eliciting subset
+     * is tracked separately because loss detection and PTO arming
+     * only care about packets the peer will explicitly acknowledge.
+     */
+    if (!(packet_out->po_flag & XQC_POF_IN_FLIGHT)
+        && XQC_CAN_IN_FLIGHT(packet_out->po_frame_types))
+    {
+        send_ctl->ctl_bytes_in_flight += packet_out->po_used_size;
         if (XQC_IS_ACK_ELICITING(packet_out->po_frame_types)) {
-            send_ctl->ctl_bytes_in_flight += packet_out->po_used_size;
             send_ctl->ctl_bytes_ack_eliciting_inflight[packet_out->po_pkt.pkt_pns] += packet_out->po_used_size;
-            packet_out->po_flag |= XQC_POF_IN_FLIGHT;
         }
+        packet_out->po_flag |= XQC_POF_IN_FLIGHT;
     }
 }
 
@@ -570,13 +580,21 @@ xqc_send_ctl_decrease_inflight(xqc_connection_t *conn, xqc_packet_out_t *packet_
         return;
     }
 
+    /*
+     * Mirror of increase_inflight. po_frame_types is set during packet
+     * build and stays constant from on_packet_sent through acked / lost /
+     * dropped, so XQC_IS_ACK_ELICITING returns the same answer on both
+     * sides and the two counters stay balanced. xqc_uint32_bounded_subtract
+     * is defensive in case that invariant ever gets broken.
+     */
     xqc_send_ctl_t *send_ctl = path->path_send_ctl;
     if (packet_out->po_flag & XQC_POF_IN_FLIGHT) {
+        send_ctl->ctl_bytes_in_flight = xqc_uint32_bounded_subtract(
+                send_ctl->ctl_bytes_in_flight, packet_out->po_used_size);
         if (XQC_IS_ACK_ELICITING(packet_out->po_frame_types)) {
             send_ctl->ctl_bytes_ack_eliciting_inflight[packet_out->po_pkt.pkt_pns] = xqc_uint32_bounded_subtract(send_ctl->ctl_bytes_ack_eliciting_inflight[packet_out->po_pkt.pkt_pns], packet_out->po_used_size);
-            send_ctl->ctl_bytes_in_flight = xqc_uint32_bounded_subtract(send_ctl->ctl_bytes_in_flight, packet_out->po_used_size);
-            packet_out->po_flag &= ~XQC_POF_IN_FLIGHT;
         }
+        packet_out->po_flag &= ~XQC_POF_IN_FLIGHT;
     }
 }
 

tests/test_client.c

@@ -4660,6 +4660,17 @@ int main(int argc, char *argv[]) {
         conn_settings.enable_pmtud = 3;
     }
 
+    /*
+     * issue #722 regression: enable PMTUD so that PADDING-bearing probe
+     * packets get scheduled during the transfer. Combined with a sizeable
+     * payload, this exercises bytes_in_flight accounting on packets that
+     * would previously have slipped past the XQC_IS_ACK_ELICITING guard
+     * in xqc_send_ctl_increase_inflight / decrease_inflight.
+     */
+    if (g_test_case == 48) {
+        conn_settings.enable_pmtud = 3;
+    }
+
     if (g_test_case == 450) {
         conn_settings.extended_ack_features = 2;
         conn_settings.max_receive_timestamps_per_ack = 45;

tests/test_server.c

@@ -2500,6 +2500,14 @@ int main(int argc, char *argv[]) {
         conn_settings.enable_pmtud = 3;
     }
 
+    /*
+     * issue #722 regression: peer side of the PMTUD-driven PADDING-only
+     * accounting test; see test_client.c case 48 for the rationale.
+     */
+    if (g_test_case == 48) {
+        conn_settings.enable_pmtud = 3;
+    }
+
     if (g_test_case == 6) {
         conn_settings.idle_time_out = 10000;
     }

tests/unittest/main.c

@@ -139,6 +139,8 @@ main()
         || !CU_add_test(pSuite, "xqc_test_vn_reject_when_scid_mismatch", xqc_test_vn_reject_when_scid_mismatch)
         || !CU_add_test(pSuite, "xqc_test_vn_reject_when_state_not_initial_sent", xqc_test_vn_reject_when_state_not_initial_sent)
         || !CU_add_test(pSuite, "xqc_test_vn_abort_on_multi_unsupported_versions", xqc_test_vn_abort_on_multi_unsupported_versions)
+        || !CU_add_test(pSuite, "xqc_test_send_ctl_inflight_padding",
+                        xqc_test_send_ctl_inflight_padding)
         /* ADD TESTS HERE */)
     {
         CU_cleanup_registry();

tests/unittest/xqc_send_ctl_test.c

@@ -13,6 +13,8 @@
 #include "src/transport/xqc_conn.h"
 #include "src/transport/xqc_multipath.h"
 #include "src/transport/xqc_transport_params.h"
+#include "src/transport/xqc_packet_out.h"
+#include "src/transport/xqc_frame.h"
 
 
 /*
@@ -319,3 +321,173 @@ xqc_test_send_ctl_update_rtt_ack_delay_cap(void)
     xqc_free(path->path_send_ctl);
     xqc_free(path);
 }
+
+
+/*
+ * Issue #722 regression test.
+ *
+ * RFC 9002 defines an in-flight packet as one that carries any frame
+ * besides ACK or CONNECTION_CLOSE. Before the fix,
+ * xqc_send_ctl_increase_inflight gated the bytes_in_flight accounting
+ * on XQC_IS_ACK_ELICITING, which excludes PADDING, so a packet that
+ * carried nothing but PADDING (PMTUD probes, Initial padding, anti-
+ * amplification fill) never reached the counter. The decrement path
+ * was symmetrically wrong, so the per-packet XQC_POF_IN_FLIGHT flag
+ * never flipped and the counters merely stayed balanced -- always
+ * understated. After the fix bytes_in_flight follows
+ * XQC_CAN_IN_FLIGHT, while bytes_ack_eliciting_inflight remains
+ * gated on XQC_IS_ACK_ELICITING.
+ *
+ * We construct minimal xqc_packet_out_t objects on the stack and
+ * drive the two counters directly; the goal is to lock the
+ * branching logic, not to exercise the surrounding scheduler.
+ */
+static void
+xqc_test_inflight_init_packet(xqc_packet_out_t *po, uint64_t types,
+    uint64_t path_id)
+{
+    memset(po, 0, sizeof(*po));
+    po->po_frame_types  = types;
+    po->po_used_size    = 1200;
+    po->po_path_id      = path_id;
+    po->po_pkt.pkt_pns  = XQC_PNS_APP_DATA;
+}
+
+void
+xqc_test_send_ctl_inflight_padding(void)
+{
+    xqc_connection_t *conn = test_engine_connect();
+    CU_ASSERT_FATAL(conn != NULL);
+    CU_ASSERT_FATAL(conn->conn_initial_path != NULL);
+
+    xqc_send_ctl_t *send_ctl = conn->conn_initial_path->path_send_ctl;
+    CU_ASSERT_FATAL(send_ctl != NULL);
+
+    uint64_t path_id = conn->conn_initial_path->path_id;
+
+    /* Baseline */
+    send_ctl->ctl_bytes_in_flight = 0;
+    send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA] = 0;
+
+    /* Case 1: pure PADDING packet must enter bytes_in_flight but
+     * leave bytes_ack_eliciting_inflight untouched. */
+    xqc_packet_out_t padding_only;
+    xqc_test_inflight_init_packet(&padding_only,
+                                  XQC_FRAME_BIT_PADDING, path_id);
+    xqc_send_ctl_increase_inflight(conn, &padding_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+    CU_ASSERT((padding_only.po_flag & XQC_POF_IN_FLIGHT) != 0);
+
+    /* Case 2: pure ACK packet must NOT be counted (XQC_CAN_IN_FLIGHT
+     * excludes ACK). */
+    xqc_packet_out_t ack_only;
+    xqc_test_inflight_init_packet(&ack_only,
+                                  XQC_FRAME_BIT_ACK, path_id);
+    xqc_send_ctl_increase_inflight(conn, &ack_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+    CU_ASSERT((ack_only.po_flag & XQC_POF_IN_FLIGHT) == 0);
+
+    /* Case 3: pure CONNECTION_CLOSE must NOT be counted. */
+    xqc_packet_out_t cc_only;
+    xqc_test_inflight_init_packet(&cc_only,
+                                  XQC_FRAME_BIT_CONNECTION_CLOSE, path_id);
+    xqc_send_ctl_increase_inflight(conn, &cc_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+
+    /* Case 4: STREAM packet bumps both counters (ack-eliciting). */
+    xqc_packet_out_t stream_pkt;
+    xqc_test_inflight_init_packet(&stream_pkt,
+                                  XQC_FRAME_BIT_STREAM, path_id);
+    xqc_send_ctl_increase_inflight(conn, &stream_pkt);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 2400);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
+
+    /* Case 5: PADDING + ACK in the same packet -- PADDING keeps
+     * XQC_CAN_IN_FLIGHT true so bytes_in_flight increases, but the
+     * packet is not ack-eliciting (PADDING and ACK are both in the
+     * XQC_IS_ACK_ELICITING exclusion list). */
+    xqc_packet_out_t padding_plus_ack;
+    xqc_test_inflight_init_packet(&padding_plus_ack,
+                                  XQC_FRAME_BIT_PADDING | XQC_FRAME_BIT_ACK,
+                                  path_id);
+    xqc_send_ctl_increase_inflight(conn, &padding_plus_ack);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 3600);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
+
+    /* Case 6: a second increase_inflight on the same packet is a
+     * no-op -- the XQC_POF_IN_FLIGHT flag must guard against double
+     * counting. */
+    xqc_send_ctl_increase_inflight(conn, &padding_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 3600);
+
+    /* Case 7: decrease_inflight on the PADDING-only packet must
+     * subtract from bytes_in_flight without touching the
+     * ack_eliciting counter -- symmetric with case 1. */
+    xqc_send_ctl_decrease_inflight(conn, &padding_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 2400);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
+    CU_ASSERT((padding_only.po_flag & XQC_POF_IN_FLIGHT) == 0);
+
+    /* Case 8: decrease the PADDING+ACK packet -- bytes_in_flight only. */
+    xqc_send_ctl_decrease_inflight(conn, &padding_plus_ack);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 1200);
+
+    /* Case 9: decrease the STREAM packet -- both counters go back to zero. */
+    xqc_send_ctl_decrease_inflight(conn, &stream_pkt);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+
+    /* Case 10: a second decrease on the same packet must not
+     * underflow either counter. xqc_uint32_bounded_subtract clamps
+     * at zero but the XQC_POF_IN_FLIGHT guard should short-circuit
+     * the call first. */
+    xqc_send_ctl_decrease_inflight(conn, &stream_pkt);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+
+    /* Case 11: pure ACK_MP packet (multipath) must NOT be counted,
+     * mirroring case 2 for ACK. XQC_CAN_IN_FLIGHT excludes ACK_MP
+     * symmetrically with ACK. */
+    xqc_packet_out_t ack_mp_only;
+    xqc_test_inflight_init_packet(&ack_mp_only,
+                                  XQC_FRAME_BIT_ACK_MP, path_id);
+    xqc_send_ctl_increase_inflight(conn, &ack_mp_only);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+    CU_ASSERT((ack_mp_only.po_flag & XQC_POF_IN_FLIGHT) == 0);
+
+    /* Case 12: PADDING + ACK_MP must follow the PADDING+ACK rule
+     * (bytes_in_flight only). This guards the multipath path that
+     * piggybacks PADDING onto ACK_MP-only datagrams. */
+    xqc_packet_out_t padding_plus_ack_mp;
+    xqc_test_inflight_init_packet(&padding_plus_ack_mp,
+                                  XQC_FRAME_BIT_PADDING | XQC_FRAME_BIT_ACK_MP,
+                                  path_id);
+    xqc_send_ctl_increase_inflight(conn, &padding_plus_ack_mp);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 1200);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+    xqc_send_ctl_decrease_inflight(conn, &padding_plus_ack_mp);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_ack_eliciting_inflight[XQC_PNS_APP_DATA], 0);
+
+    /* Case 13: zero-sized PADDING-only packet -- po_used_size == 0
+     * is a degenerate input that should still set the IN_FLIGHT flag
+     * (no value-change visible in the counter, but symmetry matters
+     * so a subsequent decrease is a no-op rather than unbalanced). */
+    xqc_packet_out_t zero_padding;
+    xqc_test_inflight_init_packet(&zero_padding,
+                                  XQC_FRAME_BIT_PADDING, path_id);
+    zero_padding.po_used_size = 0;
+    xqc_send_ctl_increase_inflight(conn, &zero_padding);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT((zero_padding.po_flag & XQC_POF_IN_FLIGHT) != 0);
+    xqc_send_ctl_decrease_inflight(conn, &zero_padding);
+    CU_ASSERT_EQUAL(send_ctl->ctl_bytes_in_flight, 0);
+    CU_ASSERT((zero_padding.po_flag & XQC_POF_IN_FLIGHT) == 0);
+
+    xqc_engine_destroy(conn->engine);
+}

tests/unittest/xqc_send_ctl_test.h

@@ -21,4 +21,11 @@ void xqc_test_pto_remote_default_when_unset(void);
  */
 void xqc_test_send_ctl_update_rtt_ack_delay_cap(void);
 
+/*
+ * Regression test for issue #722 (RFC 9002 in-flight definition):
+ * pure PADDING packets must count toward bytes_in_flight even though
+ * they are not ack-eliciting.
+ */
+void xqc_test_send_ctl_inflight_padding(void);
+
 #endif