git: 22b24dc1abb3 - stable/13 - LRO: Fix lost packets when merging 1 payload with an ACK

From: Hans Petter Selasky <hselasky_at_FreeBSD.org>
Date: Tue, 08 Feb 2022 15:13:40 UTC
The branch stable/13 has been updated by hselasky:

URL: https://cgit.FreeBSD.org/src/commit/?id=22b24dc1abb3bc41c9b54054c2242b525daa5bfc

commit 22b24dc1abb3bc41c9b54054c2242b525daa5bfc
Author:     Ryan Stone <rstone@FreeBSD.org>
AuthorDate: 2022-02-08 15:08:50 +0000
Commit:     Hans Petter Selasky <hselasky@FreeBSD.org>
CommitDate: 2022-02-08 15:08:50 +0000

    LRO: Fix lost packets when merging 1 payload with an ACK
    
    To check if it needed to regenerate a packet's header before
    sending it up the stack, LRO was checking if more than one payload
    had been merged into the packet.  This failed in the case where
    a single payload was merged with one or more pure ACKs.  This
    results in lost ACKs.
    
    Fix this by precisely tracking whether header regeneration is
    required instead of using an incorrect heuristic.
    
    Found with: Sysunit test
    Differential Revision:  https://reviews.freebsd.org/D33774
    Reviewed by: rrs
    
    (cherry picked from commit 24fe6643dac0780dc02676626f70052e525b9b22)
---
 sys/netinet/tcp_lro.c | 16 ++++++++++++----
 sys/netinet/tcp_lro.h |  3 ++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/sys/netinet/tcp_lro.c b/sys/netinet/tcp_lro.c
index 4d735f41d84d..feec8c36ab69 100644
--- a/sys/netinet/tcp_lro.c
+++ b/sys/netinet/tcp_lro.c
@@ -794,7 +794,7 @@ static void
 tcp_flush_out_entry(struct lro_ctrl *lc, struct lro_entry *le)
 {
 	/* Check if we need to recompute any checksums. */
-	if (le->m_head->m_pkthdr.lro_nsegs > 1) {
+	if (le->needs_merge) {
 		uint16_t csum;
 
 		switch (le->inner.data.lro_type) {
@@ -895,6 +895,7 @@ tcp_set_entry_to_mbuf(struct lro_ctrl *lc, struct lro_entry *le,
 	le->next_seq = ntohl(th->th_seq) + tcp_data_len;
 	le->ack_seq = th->th_ack;
 	le->window = th->th_win;
+	le->needs_merge = 0;
 
 	/* Setup new data pointers. */
 	le->m_head = m;
@@ -936,10 +937,12 @@ tcp_push_and_replace(struct lro_ctrl *lc, struct lro_entry *le, struct mbuf *m)
 }
 
 static void
-tcp_lro_mbuf_append_pkthdr(struct mbuf *m, const struct mbuf *p)
+tcp_lro_mbuf_append_pkthdr(struct lro_entry *le, const struct mbuf *p)
 {
+	struct mbuf *m;
 	uint32_t csum;
 
+	m = le->m_head;
 	if (m->m_pkthdr.lro_nsegs == 1) {
 		/* Compute relative checksum. */
 		csum = p->m_pkthdr.lro_tcp_d_csum;
@@ -956,6 +959,7 @@ tcp_lro_mbuf_append_pkthdr(struct mbuf *m, const struct mbuf *p)
 	m->m_pkthdr.lro_tcp_d_csum = csum;
 	m->m_pkthdr.lro_tcp_d_len += p->m_pkthdr.lro_tcp_d_len;
 	m->m_pkthdr.lro_nsegs += p->m_pkthdr.lro_nsegs;
+	le->needs_merge = 1;
 }
 
 static void
@@ -1074,8 +1078,12 @@ again:
 			le->next_seq += tcp_data_len;
 			le->ack_seq = th->th_ack;
 			le->window = th->th_win;
+			le->needs_merge = 1;
 		} else if (th->th_ack == le->ack_seq) {
-			le->window = WIN_MAX(le->window, th->th_win);
+			if (WIN_GT(th->th_win, le->window)) {
+				le->window = th->th_win;
+				le->needs_merge = 1;
+			}
 		}
 
 		if (tcp_data_len == 0) {
@@ -1084,7 +1092,7 @@ again:
 		}
 
 		/* Merge TCP data checksum and length to head mbuf. */
-		tcp_lro_mbuf_append_pkthdr(le->m_head, m);
+		tcp_lro_mbuf_append_pkthdr(le, m);
 
 		/*
 		 * Adjust the mbuf so that m_data points to the first byte of
diff --git a/sys/netinet/tcp_lro.h b/sys/netinet/tcp_lro.h
index 3eefd4f0537c..97819596231e 100644
--- a/sys/netinet/tcp_lro.h
+++ b/sys/netinet/tcp_lro.h
@@ -146,7 +146,8 @@ struct lro_entry {
 	uint16_t		compressed;
 	uint16_t		uncompressed;
 	uint16_t		window;
-	uint16_t		timestamp;	/* flag, not a TCP hdr field. */
+	uint16_t		timestamp : 1;
+	uint16_t		needs_merge : 1;
 	struct bintime		alloc_time;	/* time when entry was allocated */
 };