[PATCH] rxrpc: Adjust retransmission backoff

[Marc Dionne]

On Wed, Jan 5, 2022 at 7:13 AM David Howells <dhowells at redhat.com> wrote:
>
> Improve retransmission backoff by only backing off when we retransmit data
> packets rather than when we set the lost ack timer.
>
> To this end:
>
>  (1) In rxrpc_resend(), use rxrpc_get_rto_backoff() when setting the
>      retransmission timer and only tell it that we are retransmitting if we
>      actually have things to retransmit.
>
>      Note that it's possible for the retransmission algorithm to race with
>      the processing of a received ACK, so we may see no packets needing
>      retransmission.
>
>  (2) In rxrpc_send_data_packet(), don't bump the backoff when setting the
>      ack_lost_at timer, as it may then get bumped twice.
>
> With this, when looking at one particular packet, the retransmission
> intervals were seen to be 1.5ms, 2ms, 3ms, 5ms, 9ms, 17ms, 33ms, 71ms,
> 136ms, 264ms, 544ms, 1.088s, 2.1s, 4.2s and 8.3s.
>
> Suggested-by: Marc Dionne <marc.dionne at auristor.com>
> Signed-off-by: David Howells <dhowells at redhat.com>
> cc: linux-afs at lists.infradead.org
> ---
>
>  net/rxrpc/call_event.c |    8 +++-----
>  net/rxrpc/output.c     |    2 +-
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
> index 6be2672a65ea..df864e692267 100644
> --- a/net/rxrpc/call_event.c
> +++ b/net/rxrpc/call_event.c
> @@ -157,7 +157,7 @@ static void rxrpc_congestion_timeout(struct rxrpc_call *call)
>  static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
>  {
>         struct sk_buff *skb;
> -       unsigned long resend_at, rto_j;
> +       unsigned long resend_at;
>         rxrpc_seq_t cursor, seq, top;
>         ktime_t now, max_age, oldest, ack_ts;
>         int ix;
> @@ -165,10 +165,8 @@ static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
>
>         _enter("{%d,%d}", call->tx_hard_ack, call->tx_top);
>
> -       rto_j = call->peer->rto_j;
> -
>         now = ktime_get_real();
> -       max_age = ktime_sub(now, jiffies_to_usecs(rto_j));
> +       max_age = ktime_sub(now, jiffies_to_usecs(call->peer->rto_j));
>
>         spin_lock_bh(&call->lock);
>
> @@ -213,7 +211,7 @@ static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
>         }
>
>         resend_at = nsecs_to_jiffies(ktime_to_ns(ktime_sub(now, oldest)));
> -       resend_at += jiffies + rto_j;
> +       resend_at += jiffies + rxrpc_get_rto_backoff(call->peer, retrans);
>         WRITE_ONCE(call->resend_at, resend_at);
>
>         if (unacked)
> diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
> index 10f2bf2e9068..a45c83f22236 100644
> --- a/net/rxrpc/output.c
> +++ b/net/rxrpc/output.c
> @@ -468,7 +468,7 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
>                         if (call->peer->rtt_count > 1) {
>                                 unsigned long nowj = jiffies, ack_lost_at;
>
> -                               ack_lost_at = rxrpc_get_rto_backoff(call->peer, retrans);
> +                               ack_lost_at = rxrpc_get_rto_backoff(call->peer, false);
>                                 ack_lost_at += nowj;
>                                 WRITE_ONCE(call->ack_lost_at, ack_lost_at);
>                                 rxrpc_reduce_call_timer(call, ack_lost_at, nowj,

That's identical (modulo debugging code) to what I tested to produce
the observed numbers in the commit message.
Much better than continuously resending every 2-3ms until a timeout
occurs, which resulted in thousands of resent packets.

Reviewed-by: Marc Dionne <marc.dionne at auristor.com>
Tested-by: Marc Dionne <marc.dionne at auristor.com>

Marc