From 4b732459f030a568a93dbe31230c5a17caa035e5 Mon Sep 17 00:00:00 2001 From: Andrew Gallatin Date: Thu, 5 Nov 2026 11:35:41 -0500 Subject: [PATCH] iflib: Use a single tx dma map The need for a separate map for TSO and non-TSO seems to have arisen from a misunderstanding about the DMA requirements for Intel drivers like ice/ixl/iavf. Specifically, this Intel hardware has a quirk where no more than N (=8) segs can be DMA'ed per packet on the wire. This limit was enforced using a DMA map with a low segment limit for non-TSO and a driver specific check for TSO. This led to a *lot* of complexity in iflib (~80 extra lines of code that this change removes) when the limit can simply be enforced via the existing EFBIG mechanisms. This change adds a new flag, IFLIB_SPARSE_TX, that drivers for hardware with this quirk must set. When set, it causes non-TSO sends that exceed isc_tx_nsegment to be treated as if bus_dmamap_load_mbuf_sg() had returned EFBIG. Due to the API/KABI change, I've bumped the FreeBSD version as well. Note kgalazka suggested the flag, rather than a more awkward mechanism that I had in my initial patch. Reviewed by: kbowling Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D60175 --- sys/dev/iavf/if_iavf_iflib.c | 3 +- sys/dev/ice/if_ice_iflib.c | 4 +- sys/dev/ixl/if_ixl.c | 3 +- sys/net/iflib.c | 159 ++++++++--------------------------- sys/net/iflib.h | 5 +- sys/sys/param.h | 2 +- 6 files changed, 47 insertions(+), 129 deletions(-) diff --git a/sys/dev/iavf/if_iavf_iflib.c b/sys/dev/iavf/if_iavf_iflib.c index 4fb0237d2d7c..4f908a8d9a7b 100644 --- a/sys/dev/iavf/if_iavf_iflib.c +++ b/sys/dev/iavf/if_iavf_iflib.c @@ -234,7 +234,8 @@ static struct if_shared_ctx iavf_sctx = { .isc_vendor_info = iavf_vendor_info_array, .isc_driver_version = __DECONST(char *, iavf_driver_version), .isc_driver = &iavf_if_driver, - .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_NEED_ZERO_CSUM | IFLIB_TSO_INIT_IP | IFLIB_IS_VF, + .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_NEED_ZERO_CSUM | + IFLIB_TSO_INIT_IP | IFLIB_IS_VF | IFLIB_SPARSE_TX, .isc_nrxd_min = {IAVF_MIN_RING}, .isc_ntxd_min = {IAVF_MIN_RING}, diff --git a/sys/dev/ice/if_ice_iflib.c b/sys/dev/ice/if_ice_iflib.c index d5dca978601c..63c940dfa8a5 100644 --- a/sys/dev/ice/if_ice_iflib.c +++ b/sys/dev/ice/if_ice_iflib.c @@ -312,7 +312,7 @@ static struct if_shared_ctx ice_sctx = { * vectors manually instead of relying on iflib code to do this. */ .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_TSO_INIT_IP | - IFLIB_ADMIN_ALWAYS_RUN | IFLIB_SKIP_MSIX, + IFLIB_ADMIN_ALWAYS_RUN | IFLIB_SKIP_MSIX | IFLIB_SPARSE_TX, .isc_nrxd_min = {ICE_MIN_DESC_COUNT}, .isc_ntxd_min = {ICE_MIN_DESC_COUNT}, @@ -3760,7 +3760,7 @@ static struct if_shared_ctx ice_subif_sctx = { .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_TSO_INIT_IP | IFLIB_ADMIN_ALWAYS_RUN | IFLIB_SKIP_MSIX | - IFLIB_IS_VF, + IFLIB_IS_VF | IFLIB_SPARSE_TX, .isc_nrxd_min = {ICE_MIN_DESC_COUNT}, .isc_ntxd_min = {ICE_MIN_DESC_COUNT}, diff --git a/sys/dev/ixl/if_ixl.c b/sys/dev/ixl/if_ixl.c index b0f89ae9a900..b81346ec38ab 100644 --- a/sys/dev/ixl/if_ixl.c +++ b/sys/dev/ixl/if_ixl.c @@ -368,7 +368,8 @@ static struct if_shared_ctx ixl_sctx_init = { .isc_vendor_info = ixl_vendor_info_array, .isc_driver_version = IXL_DRIVER_VERSION_STRING, .isc_driver = &ixl_if_driver, - .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_NEED_ZERO_CSUM | IFLIB_TSO_INIT_IP | IFLIB_ADMIN_ALWAYS_RUN, + .isc_flags = IFLIB_NEED_SCRATCH | IFLIB_NEED_ZERO_CSUM | + IFLIB_TSO_INIT_IP | IFLIB_ADMIN_ALWAYS_RUN | IFLIB_SPARSE_TX, .isc_nrxd_min = {IXL_MIN_RING}, .isc_ntxd_min = {IXL_MIN_RING}, diff --git a/sys/net/iflib.c b/sys/net/iflib.c index 77bcedf329eb..8ab5ecb32b4e 100644 --- a/sys/net/iflib.c +++ b/sys/net/iflib.c @@ -364,7 +364,6 @@ typedef struct iflib_sw_rx_desc_array { typedef struct iflib_sw_tx_desc_array { bus_dmamap_t *ifsd_map; /* bus_dma maps for packet */ - bus_dmamap_t *ifsd_tso_map; /* bus_dma maps for TSO packet */ struct mbuf **ifsd_m; /* pkthdr mbufs */ struct mbuf **ifsd_m_defer; /* deferred mbuf ptr */ struct mbuf **ifsd_m_deferb;/* deferred mbuf backing ptr */ @@ -386,19 +385,6 @@ typedef struct iflib_sw_tx_desc_array { #define IFLIB_RESTART_BUDGET 8 -/* - * Encode TSO or !TSO in the low bits of the tx ifsd_m pointer so as - * to avoid defref'ing the mbuf to determine the correct busdma resources - * to release - */ -#define IFLIB_TSO (1ULL << 0) -#define IFLIB_NO_TSO (2ULL << 0) -#define IFLIB_FLAGS_MASK (0x3ULL) -#define IFLIB_SAVE_MBUF(mbuf, flags) ((void *)(((uintptr_t)mbuf) | flags)) -#define IFLIB_GET_FLAGS(a) ((uintptr_t)a & IFLIB_FLAGS_MASK) -#define IFLIB_GET_MBUF(a) ((struct mbuf *)((uintptr_t)a & ~IFLIB_FLAGS_MASK)) - - #define IFC_LEGACY 0x001 #define IFC_QFLUSH 0x002 #define IFC_MULTISEG 0x004 @@ -426,7 +412,8 @@ struct iflib_txq { uint8_t ift_gen; uint8_t ift_br_offset:1, ift_defer_mfree:1, - ift_spare_bits0:6; + ift_sparse_check:1, + ift_spare_bits0:5; uint16_t ift_npending; uint16_t ift_db_pending; uint16_t ift_rs_pending; @@ -491,7 +478,6 @@ struct iflib_txq { uint8_t ift_spare0[2]; /* pad to the next pointer boundary */ struct iflib_filter_info ift_filter_info; bus_dma_tag_t ift_buf_tag; - bus_dma_tag_t ift_tso_buf_tag; iflib_dma_info_t ift_ifdi; #define MTX_NAME_LEN 32 char ift_mtx_name[MTX_NAME_LEN]; @@ -1839,11 +1825,7 @@ _iflib_irq_alloc(if_ctx_t ctx, if_irq_t irq, int rid, /********************************************************************* * - * Allocate DMA resources for TX buffers as well as memory for the TX - * mbuf map. TX DMA maps (non-TSO/TSO) and TX mbuf map are kept in a - * iflib_sw_tx_desc_array structure, storing all the information that - * is needed to transmit a packet on the wire. This is called only - * once at attach, setup is done every reset. + * Allocate DMA resources for TX buffers and the TX mbuf map. * **********************************************************************/ static int @@ -1853,12 +1835,14 @@ iflib_txsd_alloc(iflib_txq_t txq) if_shared_ctx_t sctx = ctx->ifc_sctx; if_softc_ctx_t scctx = &ctx->ifc_softc_ctx; device_t dev = ctx->ifc_dev; - bus_size_t tsomaxsize; + bus_size_t maxsegsize, maxsize, tsomaxsize; bus_addr_t lowaddr; int err, nsegments, ntsosegments; bool tso; nsegments = scctx->isc_tx_nsegments; + maxsize = sctx->isc_tx_maxsize; + maxsegsize = sctx->isc_tx_maxsegsize; ntsosegments = scctx->isc_tx_tso_segments_max; tsomaxsize = scctx->isc_tx_tso_size_max; if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_VLAN_MTU) @@ -1866,48 +1850,33 @@ iflib_txsd_alloc(iflib_txq_t txq) MPASS(scctx->isc_ntxd[0] > 0); MPASS(scctx->isc_ntxd[txq->ift_br_offset] > 0); MPASS(nsegments > 0); - if (if_getcapabilities(ctx->ifc_ifp) & IFCAP_TSO) { + tso = (if_getcapabilities(ctx->ifc_ifp) & IFCAP_TSO) != 0; + if (tso) { MPASS(ntsosegments > 0); MPASS(sctx->isc_tso_maxsize >= tsomaxsize); + maxsize = MAX(maxsize, tsomaxsize); + nsegments = MAX(nsegments, ntsosegments); + maxsegsize = MAX(maxsegsize, sctx->isc_tso_maxsegsize); } lowaddr = DMA_WIDTH_TO_BUS_LOWADDR(scctx->isc_dma_width); - /* - * Set up DMA tags for TX buffers. - */ + /* Set up the TX buffer DMA tag. */ if ((err = bus_dma_tag_create(bus_get_dma_tag(dev), 1, 0, /* alignment, bounds */ lowaddr, /* lowaddr */ BUS_SPACE_MAXADDR, /* highaddr */ NULL, NULL, /* filter, filterarg */ - sctx->isc_tx_maxsize, /* maxsize */ + maxsize, /* maxsize */ nsegments, /* nsegments */ - sctx->isc_tx_maxsegsize, /* maxsegsize */ + maxsegsize, /* maxsegsize */ 0, /* flags */ NULL, /* lockfunc */ NULL, /* lockfuncarg */ &txq->ift_buf_tag))) { device_printf(dev, "Unable to allocate TX DMA tag: %d\n", err); device_printf(dev, "maxsize: %ju nsegments: %d maxsegsize: %ju\n", - (uintmax_t)sctx->isc_tx_maxsize, nsegments, (uintmax_t)sctx->isc_tx_maxsegsize); - goto fail; - } - tso = (if_getcapabilities(ctx->ifc_ifp) & IFCAP_TSO) != 0; - if (tso && (err = bus_dma_tag_create(bus_get_dma_tag(dev), - 1, 0, /* alignment, bounds */ - lowaddr, /* lowaddr */ - BUS_SPACE_MAXADDR, /* highaddr */ - NULL, NULL, /* filter, filterarg */ - tsomaxsize, /* maxsize */ - ntsosegments, /* nsegments */ - sctx->isc_tso_maxsegsize, /* maxsegsize */ - 0, /* flags */ - NULL, /* lockfunc */ - NULL, /* lockfuncarg */ - &txq->ift_tso_buf_tag))) { - device_printf(dev, "Unable to allocate TSO TX DMA tag: %d\n", - err); + (uintmax_t)maxsize, nsegments, (uintmax_t)maxsegsize); goto fail; } @@ -1940,14 +1909,6 @@ iflib_txsd_alloc(iflib_txq_t txq) err = ENOMEM; goto fail; } - if (tso && (txq->ift_sds.ifsd_tso_map = (bus_dmamap_t *)malloc( - sizeof(bus_dmamap_t) * scctx->isc_ntxd[txq->ift_br_offset], - M_IFLIB, M_NOWAIT | M_ZERO)) == NULL) { - device_printf(dev, - "Unable to allocate TSO TX buffer map memory\n"); - err = ENOMEM; - goto fail; - } for (int i = 0; i < scctx->isc_ntxd[txq->ift_br_offset]; i++) { err = bus_dmamap_create(txq->ift_buf_tag, 0, &txq->ift_sds.ifsd_map[i]); @@ -1955,14 +1916,6 @@ iflib_txsd_alloc(iflib_txq_t txq) device_printf(dev, "Unable to create TX DMA map\n"); goto fail; } - if (!tso) - continue; - err = bus_dmamap_create(txq->ift_tso_buf_tag, 0, - &txq->ift_sds.ifsd_tso_map[i]); - if (err != 0) { - device_printf(dev, "Unable to create TSO TX DMA map\n"); - goto fail; - } } return (0); fail: @@ -1983,15 +1936,6 @@ iflib_txsd_destroy(if_ctx_t ctx, iflib_txq_t txq, int i) bus_dmamap_destroy(txq->ift_buf_tag, map); txq->ift_sds.ifsd_map[i] = NULL; } - - if (txq->ift_sds.ifsd_tso_map != NULL) { - map = txq->ift_sds.ifsd_tso_map[i]; - bus_dmamap_sync(txq->ift_tso_buf_tag, map, - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(txq->ift_tso_buf_tag, map); - bus_dmamap_destroy(txq->ift_tso_buf_tag, map); - txq->ift_sds.ifsd_tso_map[i] = NULL; - } } static void @@ -2038,10 +1982,6 @@ iflib_txq_destroy(iflib_txq_t txq) free(txq->ift_sds.ifsd_map, M_IFLIB); txq->ift_sds.ifsd_map = NULL; } - if (txq->ift_sds.ifsd_tso_map != NULL) { - free(txq->ift_sds.ifsd_tso_map, M_IFLIB); - txq->ift_sds.ifsd_tso_map = NULL; - } if (txq->ift_sds.ifsd_m != NULL) { free(txq->ift_sds.ifsd_m, M_IFLIB); txq->ift_sds.ifsd_m = NULL; @@ -2054,10 +1994,6 @@ iflib_txq_destroy(iflib_txq_t txq) bus_dma_tag_destroy(txq->ift_buf_tag); txq->ift_buf_tag = NULL; } - if (txq->ift_tso_buf_tag != NULL) { - bus_dma_tag_destroy(txq->ift_tso_buf_tag); - txq->ift_tso_buf_tag = NULL; - } if (txq->ift_ifdi != NULL) { free(txq->ift_ifdi, M_IFLIB); } @@ -2068,7 +2004,7 @@ iflib_txsd_free(if_ctx_t ctx, iflib_txq_t txq, int i) { struct mbuf *m; - m = IFLIB_GET_MBUF(txq->ift_sds.ifsd_m[i]); + m = txq->ift_sds.ifsd_m[i]; if (m == NULL) return; @@ -2077,12 +2013,6 @@ iflib_txsd_free(if_ctx_t ctx, iflib_txq_t txq, int i) txq->ift_sds.ifsd_map[i], BUS_DMASYNC_POSTWRITE); bus_dmamap_unload(txq->ift_buf_tag, txq->ift_sds.ifsd_map[i]); } - if (txq->ift_sds.ifsd_tso_map != NULL) { - bus_dmamap_sync(txq->ift_tso_buf_tag, - txq->ift_sds.ifsd_tso_map[i], BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(txq->ift_tso_buf_tag, - txq->ift_sds.ifsd_tso_map[i]); - } txq->ift_sds.ifsd_m[i] = NULL; m_freem(m); DBG_COUNTER_INC(tx_frees); @@ -3813,12 +3743,9 @@ iflib_remove_mbuf(iflib_txq_t txq) ntxd = txq->ift_size; pidx = txq->ift_pidx & (ntxd - 1); ifsd_m = txq->ift_sds.ifsd_m; - m = IFLIB_GET_MBUF(ifsd_m[pidx]); + m = ifsd_m[pidx]; ifsd_m[pidx] = NULL; bus_dmamap_unload(txq->ift_buf_tag, txq->ift_sds.ifsd_map[pidx]); - if (txq->ift_sds.ifsd_tso_map != NULL) - bus_dmamap_unload(txq->ift_tso_buf_tag, - txq->ift_sds.ifsd_tso_map[pidx]); #if MEMORY_LOGGING txq->ift_dequeued++; #endif @@ -3880,7 +3807,6 @@ iflib_encap(iflib_txq_t txq, struct mbuf **m_headp, int *obytes, int *opkts) struct mbuf *m_head, **ifsd_m; bus_dmamap_t map; struct if_pkt_info pi; - uintptr_t flags; int remap = 0; int err, nsegs, ndesc, max_segs, pidx; @@ -3889,6 +3815,7 @@ iflib_encap(iflib_txq_t txq, struct mbuf **m_headp, int *obytes, int *opkts) scctx = &ctx->ifc_softc_ctx; segs = txq->ift_segs; m_head = *m_headp; + buf_tag = txq->ift_buf_tag; map = NULL; /* @@ -3898,19 +3825,6 @@ iflib_encap(iflib_txq_t txq, struct mbuf **m_headp, int *obytes, int *opkts) map = txq->ift_sds.ifsd_map[pidx]; ifsd_m = txq->ift_sds.ifsd_m; - if (m_head->m_pkthdr.csum_flags & CSUM_TSO) { - buf_tag = txq->ift_tso_buf_tag; - max_segs = scctx->isc_tx_tso_segments_max; - map = txq->ift_sds.ifsd_tso_map[pidx]; - MPASS(buf_tag != NULL); - MPASS(max_segs > 0); - flags = IFLIB_TSO; - } else { - buf_tag = txq->ift_buf_tag; - max_segs = scctx->isc_tx_nsegments; - map = txq->ift_sds.ifsd_map[pidx]; - flags = IFLIB_NO_TSO; - } if ((sctx->isc_flags & IFLIB_NEED_ETHER_PAD) && __predict_false(m_head->m_pkthdr.len < scctx->isc_min_frame_size)) { err = iflib_ether_pad(ctx->ifc_dev, m_headp, scctx->isc_min_frame_size); @@ -3941,10 +3855,20 @@ iflib_encap(iflib_txq_t txq, struct mbuf **m_headp, int *obytes, int *opkts) retry: err = bus_dmamap_load_mbuf_sg(buf_tag, map, m_head, segs, &nsegs, BUS_DMA_NOWAIT); + if (__predict_false(err == 0 && txq->ift_sparse_check && + (pi.ipi_csum_flags & CSUM_TSO) == 0 && + nsegs > scctx->isc_tx_nsegments)) { + bus_dmamap_unload(buf_tag, map); + err = EFBIG; + } defrag: if (__predict_false(err)) { switch (err) { case EFBIG: + if (pi.ipi_csum_flags & CSUM_TSO) + max_segs = scctx->isc_tx_tso_segments_max; + else + max_segs = scctx->isc_tx_nsegments; /* try collapse once and defrag once */ if (remap == 0) { m_head = m_collapse(*m_headp, M_NOWAIT, max_segs); @@ -3981,7 +3905,7 @@ defrag: DBG_COUNTER_INC(encap_txd_encap_fail); return (err); } - ifsd_m[pidx] = IFLIB_SAVE_MBUF(m_head, flags); + ifsd_m[pidx] = m_head; if (m_head->m_pkthdr.csum_flags & CSUM_SND_TAG) pi.ipi_mbuf = m_head; else @@ -4061,7 +3985,7 @@ defrag: /* * Update packets / bytes sent */ - if (flags & IFLIB_TSO) { + if (pi.ipi_csum_flags & CSUM_TSO) { int hlen = pi.ipi_ehdrlen + pi.ipi_ip_hlen + pi.ipi_tcp_hlen; int tsolen = pi.ipi_len - hlen; int nsegs = (tsolen + pi.ipi_tso_segsz - 1) / pi.ipi_tso_segsz; @@ -4109,7 +4033,6 @@ iflib_tx_desc_free(iflib_txq_t txq, int n, struct mbuf **m_defer) { uint32_t qsize, cidx, gen; struct mbuf *m, **ifsd_m; - uintptr_t flags; cidx = txq->ift_cidx; gen = txq->ift_gen; @@ -4117,22 +4040,11 @@ iflib_tx_desc_free(iflib_txq_t txq, int n, struct mbuf **m_defer) ifsd_m =txq->ift_sds.ifsd_m; while (n-- > 0) { - if ((m = IFLIB_GET_MBUF(ifsd_m[cidx])) != NULL) { - flags = IFLIB_GET_FLAGS(ifsd_m[cidx]); - MPASS(flags != 0); - if (flags & IFLIB_TSO) { - bus_dmamap_sync(txq->ift_tso_buf_tag, - txq->ift_sds.ifsd_tso_map[cidx], - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(txq->ift_tso_buf_tag, - txq->ift_sds.ifsd_tso_map[cidx]); - } else { - bus_dmamap_sync(txq->ift_buf_tag, - txq->ift_sds.ifsd_map[cidx], - BUS_DMASYNC_POSTWRITE); - bus_dmamap_unload(txq->ift_buf_tag, - txq->ift_sds.ifsd_map[cidx]); - } + if ((m = ifsd_m[cidx]) != NULL) { + bus_dmamap_sync(txq->ift_buf_tag, + txq->ift_sds.ifsd_map[cidx], BUS_DMASYNC_POSTWRITE); + bus_dmamap_unload(txq->ift_buf_tag, + txq->ift_sds.ifsd_map[cidx]); /* XXX we don't support any drivers that batch packets yet */ MPASS(m->m_nextpkt == NULL); if (m_defer == NULL) { @@ -6627,6 +6539,7 @@ iflib_queues_alloc(if_ctx_t ctx) } txq->ift_ctx = ctx; txq->ift_id = i; + txq->ift_sparse_check = (sctx->isc_flags & IFLIB_SPARSE_TX) != 0; if (sctx->isc_flags & IFLIB_HAS_TXCQ) { txq->ift_br_offset = 1; } else { diff --git a/sys/net/iflib.h b/sys/net/iflib.h index 0536fb975bd9..0c24ac614431 100644 --- a/sys/net/iflib.h +++ b/sys/net/iflib.h @@ -360,7 +360,10 @@ typedef enum { * Driver understands page/bank i2c reads */ #define IFLIB_I2C_PAGE_BANK 0x200 -#define IFLIB_SPARE6 0x400 +/* + * Non-TSO packets must not exceed isc_tx_nsegments DMA segments. + */ +#define IFLIB_SPARSE_TX 0x400 #define IFLIB_SPARE5 0x800 #define IFLIB_SPARE4 0x1000 #define IFLIB_SPARE3 0x2000 diff --git a/sys/sys/param.h b/sys/sys/param.h index 5c96c3fc1c84..04fe7b87bb0f 100644 --- a/sys/sys/param.h +++ b/sys/sys/param.h @@ -74,7 +74,7 @@ * cannot include sys/param.h and should only be updated here. */ #undef __FreeBSD_version -#define __FreeBSD_version 1600030 +#define __FreeBSD_version 1600031 /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,