]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: quic: Do not stress the peer during retransmissions of lost packets
authorFrédéric Lécaille <flecaille@haproxy.com>
Wed, 8 Mar 2023 17:23:13 +0000 (18:23 +0100)
committerFrédéric Lécaille <flecaille@haproxy.com>
Wed, 8 Mar 2023 17:50:45 +0000 (18:50 +0100)
This issue was revealed by "Multiple streams" QUIC tracker test which very often
fails (locally) with a file of about 1Mbytes (x4 streams). The log of QUIC tracker
revealed that from its point of view, the 4 files were never all received entirely:

 "results" : {
      "stream_0_rec_closed" : true,
      "stream_0_rec_offset" : 1024250,
      "stream_0_snd_closed" : true,
      "stream_0_snd_offset" : 15,
      "stream_12_rec_closed" : false,
      "stream_12_rec_offset" : 72689,
      "stream_12_snd_closed" : true,
      "stream_12_snd_offset" : 15,
      "stream_4_rec_closed" : true,
      "stream_4_rec_offset" : 1024250,
      "stream_4_snd_closed" : true,
      "stream_4_snd_offset" : 15,
      "stream_8_rec_closed" : true,
      "stream_8_rec_offset" : 1024250,
      "stream_8_snd_closed" : true,
      "stream_8_snd_offset" : 15
   },

But this in contradiction with others QUIC tracker logs which confirms that haproxy
has really (re)sent the stream at the suspected offset(stream_12_rec_offset):

       1152085,
       "transport",
       "packet_received",
       {
          "frames" : [
             {
                "frame_type" : "stream",
                "length" : "155",
                "offset" : "72689",
                "stream_id" : "12"
             }
          ],
          "header" : {
             "dcid" : "a14479169ebb9dba",
             "dcil" : "8",
             "packet_number" : "466",
             "packet_size" : 190
          },
          "packet_type" : "1RTT"
       }

When detected as losts, the packets are enlisted, then their frames are
requeued in their packet number space by qc_requeue_nacked_pkt_tx_frms().
This was done using a local list which was spliced to the packet number
frame list. This had as bad effect to retransmit the frames in the inverse
order they have been sent. This is something the QUIC tracker go client
does not like at all!

Removing the frame splicing fixes this issue and allows haproxy to pass the
"Multiple streams" test.

Must be backported to 2.7.

src/quic_conn.c

index 6d671911f7833c366b4af672f4f3c9856b2e513e..8623afefbe4a22e77764b85baf408b0fb4f45e38 100644 (file)
@@ -1858,7 +1858,6 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
                                                 struct list *pktns_frm_list)
 {
        struct quic_frame *frm, *frmbak;
-       struct list tmp = LIST_HEAD_INIT(tmp);
        struct list *pkt_frm_list = &pkt->frms;
        uint64_t pn = pkt->pn_node.key;
        int close = 0;
@@ -1932,13 +1931,11 @@ static inline int qc_requeue_nacked_pkt_tx_frms(struct quic_conn *qc,
                                close = 1;
                        }
 
-                       LIST_APPEND(&tmp, &frm->list);
+                       LIST_APPEND(pktns_frm_list, &frm->list);
                        TRACE_DEVEL("frame requeued", QUIC_EV_CONN_PRSAFRM, qc, frm);
                }
        }
 
-       LIST_SPLICE(pktns_frm_list, &tmp);
-
  end:
        TRACE_LEAVE(QUIC_EV_CONN_PRSAFRM, qc);
        return !close;