MINOR: quic: implement function to check if STREAM is fully acked

When a STREAM frame is retransmitted, a check is performed to remove
range of data already acked from it. This is useful when STREAM frames
are duplicated and splitted to cover different data ranges. The newly
retransmitted frame contains only unacked data.

This process is performed similarly in qc_dup_pkt_frms() and
qc_build_frms(). Refactor the code into a new function named
qc_stream_frm_is_acked(). It returns true if frame data are already
fully acked and retransmission can be avoided. If only a partial range
of data is acknowledged, frame content is updated to only cover the
unacked data.

This patch does not have any functional change. However, it simplifies
retransmission for STREAM frames. Also, it will be reused to fix
retransmission for empty STREAM frames with FIN set from the following
patch :
  BUG/MEDIUM: quic: handle retransmit for standalone FIN STREAM

As such, it must be backported prior to it.

diff --git a/include/haproxy/quic_retransmit.h b/include/haproxy/quic_retransmit.h
index 403a53c068f5dab0824e033642fbf735dff5bc4a..1cd404421cd80219ff75ae2493765e30d7a7795c 100644 (file)
--- a/include/haproxy/quic_retransmit.h
+++ b/include/haproxy/quic_retransmit.h
@@ -10,6 +10,10 @@
 #include <haproxy/quic_conn-t.h>
 #include <haproxy/quic_tls-t.h>
 
+struct quic_frame;
+
+int qc_stream_frm_is_acked(struct quic_conn *qc, struct quic_frame *f);
+
 void qc_prep_fast_retrans(struct quic_conn *qc,
                           struct quic_pktns *pktns,
                           struct list *frms1, struct list *frms2);

diff --git a/src/quic_retransmit.c b/src/quic_retransmit.c
index d06293f5dbfe14eb6c5a20ceba907a211925feb0..0ac57cbd567f2d73f1d1adcd7703dae8a5bfc4c7 100644 (file)
--- a/src/quic_retransmit.c
+++ b/src/quic_retransmit.c
@@ -3,12 +3,46 @@
 #include <haproxy/quic_conn.h>
 #include <haproxy/quic_frame.h>
 #include <haproxy/quic_retransmit.h>
+#include <haproxy/quic_stream-t.h>
 #include <haproxy/quic_trace.h>
 #include <haproxy/quic_tx.h>
 #include <haproxy/trace.h>
 
 #define TRACE_SOURCE &trace_quic
 
+/* Check if STREAM frame <f> content has already been acknowledged before
+ * retransmitting it. If only a subset of <f> content is acknowledged, frame is
+ * updated to only cover the unacked data.
+ *
+ * Returns true if frame content is fully acknowledged, false if partially or
+ * not at all.
+ */
+int qc_stream_frm_is_acked(struct quic_conn *qc, struct quic_frame *f)
+{
+       const struct qf_stream *frm = &f->stream;
+       const struct qc_stream_desc *s = frm->stream;
+
+       if (!eb64_lookup(&qc->streams_by_id, frm->id)) {
+               TRACE_DEVEL("STREAM frame already acked : stream released", QUIC_EV_CONN_PRSAFRM, qc, f);
+               return 1;
+       }
+
+       if (frm->offset.key + frm->len <= s->ack_offset) {
+               TRACE_DEVEL("STREAM frame already acked : fully acked range", QUIC_EV_CONN_PRSAFRM, qc, f);
+               return 1;
+       }
+
+       if (frm->offset.key < s->ack_offset &&
+           frm->offset.key + frm->len > s->ack_offset) {
+               /* Data range partially acked, remove it from STREAM frame. */
+               const uint64_t diff = s->ack_offset - frm->offset.key;
+               TRACE_DEVEL("updated partially acked frame", QUIC_EV_CONN_PRSAFRM, qc, f);
+               qc_stream_frm_mv_fwd(f, diff);
+       }
+
+       return 0;
+}
+
 /* Duplicate all frames from <pkt_frm_list> list into <out_frm_list> list
  * for <qc> QUIC connection.
  * This is a best effort function which never fails even if no memory could be
@@ -33,32 +67,11 @@ static void qc_dup_pkt_frms(struct quic_conn *qc,
                switch (frm->type) {
                case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
                {
-                       struct qf_stream *strm_frm = &frm->stream;
-                       struct eb64_node *node = NULL;
-                       struct qc_stream_desc *stream_desc;
-
-                       node = eb64_lookup(&qc->streams_by_id, strm_frm->id);
-                       if (!node) {
-                               TRACE_DEVEL("ignored frame for a released stream", QUIC_EV_CONN_PRSAFRM, qc, frm);
-                               continue;
-                       }
-
-                       stream_desc = eb64_entry(node, struct qc_stream_desc, by_id);
                        /* Do not resend this frame if in the "already acked range" */
