tcp: ensure to use the most recently sent skb when filling the rate sample

[ Upstream commit b253a0680ceadc5d7b4acca7aa2d870326cad8ad ]

If an ACK (s)acks multiple skbs, we favor the information
from the most recently sent skb by choosing the skb with
the highest prior_delivered count. But in the interval
between receiving ACKs, we send multiple skbs with the same
prior_delivered, because the tp->delivered only changes
when we receive an ACK.

We used RACK's solution, copying tcp_rack_sent_after() as
tcp_skb_sent_after() helper to determine "which packet was
sent last?". Later, we will use tcp_skb_sent_after() instead
in RACK.

Fixes: b9f64820fb22 ("tcp: track data delivery rate for a TCP connection")
Signed-off-by: Pengcheng Yang <yangpc@wangsu.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://lore.kernel.org/r/1650422081-22153-1-git-send-email-yangpc@wangsu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 31d384c3778a1792aa657a0d89f995db32f349ba..71a9aeae693d9d534e8fe5b8ae4aab8f34237738 100644 (file)
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1026,6 +1026,7 @@ struct rate_sample {
        int  losses;            /* number of packets marked lost upon ACK */
        u32  acked_sacked;      /* number of packets newly (S)ACKed upon ACK */
        u32  prior_in_flight;   /* in flight before this ACK */
+       u32  last_end_seq;      /* end_seq of most recently ACKed packet */
        bool is_app_limited;    /* is sample from packet with bubble in pipe? */
        bool is_retrans;        /* is sample from retransmission? */
        bool is_ack_delayed;    /* is this (likely) a delayed ACK? */
@@ -1148,6 +1149,11 @@ void tcp_rate_gen(struct sock *sk, u32 delivered, u32 lost,
                  bool is_sack_reneg, struct rate_sample *rs);
 void tcp_rate_check_app_limited(struct sock *sk);
 
+static inline bool tcp_skb_sent_after(u64 t1, u64 t2, u32 seq1, u32 seq2)
+{
+       return t1 > t2 || (t1 == t2 && after(seq1, seq2));
+}
+
 /* These functions determine how the current flow behaves in respect of SACK
  * handling. SACK is negotiated with the peer, and therefore it can vary
  * between different flows.

diff --git a/net/ipv4/tcp_rate.c b/net/ipv4/tcp_rate.c
index 0de6935659635f5013a26c4f28cdd06436cfa8d2..6ab197928abbcef968a72d2da5ce6667d027fce7 100644 (file)
--- a/net/ipv4/tcp_rate.c
+++ b/net/ipv4/tcp_rate.c
@@ -73,26 +73,31 @@ void tcp_rate_skb_sent(struct sock *sk, struct sk_buff *skb)
  *
  * If an ACK (s)acks multiple skbs (e.g., stretched-acks), this function is
  * called multiple times. We favor the information from the most recently
- * sent skb, i.e., the skb with the highest prior_delivered count.
+ * sent skb, i.e., the skb with the most recently sent time and the highest
+ * sequence.
  */
 void tcp_rate_skb_delivered(struct sock *sk, struct sk_buff *skb,
                            struct rate_sample *rs)
 {
        struct tcp_sock *tp = tcp_sk(sk);
        struct tcp_skb_cb *scb = TCP_SKB_CB(skb);
+       u64 tx_tstamp;
 
        if (!scb->tx.delivered_mstamp)
                return;
 
+       tx_tstamp = tcp_skb_timestamp_us(skb);
        if (!rs->prior_delivered ||
-           after(scb->tx.delivered, rs->prior_delivered)) {
+           tcp_skb_sent_after(tx_tstamp, tp->first_tx_mstamp,
+                              scb->end_seq, rs->last_end_seq)) {
                rs->prior_delivered  = scb->tx.delivered;
                rs->prior_mstamp     = scb->tx.delivered_mstamp;
                rs->is_app_limited   = scb->tx.is_app_limited;
                rs->is_retrans       = scb->sacked & TCPCB_RETRANS;
+               rs->last_end_seq     = scb->end_seq;
 
                /* Record send time of most recently ACKed packet: */
-               tp->first_tx_mstamp  = tcp_skb_timestamp_us(skb);
+               tp->first_tx_mstamp  = tx_tstamp;
                /* Find the duration of the "send phase" of this window: */
                rs->interval_us = tcp_stamp_us_delta(tp->first_tx_mstamp,
                                                     scb->tx.first_tx_mstamp);