From: Marcus Glocker Subject: Re: ether extract header IP length To: Alexander Bluhm Cc: tech@openbsd.org Date: Wed, 14 Feb 2024 22:24:22 +0100 On Wed, Feb 14, 2024 at 09:59:08PM +0100, Alexander Bluhm wrote: > Hi, > > To detect ethernet padding, LRO in ix(4) has to compare packet > length with IP length. So I would like to extract ip_len and > ip6_plen and provide it to the drivers. Also more sanitity checks > can be done, like IP packet is shorter than TCP header. Then we > don't ask the network hardware to do some offloading with bougus > packets. Now iphlen contains header lenght for IPv4 and IPv6, to > make code in drivers simpler. > > Tested on sparc64. > > ok? > > bluhm As already discussed off-list. Tested on Hrvoje's em(4), ix(4), vlan(4) amd64 setup. ok mglocker@ > Index: dev/pci/if_bnxt.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_bnxt.c,v > diff -u -p -r1.46 if_bnxt.c > --- dev/pci/if_bnxt.c 13 Feb 2024 13:58:19 -0000 1.46 > +++ dev/pci/if_bnxt.c 14 Feb 2024 20:30:52 -0000 > @@ -1432,10 +1432,8 @@ bnxt_start(struct ifqueue *ifq) > if (ext.tcp) { > lflags |= TX_BD_LONG_LFLAGS_LSO; > hdrsize = sizeof(*ext.eh); > - if (ext.ip4) > - hdrsize += ext.ip4hlen; > - else if (ext.ip6) > - hdrsize += sizeof(*ext.ip6); > + if (ext.ip4 || ext.ip6) > + hdrsize += ext.iphlen; > else > tcpstat_inc(tcps_outbadtso); > > Index: dev/pci/if_em.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_em.c,v > diff -u -p -r1.372 if_em.c > --- dev/pci/if_em.c 13 Feb 2024 13:58:19 -0000 1.372 > +++ dev/pci/if_em.c 14 Feb 2024 20:30:52 -0000 > @@ -2413,7 +2413,6 @@ em_tx_ctx_setup(struct em_queue *que, st > struct e1000_adv_tx_context_desc *TD; > uint32_t vlan_macip_lens = 0, type_tucmd_mlhl = 0, mss_l4len_idx = 0; > int off = 0; > - uint8_t iphlen; > > *olinfo_status = 0; > *cmd_type_len = 0; > @@ -2433,8 +2432,6 @@ em_tx_ctx_setup(struct em_queue *que, st > vlan_macip_lens |= (sizeof(*ext.eh) << E1000_ADVTXD_MACLEN_SHIFT); > > if (ext.ip4) { > - iphlen = ext.ip4hlen; > - > type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV4; > if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { > *olinfo_status |= E1000_TXD_POPTS_IXSM << 8; > @@ -2442,18 +2439,14 @@ em_tx_ctx_setup(struct em_queue *que, st > } > #ifdef INET6 > } else if (ext.ip6) { > - iphlen = sizeof(*ext.ip6); > - > type_tucmd_mlhl |= E1000_ADVTXD_TUCMD_IPV6; > #endif > - } else { > - iphlen = 0; > } > > *cmd_type_len |= E1000_ADVTXD_DTYP_DATA | E1000_ADVTXD_DCMD_IFCS; > *cmd_type_len |= E1000_ADVTXD_DCMD_DEXT; > *olinfo_status |= mp->m_pkthdr.len << E1000_ADVTXD_PAYLEN_SHIFT; > - vlan_macip_lens |= iphlen; > + vlan_macip_lens |= ext.iphlen; > type_tucmd_mlhl |= E1000_ADVTXD_DCMD_DEXT | E1000_ADVTXD_DTYP_CTXT; > > if (ext.tcp) { > Index: dev/pci/if_igc.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_igc.c,v > diff -u -p -r1.16 if_igc.c > --- dev/pci/if_igc.c 13 Feb 2024 13:58:19 -0000 1.16 > +++ dev/pci/if_igc.c 14 Feb 2024 20:30:52 -0000 > @@ -2005,7 +2005,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, st > struct igc_adv_tx_context_desc *txdesc; > uint32_t type_tucmd_mlhl = 0; > uint32_t vlan_macip_lens = 0; > - uint32_t iphlen; > int off = 0; > > vlan_macip_lens |= (sizeof(*ext.eh) << IGC_ADVTXD_MACLEN_SHIFT); > @@ -2028,8 +2027,6 @@ igc_tx_ctx_setup(struct tx_ring *txr, st > ether_extract_headers(mp, &ext); > > if (ext.ip4) { > - iphlen = ext.ip4hlen; > - > type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV4; > if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { > *olinfo_status |= IGC_TXD_POPTS_IXSM << 8; > @@ -2037,15 +2034,13 @@ igc_tx_ctx_setup(struct tx_ring *txr, st > } > #ifdef INET6 > } else if (ext.ip6) { > - iphlen = sizeof(*ext.ip6); > - > type_tucmd_mlhl |= IGC_ADVTXD_TUCMD_IPV6; > #endif > } else { > return 0; > } > > - vlan_macip_lens |= iphlen; > + vlan_macip_lens |= ext.iphlen; > type_tucmd_mlhl |= IGC_ADVTXD_DCMD_DEXT | IGC_ADVTXD_DTYP_CTXT; > > if (ext.tcp) { > Index: dev/pci/if_ix.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ix.c,v > diff -u -p -r1.207 if_ix.c > --- dev/pci/if_ix.c 13 Feb 2024 13:58:19 -0000 1.207 > +++ dev/pci/if_ix.c 14 Feb 2024 20:30:52 -0000 > @@ -2494,16 +2494,12 @@ ixgbe_tx_offload(struct mbuf *mp, uint32 > { > struct ether_extracted ext; > int offload = 0; > - uint32_t ethlen, iphlen; > > ether_extract_headers(mp, &ext); > - ethlen = sizeof(*ext.eh); > > - *vlan_macip_lens |= (ethlen << IXGBE_ADVTXD_MACLEN_SHIFT); > + *vlan_macip_lens |= (sizeof(*ext.eh) << IXGBE_ADVTXD_MACLEN_SHIFT); > > if (ext.ip4) { > - iphlen = ext.ip4hlen; > - > if (ISSET(mp->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT)) { > *olinfo_status |= IXGBE_TXD_POPTS_IXSM << 8; > offload = 1; > @@ -2512,8 +2508,6 @@ ixgbe_tx_offload(struct mbuf *mp, uint32 > *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV4; > #ifdef INET6 > } else if (ext.ip6) { > - iphlen = sizeof(*ext.ip6); > - > *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_IPV6; > #endif > } else { > @@ -2522,7 +2516,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32 > return offload; > } > > - *vlan_macip_lens |= iphlen; > + *vlan_macip_lens |= ext.iphlen; > > if (ext.tcp) { > *type_tucmd_mlhl |= IXGBE_ADVTXD_TUCMD_L4T_TCP; > @@ -2548,7 +2542,7 @@ ixgbe_tx_offload(struct mbuf *mp, uint32 > *mss_l4len_idx |= outlen << IXGBE_ADVTXD_MSS_SHIFT; > *mss_l4len_idx |= thlen << IXGBE_ADVTXD_L4LEN_SHIFT; > > - hdrlen = ethlen + iphlen + thlen; > + hdrlen = sizeof(*ext.eh) + ext.iphlen + thlen; > paylen = mp->m_pkthdr.len - hdrlen; > CLR(*olinfo_status, IXGBE_ADVTXD_PAYLEN_MASK > << IXGBE_ADVTXD_PAYLEN_SHIFT); > @@ -3276,10 +3270,8 @@ ixgbe_rxeof(struct rx_ring *rxr) > ext.evh) > hdrlen += ETHER_VLAN_ENCAP_LEN; > #endif > - if (ext.ip4) > - hdrlen += ext.ip4hlen; > - if (ext.ip6) > - hdrlen += sizeof(*ext.ip6); > + if (ext.ip4 || ext.ip6) > + hdrlen += ext.iphlen; > if (ext.tcp) { > hdrlen += ext.tcphlen; > tcpstat_inc(tcps_inhwlro); > Index: dev/pci/if_ixl.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ixl.c,v > diff -u -p -r1.96 if_ixl.c > --- dev/pci/if_ixl.c 13 Feb 2024 13:58:19 -0000 1.96 > +++ dev/pci/if_ixl.c 14 Feb 2024 20:30:52 -0000 > @@ -2826,18 +2826,15 @@ ixl_tx_setup_offload(struct mbuf *m0, st > offload |= ISSET(m0->m_pkthdr.csum_flags, M_IPV4_CSUM_OUT) ? > IXL_TX_DESC_CMD_IIPT_IPV4_CSUM : > IXL_TX_DESC_CMD_IIPT_IPV4; > - > - hlen = ext.ip4hlen; > #ifdef INET6 > } else if (ext.ip6) { > offload |= IXL_TX_DESC_CMD_IIPT_IPV6; > - > - hlen = sizeof(*ext.ip6); > #endif > } else { > panic("CSUM_OUT set for non-IP packet"); > /* NOTREACHED */ > } > + hlen = ext.iphlen; > > offload |= (ETHER_HDR_LEN >> 1) << IXL_TX_DESC_MACLEN_SHIFT; > offload |= (hlen >> 2) << IXL_TX_DESC_IPLEN_SHIFT; > Index: dev/pv/if_vio.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pv/if_vio.c,v > diff -u -p -r1.30 if_vio.c > --- dev/pv/if_vio.c 13 Feb 2024 13:58:19 -0000 1.30 > +++ dev/pv/if_vio.c 14 Feb 2024 20:30:52 -0000 > @@ -764,12 +764,8 @@ again: > else > hdr->csum_offset = offsetof(struct udphdr, uh_sum); > > - if (ext.ip4) > - hdr->csum_start += ext.ip4hlen; > -#ifdef INET6 > - else if (ext.ip6) > - hdr->csum_start += sizeof(*ext.ip6); > -#endif > + if (ext.ip4 || ext.ip6) > + hdr->csum_start += ext.iphlen; > hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM; > } > > Index: net/if_ethersubr.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if_ethersubr.c,v > diff -u -p -r1.292 if_ethersubr.c > --- net/if_ethersubr.c 13 Feb 2024 13:58:19 -0000 1.292 > +++ net/if_ethersubr.c 14 Feb 2024 20:30:52 -0000 > @@ -1051,7 +1051,7 @@ void > ether_extract_headers(struct mbuf *m0, struct ether_extracted *ext) > { > struct mbuf *m; > - size_t hlen; > + size_t hlen, iplen; > int hoff; > uint8_t ipproto; > uint16_t ether_type; > @@ -1143,7 +1143,19 @@ ether_extract_headers(struct mbuf *m0, s > ext->ip4 = NULL; > return; > } > - ext->ip4hlen = hlen; > + iplen = ntohs(ext->ip4->ip_len); > + if (ext->paylen < iplen) { > + DPRINTF("paylen %u, ip4len %zu", ext->paylen, iplen); > + ext->ip4 = NULL; > + return; > + } > + if (iplen < hlen) { > + DPRINTF("ip4len %zu, ip4hlen %zu", iplen, hlen); > + ext->ip4 = NULL; > + return; > + } > + ext->iplen = iplen; > + ext->iphlen = hlen; > ext->paylen -= hlen; > ipproto = ext->ip4->ip_p; > > @@ -1166,6 +1178,14 @@ ether_extract_headers(struct mbuf *m0, s > ext->ip6 = NULL; > return; > } > + iplen = hlen + ntohs(ext->ip6->ip6_plen); > + if (ext->paylen < iplen) { > + DPRINTF("paylen %u, ip6len %zu", ext->paylen, iplen); > + ext->ip6 = NULL; > + return; > + } > + ext->iplen = iplen; > + ext->iphlen = hlen; > ext->paylen -= hlen; > ipproto = ext->ip6->ip6_nxt; > break; > @@ -1192,8 +1212,9 @@ ether_extract_headers(struct mbuf *m0, s > ext->tcp = NULL; > return; > } > - if (ext->paylen < hlen) { > - DPRINTF("paylen %u, tcphlen %zu", ext->paylen, hlen); > + if (ext->iplen - ext->iphlen < hlen) { > + DPRINTF("iplen %u, iphlen %u, tcphlen %zu", > + ext->iplen, ext->iphlen, hlen); > ext->tcp = NULL; > return; > } > @@ -1211,17 +1232,18 @@ ether_extract_headers(struct mbuf *m0, s > ext->udp = (struct udphdr *)(mtod(m, caddr_t) + hoff); > > hlen = sizeof(*ext->udp); > - if (ext->paylen < hlen) { > - DPRINTF("paylen %u, udphlen %zu", ext->paylen, hlen); > + if (ext->iplen - ext->iphlen < hlen) { > + DPRINTF("iplen %u, iphlen %u, udphlen %zu", > + ext->iplen, ext->iphlen, hlen); > ext->udp = NULL; > return; > } > break; > } > > - DNPRINTF(2, "%s%s%s%s%s%s ip4h %u, tcph %u, payl %u", > + DNPRINTF(2, "%s%s%s%s%s%s ip %u, iph %u, tcph %u, payl %u", > ext->eh ? "eh," : "", ext->evh ? "evh," : "", > ext->ip4 ? "ip4," : "", ext->ip6 ? "ip6," : "", > ext->tcp ? "tcp," : "", ext->udp ? "udp," : "", > - ext->ip4hlen, ext->tcphlen, ext->paylen); > + ext->iplen, ext->iphlen, ext->tcphlen, ext->paylen); > } > Index: netinet/if_ether.h > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/if_ether.h,v > diff -u -p -r1.91 if_ether.h > --- netinet/if_ether.h 13 Feb 2024 13:58:19 -0000 1.91 > +++ netinet/if_ether.h 14 Feb 2024 20:30:52 -0000 > @@ -307,7 +307,8 @@ struct ether_extracted { > struct ip6_hdr *ip6; > struct tcphdr *tcp; > struct udphdr *udp; > - u_int ip4hlen; > + u_int iplen; > + u_int iphlen; > u_int tcphlen; > u_int paylen; > }; >