From: Frédéric Lécaille Date: Mon, 14 Mar 2022 11:21:03 +0000 (+0100) Subject: MINOR: quic: Code factorization (TX buffer reuse) X-Git-Tag: v2.6-dev4~42 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=302c2b1120902cb66baf6fce8fd9d1508519e6f8;p=thirdparty%2Fhaproxy.git MINOR: quic: Code factorization (TX buffer reuse) Add qc_may_reuse_cbuf() function used by qc_prep_pkts() and qc_prep_app_pkts(). Simplification of the factorized section code: there is no need to check there is enough room to mark the end of the data in the TX buf. This is done by the callers (qc_prep_pkts() and qc_prep_app_pkts()). Add a diagram to explain the conditions which must be verified to be able to reuse a cbuf struct. This should improve the QUIC stack implementation maintenability. --- diff --git a/src/xprt_quic.c b/src/xprt_quic.c index 34fc08423a..d53bb5f058 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -2437,6 +2437,50 @@ static int qc_parse_pkt_frms(struct quic_rx_packet *pkt, struct ssl_sock_ctx *ct return 0; } +/* Must be called only by a writer (packet builder). + * Return 1 if may be reused to build packets, depending on its and + * internal indexes, 0 if not. When this is the case, reset writer + * index after having marked the end of written data. This the responsability + * of the caller to ensure there is enough room in to write the end of + * data made of a uint16_t null field. + * + * +XXXXXXXXXXXXXXXXXXXXXXX---------------+ (cannot be reused) + * ^ ^ + * r w + * + * +-------XXXXXXXXXXXXXXXX---------------+ (can be reused) + * ^ ^ + * r w + + * +--------------------------------------+ (empty buffer, can be reused) + * ^ + * (r = w) + * + * +XXXXXXXXXXXXXXXXXXXXX-XXXXXXXXXXXXXXXX+ (full buffer, cannot be reused) + * ^ ^ + * w r + */ +static int qc_may_reuse_cbuf(struct cbuf *cbuf) +{ + int rd = HA_ATOMIC_LOAD(&cbuf->rd); + + /* We can reset the writer index only if in front of the reader index and + * if the reader index is not null. Resetting the writer when the reader + * index is null would empty the buffer. + * XXX Note than the writer index cannot reach the reader index. + * Only the reader index can reach the writer index. + */ + if (rd && rd <= cbuf->wr) { + /* Mark the end of contiguous data for the reader */ + write_u16(cb_wr(cbuf), 0); + cb_add(cbuf, sizeof(uint16_t)); + cb_wr_reset(cbuf); + return 1; + } + + return 0; +} + /* Write datagram length and first packet address into ring * buffer. This is the responsibility of the caller to check there is enough * room in . Also increase the write index consequently. @@ -2464,7 +2508,7 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct qring *qr, { struct quic_enc_level *qel; struct cbuf *cbuf; - unsigned char *end_buf, *end, *pos, *spos; + unsigned char *end_buf, *end, *pos; struct quic_tx_packet *pkt; size_t total; size_t dg_headlen; @@ -2478,7 +2522,7 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct qring *qr, total = 0; start: cbuf = qr->cbuf; - spos = pos = cb_wr(cbuf); + pos = cb_wr(cbuf); /* Leave at least bytes at the end of this buffer * to ensure there is enough room to mark the end of prepared * contiguous data with a zero length. @@ -2548,22 +2592,9 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct qring *qr, /* Reset writer index if in front of index */ if (end_buf - pos < (int)qc->path->mtu + dg_headlen) { - int rd = HA_ATOMIC_LOAD(&cbuf->rd); - TRACE_DEVEL("buffer full", QUIC_EV_CONN_PHPKTS, qc); - if (cb_contig_space(cbuf) >= sizeof(uint16_t)) { - if ((pos != spos && cbuf->wr > rd) || (pos == spos && rd <= cbuf->wr)) { - /* Mark the end of contiguous data for the reader */ - write_u16(cb_wr(cbuf), 0); - cb_add(cbuf, sizeof(uint16_t)); - } - } - - if (rd && rd <= cbuf->wr) { - cb_wr_reset(cbuf); - /* Let's try to reuse this buffer */ + if (qc_may_reuse_cbuf(cbuf)) goto start; - } } out: @@ -2589,7 +2620,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct qring *qr, { struct quic_enc_level *qel; struct cbuf *cbuf; - unsigned char *end_buf, *end, *pos, *spos; + unsigned char *end_buf, *end, *pos; struct quic_tx_packet *first_pkt, *cur_pkt, *prv_pkt; /* length of datagrams */ uint16_t dglen; @@ -2608,7 +2639,7 @@ static int qc_prep_pkts(struct quic_conn *qc, struct qring *qr, padding = 0; qel = &qc->els[tel]; cbuf = qr->cbuf; - spos = pos = cb_wr(cbuf); + pos = cb_wr(cbuf); /* Leave at least bytes at the end of this buffer * to ensure there is enough room to mark the end of prepared * contiguous data with a zero length. @@ -2749,22 +2780,9 @@ static int qc_prep_pkts(struct quic_conn *qc, struct qring *qr, stop_build: /* Reset writer index if in front of index */ if (end_buf - pos < (int)qc->path->mtu + dg_headlen) { - int rd = HA_ATOMIC_LOAD(&cbuf->rd); - TRACE_DEVEL("buffer full", QUIC_EV_CONN_PHPKTS, qc); - if (cb_contig_space(cbuf) >= sizeof(uint16_t)) { - if ((pos != spos && cbuf->wr > rd) || (pos == spos && rd <= cbuf->wr)) { - /* Mark the end of contiguous data for the reader */ - write_u16(cb_wr(cbuf), 0); - cb_add(cbuf, sizeof(uint16_t)); - } - } - - if (rd && rd <= cbuf->wr) { - cb_wr_reset(cbuf); - /* Let's try to reuse this buffer */ + if (qc_may_reuse_cbuf(cbuf)) goto start; - } } out: