On Mon, Jun 05, 2017 at 16:13 +0200, Mike Belopuhov wrote:
> 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?
>
I've heard no objections to this so the best way to test it
would be to get this in.
> > 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)
> >
