From 37ae505c217106c5686f4dfc62cff31d6e051724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Thu, 27 Jan 2022 11:31:50 +0100 Subject: [PATCH] MINOR: quic: Do not consume the RX buffer on QUIC sock i/o handler side Rename quic_lstnr_dgram_read() to quic_lstnr_dgram_dispatch() to reflect its new role. After calling this latter, the sock i/o handler must consume the buffer only if the datagram it received is detected as wrong by quic_lstnr_dgram_dispatch(). The datagram handler task mark the datagram as consumed atomically setting ->buf to NULL value. The sock i/o handler is responsible of flushing its RX buffer before using it. It also keeps a datagram among the consumed ones so that to pass it to quic_lstnr_dgram_dispatch() and prevent it from allocating a new one. --- include/haproxy/xprt_quic-t.h | 1 + include/haproxy/xprt_quic.h | 5 +++-- src/quic_sock.c | 24 +++++++++++++++++++++--- src/xprt_quic.c | 13 ++++++++++--- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/include/haproxy/xprt_quic-t.h b/include/haproxy/xprt_quic-t.h index ee2c4d4eb..e3eee60e1 100644 --- a/include/haproxy/xprt_quic-t.h +++ b/include/haproxy/xprt_quic-t.h @@ -250,6 +250,7 @@ extern struct pool_head *pool_head_quic_rxbuf; extern struct pool_head *pool_head_quic_rx_packet; extern struct pool_head *pool_head_quic_tx_packet; extern struct pool_head *pool_head_quic_frame; +extern struct pool_head *pool_head_quic_dgram; /* QUIC connection id data. * diff --git a/include/haproxy/xprt_quic.h b/include/haproxy/xprt_quic.h index 22c7a246f..715b78978 100644 --- a/include/haproxy/xprt_quic.h +++ b/include/haproxy/xprt_quic.h @@ -1185,8 +1185,9 @@ void chunk_frm_appendf(struct buffer *buf, const struct quic_frame *frm); void quic_set_tls_alert(struct quic_conn *qc, int alert); int quic_set_app_ops(struct quic_conn *qc, const unsigned char *alpn, size_t alpn_len); struct task *quic_lstnr_dghdlr(struct task *t, void *ctx, unsigned int state); -int quic_lstnr_dgram_read(unsigned char *buf, size_t len, void *owner, - struct sockaddr_storage *saddr, struct list *dgrams); +int quic_lstnr_dgram_dispatch(unsigned char *buf, size_t len, void *owner, + struct sockaddr_storage *saddr, + struct quic_dgram *new_dgram, struct list *dgrams); #endif /* USE_QUIC */ #endif /* _HAPROXY_XPRT_QUIC_H */ diff --git a/src/quic_sock.c b/src/quic_sock.c index ef03908c6..468cf64db 100644 --- a/src/quic_sock.c +++ b/src/quic_sock.c @@ -175,6 +175,7 @@ void quic_sock_fd_iocb(int fd) struct sockaddr_storage saddr = {0}; size_t max_sz; socklen_t saddrlen; + struct quic_dgram *dgram, *dgramp, *new_dgram; BUG_ON(!l); @@ -187,8 +188,23 @@ void quic_sock_fd_iocb(int fd) rxbuf = MT_LIST_POP(&l->rx.rxbuf_list, typeof(rxbuf), mt_list); if (!rxbuf) goto out; + buf = &rxbuf->buf; + new_dgram = NULL; + /* Remove all consumed datagrams of this buffer */ + list_for_each_entry_safe(dgram, dgramp, &rxbuf->dgrams, list) { + if (HA_ATOMIC_LOAD(&dgram->buf)) + break; + + LIST_DELETE(&dgram->list); + b_del(buf, dgram->len); + if (!new_dgram) + new_dgram = dgram; + else + pool_free(pool_head_quic_dgram, dgram); + } + params = &l->bind_conf->quic_params; max_sz = params->max_udp_payload_size; if (b_contig_space(buf) < max_sz) { @@ -212,9 +228,11 @@ void quic_sock_fd_iocb(int fd) } while (0); b_add(buf, ret); - quic_lstnr_dgram_read((unsigned char *)b_head(buf), ret, - l, &saddr, &rxbuf->dgrams); - b_del(buf, ret); + if (!quic_lstnr_dgram_dispatch((unsigned char *)b_head(buf), ret, + l, &saddr, new_dgram, &rxbuf->dgrams)) { + /* If wrong, consume this datagram */ + b_del(buf, ret); + } out: MT_LIST_APPEND(&l->rx.rxbuf_list, &rxbuf->mt_list); } diff --git a/src/xprt_quic.c b/src/xprt_quic.c index 95c145819..90a191661 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -5403,6 +5403,8 @@ struct task *quic_lstnr_dghdlr(struct task *t, void *ctx, unsigned int state) /* If the packet length could not be found, we cannot continue. */ break; } while (pos < end); + /* Mark this datagram as consumed */ + HA_ATOMIC_STORE(&dgram->buf, NULL); /* Increasing the received bytes counter by the UDP datagram length * if this datagram could be associated to a connection. @@ -5449,8 +5451,13 @@ static int quic_get_dgram_dcid(unsigned char *buf, const unsigned char *end, return 0; } -int quic_lstnr_dgram_read(unsigned char *buf, size_t len, void *owner, - struct sockaddr_storage *saddr, struct list *dgrams) +/* Retrieve the DCID from the datagram found in and deliver it to the + * correct datagram handler. + * Return 1 if a correct datagram could be found, 0 if not. + */ +int quic_lstnr_dgram_dispatch(unsigned char *buf, size_t len, void *owner, + struct sockaddr_storage *saddr, + struct quic_dgram *new_dgram, struct list *dgrams) { struct quic_dgram *dgram; unsigned char *dcid; @@ -5461,7 +5468,7 @@ int quic_lstnr_dgram_read(unsigned char *buf, size_t len, void *owner, if (!len || !quic_get_dgram_dcid(buf, buf + len, &dcid, &dcid_len)) goto err; - dgram = pool_alloc(pool_head_quic_dgram); + dgram = new_dgram ? new_dgram : pool_alloc(pool_head_quic_dgram); if (!dgram) goto err;