Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: octeon: fix cnmac corrupting VLAN packets transmitted by veb
To:
Sergii Rudchenko <sergii@rudchenko.nl>, OpenBSD tech <tech@openbsd.org>
Date:
Tue, 14 Jul 2026 12:52:22 +0200

Download raw body.

Thread
On Mon, 13 Jul 2026 23:30:50 +0200,
Sergii Rudchenko <sergii@rudchenko.nl> wrote:
> 
> On an EdgeRouter 6P, cnmac(4) corrupts VLAN tagged TCP and UDP packets
> transmitted by veb(4).
> 
> It happens because the TCP/UDP checksum offloading setup in cnmac(4) assumes
> that IP packet starts right after ethernet header:
> 
>        ipoffp1 = (m->m_pkthdr.csum_flags & (M_TCP_CSUM_OUT |
> M_UDP_CSUM_OUT))
>            ? (ETHER_HDR_LEN + 1) : 0;
> 
> For VLAN-encapsulated packets it produces a wrong checksum written by a
> wrong
> offset, resulting in corrupt TCP and UDP packets.
> 
> vlan(4) are not affected by this problem because unline veb(4) it does not
> declare IFCAP_CSUM_* capabilities and all checksums are calculated
> before the
> packet reaches cnmac(4).
> 
> 
> Test setup (on the EdgeRouter):
> 
> # ifconfig vport0 inet 192.168.0.1/24
> # ifconfig veb0 add cnmac1 tagged cnmac1 1 -untagged cnmac1
> # ifconfig veb0 add vport0 tagged vport0 1 untagged vport0 1 up
> # ifconfig cnmac1 up
> # nc -l 8000
> 
> On a peer machine with 192.168.0.2/24 assigned to a VLAN trunk interface
> connected to EdgeRouter:
> 
> $ echo Test | nc 192.168.0.1 8000
> 
> 
> Expected result: nc(1) on the client completes immediately and nc(1) the
> EdgeRouter prints "Test" and completes.
> 
> Actual result: nc(1) on the client hangs, nc(1) on on the EdgeRouter
> does not
> print anything. Examining the traffic on the client with Wireshark shows
> that
> TCP SYN,SYN_ACK packets from EdgeRouter have an invalid TCP option.
> Comparing
> to tcpdump on cnmac1 reveals two altered bytes at offsets 0x46 and 0x47.
> 
> 

Thanks for detailed use case and explanation of the bug!


> @@ -881,8 +884,19 @@ cnmac_send_makecmd(struct cnmac_softc *s
>         }
> 
>         /* Get the IP packet offset for TCP/UDP checksum offloading. */
> -       ipoffp1 = (m->m_pkthdr.csum_flags & (M_TCP_CSUM_OUT |
> M_UDP_CSUM_OUT))
> -           ? (ETHER_HDR_LEN + 1) : 0;
> +       if (m->m_pkthdr.csum_flags & (M_TCP_CSUM_OUT | M_UDP_CSUM_OUT)) {
> +               eh = mtod(m, struct ether_header *);
> +               ether_type = ntohs(eh->ether_type);
> +
> +               if (ether_type == ETHERTYPE_IP) {
> +                       ipoffp1 = ETHER_HDR_LEN + 1;
> +               } else if (ether_type == ETHERTYPE_VLAN) {
> +                       evh = mtod(m, struct ether_vlan_header*);
> +                       if (ntohs(evh->evl_proto) == ETHERTYPE_IP) {
> +                               ipoffp1 = sizeof(*evh) + 1;
> +                       }
> +               }
> +       }
> 

I think the issue is not cnmac related, this code reads sane for me.

My take that issue in ether_offload_ifcap()

Somethink like that, I think:

Index: sys/net/if_ethersubr.c
===================================================================
RCS file: /home/cvs/src/sys/net/if_ethersubr.c,v
diff -u -p -r1.308 if_ethersubr.c
--- sys/net/if_ethersubr.c	19 Dec 2025 02:04:13 -0000	1.308
+++ sys/net/if_ethersubr.c	14 Jul 2026 10:47:15 -0000
@@ -1267,6 +1267,7 @@ struct mbuf *
 ether_offload_ifcap(struct ifnet *ifp, struct mbuf *m)
 {
 	struct ether_extracted ext;
+	struct ifnet *csum_ifp = ifp;
 	int csum = 0;
 
 #if NVLAN > 0
@@ -1274,8 +1275,11 @@ ether_offload_ifcap(struct ifnet *ifp, s
 	    !ISSET(ifp->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
 		/*
 		 * If the underlying interface has no VLAN hardware tagging
-		 * support, inject one in software.
+		 * support, inject one in software. Calculate checksums in
+		 * software unless it supports offloading with an inline tag.
 		 */
+		if (!ISSET(ifp->if_capabilities, IFCAP_VLAN_HWOFFLOAD))
+			csum_ifp = NULL;
 		m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
 		if (m == NULL)
 			return (NULL);
@@ -1283,16 +1287,19 @@ ether_offload_ifcap(struct ifnet *ifp, s
 #endif
 
 	if (ISSET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) &&
-	    !ISSET(ifp->if_capabilities, IFCAP_CSUM_IPv4))
+	    (csum_ifp == NULL ||
+	     !ISSET(ifp->if_capabilities, IFCAP_CSUM_IPv4)))
 		csum = 1;
 
 	if (ISSET(m->m_pkthdr.csum_flags, M_TCP_CSUM_OUT) &&
-	    (!ISSET(ifp->if_capabilities, IFCAP_CSUM_TCPv4) ||
+	    (csum_ifp == NULL ||
+	     !ISSET(ifp->if_capabilities, IFCAP_CSUM_TCPv4) ||
 	     !ISSET(ifp->if_capabilities, IFCAP_CSUM_TCPv6)))
 		csum = 1;
 
 	if (ISSET(m->m_pkthdr.csum_flags, M_UDP_CSUM_OUT) &&
-	    (!ISSET(ifp->if_capabilities, IFCAP_CSUM_UDPv4) ||
+	    (csum_ifp == NULL ||
+	     !ISSET(ifp->if_capabilities, IFCAP_CSUM_UDPv4) ||
 	     !ISSET(ifp->if_capabilities, IFCAP_CSUM_UDPv6)))
 		csum = 1;
 
@@ -1320,11 +1327,11 @@ ether_offload_ifcap(struct ifnet *ifp, s
 		m->m_pkthdr.len -= ethlen;
 
 		if (ext.ip4) {
-			in_hdr_cksum_out(m, ifp);
-			in_proto_cksum_out(m, ifp);
+			in_hdr_cksum_out(m, csum_ifp);
+			in_proto_cksum_out(m, csum_ifp);
 #ifdef INET6
 		} else if (ext.ip6) {
-			in6_proto_cksum_out(m, ifp);
+			in6_proto_cksum_out(m, csum_ifp);
 #endif
 		}
 


-- 
wbr, Kirill