Latest changes:

- [API] Expose useful lsquic_ver2str[] in lsquic.h
- [BUGFIX] Do not produce packet sequence gaps due to STREAM frame elision

CHANGELOG

@@ -1,5 +1,7 @@
 2018-05-18
 
+    - [API] Expose useful lsquic_ver2str[] in lsquic.h
+    - [BUGFIX] Do not produce packet sequence gaps due to STREAM frame elision
     - Improve checks of number of incoming streams limit and associated
       error reporting.
     - [BUGFIX] Command-line option `-6` now works correctly.

include/lsquic.h

@@ -890,6 +890,9 @@ enum LSQUIC_CONN_STATUS
 enum LSQUIC_CONN_STATUS
 lsquic_conn_status (lsquic_conn_t *, char *errbuf, size_t bufsz);
 
+extern const char *const
+lsquic_ver2str[N_LSQVER];
+
 #ifdef __cplusplus
 }
 #endif

src/liblsquic/lsquic_send_ctl.c

@@ -1322,28 +1322,6 @@ lsquic_send_ctl_set_tcid0 (lsquic_send_ctl_t *ctl, int tcid0)
 }
 
 
-/* Need to assign new packet numbers to all packets following the first
- * dropped packet to eliminate packet number gap.
- */
-static void
-send_ctl_repackno_sched_tail (struct lsquic_send_ctl *ctl,
-                              struct lsquic_packet_out *pre_dropped)
-{
-    struct lsquic_packet_out *packet_out;
-
-    assert(pre_dropped);
-
-    ctl->sc_cur_packno = lsquic_senhist_largest(&ctl->sc_senhist);
-    for (packet_out = TAILQ_NEXT(pre_dropped, po_next); packet_out;
-                            packet_out = TAILQ_NEXT(packet_out, po_next))
-    {
-        packet_out->po_flags |= PO_REPACKNO;
-        if (packet_out->po_flags & PO_ENCRYPTED)
-            send_ctl_release_enc_data(ctl, packet_out);
-    }
-}
-
-
 /* The controller elides this STREAM frames of stream `stream_id' from
  * scheduled and buffered packets.  If a packet becomes empty as a result,
  * it is dropped.
@@ -1356,10 +1334,10 @@ void
 lsquic_send_ctl_elide_stream_frames (lsquic_send_ctl_t *ctl, uint32_t stream_id)
 {
     struct lsquic_packet_out *packet_out, *next;
-    struct lsquic_packet_out *pre_dropped;
     unsigned n, adj;
+    int dropped;
 
-    pre_dropped = NULL;
+    dropped = 0;
 #ifdef WIN32
     next = NULL;
 #endif
@@ -1376,19 +1354,17 @@ lsquic_send_ctl_elide_stream_frames (lsquic_send_ctl_t *ctl, uint32_t stream_id)
             ctl->sc_bytes_scheduled -= adj;
             if (0 == packet_out->po_frame_types)
             {
-                if (!pre_dropped)
-                    pre_dropped = TAILQ_PREV(packet_out, lsquic_packets_tailq,
-                                                                    po_next);
                 LSQ_DEBUG("cancel packet %"PRIu64" after eliding frames for "
                     "stream %"PRIu32, packet_out->po_packno, stream_id);
                 send_ctl_sched_remove(ctl, packet_out);
                 lsquic_packet_out_destroy(packet_out, ctl->sc_enpub);
+                ++dropped;
             }
         }
     }
 
-    if (pre_dropped)
+    if (dropped && ctl->sc_n_scheduled)
-        send_ctl_repackno_sched_tail(ctl, pre_dropped);
+        lsquic_send_ctl_reset_packnos(ctl);
 
     for (n = 0; n < sizeof(ctl->sc_buffered_packets) /
                                 sizeof(ctl->sc_buffered_packets[0]); ++n)
@@ -1490,12 +1466,12 @@ int
 lsquic_send_ctl_squeeze_sched (lsquic_send_ctl_t *ctl)
 {
     struct lsquic_packet_out *packet_out, *next;
-    struct lsquic_packet_out *pre_dropped;
+    int dropped;
 #ifndef NDEBUG
     int pre_squeeze_logged = 0;
 #endif
 
-    pre_dropped = NULL;
+    dropped = 0;
     for (packet_out = TAILQ_FIRST(&ctl->sc_scheduled_packets); packet_out;
                                                             packet_out = next)
     {
@@ -1513,18 +1489,16 @@ lsquic_send_ctl_squeeze_sched (lsquic_send_ctl_t *ctl)
                 LOG_PACKET_Q(&ctl->sc_scheduled_packets,
                                         "unacked packets before squeezing");
 #endif
-            if (!pre_dropped)
-                pre_dropped = TAILQ_PREV(packet_out, lsquic_packets_tailq,
-                                                                    po_next);
             send_ctl_sched_remove(ctl, packet_out);
             LSQ_DEBUG("Dropping packet %"PRIu64" from scheduled queue",
                 packet_out->po_packno);
             lsquic_packet_out_destroy(packet_out, ctl->sc_enpub);
+            ++dropped;
         }
     }
 
-    if (pre_dropped)
+    if (dropped && ctl->sc_n_scheduled)
-        send_ctl_repackno_sched_tail(ctl, pre_dropped);
+        lsquic_send_ctl_reset_packnos(ctl);
 
 #ifndef NDEBUG
     if (pre_squeeze_logged)

src/liblsquic/lsquic_stream.c

@@ -1354,8 +1354,11 @@ lsquic_stream_flush (lsquic_stream_t *stream)
 /* The flush threshold is the maximum size of stream data that can be sent
  * in a full packet.
  */
