On Sat, Jun 27, 2020 at 12:06 AM Yousuk Seung <[email protected]> wrote:
>
> Currently tp->delivered is updated with sacked packets but not
> cumulatively acked when SCP_TSTAMP_ACK is timestamped. This patch moves
> a tcp_ack_tstamp() call in tcp_clean_rtx_queue() to later in the loop so
> that when a skb is fully acked OPT_STATS of SCM_TSTAMP_ACK will include
> the current skb in the delivered count. When not fully acked
> tcp_ack_tstamp() is a no-op and there is no change in behavior.
>
> Signed-off-by: Yousuk Seung <[email protected]>
> Signed-off-by: Yuchung Cheng <[email protected]>
> Acked-by: Eric Dumazet <[email protected]>
> Acked-by: Neal Cardwell <[email protected]>
> Acked-by: Soheil Hassas Yeganeh <[email protected]>
> ---
> net/ipv4/tcp_input.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index f3a0eb139b76..2a683e785cca 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3078,8 +3078,6 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32
> prior_fack,
> u8 sacked = scb->sacked;
> u32 acked_pcount;
>
> - tcp_ack_tstamp(sk, skb, prior_snd_una);
> -
> /* Determine how many packets and what bytes were acked, tso
> and else */
> if (after(scb->end_seq, tp->snd_una)) {
> if (tcp_skb_pcount(skb) == 1 ||
> @@ -3143,6 +3141,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32
> prior_fack,
> if (!fully_acked)
> break;
>
> + tcp_ack_tstamp(sk, skb, prior_snd_una);
> +
> next = skb_rb_next(skb);
> if (unlikely(skb == tp->retransmit_skb_hint))
> tp->retransmit_skb_hint = NULL;
This moves tcp_ack_tstamp beyond these two breaks:
tcp_ack_tstamp(sk, skb, prior_snd_una);
/* Determine how many packets and what bytes were
acked, tso and else */
if (after(scb->end_seq, tp->snd_una)) {
if (tcp_skb_pcount(skb) == 1 ||
!after(tp->snd_una, scb->seq))
break;
acked_pcount = tcp_tso_acked(sk, skb);
if (!acked_pcount)
break;
fully_acked = false;
} else {
acked_pcount = tcp_skb_pcount(skb);
}
but tcp_ack_tstamp does not necessarily act on the end_seq in the
packet, it acts on shinfo->tskey:
if (!before(shinfo->tskey, prior_snd_una) &&
before(shinfo->tskey, tcp_sk(sk)->snd_una)) {
tcp_skb_tsorted_save(skb) {
__skb_tstamp_tx(skb, NULL, sk, SCM_TSTAMP_ACK);
} tcp_skb_tsorted_restore(skb);
}
on the next call to tcp_clean_rtx_queue tcp_sk(sk)->snd_una will be
beyond tskey, so can this result in missing notifications?
