Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
softlro refactor extract header
To:
tech@openbsd.org
Date:
Wed, 23 Apr 2025 19:11:35 +0200

Download raw body.

Thread
Hi,

The header of the tail mbuf has already been extraced.  Do all
header extraction in tcp_softlro_glue() and pass them down to
tcp_softlro().

Also do port check before address check as IPv6 address comparison
is expensive.  The empty segment check has already been done in
tcp_softlro_check(), remove it from tcp_softlro().

ok?

bluhm

Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
diff -u -p -r1.439 tcp_input.c
--- netinet/tcp_input.c	22 Apr 2025 22:34:27 -0000	1.439
+++ netinet/tcp_input.c	23 Apr 2025 17:02:26 -0000
@@ -4283,10 +4283,9 @@ tcp_softlro_check(struct mbuf *m, struct
 }
 
 int
-tcp_softlro(struct mbuf *mhead, struct mbuf *mtail)
+tcp_softlro(struct mbuf *mhead, struct ether_extracted *head,
+    struct mbuf *mtail, struct ether_extracted *tail)
 {
-	struct ether_extracted	 head;
-	struct ether_extracted	 tail;
 	struct mbuf		*m;
 	unsigned int		 hdrlen;
 	unsigned int		 cnt = 0;
@@ -4295,70 +4294,66 @@ tcp_softlro(struct mbuf *mhead, struct m
 	 * Check if head and tail are mergeable
 	 */
 
-	ether_extract_headers(mhead, &head);
-	ether_extract_headers(mtail, &tail);
-
 	/* Don't merge packets inside and outside of VLANs */
-	if (head.evh && tail.evh) {
+	if (head->evh && tail->evh) {
 		/* Don't merge packets of different VLANs */
-		if (EVL_VLANOFTAG(head.evh->evl_tag) !=
-		    EVL_VLANOFTAG(tail.evh->evl_tag))
+		if (EVL_VLANOFTAG(head->evh->evl_tag) !=
+		    EVL_VLANOFTAG(tail->evh->evl_tag))
 			return 0;
 
 		/* Don't merge packets of different priorities */
-		if (EVL_PRIOFTAG(head.evh->evl_tag) !=
-		    EVL_PRIOFTAG(tail.evh->evl_tag))
+		if (EVL_PRIOFTAG(head->evh->evl_tag) !=
+		    EVL_PRIOFTAG(tail->evh->evl_tag))
 			return 0;
-	} else if (head.evh || tail.evh)
+	} else if (head->evh || tail->evh)
+		return 0;
+
+	/* Check TCP ports. */
+	if (head->tcp->th_sport != tail->tcp->th_sport ||
+	    head->tcp->th_dport != tail->tcp->th_dport)
 		return 0;
 
 	/* Check IP header. */
-	if (head.ip4 && tail.ip4) {
+	if (head->ip4 && tail->ip4) {
 		/* Check IPv4 addresses. */
-		if (head.ip4->ip_src.s_addr != tail.ip4->ip_src.s_addr ||
-		    head.ip4->ip_dst.s_addr != tail.ip4->ip_dst.s_addr)
+		if (head->ip4->ip_src.s_addr != tail->ip4->ip_src.s_addr ||
+		    head->ip4->ip_dst.s_addr != tail->ip4->ip_dst.s_addr)
 			return 0;
 
 		/* Check max. IPv4 length. */
-		if (head.iplen + tail.iplen > IP_MAXPACKET)
+		if (head->iplen + tail->iplen > IP_MAXPACKET)
 			return 0;
-	} else if (head.ip6 && tail.ip6) {
+	} else if (head->ip6 && tail->ip6) {
 		/* Check IPv6 addresses. */
-		if (!IN6_ARE_ADDR_EQUAL(&head.ip6->ip6_src, &tail.ip6->ip6_src) ||
-		    !IN6_ARE_ADDR_EQUAL(&head.ip6->ip6_dst, &tail.ip6->ip6_dst))
+		if (!IN6_ARE_ADDR_EQUAL(&head->ip6->ip6_src,
+		    &tail->ip6->ip6_src) ||
+		    !IN6_ARE_ADDR_EQUAL(&head->ip6->ip6_dst,
+		    &tail->ip6->ip6_dst))
 			return 0;
 
 		/* Check max. IPv6 length. */
-		if ((head.iplen - head.iphlen) +
-		    (tail.iplen - tail.iphlen) > IPV6_MAXPACKET)
+		if ((head->iplen - head->iphlen) +
+		    (tail->iplen - tail->iphlen) > IPV6_MAXPACKET)
 			return 0;
 	} else {
+		/* Address family does not match. */
 		return 0;
 	}
 
-	/* Check TCP ports. */
-	if (head.tcp->th_sport != tail.tcp->th_sport ||
-	    head.tcp->th_dport != tail.tcp->th_dport)
-		return 0;
-
-	/* Don't merge empty segments. */
-	if (head.paylen == 0 || tail.paylen == 0)
-		return 0;
-
 	/* Check for continues segments. */
-	if (ntohl(head.tcp->th_seq) + head.paylen != ntohl(tail.tcp->th_seq))
+	if (ntohl(head->tcp->th_seq) + head->paylen != ntohl(tail->tcp->th_seq))
 		return 0;
 
 	/* Ignore segments with different TCP options. */
-	if (head.tcphlen - sizeof(struct tcphdr) !=
-	    tail.tcphlen - sizeof(struct tcphdr))
+	if (head->tcphlen - sizeof(struct tcphdr) !=
+	    tail->tcphlen - sizeof(struct tcphdr))
 		return 0;
 
 	/* Check for TCP options */
-	if (head.tcphlen > sizeof(struct tcphdr)) {
-		char *hopt = (char *)(head.tcp) + sizeof(struct tcphdr);
-		char *topt = (char *)(tail.tcp) + sizeof(struct tcphdr);
-		int optsize = head.tcphlen - sizeof(struct tcphdr);
+	if (head->tcphlen > sizeof(struct tcphdr)) {
+		char *hopt = (char *)(head->tcp) + sizeof(struct tcphdr);
+		char *topt = (char *)(tail->tcp) + sizeof(struct tcphdr);
+		int optsize = head->tcphlen - sizeof(struct tcphdr);
 		int optlen;
 
 		for (; optsize > 0; optsize -= optlen) {
@@ -4395,27 +4390,27 @@ tcp_softlro(struct mbuf *mhead, struct m
 	 */
 
 	/* Adjust IP header. */
-	if (head.ip4) {
-		head.ip4->ip_len = htons(head.iplen + tail.paylen);
-	} else if (head.ip6) {
-		head.ip6->ip6_plen =
-		    htons(head.iplen - head.iphlen + tail.paylen);
+	if (head->ip4) {
+		head->ip4->ip_len = htons(head->iplen + tail->paylen);
+	} else if (head->ip6) {
+		head->ip6->ip6_plen =
+		    htons(head->iplen - head->iphlen + tail->paylen);
 	}
 
 	/* Combine TCP flags from head and tail. */
-	if (ISSET(tail.tcp->th_flags, TH_PUSH))
-		SET(head.tcp->th_flags, TH_PUSH);
+	if (ISSET(tail->tcp->th_flags, TH_PUSH))
+		SET(head->tcp->th_flags, TH_PUSH);
 
 	/* Adjust TCP header. */
-	head.tcp->th_win = tail.tcp->th_win;
-	head.tcp->th_ack = tail.tcp->th_ack;
+	head->tcp->th_win = tail->tcp->th_win;
+	head->tcp->th_ack = tail->tcp->th_ack;
 
 	/* Calculate header length of tail packet. */
-	hdrlen = sizeof(*tail.eh);
-	if (tail.evh)
-		hdrlen = sizeof(*tail.evh);
-	hdrlen += tail.iphlen;
-	hdrlen += tail.tcphlen;
+	hdrlen = sizeof(*tail->eh);
+	if (tail->evh)
+		hdrlen = sizeof(*tail->evh);
+	hdrlen += tail->iphlen;
+	hdrlen += tail->tcphlen;
 
 	/* Skip protocol headers in tail. */
 	m_adj(mtail, hdrlen);
@@ -4425,24 +4420,24 @@ tcp_softlro(struct mbuf *mhead, struct m
 	for (m = mhead; m->m_next;)
 		m = m->m_next;
 	m->m_next = mtail;
-	mhead->m_pkthdr.len += tail.paylen;
+	mhead->m_pkthdr.len += tail->paylen;
 
 	/* Flag mbuf as TSO packet with MSS. */
 	if (!ISSET(mhead->m_pkthdr.csum_flags, M_TCP_TSO)) {
 		/* Set CSUM_OUT flags in case of forwarding. */
 		SET(mhead->m_pkthdr.csum_flags, M_TCP_CSUM_OUT);
-		head.tcp->th_sum = 0;
-		if (head.ip4) {
+		head->tcp->th_sum = 0;
+		if (head->ip4) {
 			SET(mhead->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT);
-			head.ip4->ip_sum = 0;
+			head->ip4->ip_sum = 0;
 		}
 
 		SET(mhead->m_pkthdr.csum_flags, M_TCP_TSO);
-		mhead->m_pkthdr.ph_mss = head.paylen;
+		mhead->m_pkthdr.ph_mss = head->paylen;
 		tcpstat_inc(tcps_inswlro);
 		tcpstat_inc(tcps_inpktlro);	/* count head */
 	}
-	mhead->m_pkthdr.ph_mss = MAX(mhead->m_pkthdr.ph_mss, tail.paylen);
+	mhead->m_pkthdr.ph_mss = MAX(mhead->m_pkthdr.ph_mss, tail->paylen);
 	tcpstat_inc(tcps_inpktlro);	/* count tail */
 
 	return 1;
@@ -4451,7 +4446,7 @@ tcp_softlro(struct mbuf *mhead, struct m
 void
 tcp_softlro_glue(struct mbuf_list *ml, struct mbuf *mtail, struct ifnet *ifp)
 {
-	struct ether_extracted	 tail;
+	struct ether_extracted head, tail;
 	struct mbuf *mhead;
 
 	if (!ISSET(ifp->if_xflags, IFXF_LRO))
@@ -4492,7 +4487,8 @@ tcp_softlro_glue(struct mbuf_list *ml, s
 				continue;
 		}
 
-		if (tcp_softlro(mhead, mtail))
+		ether_extract_headers(mhead, &head);
+		if (tcp_softlro(mhead, &head, mtail, &tail))
 			return;
 	}
  out: