On Wed, May 31, 2017 at 20:40 +0200, Mike Belopuhov wrote: > According to the FreeBSD driver, txp(4) is not setting up its TX > ring correctly. FreeBSD driver uses up to 16 fragments, while we > use up to 252 which is suspicious. > > This gets us in line with FreeBSD, introduces goodness of m_defrag > and removes pesky if_deq_* thingies. > > Does anyone still have the hardware (3com 3CR900 Typhoon) to test? > OK's are welcome. >
Any OKs? Tests? Should I go ahead with this? > diff --git sys/dev/pci/if_txp.c sys/dev/pci/if_txp.c > index deede70e9de..1aed06765c0 100644 > --- sys/dev/pci/if_txp.c > +++ sys/dev/pci/if_txp.c > @@ -883,12 +883,12 @@ txp_alloc_rings(struct txp_softc *sc) > sc->sc_txhir.r_desc = (struct txp_tx_desc > *)sc->sc_txhiring_dma.dma_vaddr; > sc->sc_txhir.r_cons = sc->sc_txhir.r_prod = sc->sc_txhir.r_cnt = 0; > sc->sc_txhir.r_off = &sc->sc_hostvar->hv_tx_hi_desc_read_idx; > for (i = 0; i < TX_ENTRIES; i++) { > if (bus_dmamap_create(sc->sc_dmat, TXP_MAX_PKTLEN, > - TX_ENTRIES - 4, TXP_MAX_SEGLEN, 0, > - BUS_DMA_NOWAIT, &sc->sc_txd[i].sd_map) != 0) { > + TXP_MAXTXSEGS, MCLBYTES, 0, BUS_DMA_NOWAIT, > + &sc->sc_txd[i].sd_map) != 0) { > for (j = 0; j < i; j++) { > bus_dmamap_destroy(sc->sc_dmat, > sc->sc_txd[j].sd_map); > sc->sc_txd[j].sd_map = NULL; > } > @@ -1261,57 +1261,48 @@ txp_start(struct ifnet *ifp) > struct txp_softc *sc = ifp->if_softc; > struct txp_tx_ring *r = &sc->sc_txhir; > struct txp_tx_desc *txd; > int txdidx; > struct txp_frag_desc *fxd; > - struct mbuf *m, *mnew; > + struct mbuf *m; > struct txp_swdesc *sd; > u_int32_t firstprod, firstcnt, prod, cnt, i; > > if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(&ifp->if_snd)) > return; > > prod = r->r_prod; > cnt = r->r_cnt; > > while (1) { > - m = ifq_deq_begin(&ifp->if_snd); > + if (cnt >= TX_ENTRIES - TXP_MAXTXSEGS - 4) > + goto oactive; > + > + m = ifq_dequeue(&ifp->if_snd); > if (m == NULL) > break; > - mnew = NULL; > > firstprod = prod; > firstcnt = cnt; > > sd = sc->sc_txd + prod; > sd->sd_mbuf = m; > > - if (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m, > + switch (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m, > BUS_DMA_NOWAIT)) { > - MGETHDR(mnew, M_DONTWAIT, MT_DATA); > - if (mnew == NULL) > - goto oactive1; > - if (m->m_pkthdr.len > MHLEN) { > - MCLGET(mnew, M_DONTWAIT); > - if ((mnew->m_flags & M_EXT) == 0) { > - m_freem(mnew); > - goto oactive1; > - } > - } > - m_copydata(m, 0, m->m_pkthdr.len, mtod(mnew, caddr_t)); > - mnew->m_pkthdr.len = mnew->m_len = m->m_pkthdr.len; > - ifq_deq_commit(&ifp->if_snd, m); > + case 0: > + break; > + case EFBIG: > + if (m_defrag(m, M_DONTWAIT) == 0 && > + bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m, > + BUS_DMA_NOWAIT) == 0) > + break; > + default: > m_freem(m); > - m = mnew; > - if (bus_dmamap_load_mbuf(sc->sc_dmat, sd->sd_map, m, > - BUS_DMA_NOWAIT)) > - goto oactive1; > + continue; > } > > - if ((TX_ENTRIES - cnt) < 4) > - goto oactive; > - > txd = r->r_desc + prod; > txdidx = prod; > txd->tx_flags = TX_FLAGS_TYPE_DATA; > txd->tx_numdesc = 0; > txd->tx_addrlo = 0; > @@ -1321,13 +1312,10 @@ txp_start(struct ifnet *ifp) > txd->tx_numdesc = sd->sd_map->dm_nsegs; > > if (++prod == TX_ENTRIES) > prod = 0; > > - if (++cnt >= (TX_ENTRIES - 4)) > - goto oactive; > - > #if NVLAN > 0 > if (m->m_flags & M_VLANTAG) { > txd->tx_pflags = TX_PFLAGS_VLAN | > (htons(m->m_pkthdr.ether_vtag) << > TX_PFLAGS_VLANTAG_S); > } > @@ -1351,10 +1339,12 @@ txp_start(struct ifnet *ifp) > for (i = 0; i < sd->sd_map->dm_nsegs; i++) { > if (++cnt >= (TX_ENTRIES - 4)) { > bus_dmamap_sync(sc->sc_dmat, sd->sd_map, > 0, sd->sd_map->dm_mapsize, > BUS_DMASYNC_POSTWRITE); > + bus_dmamap_unload(sc->sc_dmat, sd->sd_map); > + m_freem(m); > goto oactive; > } > > fxd->frag_flags = FRAG_FLAGS_TYPE_FRAG | > FRAG_FLAGS_VALID; > @@ -1379,17 +1369,10 @@ txp_start(struct ifnet *ifp) > } else > fxd++; > > } > > - /* > - * if mnew isn't NULL, we already dequeued and copied > - * the packet. > - */ > - if (mnew == NULL) > - ifq_deq_commit(&ifp->if_snd, m); > - > ifp->if_timer = 5; > > #if NBPFILTER > 0 > if (ifp->if_bpf) > bpf_mtap_ether(ifp->if_bpf, m, BPF_DIRECTION_OUT); > @@ -1424,13 +1407,10 @@ txp_start(struct ifnet *ifp) > r->r_prod = prod; > r->r_cnt = cnt; > return; > > oactive: > - bus_dmamap_unload(sc->sc_dmat, sd->sd_map); > -oactive1: > - ifq_deq_rollback(&ifp->if_snd, m); > ifq_set_oactive(&ifp->if_snd); > r->r_prod = firstprod; > r->r_cnt = firstcnt; > } > > diff --git sys/dev/pci/if_txpreg.h sys/dev/pci/if_txpreg.h > index cc4d5f1c60c..b6962872f34 100644 > --- sys/dev/pci/if_txpreg.h > +++ sys/dev/pci/if_txpreg.h > @@ -607,10 +607,12 @@ struct txp_fw_section_header { > }; > > #define TXP_MAX_SEGLEN 0xffff > #define TXP_MAX_PKTLEN 0x0800 > > +#define TXP_MAXTXSEGS 16 > + > #define WRITE_REG(sc,reg,val) \ > bus_space_write_4((sc)->sc_bt, (sc)->sc_bh, reg, val) > #define READ_REG(sc,reg) \ > bus_space_read_4((sc)->sc_bt, (sc)->sc_bh, reg) >