Download raw body.
ixl/ice(4): Avoid unnecessary defrag of TSO packets
> Date: Tue, 16 Sep 2025 21:20:43 +0200
> From: Alexander Bluhm <alexander.bluhm@gmx.net>
>
> On Sat, Sep 13, 2025 at 04:00:04PM +0200, Jan Klemkow wrote:
> > On Fri, Sep 12, 2025 at 05:57:49PM +0200, Mark Kettenis wrote:
> > > > Date: Fri, 12 Sep 2025 16:00:39 +0200
> > > > From: Jan Klemkow <jan@openbsd.org>
> > > > ixl(4) and ice(4) network cards can handle just up to 8 DMA segments for
> > > > a regular packet, but 128 segments for a TSO packet. TSO packets reach
> > > > the 8 segments limit very fast. Thus, we run into unnecessary
> > > > m_defrag() calls, which cost throughput.
> > > >
> > > > This diff sets the limit to 128 segments for ice(4) and ixl(4) and adds an
> > > > additional check for non-TSO packets.
> > > >
> > > > ok?
> > >
> > > I don't think so. This bypasses all the bus_dma(9) logic. There
> > > isn't necessarily a relationship between the number of elements in an
> > > mbuf chain and the number of bus_dma(9) segments.
> > >
> > > I think you should use separate bus_dma(9) maps for TSO.
> >
> > Like the following diff?
>
> I have tested it successfully. Comments inline.
>
> bluhm
>
> > Index: dev/pci/if_ice.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/if_ice.c,v
> > diff -u -p -r1.58 if_ice.c
> > --- dev/pci/if_ice.c 19 Aug 2025 11:46:52 -0000 1.58
> > +++ dev/pci/if_ice.c 13 Sep 2025 13:29:55 -0000
> > @@ -13432,8 +13432,10 @@ ice_txq_clean(struct ice_softc *sc, stru
> >
> > if (txm->txm_m == NULL)
> > continue;
> > -
> > - map = txm->txm_map;
> > + if (ISSET(txm->txm_m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> > bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
> > BUS_DMASYNC_POSTWRITE);
> > bus_dmamap_unload(sc->sc_dmat, map);
> > @@ -13868,7 +13870,7 @@ ice_tso_detect_sparse(struct mbuf *m, st
> > hlen -= seglen;
> > }
> >
> > - maxsegs = ICE_MAX_TX_SEGS - hdrs;
> > + maxsegs = ICE_MAX_TSO_SEGS - hdrs;
> >
> > /* We must count the headers, in order to verify that they take up
> > * 3 or fewer descriptors. However, we don't need to check the data
>
> The comment above ice_tso_detect_sparse() talks about 8. That
> should be 128 in some cases. Please adapt.
>
> In general, parts of ice_tso_detect_sparse() do what bus_dmamap_load_mbuf()
> checks anyway. Other drivers don't have theses checks although I
> guess they have similar hardware limitations. I think this function
> has to be reconsidered. But not in this diff.
Yes, that function is weird and I don't think it does what the comment
says. And yes, that is another diff.
> > @@ -14066,7 +14068,11 @@ ice_start(struct ifqueue *ifq)
> > }
> >
> > txm = &txq->tx_map[prod];
> > - map = txm->txm_map;
> > +
> > + if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> >
> > if (ice_load_mbuf(sc->sc_dmat, map, m) != 0) {
> > ifq->ifq_errors++;
> > @@ -29478,7 +29484,10 @@ ice_txeof(struct ice_softc *sc, struct i
> > if (dtype != htole64(ICE_TX_DESC_DTYPE_DESC_DONE))
> > break;
> >
> > - map = txm->txm_map;
> > + if (ISSET(txm->txm_m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> >
> > bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
> > BUS_DMASYNC_POSTWRITE);
> > @@ -29601,6 +29610,11 @@ ice_free_tx_queues(struct ice_softc *sc)
> > bus_dmamap_destroy(sc->sc_dmat, map->txm_map);
> > map->txm_map = NULL;
> > }
> > + if (map->txm_map_tso != NULL) {
> > + bus_dmamap_destroy(sc->sc_dmat,
> > + map->txm_map_tso);
> > + map->txm_map_tso = NULL;
> > + }
> > }
> > free(txq->tx_map, M_DEVBUF, txq->desc_count * sizeof(*map));
> > txq->tx_map = NULL;
> > @@ -29674,6 +29688,16 @@ ice_tx_queues_alloc(struct ice_softc *sc
> > ICE_MAX_TX_SEGS, ICE_MAX_DMA_SEG_SIZE, 0,
> > BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW | BUS_DMA_64BIT,
> > &map->txm_map) != 0) {
> > + printf("%s: could not allocate Tx DMA map\n",
> > + sc->sc_dev.dv_xname);
> > + err = ENOMEM;
> > + goto free_tx_queues;
> > + }
> > +
> > + if (bus_dmamap_create(sc->sc_dmat, MAXMCLBYTES,
> > + ICE_MAX_TSO_SEGS, ICE_MAX_DMA_SEG_SIZE, 0,
> > + BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW | BUS_DMA_64BIT,
> > + &map->txm_map_tso) != 0) {
> > printf("%s: could not allocate Tx DMA map\n",
> > sc->sc_dev.dv_xname);
> > err = ENOMEM;
>
> Plase use unique error messages. Rename the second one to
> "%s: could not allocate TSO DMA map\n"
>
> > Index: dev/pci/if_icevar.h
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/pci/if_icevar.h,v
> > diff -u -p -r1.8 if_icevar.h
> > --- dev/pci/if_icevar.h 19 Aug 2025 11:46:52 -0000 1.8
> > +++ dev/pci/if_icevar.h 13 Sep 2025 10:27:14 -0000
> > @@ -4537,6 +4537,7 @@ struct ice_pf_sw_stats {
> > struct ice_tx_map {
> > struct mbuf *txm_m;
> > bus_dmamap_t txm_map;
> > + bus_dmamap_t txm_map_tso;
> > unsigned int txm_eop;
> > };
> >
> > 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 13 Sep 2025 13:30:47 -0000
> > @@ -900,6 +900,7 @@ struct ixl_rx_wb_desc_32 {
> > } __packed __aligned(16);
> >
> > #define IXL_TX_PKT_DESCS 8
> > +#define IXL_TX_TSO_PKT_DESCS 128
> > #define IXL_TX_QUEUE_ALIGN 128
> > #define IXL_RX_QUEUE_ALIGN 128
> >
> > @@ -1142,6 +1143,7 @@ struct ixl_chip;
> > struct ixl_tx_map {
> > struct mbuf *txm_m;
> > bus_dmamap_t txm_map;
> > + bus_dmamap_t txm_map_tso;
> > unsigned int txm_eop;
> > };
> >
> > @@ -2584,6 +2586,12 @@ ixl_txr_alloc(struct ixl_softc *sc, unsi
> > &txm->txm_map) != 0)
> > goto uncreate;
> >
> > + if (bus_dmamap_create(sc->sc_dmat,
> > + MAXMCLBYTES, IXL_TX_TSO_PKT_DESCS, IXL_MAX_DMA_SEG_SIZE, 0,
> > + BUS_DMA_WAITOK | BUS_DMA_ALLOCNOW | BUS_DMA_64BIT,
> > + &txm->txm_map_tso) != 0)
> > + goto uncreate;
> > +
> > txm->txm_eop = -1;
> > txm->txm_m = NULL;
> > }
> > @@ -2604,6 +2612,11 @@ uncreate:
> > continue;
> >
> > bus_dmamap_destroy(sc->sc_dmat, txm->txm_map);
> > +
> > + if (txm->txm_map_tso == NULL)
> > + continue;
> > +
> > + bus_dmamap_destroy(sc->sc_dmat, txm->txm_map_tso);
> > }
> >
> > ixl_dmamem_free(sc, &txr->txr_mem);
>
> This continue is wrong as the first one also affects the second map.
>
> if (txm->txm_map != NULL)
> bus_dmamap_destroy(sc->sc_dmat, txm->txm_map);
> if (txm->txm_map_tso != NULL)
> bus_dmamap_destroy(sc->sc_dmat, txm->txm_map_tso);
>
> > @@ -2680,7 +2693,10 @@ ixl_txr_clean(struct ixl_softc *sc, stru
> > if (txm->txm_m == NULL)
> > continue;
> >
> > - map = txm->txm_map;
> > + if (ISSET(txm->txm_m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> > bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
> > BUS_DMASYNC_POSTWRITE);
> > bus_dmamap_unload(sc->sc_dmat, map);
> > @@ -2739,6 +2755,7 @@ ixl_txr_free(struct ixl_softc *sc, struc
> > txm = &maps[i];
> >
> > bus_dmamap_destroy(sc->sc_dmat, txm->txm_map);
> > + bus_dmamap_destroy(sc->sc_dmat, txm->txm_map_tso);
> > }
> >
> > ixl_dmamem_free(sc, &txr->txr_mem);
> > @@ -2885,7 +2902,7 @@ ixl_start(struct ifqueue *ifq)
> >
> > for (;;) {
> > /* We need one extra descriptor for TSO packets. */
> > - if (free <= (IXL_TX_PKT_DESCS + 1)) {
> > + if (free <= (IXL_TX_TSO_PKT_DESCS + 1)) {
> > ifq_set_oactive(ifq);
> > break;
> > }
> > @@ -2897,7 +2914,10 @@ ixl_start(struct ifqueue *ifq)
> > offload = ixl_tx_setup_offload(m, txr, prod);
> >
> > txm = &txr->txr_maps[prod];
> > - map = txm->txm_map;
> > + if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> >
> > if (ISSET(m->m_pkthdr.csum_flags, M_TCP_TSO)) {
> > prod++;
> > @@ -2988,7 +3008,10 @@ ixl_txeof(struct ixl_softc *sc, struct i
> > if (dtype != htole64(IXL_TX_DESC_DTYPE_DONE))
> > break;
> >
> > - map = txm->txm_map;
> > + if (ISSET(txm->txm_m->m_pkthdr.csum_flags, M_TCP_TSO))
> > + map = txm->txm_map_tso;
> > + else
> > + map = txm->txm_map;
> >
> > bus_dmamap_sync(sc->sc_dmat, map, 0, map->dm_mapsize,
> > BUS_DMASYNC_POSTWRITE);
>
ixl/ice(4): Avoid unnecessary defrag of TSO packets