From 7cc8b3166a4fcbb3f4e473e2b1bbea5991f4cb67 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Thu, 5 May 2022 12:04:28 +0200 Subject: [PATCH] MINOR: quic: Add correct ack delay values to ACK frames A ->time_received new member is added to quic_rx_packet to store the time the packet are received. ->largest_time_received is added the the packet number space structure to store this timestamp for the packet with a new largest packet number to be acknowledged. QUIC_FL_PKTNS_NEW_LARGEST_PN new flag is added to mark a packet number space as having to acknowledged a packet wih a new largest packet number. In this case, the packet number space ack delay must be recalculated. Add quic_compute_ack_delay_us() function to compute the ack delay from the value of the time a packet was received. Used only when a packet with a new largest packet number. --- include/haproxy/xprt_quic-t.h | 9 +++++++++ include/haproxy/xprt_quic.h | 11 +++++++++++ src/xprt_quic.c | 25 ++++++++++++++++++++----- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/include/haproxy/xprt_quic-t.h b/include/haproxy/xprt_quic-t.h index 504294f..4db01fc 100644 --- a/include/haproxy/xprt_quic-t.h +++ b/include/haproxy/xprt_quic-t.h @@ -395,6 +395,10 @@ struct quic_arngs { #define QUIC_FL_PKTNS_ACK_REQUIRED (1UL << 1) /* Flag the packet number space as needing probing */ #define QUIC_FL_PKTNS_PROBE_NEEDED (1UL << 2) +/* Flag the packet number space as having received a packet with a new largest + * packet number, to be acknowledege + */ +#define QUIC_FL_PKTNS_NEW_LARGEST_PN (1UL << 3) /* The maximum number of dgrams which may be sent upon PTO expirations. */ #define QUIC_MAX_NB_PTO_DGRAMS 2 @@ -416,6 +420,8 @@ struct quic_pktns { unsigned int pto_probe; /* In flight bytes for this packet number space. */ size_t in_flight; + /* The acknowledgement delay of the packet with the largest packet number */ + uint64_t ack_delay; } tx; struct { /* Largest packet number */ @@ -424,6 +430,8 @@ struct quic_pktns { int64_t largest_acked_pn; struct quic_arngs arngs; unsigned int nb_aepkts_since_last_ack; + /* The time the packet with the largest packet number was received */ + uint64_t largest_time_received; } rx; unsigned int flags; }; @@ -483,6 +491,7 @@ struct quic_rx_packet { /* Source address of this packet. */ struct sockaddr_storage saddr; unsigned int flags; + unsigned int time_received; }; /* QUIC datagram handler */ diff --git a/include/haproxy/xprt_quic.h b/include/haproxy/xprt_quic.h index 6457fa4..2c98ed6 100644 --- a/include/haproxy/xprt_quic.h +++ b/include/haproxy/xprt_quic.h @@ -477,6 +477,15 @@ static inline unsigned int quic_ack_delay_ms(struct quic_ack *ack_frm, return (ack_frm->ack_delay << conn->tx.params.ack_delay_exponent) / 1000; } +/* Returns the field value in microsecond to be set in an ACK frame + * depending on the time the packet with a new largest packet number was received. + */ +static inline uint64_t quic_compute_ack_delay_us(unsigned int time_received, + struct quic_conn *conn) +{ + return ((now_ms - time_received) * 1000) >> conn->tx.params.ack_delay_exponent; +} + /* Initialize transport parameters with default values (when absent) * from . * Never fails. @@ -981,6 +990,7 @@ static inline void quic_pktns_init(struct quic_pktns *pktns) pktns->tx.time_of_last_eliciting = 0; pktns->tx.loss_time = TICK_ETERNITY; pktns->tx.in_flight = 0; + pktns->tx.ack_delay = 0; pktns->rx.largest_pn = -1; pktns->rx.largest_acked_pn = -1; @@ -988,6 +998,7 @@ static inline void quic_pktns_init(struct quic_pktns *pktns) pktns->rx.arngs.sz = 0; pktns->rx.arngs.enc_sz = 0; pktns->rx.nb_aepkts_since_last_ack = 0; + pktns->rx.largest_time_received = 0; pktns->flags = 0; } diff --git a/src/xprt_quic.c b/src/xprt_quic.c index ffdbe77..66c3555 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -3621,6 +3621,7 @@ int qc_treat_rx_pkts(struct quic_enc_level *cur_el, struct quic_enc_level *next_ { struct eb64_node *node; int64_t largest_pn = -1; + unsigned int largest_pn_time_received = 0; struct quic_conn *qc = ctx->qc; struct quic_enc_level *qel = cur_el; @@ -3657,8 +3658,10 @@ int qc_treat_rx_pkts(struct quic_enc_level *cur_el, struct quic_enc_level *next_ qel->pktns->rx.nb_aepkts_since_last_ack++; qc_idle_timer_rearm(qc, 1); } - if (pkt->pn > largest_pn) + if (pkt->pn > largest_pn) { largest_pn = pkt->pn; + largest_pn_time_received = pkt->time_received; + } /* Update the list of ranges to acknowledge. */ if (!quic_update_ack_ranges_list(&qel->pktns->rx.arngs, &ar)) TRACE_DEVEL("Could not update ack range list", @@ -3671,9 +3674,14 @@ int qc_treat_rx_pkts(struct quic_enc_level *cur_el, struct quic_enc_level *next_ } HA_RWLOCK_WRUNLOCK(QUIC_LOCK, &qel->rx.pkts_rwlock); - /* Update the largest packet number. */ - if (largest_pn != -1 && largest_pn > qel->pktns->rx.largest_pn) + if (largest_pn != -1 && largest_pn > qel->pktns->rx.largest_pn) { + /* Update the largest packet number. */ qel->pktns->rx.largest_pn = largest_pn; + /* Update the largest acknowledged packet timestamps */ + qel->pktns->rx.largest_time_received = largest_pn_time_received; + qel->pktns->flags |= QUIC_FL_PKTNS_NEW_LARGEST_PN; + } + if (!qc_treat_rx_crypto_frms(qel, ctx)) goto err; @@ -5787,9 +5795,15 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, /* Build an ACK frame if required. */ ack_frm_len = 0; if ((qel->pktns->flags & QUIC_FL_PKTNS_ACK_REQUIRED) && !qel->pktns->tx.pto_probe) { + struct quic_arngs *arngs = &qel->pktns->rx.arngs; BUG_ON(eb_is_empty(&qel->pktns->rx.arngs.root)); - ack_frm.tx_ack.ack_delay = 0; - ack_frm.tx_ack.arngs = &qel->pktns->rx.arngs; + ack_frm.tx_ack.arngs = arngs; + if (qel->pktns->flags & QUIC_FL_PKTNS_NEW_LARGEST_PN) { + qel->pktns->tx.ack_delay = + quic_compute_ack_delay_us(qel->pktns->rx.largest_time_received, qc); + qel->pktns->flags &= ~QUIC_FL_PKTNS_NEW_LARGEST_PN; + } + ack_frm.tx_ack.ack_delay = qel->pktns->tx.ack_delay; /* XXX BE CAREFUL XXX : here we reserved at least one byte for the * smallest frame (PING) and <*pn_len> more for the packet number. Note * that from here, we do not know if we will have to send a PING frame. @@ -6201,6 +6215,7 @@ struct task *quic_lstnr_dghdlr(struct task *t, void *ctx, unsigned int state) if (!pkt) goto err; + pkt->time_received = now_ms; quic_rx_packet_refinc(pkt); qc_lstnr_pkt_rcv(pos, end, pkt, first_pkt, dgram); first_pkt = 0; -- 1.7.10.4