linux-kernel - [PATCH 7/7] rxrpc: Limit retransmission by acked serial

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 May 2022 12:00:19 +0100
From:   David Howells <dhowells@...hat.com>
To:     marc.dionne@...istor.com
Cc:     dhowells@...hat.com, linux-afs@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH 7/7] rxrpc: Limit retransmission by acked serial

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@...hat.com>
cc: Marc Dionne <marc.dionne@...istor.com>
cc: linux-afs@...ts.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);