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)
>