-                       if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) {
-                               TRACE_DEVEL("ignored frame in already acked range",
-                                           QUIC_EV_CONN_PRSAFRM, qc, frm);
+                       if (qc_stream_frm_is_acked(qc, frm))
                                continue;
-                       }
-                       else if (strm_frm->offset.key < stream_desc->ack_offset) {
-                               uint64_t diff = stream_desc->ack_offset - strm_frm->offset.key;
-
-                               qc_stream_frm_mv_fwd(frm, diff);
-                               TRACE_DEVEL("updated partially acked frame",
-                                           QUIC_EV_CONN_PRSAFRM, qc, frm);
-                       }
 
-                       strm_frm->dup = 1;
+                       frm->stream.dup = 1;
                        break;
                }
 

diff --git a/src/quic_rx.c b/src/quic_rx.c
index fd01c8f8bfb3710d842f02a90e92c3a396bde837..38faafb23805d910f73bd24a283c3f5e00d07062 100644 (file)
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -412,34 +412,10 @@ int qc_handle_frms_of_lost_pkt(struct quic_conn *qc,
                switch (frm->type) {
                case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
                {
-                       struct qf_stream *strm_frm = &frm->stream;
-                       struct eb64_node *node = NULL;
-                       struct qc_stream_desc *stream_desc;
-
-                       node = eb64_lookup(&qc->streams_by_id, strm_frm->id);
-                       if (!node) {
-                               TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, frm);
-                               TRACE_DEVEL("freeing frame from packet", QUIC_EV_CONN_PRSAFRM,
-                                           qc, frm, &pn);
+                       if (qc_stream_frm_is_acked(qc, frm)) {
                                qc_frm_free(qc, &frm);
                                continue;
                        }
-
-                       stream_desc = eb64_entry(node, struct qc_stream_desc, by_id);
-                       /* Do not resend this frame if in the "already acked range" */
-                       if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) {
-                               TRACE_DEVEL("ignored frame in already acked range",
-                                           QUIC_EV_CONN_PRSAFRM, qc, frm);
-                               qc_frm_free(qc, &frm);
-                               continue;
-                       }
-                       else if (strm_frm->offset.key < stream_desc->ack_offset) {
-                               uint64_t diff = stream_desc->ack_offset - strm_frm->offset.key;
-
-                               qc_stream_frm_mv_fwd(frm, diff);
-                               TRACE_DEVEL("updated partially acked frame",
-                                           QUIC_EV_CONN_PRSAFRM, qc, frm);
-                       }
                        break;
                }
 

diff --git a/src/quic_tx.c b/src/quic_tx.c
index 4126d6423185cf5c6c2ebc105d008e3eadb5b026..8ad3d515e6d5c8494ba76b6a3e9059b05b487e4f 100644 (file)
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -1605,35 +1605,10 @@ static int qc_build_frms(struct list *outlist, struct list *inlist,
 
                case QUIC_FT_STREAM_8 ... QUIC_FT_STREAM_F:
                        if (cf->stream.dup) {
-                               struct eb64_node *node = NULL;
-                               struct qc_stream_desc *stream_desc = NULL;
-                               struct qf_stream *strm_frm = &cf->stream;
-
-                               /* As this frame has been already lost, ensure the stream is always
-                                * available or the range of this frame is not consumed before
-                                * resending it.
-                                */
-                               node = eb64_lookup(&qc->streams_by_id, strm_frm->id);
-                               if (!node) {
-                                       TRACE_DEVEL("released stream", QUIC_EV_CONN_PRSAFRM, qc, cf);
-                                       qc_frm_free(qc, &cf);
-                                       continue;
-                               }
-
-                               stream_desc = eb64_entry(node, struct qc_stream_desc, by_id);
-                               if (strm_frm->offset.key + strm_frm->len <= stream_desc->ack_offset) {
-                                       TRACE_DEVEL("ignored frame frame in already acked range",
-                                                   QUIC_EV_CONN_PRSAFRM, qc, cf);
+                               if (qc_stream_frm_is_acked(qc, cf)) {
                                        qc_frm_free(qc, &cf);
                                        continue;
                                }
-                               else if (strm_frm->offset.key < stream_desc->ack_offset) {
-                                       uint64_t diff = stream_desc->ack_offset - strm_frm->offset.key;
-
-                                       qc_stream_frm_mv_fwd(cf, diff);
-                                       TRACE_DEVEL("updated partially acked frame",
-                                                   QUIC_EV_CONN_PRSAFRM, qc, cf);
-                               }
                        }
                        /* Note that these frames are accepted in short packets only without
                         * "Length" packet field. Here, <*len> is used only to compute the