From: Kirill A. Korinsky Subject: Re: octeon: fix cnmac corrupting VLAN packets transmitted by veb To: Visa Hankala Cc: sergii@rudchenko.nl, tech@openbsd.org Date: Tue, 14 Jul 2026 14:17:21 +0200 On Tue, 14 Jul 2026 13:52:44 +0200, Visa Hankala wrote: > > 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. > Indeed, your version is clean cnmac specific, I like it, FWIW OK kirill@ But I still think we need a global fix of ether_offload_ifcap() > 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) -- wbr, Kirill