From: Jan Klemkow <jan@openbsd.org>
Subject: Re: vport/veb(4): use/fix checksum offload
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Sat, 2 Aug 2025 19:20:34 +0200

On Fri, Aug 01, 2025 at 05:43:03PM +0200, Jan Klemkow wrote:
> On Fri, Aug 01, 2025 at 03:02:37PM +0200, Jan Klemkow wrote:
> > On Sun, Jun 29, 2025 at 07:57:15PM +0200, Alexander Bluhm wrote:
> > > On Fri, Jun 27, 2025 at 08:06:42PM +0200, Jan Klemkow wrote:
> > > > This diff enables checksum offload in vport(4) and fixes checksum
> > > > offload in veb(4) in some corner cases.  If we get packages with
> > > > M_TCP_CSUM_OUT from an interface.
> > > > 
> > > > When we bridge two vio(4) interfaces via veb(4), we will lost the
> > > > VIRTIO_NET_HDR_F_NEEDS_CSUM flags, which is encodes in our mbuf via
> > > > M_TCP_CSUM_OUT flag.
> > > > 
> > > > In the case of bridging vio(4) with an physical interface (e.g. em(4),
> > > > we also have to M_TCP_CSUM_OUT flag.  So, the hardware is able to
> > > > calculate the correct checksum.  Or, when we bridge vio(4) with an
> > > > interface unable to calculate we have to do this here, before sending
> > > > the packet to the interface.
> > > > 
> > > Crashes in my veb(4) test with vio(4)
> > 
> > Fix it and improved error handling.
> 
> bluhm pointed out several bugs.  So, here is a fixes version:

New diff after last fixes.

ok?

Thanks,
Jan

Index: net/if_veb.c
===================================================================
RCS file: /cvs/src/sys/net/if_veb.c,v
diff -u -p -r1.42 if_veb.c
--- net/if_veb.c	2 Aug 2025 10:19:37 -0000	1.42
+++ net/if_veb.c	2 Aug 2025 17:00:04 -0000
@@ -940,6 +940,9 @@ veb_ipsec_out(struct ifnet *ifp0, struct
 static struct mbuf *
 veb_offload(struct ifnet *ifp, struct ifnet *ifp0, struct mbuf *m)
 {
+	struct ether_extracted ext;
+	int csum = 0;
+
 #if NVLAN > 0
 	if (ISSET(m->m_flags, M_VLANTAG) &&
 	    !ISSET(ifp0->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
@@ -948,14 +951,67 @@ veb_offload(struct ifnet *ifp, struct if
 		 * support, inject one in software.
 		 */
 		m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
-		if (m == NULL) {
-			counters_inc(ifp->if_counters, ifc_ierrors);
-			return NULL;
-		}
+		if (m == NULL)
+			goto err;
 	}
 #endif
 
+	if (ISSET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) &&
+	    !ISSET(ifp0->if_capabilities, IFCAP_CSUM_IPv4))
+		csum = 1;
+
+	if (ISSET(m->m_pkthdr.csum_flags, M_TCP_CSUM_OUT) &&
+	    (!ISSET(ifp0->if_capabilities, IFCAP_CSUM_TCPv4) ||
+	     !ISSET(ifp0->if_capabilities, IFCAP_CSUM_TCPv6)))
+		csum = 1;
+
+	if (ISSET(m->m_pkthdr.csum_flags, M_UDP_CSUM_OUT) &&
+	    (!ISSET(ifp0->if_capabilities, IFCAP_CSUM_UDPv4) ||
+	     !ISSET(ifp0->if_capabilities, IFCAP_CSUM_UDPv6)))
+		csum = 1;
+
+	if (csum) {
+		int ethlen;
+		int hlen;
+
+		ether_extract_headers(m, &ext);
+
+		ethlen = sizeof *ext.eh;
+		if (ext.evh)
+			ethlen = sizeof *ext.evh;
+
+		hlen = m->m_pkthdr.len - ext.paylen;
+
+		if (m->m_len < hlen) {
+			m = m_pullup(m, hlen);
+			if (m == NULL)
+				goto err;
+		}
+
+		/* hide ethernet header */
+		m->m_data += ethlen;
+		m->m_len -= ethlen;
+		m->m_pkthdr.len -= ethlen;
+
+		if (ext.ip4) {
+			in_hdr_cksum_out(m, ifp0);
+			in_proto_cksum_out(m, ifp0);
+#ifdef INET6
+		} else if (ext.ip6) {
+			in6_proto_cksum_out(m, ifp0);
+#endif
+		}
+
+		/* show ethernet header again */
+		m->m_data -= ethlen;
+		m->m_len += ethlen;
+		m->m_pkthdr.len += ethlen;
+	}
+
 	return m;
+ err:
+	counters_inc(ifp->if_counters, ifc_ierrors);
+	return NULL;
 }
 
 static void
@@ -2373,7 +2429,15 @@ vport_clone_create(struct if_clone *ifc,
 	ifp->if_qstart = vport_start;
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
 	ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
-	ifp->if_capabilities = IFCAP_VLAN_HWTAGGING;
+
+	ifp->if_capabilities = 0;
+#if NVLAN > 0
+	ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
+#endif
+	ifp->if_capabilities |= IFCAP_CSUM_IPv4;
+	ifp->if_capabilities |= IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4;
+	ifp->if_capabilities |= IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6;
+
 	ether_fakeaddr(ifp);
 
 	if_counters_alloc(ifp);