[net-next,2/2] tcp: change data receiver flowlabel after one dup

Commit Message

David Morley Sept. 27, 2023, 6:27 p.m. UTC

From: David Morley <[e-mail]>

This commit changes the data receiver repath behavior to occur after
receiving a single duplicate. This can help recover ACK connectivity
quicker if a TLP was sent along a nonworking path.

For instance, consider the case where we have an initially nonworking
forward path and reverse path and subsequently switch to only working
forward paths. Before this patch we would have the following behavior.

+---------+--------+--------+----------+----------+----------+
| Event   | For FL | Rev FL | FP Works | RP Works | Data Del |
+---------+--------+--------+----------+----------+----------+
| Initial | A      | 1      | N        | N        | 0        |
+---------+--------+--------+----------+----------+----------+
| TLP     | A      | 1      | N        | N        | 0        |
+---------+--------+--------+----------+----------+----------+
| RTO 1   | B      | 1      | Y        | N        | 1        |
+---------+--------+--------+----------+----------+----------+
| RTO 2   | C      | 1      | Y        | N        | 2        |
+---------+--------+--------+----------+----------+----------+
| RTO 3   | D      | 2      | Y        | Y        | 3        |
+---------+--------+--------+----------+----------+----------+

This patch gets rid of at least RTO 3, avoiding additional unnecessary
repaths of a working forward path to a (potentially) nonworking one.

In addition, this commit changes the behavior to avoid repathing upon
rx of duplicate data if the local endpoint is in CA_Loss (in which
case the RTOs will already be changing the outgoing flowlabel).

Signed-off-by: David Morley <[e-mail]>
Signed-off-by: Neal Cardwell <[e-mail]>
Signed-off-by: Yuchung Cheng <[e-mail]>
Tested-by: David Morley <[e-mail]>
---
 net/ipv4/tcp_input.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

Eric Dumazet Sept. 29, 2023, 12:54 a.m. UTC | #1

On Wed, Sep 27, 2023 at 8:28 PM David Morley <[e-mail]> wrote:
>
> From: David Morley <[e-mail]>
>
> This commit changes the data receiver repath behavior to occur after
> receiving a single duplicate. This can help recover ACK connectivity
> quicker if a TLP was sent along a nonworking path.
>
> For instance, consider the case where we have an initially nonworking
> forward path and reverse path and subsequently switch to only working
> forward paths. Before this patch we would have the following behavior.
>
> +---------+--------+--------+----------+----------+----------+
> | Event   | For FL | Rev FL | FP Works | RP Works | Data Del |
> +---------+--------+--------+----------+----------+----------+
> | Initial | A      | 1      | N        | N        | 0        |
> +---------+--------+--------+----------+----------+----------+
> | TLP     | A      | 1      | N        | N        | 0        |
> +---------+--------+--------+----------+----------+----------+
> | RTO 1   | B      | 1      | Y        | N        | 1        |
> +---------+--------+--------+----------+----------+----------+
> | RTO 2   | C      | 1      | Y        | N        | 2        |
> +---------+--------+--------+----------+----------+----------+
> | RTO 3   | D      | 2      | Y        | Y        | 3        |
> +---------+--------+--------+----------+----------+----------+
>
> This patch gets rid of at least RTO 3, avoiding additional unnecessary
> repaths of a working forward path to a (potentially) nonworking one.
>
> In addition, this commit changes the behavior to avoid repathing upon
> rx of duplicate data if the local endpoint is in CA_Loss (in which
> case the RTOs will already be changing the outgoing flowlabel).
>
> Signed-off-by: David Morley <[e-mail]>
> Signed-off-by: Neal Cardwell <[e-mail]>
> Signed-off-by: Yuchung Cheng <[e-mail]>
> Tested-by: David Morley <[e-mail]>

Reviewed-by: Eric Dumazet <[e-mail]>

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index abe7494361c0..f77fbdb3103d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4511,15 +4511,23 @@  static void tcp_rcv_spurious_retrans(struct sock *sk, const struct sk_buff *skb)
 {
 	/* When the ACK path fails or drops most ACKs, the sender would
 	 * timeout and spuriously retransmit the same segment repeatedly.
-	 * The receiver remembers and reflects via DSACKs. Leverage the
-	 * DSACK state and change the txhash to re-route speculatively.
+	 * If it seems our ACKs are not reaching the other side,
+	 * based on receiving a duplicate data segment with new flowlabel
+	 * (suggesting the sender suffered an RTO), and we are not already
+	 * repathing due to our own RTO, then rehash the socket to repath our
+	 * packets.
 	 */
-	if (TCP_SKB_CB(skb)->seq == tcp_sk(sk)->duplicate_sack[0].start_seq &&
+#if IS_ENABLED(CONFIG_IPV6)
+	if (inet_csk(sk)->icsk_ca_state != TCP_CA_Loss &&
+	    skb->protocol == htons(ETH_P_IPV6) &&
+	    (tcp_sk(sk)->inet_conn.icsk_ack.lrcv_flowlabel !=
+	     ntohl(ip6_flowlabel(ipv6_hdr(skb)))) &&
 	    sk_rethink_txhash(sk))
 		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPDUPLICATEDATAREHASH);
 
 	/* Save last flowlabel after a spurious retrans. */
 	tcp_save_lrcv_flowlabel(sk, skb);
+#endif
 }
 
 static void tcp_send_dupack(struct sock *sk, const struct sk_buff *skb)