From: Alexander Bluhm <bluhm@openbsd.org>
Subject: TCP softlro split compare and concat function
To: tech@openbsd.org
Date: Thu, 8 May 2025 23:50:19 +0200

Hi,

I would like to split the function that compares two mbuf chains
if their TCP segments fit together from the actual concatenation.
This clarifies where the point of no return is.

The loops that limit the length of the mbuf chains can be made a
bit more efficient.

Also do some minor style nits.

ok?

bluhm

Index: netinet/tcp_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
diff -u -p -r1.445 tcp_input.c
--- netinet/tcp_input.c	8 May 2025 20:22:56 -0000	1.445
+++ netinet/tcp_input.c	8 May 2025 20:55:12 -0000
@@ -4323,7 +4323,7 @@ tcp_softlro_check(struct mbuf *m, struct
 	if (!ext->tcp)
 		return 0;
 
-	/* Don't merge empty segments. */
+	/* Don't merge empty TCP segments. */
 	if (ext->paylen == 0)
 		return 0;
 
@@ -4351,18 +4351,9 @@ tcp_softlro_check(struct mbuf *m, struct
 	return 1;
 }
 
-int
-tcp_softlro(struct mbuf *mhead, struct ether_extracted *head,
-    struct mbuf *mtail, struct ether_extracted *tail)
+static int
+tcp_softlro_compare(struct ether_extracted *head, struct ether_extracted *tail)
 {
-	struct mbuf		*m;
-	unsigned int		 hdrlen;
-	unsigned int		 cnt = 0;
-
-	/*
-	 * Check if head and tail are mergeable
-	 */
-
 	/* Don't merge packets inside and outside of VLANs */
 	if (head->evh && tail->evh) {
 		/* Don't merge packets of different VLANs */
@@ -4409,7 +4400,7 @@ tcp_softlro(struct mbuf *mhead, struct e
 		return 0;
 	}
 
-	/* Check for continues segments. */
+	/* Check for contiguous segments. */
 	if (ntohl(head->tcp->th_seq) + head->paylen != ntohl(tail->tcp->th_seq))
 		return 0;
 
@@ -4428,19 +4419,17 @@ tcp_softlro(struct mbuf *mhead, struct e
 			return 0;
 	}
 
-	/* Limit mbuf chain len to avoid m_defrag calls on forwarding. */
-	for (m = mhead; m != NULL; m = m->m_next)
-		if (cnt++ >= 8)
-			return 0;
-	for (m = mtail; m != NULL; m = m->m_next)
-		if (cnt++ >= 8)
-			return 0;
+	return 1;
+}
 
-	/*
-	 * Prepare concatenation of head and tail.
-	 */
+static void
+tcp_softlro_concat(struct mbuf *mhead, struct ether_extracted *head,
+    struct mbuf *mtail, struct ether_extracted *tail)
+{
+	struct mbuf *m;
+	unsigned int hdrlen;
 
-	/* Adjust IP header. */
+	/* Adjust IP header length. */
 	if (head->ip4) {
 		head->ip4->ip_len = htons(head->iplen + tail->paylen);
 	} else if (head->ip6) {
@@ -4477,8 +4466,8 @@ tcp_softlro(struct mbuf *mhead, struct e
 	CLR(mtail->m_flags, M_PKTHDR);
 
 	/* Concatenate */
-	for (m = mhead; m->m_next;)
-		m = m->m_next;
+	for (m = mhead; m->m_next != NULL; m = m->m_next)
+		;
 	m->m_next = mtail;
 	mhead->m_pkthdr.len += tail->paylen;
 
@@ -4499,18 +4488,19 @@ tcp_softlro(struct mbuf *mhead, struct e
 	}
 	mhead->m_pkthdr.ph_mss = MAX(mhead->m_pkthdr.ph_mss, tail->paylen);
 	tcpstat_inc(tcps_inpktlro);	/* count tail */
-
-	return 1;
 }
 
 void
 tcp_softlro_glue(struct mbuf_list *ml, struct mbuf *mtail, struct ifnet *ifp)
 {
 	struct ether_extracted head, tail;
-	struct mbuf *mhead;
+	struct mbuf *m, *mhead;
+	unsigned int headcnt, tailcnt;
 
 	if (!ISSET(ifp->if_xflags, IFXF_LRO))
-		goto out;
+		goto dontmerge;
+
+	mtail->m_pkthdr.ph_mss = 0;
 
 	ether_extract_headers(mtail, &tail);
 
@@ -4525,10 +4515,15 @@ tcp_softlro_glue(struct mbuf_list *ml, s
 		}
 	}
 
-	if (!tcp_softlro_check(mtail, &tail)) {
-		mtail->m_pkthdr.ph_mss = 0;
-		goto out;
+	if (!tcp_softlro_check(mtail, &tail))
+		goto dontmerge;
+
+	tailcnt = 0;
+	for (m = mtail; m != NULL; m = m->m_next) {
+		if (tailcnt++ >= 8)
+			goto dontmerge;
 	}
+
 	mtail->m_pkthdr.ph_mss = tail.paylen;
 
 	for (mhead = ml->ml_head; mhead != NULL; mhead = mhead->m_nextpkt) {
@@ -4560,9 +4555,19 @@ tcp_softlro_glue(struct mbuf_list *ml, s
 		}
 
 		ether_extract_headers(mhead, &head);
-		if (tcp_softlro(mhead, &head, mtail, &tail))
-			return;
+		if (!tcp_softlro_compare(&head, &tail))
+			continue;
+
+		/* Limit mbuf chain to avoid m_defrag calls when forwarding. */
+		headcnt = tailcnt;
+		for (m = mhead; m != NULL; m = m->m_next) {
+			if (headcnt++ >= 8)
+				goto dontmerge;
+		}
+
+		tcp_softlro_concat(mhead, &head, mtail, &tail);
+		return;
 	}
- out:
+ dontmerge:
 	ml_enqueue(ml, mtail);
 }