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: Fri, 1 Aug 2025 17:43:03 +0200

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:

ok?

Thanks,
Jan

Index: net/if_veb.c
===================================================================
RCS file: /cvs/src/sys/net/if_veb.c,v
diff -u -p -r1.41 if_veb.c
--- net/if_veb.c	7 Jul 2025 02:28:50 -0000	1.41
+++ net/if_veb.c	1 Aug 2025 15:33:03 -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,13 +951,63 @@ 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)
+			return (NULL);
 	}
 #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)
+				return NULL;
+		}
+
+		/* 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;
 }
 
@@ -1027,8 +1080,10 @@ veb_broadcast(struct veb_softc *sc, stru
 		if (veb_rule_filter(tp, VEB_RULE_LIST_OUT, m0, src, dst))
 			continue;
 
-		if ((m0 = veb_offload(ifp, ifp0, m0)) == NULL)
-			goto done;
+		if ((m0 = veb_offload(ifp, ifp0, m0)) == NULL) {
+			counters_inc(ifp->if_counters, ifc_ierrors);
+			return;
+		}
 
 		m = m_dup_pkt(m0, max_linkhdr + ETHER_ALIGN, M_NOWAIT);
 		if (m == NULL) {
@@ -1082,8 +1137,10 @@ veb_transmit(struct veb_softc *sc, struc
 	counters_pkt(ifp->if_counters, ifc_opackets, ifc_obytes,
 	    m->m_pkthdr.len);
 
-	if ((m = veb_offload(ifp, ifp0, m)) == NULL)
-		goto drop;
+	if ((m = veb_offload(ifp, ifp0, m)) == NULL) {
+		counters_inc(ifp->if_counters, ifc_ierrors);
+		return (NULL);
+	}
 
 	(*tp->p_enqueue)(ifp0, m); /* XXX count error */
 
@@ -2372,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);