Merge pull request #1867 from private-octopus/fix-excess-fragment

Avoid creating small frame fragments

Author: huitema

CMakeLists.txt

@@ -8,7 +8,7 @@ else()
 endif()
 
 project(picoquic
-        VERSION 1.1.33.1
+        VERSION 1.1.33.2
         DESCRIPTION "picoquic library"
         LANGUAGES C CXX)
 

picoquic/frames.c

@@ -2112,10 +2112,16 @@ uint8_t* picoquic_copy_stream_frame_for_retransmit(
                 else {
                     /* buffer is too short -- do not send the FIN bit, do not set the length, just copy bytes */
                     size_t available = bytes_max - before_length;
-                    bytes_next = before_length;
-                    memcpy(bytes_next, frame_bytes, available);
-                    bytes_next += available;
-                    bytes_not_sent = data_available - available;
+                    if (available < PICOQUIC_MIN_STREAM_DATA_FRAGMENT) {
+                        bytes_not_sent = data_available;
+                        bytes_next = bytes_first;
+                    }
+                    else {
+                        bytes_next = before_length;
+                        memcpy(bytes_next, frame_bytes, available);
+                        bytes_next += available;
+                        bytes_not_sent = data_available - available;
+                    }
                 }
             }
         }

picoquic/picoquic.h

@@ -40,7 +40,7 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_VERSION "1.1.33.1"
+#define PICOQUIC_VERSION "1.1.33.2"
 #define PICOQUIC_ERROR_CLASS 0x400
 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1)
 #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)

picoquic/picoquic_internal.h

