gamzinav/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: quic: MIssing check when building TX packets

Browse Source 
When building an ack-eliciting frame only packet, if we did not manage to add
at least one such a frame to the packet, we did not notify the caller about
the fact the packet is empty. This could lead the caller to believe
everything was ok and make it endlessly try to build packet again and again.
This issue was amplified by the recent changes where a while(1) loop has been
added to qc_send_app_pkt() which calls qc_do_build_pkt() through qc_prep_app_pkts()
until we could not prepare packets. Before this recent change, I guess only
one empty packet was sent.
This patch checks that non empty packets could be built by qc_do_build_pkt()
and makes this function return an error if this was the case. Also note that
such an issue could happened only when the packet building was limited by
the congestion control.

Thank you to Tristan for having reported this issue in GH #1808.

Must be backported to 2.6.
This commit is contained in:
Frédéric Lécaille 2022-08-16 12:03:00 +02:00
parent 35a66c0a36
commit ebb1070721
1 changed files with 13 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/xprt_quic.c
View File

@ -6592,7 +6592,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
struct quic_enc_level *qel, struct quic_conn *qc,
const struct quic_version *ver, struct list *frms)
{
unsigned char *beg;
unsigned char *beg, *payload;
size_t len, len_sz, len_frms, padding_len;
struct quic_frame frm = { .type = QUIC_FT_CRYPTO, };
struct quic_frame ack_frm = { .type = QUIC_FT_ACK, };
@ -6743,6 +6743,8 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
if (!quic_packet_number_encode(&pos, end, pn, *pn_len))
goto no_room;
/* payload building (ack-eliciting or not frames) */
payload = pos;
if (ack_frm_len) {
if (!qc_build_frm(&pos, end, &ack_frm, pkt, qc))
goto no_room;
@ -6760,10 +6762,10 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
ssize_t room = end - pos;
TRACE_DEVEL("Not enough room", QUIC_EV_CONN_TXPKT,
qc, NULL, NULL, &room);
/* TODO: this should not have happened if qc_build_frms()
* had correctly computed and sized the frames to be
* added to this packet. Note that <cf> was added
* from <frm_list> to <frms> list by qc_build_frms().
/* TODO: this should not have happened except if we
* are limited by the congestion control.
* Note that <cf> was added from <frm_list> to <frms> list by
* qc_build_frms().
*/
LIST_DELETE(&cf->list);
LIST_INSERT(frms, &cf->list);
@ -6799,6 +6801,12 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
goto no_room;
}
if (pos == payload) {
/* No payload was built because of congestion control */
TRACE_DEVEL("limited by congestion control", QUIC_EV_CONN_TXPKT, qc);
goto no_room;
}
/* If this packet is ack-eliciting and we are probing let's
* decrement the PTO probe counter.
*/