Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: ether extract header IP length
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
tech@openbsd.org
Date:
Wed, 14 Feb 2024 22:24:22 +0100

Download raw body.

Thread
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;
>  };
>