From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: vmx(4): refactor rx offload
To: jan@openbsd.org
Cc: tech@openbsd.org
Date: Tue, 21 May 2024 20:58:07 +0200

On Tue, Apr 23, 2024 at 11:30:26AM +0200, jan@openbsd.org wrote:
> Hi,
> 
> I'm working on an LRO diff for vmx(4).  But, first I want to refactor
> the receive offload function.  Thus, the LRO diff fits better and the
> code it more uniform to other drivers.
> 
> Improvements are:
>  - move VLAN code into vmx_rx_offload(), as in vmx_tx_offload()
>  - remove useless letoh32() calls
>  - simplify the IPv4 checksum flag check
>  - avoid a combination of M_TCP_CSUM_IN_OK and M_UDP_CSUM_IN_OK
> 
> ok?

tested and OK bluhm@

> Index: dev/pci/if_vmx.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
> diff -u -p -r1.83 if_vmx.c
> --- dev/pci/if_vmx.c	2 Apr 2024 20:59:48 -0000	1.83
> +++ dev/pci/if_vmx.c	23 Apr 2024 09:15:39 -0000
> @@ -203,7 +203,7 @@ void vmxnet3_rxintr(struct vmxnet3_softc
>  void vmxnet3_rxfill_tick(void *);
>  void vmxnet3_rxfill(struct vmxnet3_rxring *);
>  void vmxnet3_iff(struct vmxnet3_softc *);
> -void vmxnet3_rx_csum(struct vmxnet3_rxcompdesc *, struct mbuf *);
> +void vmxnet3_rx_offload(struct vmxnet3_rxcompdesc *, struct mbuf *);
>  void vmxnet3_stop(struct ifnet *);
>  void vmxnet3_reset(struct vmxnet3_softc *);
>  int vmxnet3_init(struct vmxnet3_softc *);
> @@ -1129,14 +1129,8 @@ vmxnet3_rxintr(struct vmxnet3_softc *sc,
>  		}
>  		m->m_pkthdr.len = m->m_len = len;
>  
> -		vmxnet3_rx_csum(rxcd, m);
> -#if NVLAN > 0
> -		if (letoh32(rxcd->rxc_word2 & VMXNET3_RXC_VLAN)) {
> -			m->m_flags |= M_VLANTAG;
> -			m->m_pkthdr.ether_vtag = letoh32((rxcd->rxc_word2 >>
> -			    VMXNET3_RXC_VLANTAG_S) & VMXNET3_RXC_VLANTAG_M);
> -		}
> -#endif
> +		vmxnet3_rx_offload(rxcd, m);
> +
>  		if (((letoh32(rxcd->rxc_word0) >> VMXNET3_RXC_RSSTYPE_S) &
>  		    VMXNET3_RXC_RSSTYPE_M) != VMXNET3_RXC_RSSTYPE_NONE) {
>  			m->m_pkthdr.ph_flowid = letoh32(rxcd->rxc_word1);
> @@ -1217,22 +1211,39 @@ vmxnet3_iff(struct vmxnet3_softc *sc)
>  
>  
>  void
> -vmxnet3_rx_csum(struct vmxnet3_rxcompdesc *rxcd, struct mbuf *m)
> +vmxnet3_rx_offload(struct vmxnet3_rxcompdesc *rxcd, struct mbuf *m)
>  {
> -	if (letoh32(rxcd->rxc_word0 & VMXNET3_RXC_NOCSUM))
> +	/*
> +	 * VLAN Offload
> +	 */
> +
> +#if NVLAN > 0
> +	if (ISSET(rxcd->rxc_word2, VMXNET3_RXC_VLAN)) {
> +		SET(m->m_flags, M_VLANTAG);
> +		m->m_pkthdr.ether_vtag = letoh32((rxcd->rxc_word2 >>
> +		    VMXNET3_RXC_VLANTAG_S) & VMXNET3_RXC_VLANTAG_M);
> +	}
> +#endif
> +
> +	/*
> +	 * Checksum Offload
> +	 */
> +
> +	if (ISSET(rxcd->rxc_word0, VMXNET3_RXC_NOCSUM))
>  		return;
>  
> -	if ((rxcd->rxc_word3 & (VMXNET3_RXC_IPV4 | VMXNET3_RXC_IPSUM_OK)) ==
> -	    (VMXNET3_RXC_IPV4 | VMXNET3_RXC_IPSUM_OK))
> -		m->m_pkthdr.csum_flags |= M_IPV4_CSUM_IN_OK;
> +	if (ISSET(rxcd->rxc_word3, VMXNET3_RXC_IPV4) &&
> +	    ISSET(rxcd->rxc_word3, VMXNET3_RXC_IPSUM_OK))
> +		SET(m->m_pkthdr.csum_flags, M_IPV4_CSUM_IN_OK);
>  
> -	if (rxcd->rxc_word3 & VMXNET3_RXC_FRAGMENT)
> +	if (ISSET(rxcd->rxc_word3, VMXNET3_RXC_FRAGMENT))
>  		return;
>  
> -	if (rxcd->rxc_word3 & (VMXNET3_RXC_TCP | VMXNET3_RXC_UDP)) {
> -		if (rxcd->rxc_word3 & VMXNET3_RXC_CSUM_OK)
> -			m->m_pkthdr.csum_flags |=
> -			    M_TCP_CSUM_IN_OK | M_UDP_CSUM_IN_OK;
> +	if (ISSET(rxcd->rxc_word3, VMXNET3_RXC_CSUM_OK)) {
> +		if (ISSET(rxcd->rxc_word3, VMXNET3_RXC_TCP))
> +			SET(m->m_pkthdr.csum_flags, M_TCP_CSUM_IN_OK);
> +		else if (ISSET(rxcd->rxc_word3, VMXNET3_RXC_UDP))
> +			SET(m->m_pkthdr.csum_flags, M_UDP_CSUM_IN_OK);
>  	}
>  }
>