-static size_t
+#ifdef NDEBUG
-flush_threshold (const lsquic_stream_t *stream)
+static
+#endif
+       size_t
+lsquic_stream_flush_threshold (const struct lsquic_stream *stream)
 {
     enum packet_out_flags flags;
     enum lsquic_packno_bits bits;
@@ -1716,7 +1719,7 @@ stream_write (lsquic_stream_t *stream, struct lsquic_reader *reader)
 {
     size_t thresh, len;
 
-    thresh = flush_threshold(stream);
+    thresh = lsquic_stream_flush_threshold(stream);
     len = reader->lsqr_size(reader->lsqr_ctx);
     if (stream->sm_n_buffered + len <= SM_BUF_SIZE &&
                                     stream->sm_n_buffered + len < thresh)

src/liblsquic/lsquic_stream.h

@@ -295,4 +295,9 @@ lsquic_stream_readable (const lsquic_stream_t *);
 size_t
 lsquic_stream_write_avail (const struct lsquic_stream *);
 
+#ifndef NDEBUG
+size_t
+lsquic_stream_flush_threshold (const struct lsquic_stream *);
+#endif
+
 #endif

test/unittests/test_stream.c

@@ -915,6 +915,148 @@ test_loc_RST_rem_FIN (struct test_objs *tobjs)
 }
 
 
+/* Test that when stream frame is elided and the packet is dropped,
+ * the send controller produces a gapless sequence.
+ *
+ * Case "middle": 3 packets with STREAM frames for streams A, B, and A.
+ *          Stream B is reset.  We should get a gapless sequence
+ *          of packets 1, 2.
+ */
+static void
+test_gapless_elision_middle (struct test_objs *tobjs)
+{
+    lsquic_stream_t *streamA, *streamB;
+    unsigned char buf[0x1000], buf_out[0x1000];
+    size_t n, thresh, written_to_A = 0;
+    int s, fin;
+    lsquic_packet_out_t *packet_out;
+
+    streamA = new_stream(tobjs, 345);
+    streamB = new_stream(tobjs, 347);
+
+    init_buf(buf_out, sizeof(buf_out));
+    thresh = lsquic_stream_flush_threshold(streamA);
+    n = lsquic_stream_write(streamA, buf_out, thresh);
+    assert(n == thresh);
+    assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    written_to_A += n;
+
+    thresh = lsquic_stream_flush_threshold(streamB);
+    n = lsquic_stream_write(streamB, buf_out, thresh);
+    assert(n == thresh);
+    assert(2 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+
+    thresh = lsquic_stream_flush_threshold(streamA);
+    n = lsquic_stream_write(streamA, buf_out + written_to_A, thresh);
+    assert(n == thresh);
+    assert(3 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    written_to_A += n;
+
+    /* Verify contents of A: */
+    n = read_from_scheduled_packets(&tobjs->send_ctl, streamA->id, buf,
+                                                    sizeof(buf), 0, &fin, 0);
+    assert(n == written_to_A);
+    assert(0 == memcmp(buf, buf_out, written_to_A));
+
+    /* Now reset stream A: */
+    s = lsquic_stream_rst_in(streamB, 0, 0);
+    assert(s == 0);
+    assert(2 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    /* Verify A again: */
+    n = read_from_scheduled_packets(&tobjs->send_ctl, streamA->id, buf,
+                                                    sizeof(buf), 0, &fin, 0);
+    assert(n == written_to_A);
+    assert(0 == memcmp(buf, buf_out, written_to_A));
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(packet_out->po_packno == 1);
+    lsquic_send_ctl_sent_packet(&tobjs->send_ctl, packet_out, 1);
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(packet_out->po_packno == 2);
+    lsquic_send_ctl_sent_packet(&tobjs->send_ctl, packet_out, 1);
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(!packet_out);
+
+    /* Now we can call on_close: */
+    lsquic_stream_destroy(streamA);
+    lsquic_stream_destroy(streamB);
+}
+
+
+/* Test that when stream frame is elided and the packet is dropped,
+ * the send controller produces a gapless sequence.
+ *
+ * Case "beginnig": 3 packets with STREAM frames for streams B, A, and A.
+ *          Stream B is reset.  We should get a gapless sequence
+ *          of packets 1, 2.
+ */
+static void
+test_gapless_elision_beginning (struct test_objs *tobjs)
+{
+    lsquic_stream_t *streamA, *streamB;
+    unsigned char buf[0x1000], buf_out[0x1000];
+    size_t n, thresh, written_to_A = 0;
+    int s, fin;
+    lsquic_packet_out_t *packet_out;
+
+    streamA = new_stream(tobjs, 345);
+    streamB = new_stream(tobjs, 347);
+
+    init_buf(buf_out, sizeof(buf_out));
+
+    thresh = lsquic_stream_flush_threshold(streamB);
+    n = lsquic_stream_write(streamB, buf_out, thresh);
+    assert(n == thresh);
+    assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+
+    thresh = lsquic_stream_flush_threshold(streamA);
+    n = lsquic_stream_write(streamA, buf_out, thresh);
+    assert(n == thresh);
+    assert(2 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    written_to_A += n;
+
+    thresh = lsquic_stream_flush_threshold(streamA);
+    n = lsquic_stream_write(streamA, buf_out + written_to_A, thresh);
+    assert(n == thresh);
+    assert(3 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    written_to_A += n;
+
+    /* Verify contents of A: */
+    n = read_from_scheduled_packets(&tobjs->send_ctl, streamA->id, buf,
+                                                    sizeof(buf), 0, &fin, 0);
+    assert(n == written_to_A);
+    assert(0 == memcmp(buf, buf_out, written_to_A));
+
+    /* Now reset stream A: */
+    s = lsquic_stream_rst_in(streamB, 0, 0);
+    assert(s == 0);
+    assert(2 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
+    /* Verify A again: */
+    n = read_from_scheduled_packets(&tobjs->send_ctl, streamA->id, buf,
+                                                    sizeof(buf), 0, &fin, 0);
+    assert(n == written_to_A);
+    assert(0 == memcmp(buf, buf_out, written_to_A));
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(packet_out->po_packno == 1);
+    lsquic_send_ctl_sent_packet(&tobjs->send_ctl, packet_out, 1);
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(packet_out->po_packno == 2);
+    lsquic_send_ctl_sent_packet(&tobjs->send_ctl, packet_out, 1);
+
+    packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl);
+    assert(!packet_out);
+
+    /* Now we can call on_close: */
+    lsquic_stream_destroy(streamA);
+    lsquic_stream_destroy(streamB);
+}
+
+
+
 /* Write data to the stream, but do not flush: connection cap take a hit.
  * After stream is destroyed, connection cap should go back up.
  */
@@ -1045,6 +1187,8 @@ test_termination (void)
         test_loc_FIN_rem_RST,
         test_loc_data_rem_RST,
         test_loc_RST_rem_FIN,
+        test_gapless_elision_beginning,
+        test_gapless_elision_middle,
     };
 
     for (i = 0; i < sizeof(test_funcs) / sizeof(test_funcs[0]); ++i)