@@ -42,6 +42,7 @@ extern "C" {
 #define PICOQUIC_ENFORCED_INITIAL_MTU 1200
 #define PICOQUIC_ENFORCED_INITIAL_CID_LENGTH 8
 #define PICOQUIC_PRACTICAL_MAX_MTU 1440
+#define PICOQUIC_MIN_STREAM_DATA_FRAGMENT 512
 #define PICOQUIC_RETRY_SECRET_SIZE 64
 #define PICOQUIC_RETRY_TOKEN_PAD_SIZE 26
 #define PICOQUIC_DEFAULT_0RTT_WINDOW (10*PICOQUIC_ENFORCED_INITIAL_MTU)

picoquictest/app_limited.c

@@ -612,7 +612,7 @@ int app_limited_rpr_test()
     config.ccalgo = picoquic_cubic_algorithm;
     config.do_preemptive_repeat = 1;
     config.loss_mask = 0x1482481224818214ull;
-    config.completion_target = 46000000;
+    config.completion_target = 47000000;
     config.nb_losses_max = 1980;
     config.rtt_max = 275000;
 

picoquictest/congestion_test.c

@@ -728,9 +728,12 @@ static int blackhole_test_one(picoquic_congestion_algorithm_t* ccalgo, uint64_t
     uint64_t latency = 15000;
     uint64_t picoseq_per_byte_10 = (1000000ull * 8) / 10;
     picoquic_test_tls_api_ctx_t* test_ctx = NULL;
+    picoquic_connection_id_t initial_cid = { {0xb1, 0xac, 2, 3, 4, 5, 6, 7}, 8 };
     int ret = 0;
 
-    ret = tls_api_one_scenario_init(&test_ctx, &simulated_time, PICOQUIC_INTERNAL_TEST_VERSION_1, NULL, NULL);
+    initial_cid.id[2] = ccalgo->congestion_algorithm_number;
+
+    ret = tls_api_init_ctx_ex(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 1, 0, &initial_cid);
 
     if (ret == 0 && test_ctx == NULL) {
         ret = -1;
@@ -750,6 +753,9 @@ static int blackhole_test_one(picoquic_congestion_algorithm_t* ccalgo, uint64_t
         test_ctx->blackhole_end = 7000000;
         test_ctx->blackhole_start = 5000000;
 
+
+        picoquic_set_binlog(test_ctx->qserver, ".");
+
         if (ret == 0) {
             ret = tls_api_one_scenario_body(test_ctx, &simulated_time, test_scenario_10mb, sizeof(test_scenario_10mb), 0, 0, 0, 2 * latency, max_completion_time);
         }

picoquictest/perf_trace_client_ref.txt

@@ -1,2 +1,2 @@
 Log_v, PQ_v, Duration, Sent, Received, Mpbs_S, Mbps_R, QUIC_v, ALPN, CNX_ID, T64, is_client, pkt_recv, trains_s, t_short, tb_cwin, tb_pacing, tb_others, pkt_sent, retrans., spurious, delayed_ack_option, min_ack_delay_remote, max_ack_delay_remote, max_ack_gap_remote, min_ack_delay_local, max_ack_delay_local, max_ack_gap_local, max_mtu_sent, max_mtu_received, zero_rtt, srtt, minrtt, cwin, ccalgo, bwe_max, p_quantum, p_rate
-1, $V, 1.457960, 2056, 8000000, 0.011282, 43.896952, 0x50435130, picoquic-test, 0x9e8f088a8ce00000, 0, 1, 5789, 300, 298, 0, 0, 2, 301, 0, 0, 1, 1000, 10000, 64, 10000, 25000, 2, 1440, 1440, 0, 73645, 70008, 15360, 1, 68050, 32768, 273543
+1, $V, 1.462692, 2056, 8000000, 0.011245, 43.754940, 0x50435130, picoquic-test, 0x9e8f088a8ce00000, 0, 1, 5789, 311, 309, 0, 0, 2, 312, 0, 0, 1, 1000, 10000, 64, 10000, 25000, 2, 1440, 1440, 0, 74166, 70113, 15360, 1, 72716, 32768, 273543

picoquictest/perf_trace_server_ref.txt

@@ -1,2 +1,2 @@
 Log_v, PQ_v, Duration, Sent, Received, Mpbs_S, Mbps_R, QUIC_v, ALPN, CNX_ID, T64, is_client, pkt_recv, trains_s, t_short, tb_cwin, tb_pacing, tb_others, pkt_sent, retrans., spurious, delayed_ack_option, min_ack_delay_remote, max_ack_delay_remote, max_ack_gap_remote, min_ack_delay_local, max_ack_delay_local, max_ack_gap_local, max_mtu_sent, max_mtu_received, zero_rtt, srtt, minrtt, cwin, ccalgo, bwe_max, p_quantum, p_rate
-1, $V, 1.422865, 8000000, 2056, 44.979671, 0.011560, 0x50435130, picoquic-test, 0x9e8f088a8ce00000, 35095, 0, 302, 5715, 45, 74, 5595, 1, 6417, 629, 0, 1, 1000, 10000, 2, 10000, 25000, 64, 1440, 1440, 0, 71305, 70098, 1802062, 5, 12788946, 35826, 35826010
+1, $V, 1.427597, 8000000, 2056, 44.830579, 0.011521, 0x50435130, picoquic-test, 0x9e8f088a8ce00000, 35095, 0, 313, 5715, 44, 74, 5596, 1, 6417, 629, 0, 1, 1000, 10000, 2, 10000, 25000, 64, 1440, 1440, 0, 71335, 70098, 1859903, 5, 12789325, 35826, 35826010

picoquictest/skip_frame_test.c

@@ -3133,8 +3133,10 @@ int dataqueue_copy_test()
                 {
                     size_t output_length = next_byte - output;
                     if (dataqueue_verify_test(&packet, next_frame, next_index, frame_length, data, output, output_length) != 0) {
-                        DBG_PRINTF("Verify data fails for case %d, option &x", basic_case, case_opt);
-                        ret = -1;
+                        if (frame_length >= PICOQUIC_MIN_STREAM_DATA_FRAGMENT || output_length != 0) {
+                            DBG_PRINTF("Verify data fails for case %d, option &x", basic_case, case_opt);
+                            ret = -1;
+                        }
                     }
                 }
             }
@@ -3250,7 +3252,7 @@ int dataqueue_packet_test()
 
     if (ret == 0 &&
         (ret = dataqueue_packet_test_iterate(1, cnx, 2, 0, 1, 1)) == 0 &&
-        (ret = dataqueue_packet_test_iterate(2, cnx, 128, 1, 1, 0)) == 0 &&
+        (ret = dataqueue_packet_test_iterate(2, cnx, 128, 0, 1, 1)) == 0 &&
         (ret = dataqueue_packet_test_iterate(3, cnx, 1024, 1, 0, 0)) == 0) {
         ret = dataqueue_packet_test_iterate(4, cnx, 1024, 0, 0, 1);
     }