Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: reduce redundant load_mbuf code
To:
Jan Klemkow <jan@openbsd.org>
Cc:
tech@openbsd.org
Date:
Tue, 12 Aug 2025 20:42:36 +0200

Download raw body.

Thread
On Tue, Aug 12, 2025 at 08:39:02PM +0200, Jan Klemkow wrote:
> Hi,
> 
> This diff reduces the redundant bus_dmamap_load_mbuf(), m_defrag() and
> bus_dmamap_load_mbuf() call chain.  In many other interface drivers are
> similar implementations with switch/case statements, which could be
> reduced with the same function but in a separate diff.
> 
> ok?

Please don't do that. The goal is to put m_defrag into
bus_dmamap_load_mbuf() and not hide it behind yet another abstraction.
 
> bye,
> Jan
> 
> Index: dev/pci/if_iavf.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_iavf.c,v
> diff -u -p -r1.25 if_iavf.c
> --- dev/pci/if_iavf.c	24 Jun 2025 10:59:15 -0000	1.25
> +++ dev/pci/if_iavf.c	12 Aug 2025 17:30:35 -0000
> @@ -1915,24 +1915,6 @@ iavf_txr_free(struct iavf_softc *sc, str
>  	free(txr, M_DEVBUF, sizeof(*txr));
>  }
>  
> -static inline int
> -iavf_load_mbuf(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> -{
> -	int error;
> -
> -	error = bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> -	if (error != EFBIG)
> -		return (error);
> -
> -	error = m_defrag(m, M_DONTWAIT);
> -	if (error != 0)
> -		return (error);
> -
> -	return (bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
> -}
> -
>  static uint64_t
>  iavf_tx_offload(struct mbuf *m, struct iavf_tx_ring *txr, unsigned int prod)
>  {
> @@ -2072,7 +2054,7 @@ iavf_start(struct ifqueue *ifq)
>  			free--;
>  		}
>  
> -		if (iavf_load_mbuf(sc->sc_dmat, map, m) != 0) {
> +		if (m_load_dmamap(sc->sc_dmat, map, m) != 0) {
>  			ifq->ifq_errors++;
>  			m_freem(m);
>  			continue;
> Index: dev/pci/if_ice.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_ice.c,v
> diff -u -p -r1.56 if_ice.c
> --- dev/pci/if_ice.c	17 Jul 2025 09:19:21 -0000	1.56
> +++ dev/pci/if_ice.c	12 Aug 2025 17:07:02 -0000
> @@ -13791,24 +13791,6 @@ ice_tx_setup_offload(struct mbuf *m0, st
>  	return offload;
>  }
>  
> -static inline int
> -ice_load_mbuf(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> -{
> -	int error;
> -
> -	error = bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> -	if (error != EFBIG)
> -		return (error);
> -
> -	error = m_defrag(m, M_DONTWAIT);
> -	if (error != 0)
> -		return (error);
> -
> -	return (bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
> -}
> -
>  void
>  ice_set_tso_context(struct mbuf *m0, struct ice_tx_queue *txq,
>      unsigned int prod, struct ether_extracted *ext)
> @@ -13905,7 +13887,7 @@ ice_start(struct ifqueue *ifq)
>  		txm = &txq->tx_map[prod];
>  		map = txm->txm_map;
>  
> -		if (ice_load_mbuf(sc->sc_dmat, map, m) != 0) {
> +		if (m_load_dmamap(sc->sc_dmat, map, m) != 0) {
>  			ifq->ifq_errors++;
>  			m_freem(m);
>  			continue;
> Index: dev/pci/if_igc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_igc.c,v
> diff -u -p -r1.28 if_igc.c
> --- dev/pci/if_igc.c	24 Jun 2025 11:00:27 -0000	1.28
> +++ dev/pci/if_igc.c	12 Aug 2025 17:08:18 -0000
> @@ -937,24 +937,6 @@ igc_init(void *arg)
>  	splx(s);
>  }
>  
> -static inline int
> -igc_load_mbuf(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> -{
> -	int error;
> -
> -	error = bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> -	if (error != EFBIG)
> -		return (error);
> -
> -	error = m_defrag(m, M_DONTWAIT);
> -	if (error != 0)
> -		return (error);
> -
> -	return (bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
> -}
> -
>  void
>  igc_start(struct ifqueue *ifq)
>  {
> @@ -1003,7 +985,7 @@ igc_start(struct ifqueue *ifq)
>  		txbuf = &txr->tx_buffers[prod];
>  		map = txbuf->map;
>  
> -		if (igc_load_mbuf(txr->txdma.dma_tag, map, m) != 0) {
> +		if (m_load_dmamap(txr->txdma.dma_tag, map, m) != 0) {
>  			ifq->ifq_errors++;
>  			m_freem(m);
>  			continue;
> Index: dev/pci/if_ixl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v
> diff -u -p -r1.108 if_ixl.c
> --- dev/pci/if_ixl.c	24 Jun 2025 11:03:10 -0000	1.108
> +++ dev/pci/if_ixl.c	12 Aug 2025 17:06:16 -0000
> @@ -2746,24 +2746,6 @@ ixl_txr_free(struct ixl_softc *sc, struc
>  	free(txr, M_DEVBUF, sizeof(*txr));
>  }
>  
> -static inline int
> -ixl_load_mbuf(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> -{
> -	int error;
> -
> -	error = bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> -	if (error != EFBIG)
> -		return (error);
> -
> -	error = m_defrag(m, M_DONTWAIT);
> -	if (error != 0)
> -		return (error);
> -
> -	return (bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
> -}
> -
>  static uint64_t
>  ixl_tx_setup_offload(struct mbuf *m0, struct ixl_tx_ring *txr,
>      unsigned int prod)
> @@ -2905,7 +2887,7 @@ ixl_start(struct ifqueue *ifq)
>  			free--;
>  		}
>  
> -		if (ixl_load_mbuf(sc->sc_dmat, map, m) != 0) {
> +		if (m_load_dmamap(sc->sc_dmat, map, m) != 0) {
>  			ifq->ifq_errors++;
>  			m_freem(m);
>  			continue;
> Index: dev/pci/if_vmx.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
> diff -u -p -r1.93 if_vmx.c
> --- dev/pci/if_vmx.c	19 Jun 2025 09:36:21 -0000	1.93
> +++ dev/pci/if_vmx.c	12 Aug 2025 17:07:26 -0000
> @@ -1535,24 +1535,6 @@ vmxnet3_ioctl(struct ifnet *ifp, u_long 
>  	return error;
>  }
>  
> -static inline int
> -vmx_load_mbuf(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> -{
> -	int error;
> -
> -	error = bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> -	if (error != EFBIG)
> -		return (error);
> -
> -	error = m_defrag(m, M_DONTWAIT);
> -	if (error != 0)
> -		return (error);
> -
> -	return (bus_dmamap_load_mbuf(dmat, map, m,
> -	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
> -}
> -
>  void
>  vmxnet3_tx_offload(struct vmxnet3_txdesc *sop, struct mbuf *m)
>  {
> @@ -1695,7 +1677,7 @@ vmxnet3_start(struct ifqueue *ifq)
>  
>  		map = ring->dmap[prod];
>  
> -		if (vmx_load_mbuf(sc->sc_dmat, map, m) != 0) {
> +		if (m_load_dmamap(sc->sc_dmat, map, m) != 0) {
>  			ifq->ifq_errors++;
>  			m_freem(m);
>  			continue;
> Index: sys/mbuf.h
> ===================================================================
> RCS file: /cvs/src/sys/sys/mbuf.h,v
> diff -u -p -r1.267 mbuf.h
> --- sys/mbuf.h	25 Jun 2025 20:26:32 -0000	1.267
> +++ sys/mbuf.h	12 Aug 2025 17:10:52 -0000
> @@ -35,6 +35,7 @@
>  #ifndef _SYS_MBUF_H_
>  #define _SYS_MBUF_H_
>  
> +#include <machine/bus.h>
>  #include <sys/queue.h>
>  
>  /*
> @@ -464,6 +465,24 @@ m_freemp(struct mbuf **mp)
>  
>  	*mp = NULL;
>  	return m_freem(m);
> +}
> +
> +static inline int
> +m_load_dmamap(bus_dma_tag_t dmat, bus_dmamap_t map, struct mbuf *m)
> +{
> +	int error;
> +
> +	error = bus_dmamap_load_mbuf(dmat, map, m,
> +	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT);
> +	if (error != EFBIG)
> +		return (error);
> +
> +	error = m_defrag(m, M_DONTWAIT);
> +	if (error != 0)
> +		return (error);
> +
> +	return (bus_dmamap_load_mbuf(dmat, map, m,
> +	    BUS_DMA_STREAMING | BUS_DMA_NOWAIT));
>  }
>  
>  #include <sys/percpu.h>
> 

-- 
:wq Claudio