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 15:02:37 +0200

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.
> > 
> > ok?
> 
> Crashes in my veb(4) test with vio(4)

Fix it and improved error handling.

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 12:28:40 -0000
@@ -938,24 +938,84 @@ veb_ipsec_out(struct ifnet *ifp0, struct
 #endif /* IPSEC */
 
 static struct mbuf *
-veb_offload(struct ifnet *ifp, struct ifnet *ifp0, struct mbuf *m)
+veb_offload(struct ifnet *ifp, struct ifnet *ifp0, struct mbuf *m0)
 {
+	struct ether_extracted ext;
+	int csum = 0;
+
 #if NVLAN > 0
-	if (ISSET(m->m_flags, M_VLANTAG) &&
+	if (ISSET(m0->m_flags, M_VLANTAG) &&
 	    !ISSET(ifp0->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
 		/*
 		 * If the underlying interface has no VLAN hardware tagging
 		 * 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;
-		}
+		m0 = vlan_inject(m0, ETHERTYPE_VLAN, m0->m_pkthdr.ether_vtag);
+		if (m0 == NULL)
+			goto drop;
 	}
 #endif
 
-	return m;
+	if (ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) &&
+	    !ISSET(ifp0->if_capabilities, IFCAP_CSUM_IPv4))
+		csum = 1;
+
+	if (ISSET(m0->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(m0->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(m0, &ext);
+
+		ethlen = sizeof *ext.eh;
+		if (ext.evh)
+			ethlen = sizeof *ext.evh;
+
+		hlen = m0->m_pkthdr.len - ext.paylen;
+
+		if (m0->m_len < hlen) {
+			struct mbuf *m = m_pullup(m0, hlen);
+			if (m == NULL)
+				goto drop;
+
+			if (m != m0)	/* should not happen */
+				goto drop;
+		}
+		/* hide ethernet header */
+		m0->m_data += ethlen;
+		m0->m_len -= ethlen;
+		m0->m_pkthdr.len -= ethlen;
+
+		if (ext.ip4) {
+			in_hdr_cksum_out(m0, ifp0);
+			in_proto_cksum_out(m0, ifp0);
+#ifdef INET6
+		} else if (ext.ip6) {
+			in6_proto_cksum_out(m0, ifp0);
+#endif
+		}
+
+		/* show ethernet header again */
+		m0->m_data -= ethlen;
+		m0->m_len += ethlen;
+		m0->m_pkthdr.len += ethlen;
+	}
+
+	return m0;
+
+ drop:
+	m_freem(m0);
+	counters_inc(ifp->if_counters, ifc_ierrors);
+	return NULL;
 }
 
 static void
@@ -1028,7 +1088,7 @@ veb_broadcast(struct veb_softc *sc, stru
 			continue;
 
 		if ((m0 = veb_offload(ifp, ifp0, m0)) == NULL)
-			goto done;
+			return;
 
 		m = m_dup_pkt(m0, max_linkhdr + ETHER_ALIGN, M_NOWAIT);
 		if (m == NULL) {
@@ -1083,7 +1143,7 @@ veb_transmit(struct veb_softc *sc, struc
 	    m->m_pkthdr.len);
 
 	if ((m = veb_offload(ifp, ifp0, m)) == NULL)
-		goto drop;
+		return (NULL);
 
 	(*tp->p_enqueue)(ifp0, m); /* XXX count error */
 
@@ -2372,7 +2432,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);