From: Visa Hankala Subject: Re: octeon: fix cnmac corrupting VLAN packets transmitted by veb To: "Kirill A. Korinsky" Cc: sergii@rudchenko.nl, tech@openbsd.org Date: Tue, 14 Jul 2026 11:52:44 +0000 On Tue, Jul 14, 2026 at 12:52:22PM +0200, Kirill A. Korinsky wrote: > On Mon, 13 Jul 2026 23:30:50 +0200, > Sergii Rudchenko 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 > } > It is relatively common that a NIC can compute the TCP/UDP checksum in the presence of a VLAN header even if the NIC did not support VLAN tag insertion. I think the initial patch could be improved a little by using ether_extract_headers(). This avoids adding another custom packet peeker, and adds IPv6 handling. Index: arch/octeon/dev/if_cnmac.c =================================================================== RCS file: src/sys/arch/octeon/dev/if_cnmac.c,v retrieving revision 1.91 diff -u -p -r1.91 if_cnmac.c --- arch/octeon/dev/if_cnmac.c +++ arch/octeon/dev/if_cnmac.c @@ -868,8 +868,9 @@ int cnmac_send_makecmd(struct cnmac_softc *sc, struct mbuf *m, uint64_t *gbuf, uint64_t *rpko_cmd_w0, uint64_t *rpko_cmd_w1) { + struct ether_extracted ext; uint64_t pko_cmd_w0, pko_cmd_w1; - int ipoffp1; + int ipoffp1 = 0; int segs; int result = 0; @@ -881,8 +882,16 @@ 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)) { + ether_extract_headers(m, &ext); + + if (ext.tcp || ext.udp) { + if (ext.evh) + ipoffp1 = sizeof(*ext.evh) + 1; + else + ipoffp1 = sizeof(*ext.eh) + 1; + } + } /* * segs == 1 -> link mode (single continuous buffer)