On Mon, Jul 27, 2020 at 11:24:55AM -0700, Jonathon Fletcher wrote: > > I have attached an updated patch - which includes Kevin's changes from > his earlier email - that also corrects the tx buffer sizes for RTL8153 > / RTL8153B devices. >
Unfortunately below diff fails to apply. 2 out of 2 hunks failed 15 out of 15 hunks failed I've tried on fresh cvs version of if_urereg.h and if_ure.c I usually, for convenience take the diffs from marc.info: https://marc.info/?l=openbsd-tech&m=159587449018248&q=mbox > > Index: sys/dev/usb/if_urereg.h > =================================================================== > RCS file: /cvs/src/sys/dev/usb/if_urereg.h,v > retrieving revision 1.8 > diff -u -p -u -r1.8 if_urereg.h > --- sys/dev/usb/if_urereg.h 7 Dec 2019 08:45:28 -0000 1.8 > +++ sys/dev/usb/if_urereg.h 27 Jul 2020 17:10:09 -0000 > @@ -494,28 +494,32 @@ struct ure_txpkt { > #define URE_ENDPT_TX 1 > #define URE_ENDPT_MAX 2 > > -#define URE_TX_LIST_CNT 8 > +#define URE_TX_LIST_CNT 1 > #define URE_RX_LIST_CNT 1 > -#define URE_RX_BUF_ALIGN sizeof(uint64_t) > +#define URE_TX_BUF_ALIGN 4 > +#define URE_RX_BUF_ALIGN 8 > > -#define URE_TXBUFSZ 16384 > -#define URE_8152_RXBUFSZ 16384 > -#define URE_8153_RXBUFSZ 32768 > +#define URE_8152_TX_BUFSZ 16384 > +#define URE_8152_RX_BUFSZ 16384 > + > +#define URE_8153_TX_BUFSZ 16384 > +#define URE_8153_RX_BUFSZ 32768 > > struct ure_chain { > struct ure_softc *uc_sc; > struct usbd_xfer *uc_xfer; > char *uc_buf; > - struct mbuf *uc_mbuf; > - int uc_accum; > - int uc_idx; > + uint32_t uc_buflen; > + uint32_t uc_bufmax; > + struct mbuf_list uc_mbuf_list; > + SLIST_ENTRY(ure_chain) ure_list; > + uint8_t uc_idx; > }; > > struct ure_cdata { > - struct ure_chain tx_chain[URE_TX_LIST_CNT]; > - struct ure_chain rx_chain[URE_RX_LIST_CNT]; > - int tx_prod; > - int tx_cnt; > + struct ure_chain ure_rx_chain[URE_RX_LIST_CNT]; > + struct ure_chain ure_tx_chain[URE_TX_LIST_CNT]; > + SLIST_HEAD(ure_list_head, ure_chain) ure_tx_free; > }; > > struct ure_softc { > @@ -541,7 +545,7 @@ struct ure_softc { > > struct timeval ure_rx_notice; > int ure_rxbufsz; > - int ure_tx_list_cnt; > + int ure_txbufsz; > > int ure_phyno; > > Index: sys/dev/usb/if_ure.c > =================================================================== > RCS file: /cvs/src/sys/dev/usb/if_ure.c,v > retrieving revision 1.16 > diff -u -p -u -r1.16 if_ure.c > --- sys/dev/usb/if_ure.c 10 Jul 2020 13:26:41 -0000 1.16 > +++ sys/dev/usb/if_ure.c 27 Jul 2020 17:10:39 -0000 > @@ -117,11 +117,13 @@ void ure_miibus_writereg(struct device > void ure_lock_mii(struct ure_softc *); > void ure_unlock_mii(struct ure_softc *); > > -int ure_encap(struct ure_softc *, struct mbuf *); > +int ure_encap_txpkt(struct mbuf *, char *, uint32_t); > +int ure_encap_xfer(struct ifnet *, struct ure_softc *, struct ure_chain *); > void ure_rxeof(struct usbd_xfer *, void *, usbd_status); > void ure_txeof(struct usbd_xfer *, void *, usbd_status); > -int ure_rx_list_init(struct ure_softc *); > -int ure_tx_list_init(struct ure_softc *); > +int ure_xfer_list_init(struct ure_softc *, struct ure_chain *, > + uint32_t, int); > +void ure_xfer_list_free(struct ure_softc *, struct ure_chain *, int); > > void ure_tick_task(void *); > void ure_tick(void *); > @@ -625,12 +627,36 @@ void > ure_watchdog(struct ifnet *ifp) > { > struct ure_softc *sc = ifp->if_softc; > + struct ure_chain *c; > + usbd_status err; > + int i, s; > + > + ifp->if_timer = 0; > > if (usbd_is_dying(sc->ure_udev)) > return; > > + if ((ifp->if_flags & (IFF_RUNNING|IFF_UP)) != (IFF_RUNNING|IFF_UP)) > + return; > + > + sc = ifp->if_softc; > + s = splnet(); > + > ifp->if_oerrors++; > - printf("%s: watchdog timeout\n", sc->ure_dev.dv_xname); > + DPRINTF(("watchdog timeout\n")); > + > + for (i = 0; i < URE_TX_LIST_CNT; i++) { > + c = &sc->ure_cdata.ure_tx_chain[i]; > + if (ml_len(&c->uc_mbuf_list) > 0) { > + usbd_get_xfer_status(c->uc_xfer, NULL, NULL, NULL, > + &err); > + ure_txeof(c->uc_xfer, c, err); > + } > + } > + > + if (ifq_is_oactive(&ifp->if_snd)) > + ifq_restart(&ifp->if_snd); > + splx(s); > } > > void > @@ -653,18 +679,26 @@ ure_init(void *xsc) > else > ure_rtl8153_nic_reset(sc); > > - if (ure_rx_list_init(sc) == ENOBUFS) { > + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_rx_chain, > + sc->ure_rxbufsz, URE_RX_LIST_CNT) == ENOBUFS) { > printf("%s: rx list init failed\n", sc->ure_dev.dv_xname); > splx(s); > return; > } > > - if (ure_tx_list_init(sc) == ENOBUFS) { > + if (ure_xfer_list_init(sc, sc->ure_cdata.ure_tx_chain, > + sc->ure_txbufsz, URE_TX_LIST_CNT) == ENOBUFS) { > printf("%s: tx list init failed\n", sc->ure_dev.dv_xname); > splx(s); > return; > } > > + /* Initialize the SLIST we are using for the multiple tx buffers */ > + SLIST_INIT(&sc->ure_cdata.ure_tx_free); > + for (i = 0; i < URE_TX_LIST_CNT; i++) > + SLIST_INSERT_HEAD(&sc->ure_cdata.ure_tx_free, > + &sc->ure_cdata.ure_tx_chain[i], ure_list); > + > /* Set MAC address. */ > ure_write_1(sc, URE_PLA_CRWECR, URE_MCU_TYPE_PLA, URE_CRWECR_CONFIG); > ure_write_mem(sc, URE_PLA_IDR, URE_MCU_TYPE_PLA | URE_BYTE_EN_SIX_BYTES, > @@ -739,9 +773,9 @@ ure_init(void *xsc) > > /* Start up the receive pipe. */ > for (i = 0; i < URE_RX_LIST_CNT; i++) { > - c = &sc->ure_cdata.rx_chain[i]; > + c = &sc->ure_cdata.ure_rx_chain[i]; > usbd_setup_xfer(c->uc_xfer, sc->ure_ep[URE_ENDPT_RX], > - c, c->uc_buf, sc->ure_rxbufsz, > + c, c->uc_buf, c->uc_bufmax, > USBD_SHORT_XFER_OK | USBD_NO_COPY, > USBD_NO_TIMEOUT, ure_rxeof); > usbd_transfer(c->uc_xfer); > @@ -763,34 +797,83 @@ void > ure_start(struct ifnet *ifp) > { > struct ure_softc *sc = ifp->if_softc; > - struct mbuf *m_head = NULL; > - > - if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(&ifp->if_snd) || > - !(sc->ure_flags & URE_FLAG_LINK)) > + struct ure_cdata *cd = &sc->ure_cdata; > + struct ure_chain *c; > + struct mbuf *m = NULL; > + uint32_t new_buflen; > + int s, mlen; > + > + if (!(sc->ure_flags & URE_FLAG_LINK) || > + (ifp->if_flags & (IFF_RUNNING|IFF_UP)) != > + (IFF_RUNNING|IFF_UP)) { > return; > + } > + > + s = splnet(); > > - for (;;) { > - if (sc->ure_cdata.tx_cnt == sc->ure_tx_list_cnt) { > - ifq_set_oactive(&ifp->if_snd); > + c = SLIST_FIRST(&cd->ure_tx_free); > + while (c != NULL) { > + m = NULL; > + mlen = ifq_hdatalen(&ifp->if_snd); > + if (mlen <= 0) > break; > + > + /* > + * If packet larger than remaining space, send buffer and > + * continue. > + */ > + new_buflen = roundup(c->uc_buflen, URE_TX_BUF_ALIGN); > + if (new_buflen + sizeof(struct ure_txpkt) + mlen >= > + c->uc_bufmax) { > + SLIST_REMOVE_HEAD(&cd->ure_tx_free, ure_list); > + if (ure_encap_xfer(ifp, sc, c)) { > + SLIST_INSERT_HEAD(&cd->ure_tx_free, c, > + ure_list); > + break; > + } > + c = SLIST_FIRST(&cd->ure_tx_free); > + continue; > } > > - m_head = ifq_dequeue(&ifp->if_snd); > - if (m_head == NULL) > + m = ifq_dequeue(&ifp->if_snd); > + if (m == NULL) > break; > > - if (ure_encap(sc, m_head)) { > - m_freem(m_head); > - ifq_set_oactive(&ifp->if_snd); > + /* Discard packet larger than buffer. */ > + if (mlen + sizeof(struct ure_txpkt) >= c->uc_bufmax) { > + m_freem(m); > + continue; > + } > + > + /* Append packet to current buffer. */ > + mlen = ure_encap_txpkt(m, c->uc_buf + new_buflen, > + c->uc_bufmax - new_buflen); > + if (mlen <= 0) { > + m_freem(m); > break; > } > + ml_enqueue(&c->uc_mbuf_list, m); > + c->uc_buflen = new_buflen + mlen; > + } > > -#if NBPFILTER > 0 > - if (ifp->if_bpf) > - bpf_mtap(ifp->if_bpf, m_head, BPF_DIRECTION_OUT); > -#endif > - ifp->if_timer = 5; > + if (c != NULL) { > + /* Send current buffer unless empty */ > + if (c->uc_buflen > 0 && ml_len(&c->uc_mbuf_list) > 0) { > + SLIST_REMOVE_HEAD(&cd->ure_tx_free, ure_list); > + if (ure_encap_xfer(ifp, sc, c)) { > + SLIST_INSERT_HEAD(&cd->ure_tx_free, c, > + ure_list); > + } > + c = SLIST_FIRST(&cd->ure_tx_free); > + } > } > + > + if (c == NULL) > + ifq_set_oactive(&ifp->if_snd); > + > + ifp->if_timer = 5; > + > + splx(s); > } > > void > @@ -810,9 +893,9 @@ ure_tick(void *xsc) > void > ure_stop(struct ure_softc *sc) > { > - usbd_status err; > + struct ure_cdata *cd; > struct ifnet *ifp; > - int i; > + usbd_status err; > > ure_reset(sc); > > @@ -844,25 +927,54 @@ ure_stop(struct ure_softc *sc) > sc->ure_ep[URE_ENDPT_TX] = NULL; > } > > - for (i = 0; i < URE_RX_LIST_CNT; i++) { > - if (sc->ure_cdata.rx_chain[i].uc_mbuf != NULL) { > - m_freem(sc->ure_cdata.rx_chain[i].uc_mbuf); > - sc->ure_cdata.rx_chain[i].uc_mbuf = NULL; > - } > - if (sc->ure_cdata.rx_chain[i].uc_xfer != NULL) { > - usbd_free_xfer(sc->ure_cdata.rx_chain[i].uc_xfer); > - sc->ure_cdata.rx_chain[i].uc_xfer = NULL; > + cd = &sc->ure_cdata; > + ure_xfer_list_free(sc, cd->ure_rx_chain, URE_RX_LIST_CNT); > + ure_xfer_list_free(sc, cd->ure_tx_chain, URE_TX_LIST_CNT); > +} > + > +int > +ure_xfer_list_init(struct ure_softc *sc, struct ure_chain *ch, > + uint32_t bufsize, int listlen) > +{ > + struct ure_chain *c; > + int i; > + > + for (i = 0; i < listlen; i++) { > + c = &ch[i]; > + c->uc_sc = sc; > + c->uc_idx = i; > + c->uc_buflen = 0; > + c->uc_bufmax = bufsize; > + ml_init(&c->uc_mbuf_list); > + if (c->uc_xfer == NULL) { > + c->uc_xfer = usbd_alloc_xfer(sc->ure_udev); > + if (c->uc_xfer == NULL) > + return (ENOBUFS); > + c->uc_buf = usbd_alloc_buffer(c->uc_xfer, c->uc_bufmax); > + if (c->uc_buf == NULL) { > + usbd_free_xfer(c->uc_xfer); > + c->uc_xfer = NULL; > + return (ENOBUFS); > + } > } > } > > - for (i = 0; i < sc->ure_tx_list_cnt; i++) { > - if (sc->ure_cdata.tx_chain[i].uc_mbuf != NULL) { > - m_freem(sc->ure_cdata.tx_chain[i].uc_mbuf); > - sc->ure_cdata.tx_chain[i].uc_mbuf = NULL; > - } > - if (sc->ure_cdata.tx_chain[i].uc_xfer != NULL) { > - usbd_free_xfer(sc->ure_cdata.tx_chain[i].uc_xfer); > - sc->ure_cdata.tx_chain[i].uc_xfer = NULL; > + return (0); > +} > + > +void > +ure_xfer_list_free(struct ure_softc *sc, struct ure_chain *ch, int listlen) > +{ > + int i; > + > + for (i = 0; i < listlen; i++) { > + if (ch[i].uc_buf != NULL) { > + ch[i].uc_buf = NULL; > + } > + ml_purge(&ch[i].uc_mbuf_list); > + if (ch[i].uc_xfer != NULL) { > + usbd_free_xfer(ch[i].uc_xfer); > + ch[i].uc_xfer = NULL; > } > } > } > @@ -1458,20 +1570,12 @@ ure_attach(struct device *parent, struct > } > } > > - switch (uaa->product) { > - case USB_PRODUCT_REALTEK_RTL8152: > - sc->ure_flags |= URE_FLAG_8152; > - sc->ure_rxbufsz = URE_8152_RXBUFSZ; > - sc->ure_tx_list_cnt = 1; > - break; > - case USB_PRODUCT_REALTEK_RTL8156: > - sc->ure_flags |= URE_FLAG_8156; > - sc->ure_rxbufsz = URE_8153_RXBUFSZ; > - sc->ure_tx_list_cnt = URE_TX_LIST_CNT; > - break; > - default: > - sc->ure_rxbufsz = URE_8153_RXBUFSZ; > - sc->ure_tx_list_cnt = 1; > + if (uaa->product == USB_PRODUCT_REALTEK_RTL8152) { > + sc->ure_txbufsz = URE_8152_TX_BUFSZ; > + sc->ure_rxbufsz = URE_8152_RX_BUFSZ; > + } else { > + sc->ure_txbufsz = URE_8153_TX_BUFSZ; > + sc->ure_rxbufsz = URE_8153_RX_BUFSZ; > } > > s = splnet(); > @@ -1482,10 +1586,12 @@ ure_attach(struct device *parent, struct > ver = ure_read_2(sc, URE_PLA_TCR1, URE_MCU_TYPE_PLA) & URE_VERSION_MASK; > switch (ver) { > case 0x4c00: > + sc->ure_flags = URE_FLAG_8152; > sc->ure_chip |= URE_CHIP_VER_4C00; > printf("RTL8152 (0x4c00)"); > break; > case 0x4c10: > + sc->ure_flags = URE_FLAG_8152; > sc->ure_chip |= URE_CHIP_VER_4C10; > printf("RTL8152 (0x4c10)"); > break; > @@ -1507,18 +1613,18 @@ ure_attach(struct device *parent, struct > break; > case 0x6000: > sc->ure_flags = URE_FLAG_8153B; > - sc->ure_tx_list_cnt = URE_TX_LIST_CNT; > printf("RTL8153B (0x6000)"); > break; > case 0x6010: > sc->ure_flags = URE_FLAG_8153B; > - sc->ure_tx_list_cnt = URE_TX_LIST_CNT; > printf("RTL8153B (0x6010)"); > break; > case 0x7020: > + sc->ure_flags = URE_FLAG_8156; > printf("RTL8156 (0x7020)"); > break; > case 0x7030: > + sc->ure_flags = URE_FLAG_8156; > printf("RTL8156 (0x7030)"); > break; > default: > @@ -1721,7 +1827,7 @@ ure_rxeof(struct usbd_xfer *xfer, void * > goto done; > } > > - buf += roundup(pktlen, 8); > + buf += roundup(pktlen, URE_RX_BUF_ALIGN); > > memcpy(&rxhdr, buf, sizeof(rxhdr)); > total_len -= sizeof(rxhdr); > @@ -1734,7 +1840,7 @@ ure_rxeof(struct usbd_xfer *xfer, void * > goto done; > } > > - total_len -= roundup(pktlen, 8); > + total_len -= roundup(pktlen, URE_RX_BUF_ALIGN); > buf += sizeof(rxhdr); > > m = m_devget(buf, pktlen, ETHER_ALIGN); > @@ -1799,18 +1905,27 @@ ure_txeof(struct usbd_xfer *xfer, void * > if (usbd_is_dying(sc->ure_udev)) > return; > > - DPRINTFN(2, ("tx completion\n")); > + if (status != USBD_NORMAL_COMPLETION) > + DPRINTF(("%s: %s uc_idx=%u : %s\n", sc->ure_dev.dv_xname, > + __func__, c->uc_idx, usbd_errstr(status))); > > s = splnet(); > - sc->ure_cdata.tx_cnt--; > + > + ml_purge(&c->uc_mbuf_list); > + c->uc_buflen = 0; > + > + SLIST_INSERT_HEAD(&sc->ure_cdata.ure_tx_free, c, ure_list); > + > if (status != USBD_NORMAL_COMPLETION) { > if (status == USBD_NOT_STARTED || status == USBD_CANCELLED) { > splx(s); > return; > } > + > ifp->if_oerrors++; > printf("%s: usb error on tx: %s\n", sc->ure_dev.dv_xname, > usbd_errstr(status)); > + > if (status == USBD_STALLED) > usbd_clear_endpoint_stall_async( > sc->ure_ep[URE_ENDPT_TX]); > @@ -1819,83 +1934,18 @@ ure_txeof(struct usbd_xfer *xfer, void * > } > > ifp->if_timer = 0; > - ifq_clr_oactive(&ifp->if_snd); > - > - m_freem(c->uc_mbuf); > - c->uc_mbuf = NULL; > - > - if (!ifq_empty(&ifp->if_snd)) > - ure_start(ifp); > - > - splx(s); > -} > - > -int > -ure_tx_list_init(struct ure_softc *sc) > -{ > - struct ure_cdata *cd; > - struct ure_chain *c; > - int i; > - > - cd = &sc->ure_cdata; > - for (i = 0; i < sc->ure_tx_list_cnt; i++) { > - c = &cd->tx_chain[i]; > - c->uc_sc = sc; > - c->uc_idx = i; > - c->uc_mbuf = NULL; > - if (c->uc_xfer == NULL) { > - c->uc_xfer = usbd_alloc_xfer(sc->ure_udev); > - if (c->uc_xfer == NULL) > - return ENOBUFS; > - c->uc_buf = usbd_alloc_buffer(c->uc_xfer, URE_TXBUFSZ); > - if (c->uc_buf == NULL) { > - usbd_free_xfer(c->uc_xfer); > - return ENOBUFS; > - } > - } > - } > - > - cd->tx_prod = cd->tx_cnt = 0; > - > - return (0); > + if (ifq_is_oactive(&ifp->if_snd)) > + ifq_restart(&ifp->if_snd); > } > > int > -ure_rx_list_init(struct ure_softc *sc) > +ure_encap_txpkt(struct mbuf *m, char *buf, uint32_t maxlen) > { > - struct ure_cdata *cd; > - struct ure_chain *c; > - int i; > - > - cd = &sc->ure_cdata; > - for (i = 0; i < URE_RX_LIST_CNT; i++) { > - c = &cd->rx_chain[i]; > - c->uc_sc = sc; > - c->uc_idx = i; > - c->uc_mbuf = NULL; > - if (c->uc_xfer == NULL) { > - c->uc_xfer = usbd_alloc_xfer(sc->ure_udev); > - if (c->uc_xfer == NULL) > - return ENOBUFS; > - c->uc_buf = usbd_alloc_buffer(c->uc_xfer, > - sc->ure_rxbufsz); > - if (c->uc_buf == NULL) { > - usbd_free_xfer(c->uc_xfer); > - return ENOBUFS; > - } > - } > - } > - > - return (0); > -} > - > -int > -ure_encap(struct ure_softc *sc, struct mbuf *m) > -{ > - struct ure_chain *c; > - usbd_status err; > struct ure_txpkt txhdr; > - uint32_t frm_len = 0, cflags = 0; > + uint32_t len = sizeof(txhdr), cflags = 0; > + > + if (len + m->m_pkthdr.len > maxlen) > + return (-1); > > if ((m->m_pkthdr.csum_flags & > (M_IPV4_CSUM_OUT | M_TCP_CSUM_OUT | M_UDP_CSUM_OUT)) != 0) { > @@ -1911,35 +1961,41 @@ ure_encap(struct ure_softc *sc, struct m > cflags |= swap16(m->m_pkthdr.ether_vtag | URE_TXPKT_VLAN_TAG); > #endif > > - c = &sc->ure_cdata.tx_chain[sc->ure_cdata.tx_prod]; > - > - /* header */ > txhdr.ure_pktlen = htole32(m->m_pkthdr.len | URE_TXPKT_TX_FS | > URE_TXPKT_TX_LS); > txhdr.ure_vlan = htole32(cflags); > - memcpy(c->uc_buf, &txhdr, sizeof(txhdr)); > - frm_len = sizeof(txhdr); > + memcpy(buf, &txhdr, len); > > - /* packet */ > - m_copydata(m, 0, m->m_pkthdr.len, c->uc_buf + frm_len); > - frm_len += m->m_pkthdr.len; > + m_copydata(m, 0, m->m_pkthdr.len, buf + len); > + len += m->m_pkthdr.len; > > - c->uc_mbuf = m; > + return (len); > +} > + > +int > +ure_encap_xfer(struct ifnet *ifp, struct ure_softc *sc, struct ure_chain *c) > +{ > + usbd_status err; > > - DPRINTFN(2, ("tx %d bytes\n", frm_len)); > usbd_setup_xfer(c->uc_xfer, sc->ure_ep[URE_ENDPT_TX], c, c->uc_buf, > - frm_len, USBD_FORCE_SHORT_XFER | USBD_NO_COPY, 10000, ure_txeof); > + c->uc_buflen, USBD_FORCE_SHORT_XFER | USBD_NO_COPY, 10000, > + ure_txeof); > > err = usbd_transfer(c->uc_xfer); > - if (err != 0 && err != USBD_IN_PROGRESS) { > - c->uc_mbuf = NULL; > + if (err != USBD_IN_PROGRESS) { > + ml_purge(&c->uc_mbuf_list); > + c->uc_buflen = 0; > ure_stop(sc); > return (EIO); > } > > - sc->ure_cdata.tx_cnt++; > - sc->ure_cdata.tx_prod = (sc->ure_cdata.tx_prod + 1) % > - sc->ure_tx_list_cnt; > +#if NBPFILTER > 0 > + struct mbuf *m; > + > + if (ifp->if_bpf) > + MBUF_LIST_FOREACH(&c->uc_mbuf_list, m) > + bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT); > +#endif > > return (0); > } -- Regards, Mikolaj
