linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: marc.dionne@auristor.com
Cc: dhowells@redhat.com, linux-afs@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 7/7] rxrpc: Limit retransmission by acked serial
Date: Thu, 19 May 2022 12:00:19 +0100	[thread overview]
Message-ID: <165295801939.3423686.15170060927390373804.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <165295798034.3423686.10148594094912570224.stgit@warthog.procyon.org.uk>

The Rx protocol marks each packet on a connection with a monotonically
increasing serial number.  This serial number is referenced in ACK packets
to indicate the packet that caused it for the purpose of RTT determination.

When a DATA packet is lost, we get an out-of-sequence ACK in response to
the next ACK received and we can use this to determine what packets we need
to retransmit.  However, the OOS ACK might not mention DATA packets later
than the triggering DATA packet because at the time it generated the ACK,
it hadn't received them yet.

This causes a problem in the retransmission algorithm as it has to try and
determine which packets to retransmit.  It sees a bunch of un-ACK'd
packets, both the explicitly NACK'd ones and the ones that the OOS ACK
didn't mention - and, currently, it will try to retransmit all of them.

A further problem can also be seen in which a stream of OOS ACKs (because
several DATA packets arrive after the missing one) causes a bunch of
retransmissions of the same DATA packet.

Fix this by tracking the highest serial number referenced in an ACK packet
on each call, and don't retransmit any DATA packet whose last transmission
serial number exceeded that.  A retransmitted packet will have its
REQUEST_ACK flag set and this will elicit another ACK that should advance
the serial number when we receive it, allowing further retransmission.

There are two gotchas with this, though: PING ACKs don't have the serial
number set, and DELAY ACKs may not have it set (it'll be 0 in both cases).

Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
---

 net/rxrpc/ar-internal.h |    1 +
 net/rxrpc/call_event.c  |    5 +++++
 net/rxrpc/input.c       |    4 ++++
 3 files changed, 10 insertions(+)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index dce056adb78c..3b08075c71fd 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -698,6 +698,7 @@ struct rxrpc_call {
 	rxrpc_seq_t		acks_lowest_nak; /* Lowest NACK in the buffer (or ==tx_hard_ack) */
 	rxrpc_seq_t		acks_lost_top;	/* tx_top at the time lost-ack ping sent */
 	rxrpc_serial_t		acks_lost_ping;	/* Serial number of probe ACK */
+	rxrpc_serial_t		acks_highest_serial; /* Highest serial number ACK'd */
 };
 
 /*
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 31761084a76f..4a381de0b4dd 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -156,6 +156,7 @@ static void rxrpc_congestion_timeout(struct rxrpc_call *call)
  */
 static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
 {
+	struct rxrpc_skb_priv *sp;
 	struct sk_buff *skb;
 	unsigned long resend_at;
 	rxrpc_seq_t cursor, seq, top;
@@ -191,6 +192,10 @@ static void rxrpc_resend(struct rxrpc_call *call, unsigned long now_j)
 			continue;
 
 		skb = call->rxtx_buffer[ix];
+		sp = rxrpc_skb(skb);
+
+		if (after(sp->hdr.serial, call->acks_highest_serial))
+			continue;
 		rxrpc_see_skb(skb, rxrpc_skb_seen);
 
 		if (anno_type == RXRPC_TX_ANNO_UNACK) {
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 1145cb14d86f..6c2db1d58aa8 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -940,6 +940,10 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
 	call->acks_first_seq = first_soft_ack;
 	call->acks_prev_seq = prev_pkt;
 
+	if (buf.ack.reason != RXRPC_ACK_PING &&
+	    after(acked_serial, call->acks_highest_serial))
+		call->acks_highest_serial = acked_serial;
+
 	/* Parse rwind and mtu sizes if provided. */
 	if (buf.info.rxMTU)
 		rxrpc_input_ackinfo(call, skb, &buf.info);



      parent reply	other threads:[~2022-05-19 11:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 10:59 [PATCH 1/7] rxrpc: Enable IPv6 checksums on transport socket David Howells
2022-05-19 10:59 ` [PATCH 2/7] rxrpc: Fix listen() setting the bar too high for the prealloc rings David Howells
2022-05-19 10:59 ` [PATCH 3/7] rxrpc: Don't try to resend the request if we're receiving the reply David Howells
2022-05-19 10:59 ` [PATCH 4/7] rxrpc: Fix overlapping ACK accounting David Howells
2022-05-19 11:00 ` [PATCH 5/7] rxrpc: Don't let ack.previousPacket regress David Howells
2022-05-19 11:00 ` [PATCH 6/7] rxrpc: Fix decision on when to generate an IDLE ACK David Howells
2022-05-19 11:00 ` David Howells [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=165295801939.3423686.15170060927390373804.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).