# HG changeset patch # User Taylor R Campbell # Date 1769392552 0 # Mon Jan 26 01:55:52 2026 +0000 # Branch trunk # Node ID 6100a22b209db0d764ab149f7f79961fcb9f7846 # Parent 0ec2f07fad37df0769bb142f8c4b9d17de4e8c77 # EXP-Topic riastradh-pr59940-usbnettxlenaudit usbnet(9): Tighten tx path. 1. Verify, don't just assert, that the packet length is below the buffer size. Even if all the callers enforce the interface's MTU, I can't prove that the usbnet(9) tx buffer size is an upper bound enforced on the interface's MTU. We can remove the check later if we do enforce that upper bound at some point, which would probably be worth doing anyway since the MTU is checked earlier in the tx path. 2. Assert, don't check, that c->unc_xfer is nonnull. We can only reach the tx path if we cross if_init=usbnet_init_rx_tx, and that (via usbnet_tx_list_init) is guaranteed to fail and back out unless all of the usbnet_chain unc_xfers get initialized. 3. If we can't fit the packet into a buffer, drop it -- don't leave it in the queue to try again when it still won't fit in the buffer. 4. If the transfer for this packet fails, drop it -- don't leave it in the queue to try again just in case it might work better the next time. PR kern/59940: usbnet(9): uno_tx_prepare buffer overrun audit PR kern/59943: usbnet(9) keeps failed packet in queue to retry indefinitely diff -r 0ec2f07fad37 -r 6100a22b209d sys/dev/usb/usbnet.c --- a/sys/dev/usb/usbnet.c Sat Jan 31 17:45:10 2026 +0000 +++ b/sys/dev/usb/usbnet.c Mon Jan 26 01:55:52 2026 +0000 @@ -521,26 +521,31 @@ usbnet_start_locked(struct ifnet *ifp) idx = cd->uncd_tx_prod; count = 0; while (cd->uncd_tx_cnt < un->un_tx_list_cnt) { - IFQ_POLL(&ifp->if_snd, m); + IFQ_DEQUEUE(&ifp->if_snd, m); if (m == NULL) { DPRINTF("start called, queue empty", 0, 0, 0, 0); break; } - KASSERT(m->m_pkthdr.len <= un->un_tx_bufsz); + if ((unsigned)m->m_pkthdr.len > un->un_tx_bufsz) { + DPRINTF("oversize packet, %ju > %ju", + (unsigned)m->m_pkthdr.len, un->un_tx_bufsz, 0, 0); + if_statinc(ifp, if_oerrors); + m_freem(m); + continue; + } + KASSERTMSG((unsigned)m->m_pkthdr.len <= un->un_tx_bufsz, + "m->m_pkthdr.len=%d bufsz=%u", + m->m_pkthdr.len, un->un_tx_bufsz); - struct usbnet_chain *c = &cd->uncd_tx_chain[idx]; + struct usbnet_chain *const c = &cd->uncd_tx_chain[idx]; + KASSERT(c->unc_xfer != NULL); length = uno_tx_prepare(un, m, c); if (length == 0) { DPRINTF("uno_tx_prepare gave zero length", 0, 0, 0, 0); if_statinc(ifp, if_oerrors); - break; - } - - if (__predict_false(c->unc_xfer == NULL)) { - DPRINTF("unc_xfer is NULL", 0, 0, 0, 0); - if_statinc(ifp, if_oerrors); - break; + m_freem(m); + continue; } usbd_setup_xfer(c->unc_xfer, c, c->unc_buf, length, @@ -552,12 +557,11 @@ usbnet_start_locked(struct ifnet *ifp) DPRINTF("usbd_transfer on %#jx for %ju bytes: %jd", (uintptr_t)c->unc_buf, length, err, 0); if_statinc(ifp, if_oerrors); - break; + m_freem(m); + continue; } done_transmit = true; - IFQ_DEQUEUE(&ifp->if_snd, m); - /* * If there's a BPF listener, bounce a copy of this frame * to him. # HG changeset patch # User Taylor R Campbell # Date 1769290484 0 # Sat Jan 24 21:34:44 2026 +0000 # Branch trunk # Node ID 35fbb70de7c6934cb0b574504ef31dccbb174506 # Parent 6100a22b209db0d764ab149f7f79961fcb9f7846 # EXP-Topic riastradh-pr59939-usbnetmultipkttxxfer WIP: usbnet(9): Support multi-packet tx transfers. New usbnet_ops: overhead = uno_tx_begin(un) Returns the number of bytes of overhead in a USB transfer, independent of the packets being transmitted, or -1 if tx is blocked for any reason. If successful (i.e., if returns >=0), always followed by a sequence of calls to uno_tx_add and then either uno_tx_abort or uno_tx_commit. overhead = uno_tx_add(un, m, pktno, offset) Returns the number of bytes of overhead in a USB transfer to transmit the packet m _beyond_ m->m_pkthdr.len, or -1 if tx is blocked for any reason. pktno is the zero-based sequence number of m in the transfer, and offset is the number of bytes reserved so far by uno_tx_begin and uno_tx_add and all the packet lengths. In other words, if packets m0, m1, ..., mk have already passed through uno_tx_add, then pktno = k + 1 offset = uno_tx_begin(un) + uno_tx_add(un, m0, ...) + m0->m_pkthdr.len + uno_tx_add(un, m1, ...) + m1->m_pkthdr.len ... + uno_tx_add(un, mk, ...) + mk->m_pkthdr.len Caller has guaranteed the transfer buffer already has space for offset + m->m_pkthdr.len bytes. uno_tx_abort(un) Called if the caller has decided not to transmit the packets it previously passed to uno_tx_begin/add. Optional; may be omitted if it has nothing to do. uno_tx_commit(un, m0, c, npkt, nbyte) Fill usbnet transfer chain c's buffer with any device-specific headers and payloads of the linked list of npkt packets headed by m0 linked through m_nexptkt previously passed in order to uno_tx_add, with a total of nbyte bytes reserved in the transfer, guaranteed to be at most un->un_tx_bufsz. XXX kernel revbump -- breaks usbnet(9) module ABI by expanding struct usbnet_ops with no version or feature flag on which to gate access kern/59939: usbnet(9): support multi-packet tx transfers diff -r 6100a22b209d -r 35fbb70de7c6 sys/dev/usb/usbnet.c --- a/sys/dev/usb/usbnet.c Mon Jan 26 01:55:52 2026 +0000 +++ b/sys/dev/usb/usbnet.c Sat Jan 24 21:34:44 2026 +0000 @@ -229,6 +229,45 @@ uno_tx_prepare(struct usbnet *un, struct return (*un->un_ops->uno_tx_prepare)(un, m, c); } +static int +uno_tx_begin(struct usbnet *un) +{ + usbnet_isowned_tx(un); + return (*un->un_ops->uno_tx_begin)(un); +} + +static int +uno_tx_add(struct usbnet *un, struct mbuf *m, unsigned pktno, unsigned offset) +{ + usbnet_isowned_tx(un); + return (*un->un_ops->uno_tx_add)(un, m, pktno, offset); +} + +static void +uno_tx_abort(struct usbnet *un) +{ + usbnet_isowned_tx(un); + if (un->un_ops->uno_tx_abort) + return (*un->un_ops->uno_tx_abort)(un); +} + +static void +uno_tx_commit(struct usbnet *un, struct mbuf *m, struct usbnet_chain *c, + unsigned npkt, unsigned nbyte) +{ + usbnet_isowned_tx(un); + return (*un->un_ops->uno_tx_commit)(un, m, c, npkt, nbyte); +} + +static bool +usbnet_batch_tx(struct usbnet *un) +{ + + return un->un_ops->uno_tx_begin != NULL && + un->un_ops->uno_tx_add != NULL && + un->un_ops->uno_tx_commit != NULL; +} + static void uno_rx_loop(struct usbnet *un, struct usbnet_chain *c, uint32_t total_len) { @@ -521,11 +560,16 @@ usbnet_start_locked(struct ifnet *ifp) idx = cd->uncd_tx_prod; count = 0; while (cd->uncd_tx_cnt < un->un_tx_list_cnt) { + struct mbuf *batch = NULL, **batch_tail = &batch; + unsigned npkt = 0; + IFQ_DEQUEUE(&ifp->if_snd, m); if (m == NULL) { DPRINTF("start called, queue empty", 0, 0, 0, 0); break; } + npkt++; + if ((unsigned)m->m_pkthdr.len > un->un_tx_bufsz) { DPRINTF("oversize packet, %ju > %ju", (unsigned)m->m_pkthdr.len, un->un_tx_bufsz, 0, 0); @@ -540,6 +584,127 @@ usbnet_start_locked(struct ifnet *ifp) struct usbnet_chain *const c = &cd->uncd_tx_chain[idx]; KASSERT(c->unc_xfer != NULL); + if (usbnet_batch_tx(un)) { + unsigned offset = 0; + int overhead; + + overhead = uno_tx_begin(un); + KASSERTMSG((overhead == -1 || + (unsigned)overhead <= un->un_tx_bufsz), + "overhead=%d bufsz=%u", overhead, un->un_tx_bufsz); + if (overhead == -1) { + DPRINTF("uno_tx_begin failed", 0, 0, 0, 0); + if_statinc(ifp, if_oerrors); + m_freem(m); + continue; + } + if ((unsigned)overhead > + (un->un_tx_bufsz - (unsigned)m->m_pkthdr.len)) { + DPRINTF("uno_tx_begin overhead too large", + 0, 0, 0, 0); + uno_tx_abort(un); + if_statinc(ifp, if_oerrors); + m_freem(m); + continue; + } + offset = (unsigned)overhead; + + KASSERTMSG(((unsigned)m->m_pkthdr.len <= + un->un_tx_bufsz - offset), + "offset=%u m->m_pkthdr.len=%d bufsz=%u", + offset, m->m_pkthdr.len, un->un_tx_bufsz); + overhead = uno_tx_add(un, m, 0, offset); + KASSERTMSG((overhead == -1 || + (unsigned)overhead <= un->un_tx_bufsz), + "overhead=%d bufsz=%u", + overhead, un->un_tx_bufsz); + if (overhead == -1 || + (unsigned)overhead > (un->un_tx_bufsz - offset - + (unsigned)m->m_pkthdr.len)) { + DPRINTF("uno_tx_add failed", 0, 0, 0, 0); + uno_tx_abort(un); + if_statinc(ifp, if_oerrors); + m_freem(m); + continue; + } + for (;;) { + /* + * Loop invariant: We have a packet m, + * and we know its overhead and length + * both fit in the remaining buffer + * space. + * + * Queue it up, and then examine the + * next packet. + */ + KASSERT(m != NULL); + KASSERTMSG(((unsigned)overhead <= + un->un_tx_bufsz - offset), + "offset=%u overhead=%d len=%d bufsz=%u", + offset, overhead, m->m_pkthdr.len, + un->un_tx_bufsz); + KASSERTMSG(((unsigned)m->m_pkthdr.len <= + un->un_tx_bufsz - offset - + (unsigned)overhead), + "offset=%u overhead=%d len=%d bufsz=%u", + offset, overhead, m->m_pkthdr.len, + un->un_tx_bufsz); + offset += (unsigned)overhead; + offset += (unsigned)m->m_pkthdr.len; + KASSERTMSG(offset <= un->un_tx_bufsz, + "offset=%u bufsz=%u", + offset, un->un_tx_bufsz); + m->m_nextpkt = NULL; + *batch_tail = m; + batch_tail = &m->m_nextpkt; + + IFQ_POLL(&ifp->if_snd, m); + if (m == NULL) + break; + KASSERTMSG(((unsigned)m->m_pkthdr.len <= + un->un_tx_bufsz), + "m->m_pkthdr.len=%d bufsz=%u", + m->m_pkthdr.len, un->un_tx_bufsz); + if ((unsigned)m->m_pkthdr.len > + un->un_tx_bufsz - offset) { + /* packet payload too large */ + m = NULL; /* leave in ifp->if_snd q */ + break; + } + KASSERTMSG(((unsigned)m->m_pkthdr.len <= + un->un_tx_bufsz - offset), + "offset=%u m->m_pkthdr.len=%d bufsz=%u", + offset, m->m_pkthdr.len, un->un_tx_bufsz); + overhead = uno_tx_add(un, m, npkt, offset); + KASSERTMSG((overhead == -1 || + (unsigned)overhead <= un->un_tx_bufsz), + "overhead=%d bufsz=%u", + overhead, un->un_tx_bufsz); + if (overhead == -1 || + (unsigned)overhead > (un->un_tx_bufsz - + offset - (unsigned)m->m_pkthdr.len)) { + /* + * driver didn't like the + * packet, or packet overhead + * too large + */ + m = NULL; /* leave in ifp->if_snd q */ + break; + } + IFQ_DEQUEUE(&ifp->if_snd, m); + npkt++; + } + + /* + * We have gathered a nonempty batch of packets + * to transfer. Ask the driver to fill up the + * transfer buffer with the packets. + */ + uno_tx_commit(un, batch, c, npkt, offset); + length = offset; + goto xfer; + } + length = uno_tx_prepare(un, m, c); if (length == 0) { DPRINTF("uno_tx_prepare gave zero length", 0, 0, 0, 0); @@ -548,6 +713,8 @@ usbnet_start_locked(struct ifnet *ifp) continue; } +xfer: KASSERTMSG(length <= un->un_tx_bufsz, + "length=%u bufsz=%u", length, un->un_tx_bufsz); usbd_setup_xfer(c->unc_xfer, c, c->unc_buf, length, un->un_tx_xfer_flags, 10000, usbnet_txeof); @@ -556,18 +723,35 @@ usbnet_start_locked(struct ifnet *ifp) if (err != USBD_IN_PROGRESS) { DPRINTF("usbd_transfer on %#jx for %ju bytes: %jd", (uintptr_t)c->unc_buf, length, err, 0); - if_statinc(ifp, if_oerrors); - m_freem(m); + if_statadd(ifp, if_oerrors, npkt); + if (batch) { + while (batch != NULL) { + m = batch; + batch = m->m_nextpkt; + m_freem(m); + } + } else { + m_freem(m); + } continue; } done_transmit = true; /* - * If there's a BPF listener, bounce a copy of this frame - * to him. + * If there's a BPF listener, bounce a copy of these + * frames to them, and free them all. */ - bpf_mtap(ifp, m, BPF_D_OUT); - m_freem(m); + if (batch) { + while (batch != NULL) { + m = batch; + batch = m->m_nextpkt; + bpf_mtap(ifp, m, BPF_D_OUT); + m_freem(m); + } + } else { + bpf_mtap(ifp, m, BPF_D_OUT); + m_freem(m); + } idx = (idx + 1) % un->un_tx_list_cnt; cd->uncd_tx_cnt++; diff -r 6100a22b209d -r 35fbb70de7c6 sys/dev/usb/usbnet.h --- a/sys/dev/usb/usbnet.h Mon Jan 26 01:55:52 2026 +0000 +++ b/sys/dev/usb/usbnet.h Sat Jan 24 21:34:44 2026 +0000 @@ -141,6 +141,12 @@ typedef void (*usbnet_mii_statchg_cb)(st /* Prepare packet to send callback, returns length. */ typedef unsigned (*usbnet_tx_prepare_cb)(struct usbnet *, struct mbuf *, struct usbnet_chain *); +typedef int (*usbnet_tx_begin_cb)(struct usbnet *); +typedef int (*usbnet_tx_add_cb)(struct usbnet *, struct mbuf *, + unsigned, unsigned); +typedef void (*usbnet_tx_abort_cb)(struct usbnet *); +typedef void (*usbnet_tx_commit_cb)(struct usbnet *, struct mbuf *, + struct usbnet_chain *, unsigned, unsigned); /* Receive some packets callback. */ typedef void (*usbnet_rx_loop_cb)(struct usbnet *, struct usbnet_chain *, uint32_t); @@ -181,6 +187,10 @@ struct usbnet_ops { usbnet_rx_loop_cb uno_rx_loop; /* R */ usbnet_tick_cb uno_tick; /* n */ usbnet_intr_cb uno_intr; /* n */ + usbnet_tx_begin_cb uno_tx_begin; /* T */ + usbnet_tx_add_cb uno_tx_add; /* T */ + usbnet_tx_abort_cb uno_tx_abort; /* T */ + usbnet_tx_commit_cb uno_tx_commit; /* T */ }; /* # HG changeset patch # User Taylor R Campbell # Date 1769290712 0 # Sat Jan 24 21:38:32 2026 +0000 # Branch trunk # Node ID 34f6e8366a6c0c73b522a5ff14340eb10e84131b # Parent 35fbb70de7c6934cb0b574504ef31dccbb174506 # EXP-Topic riastradh-pr59939-usbnetmultipkttxxfer WIP: ure(4): Support multi-packet tx transfers. PR kern/59939: usbnet(9): support multi-packet tx transfers diff -r 35fbb70de7c6 -r 34f6e8366a6c sys/dev/usb/if_ure.c --- a/sys/dev/usb/if_ure.c Sat Jan 24 21:34:44 2026 +0000 +++ b/sys/dev/usb/if_ure.c Sat Jan 24 21:38:32 2026 +0000 @@ -95,6 +95,11 @@ static int ure_uno_mii_write_reg(struct static void ure_uno_miibus_statchg(struct ifnet *); static unsigned ure_uno_tx_prepare(struct usbnet *, struct mbuf *, struct usbnet_chain *); +static int ure_uno_tx_begin(struct usbnet *); +static int ure_uno_tx_add(struct usbnet *, struct mbuf *, + unsigned, unsigned); +static void ure_uno_tx_commit(struct usbnet *, struct mbuf *, + struct usbnet_chain *, unsigned, unsigned); static void ure_uno_rx_loop(struct usbnet *, struct usbnet_chain *, uint32_t); static int ure_uno_init(struct ifnet *); @@ -112,6 +117,9 @@ static const struct usbnet_ops ure_ops = .uno_write_reg = ure_uno_mii_write_reg, .uno_statchg = ure_uno_miibus_statchg, .uno_tx_prepare = ure_uno_tx_prepare, + .uno_tx_begin = ure_uno_tx_begin, + .uno_tx_add = ure_uno_tx_add, + .uno_tx_commit = ure_uno_tx_commit, .uno_rx_loop = ure_uno_rx_loop, .uno_init = ure_uno_init, }; @@ -1035,14 +1043,20 @@ ure_rxcsum(struct ifnet *ifp, struct ure } static unsigned -ure_uno_tx_prepare(struct usbnet *un, struct mbuf *m, struct usbnet_chain *c) +ure_uno_tx_prepare_offset(struct usbnet *un, struct mbuf *m, + struct usbnet_chain *c, unsigned offset) { struct ure_txpkt txhdr; uint32_t frm_len = 0; - uint8_t *buf = c->unc_buf; + uint8_t *buf = c->unc_buf + offset; - if ((unsigned)m->m_pkthdr.len > un->un_tx_bufsz - sizeof(txhdr)) - return 0; + __CTASSERT(sizeof(txhdr) <= URE_TX_BUFSZ); + KASSERTMSG((sizeof(txhdr) <= un->un_tx_bufsz && + offset <= un->un_tx_bufsz - sizeof(txhdr) && + (unsigned)m->m_pkthdr.len <= (un->un_tx_bufsz - sizeof(txhdr) - + offset)), + "offset=%u sizeof(txhdr)=%zu len=%u bufsz=%u", + offset, sizeof(txhdr), m->m_pkthdr.len, un->un_tx_bufsz); /* header */ txhdr.ure_pktlen = htole32(m->m_pkthdr.len | URE_TXPKT_TX_FS | @@ -1061,6 +1075,53 @@ ure_uno_tx_prepare(struct usbnet *un, st return frm_len; } +static unsigned +ure_uno_tx_prepare(struct usbnet *un, struct mbuf *m, struct usbnet_chain *c) +{ + + __CTASSERT(sizeof(struct ure_txpkt) <= URE_TX_BUFSZ); + KASSERTMSG(sizeof(struct ure_txpkt) <= un->un_tx_bufsz, + "sizeof(struct ure_txpkt)=%zu bufsz=%u", + sizeof(struct ure_txpkt), un->un_tx_bufsz); + if ((unsigned)m->m_pkthdr.len > + un->un_tx_bufsz - sizeof(struct ure_txpkt)) + return 0; + + return ure_uno_tx_prepare_offset(un, m, c, 0); +} + +static int +ure_uno_tx_begin(struct usbnet *un) +{ + + /* zero per-packet overhead */ + return 0; +} + +static int +ure_uno_tx_add(struct usbnet *un, struct mbuf *m, + unsigned pktno, unsigned offset) +{ + + return sizeof(struct ure_txpkt); +} + +static void +ure_uno_tx_commit(struct usbnet *un, struct mbuf *m0, struct usbnet_chain *c, + unsigned npkt, unsigned nbyte) +{ + struct mbuf *m; + unsigned offset = 0; + + for (m = m0; m != NULL; m = m->m_nextpkt) { + KASSERT(offset <= nbyte - + (sizeof(struct ure_txpkt) + (unsigned)m->m_pkthdr.len)); + offset += ure_uno_tx_prepare_offset(un, m, c, offset); + KASSERT(offset <= nbyte); + } + KASSERT(offset == nbyte); +} + /* * We need to calculate L4 checksum in software, if the offset of * L4 header is larger than 0x7ff = 2047. # HG changeset patch # User Taylor R Campbell # Date 1769297008 0 # Sat Jan 24 23:23:28 2026 +0000 # Branch trunk # Node ID 0f00feeede5f58f00c1f3e1a7992e8e91d5bd028 # Parent 34f6e8366a6c0c73b522a5ff14340eb10e84131b # EXP-Topic riastradh-pr59939-usbnetmultipkttxxfer WIP: ncm(4): Support multi-packet tx transfers. PR kern/59939: usbnet(9): support multi-packet tx transfers diff -r 34f6e8366a6c -r 0f00feeede5f sys/dev/usb/if_ncm.c --- a/sys/dev/usb/if_ncm.c Sat Jan 24 21:38:32 2026 +0000 +++ b/sys/dev/usb/if_ncm.c Sat Jan 24 23:23:28 2026 +0000 @@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_ncm.c,v 1 struct ncm_softc { struct usbnet ncm_un; + struct ncm_ntb_parameters sc_ntb_param; uint32_t sc_tx_seq; }; @@ -67,9 +68,17 @@ static void ncm_uno_rx_loop(struct usbne uint32_t); static unsigned ncm_uno_tx_prepare(struct usbnet *, struct mbuf *, struct usbnet_chain *); +static int ncm_uno_tx_begin(struct usbnet *); +static int ncm_uno_tx_add(struct usbnet *, struct mbuf *, + unsigned, unsigned); +static void ncm_uno_tx_commit(struct usbnet *, struct mbuf *, + struct usbnet_chain *, unsigned, unsigned); static const struct usbnet_ops ncm_ops = { .uno_tx_prepare = ncm_uno_tx_prepare, + .uno_tx_begin = ncm_uno_tx_begin, + .uno_tx_add = ncm_uno_tx_add, + .uno_tx_commit = ncm_uno_tx_commit, .uno_rx_loop = ncm_uno_rx_loop, }; @@ -102,8 +111,6 @@ ncm_attach(device_t parent, device_t sel const usb_cdc_ethernet_descriptor_t *ue; char eaddr_str[USB_MAX_ENCODED_STRING_LEN]; usb_device_request_t req; - struct ncm_ntb_parameters np; - aprint_naive("\n"); aprint_normal("\n"); @@ -226,18 +233,56 @@ ncm_attach(device_t parent, device_t sel req.bRequest = NCM_GET_NTB_PARAMETERS; USETW(req.wValue, 0); USETW(req.wIndex, uiaa->uiaa_ifaceno); - USETW(req.wLength, sizeof(np)); - if (usbd_do_request(un->un_udev, &req, &np) != USBD_NORMAL_COMPLETION || - UGETW(np.wLength) != sizeof(np)) { + USETW(req.wLength, sizeof(sc->sc_ntb_param)); + if (usbd_do_request(un->un_udev, &req, &sc->sc_ntb_param) != + USBD_NORMAL_COMPLETION || + UGETW(sc->sc_ntb_param.wLength) != sizeof(sc->sc_ntb_param)) { aprint_error_dev(un->un_dev, "NCM_GET_NTB_PARAMETERS failed\n"); return; } + aprint_verbose_dev(un->un_dev, "formats: 0x%02x%02x\n", + sc->sc_ntb_param.bmNtbFormatsSupported[1], + sc->sc_ntb_param.bmNtbFormatsSupported[0]); + aprint_verbose_dev(un->un_dev, + "in: max %u, =%u mod %u, align %u\n", + UGETDW(sc->sc_ntb_param.dwNtbInMaxSize), + UGETW(sc->sc_ntb_param.wNdpInPayloadRemainder), + UGETW(sc->sc_ntb_param.wNdpInDivisor), + UGETW(sc->sc_ntb_param.wNdpInAlignment)); + aprint_verbose_dev(un->un_dev, + "out: max %u (%u pkt), =%u mod %u, align %u", + UGETDW(sc->sc_ntb_param.dwNtbOutMaxSize), + UGETW(sc->sc_ntb_param.wNtbOutMaxDatagrams), + UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder), + UGETW(sc->sc_ntb_param.wNdpOutDivisor), + UGETW(sc->sc_ntb_param.wNdpOutAlignment)); + + /* paranoia */ + if (UGETW(sc->sc_ntb_param.wNdpInDivisor) == 0) + USETW(sc->sc_ntb_param.wNdpInDivisor, 1); + if (UGETW(sc->sc_ntb_param.wNdpOutDivisor) == 0) + USETW(sc->sc_ntb_param.wNdpOutDivisor, 1); + if (UGETW(sc->sc_ntb_param.wNdpInPayloadRemainder) >= + UGETW(sc->sc_ntb_param.wNdpInDivisor)) { + USETW(sc->sc_ntb_param.wNdpInPayloadRemainder, + UGETW(sc->sc_ntb_param.wNdpInPayloadRemainder) % + UGETW(sc->sc_ntb_param.wNdpInDivisor)); + } + if (UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder) >= + UGETW(sc->sc_ntb_param.wNdpOutDivisor)) { + USETW(sc->sc_ntb_param.wNdpOutPayloadRemainder, + UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder) % + UGETW(sc->sc_ntb_param.wNdpOutDivisor)); + } + un->un_rx_list_cnt = 1; un->un_tx_list_cnt = 1; - un->un_rx_bufsz = UGETDW(np.dwNtbInMaxSize); - un->un_tx_bufsz = UGETDW(np.dwNtbOutMaxSize); - if (un->un_tx_bufsz < NCM_MIN_TX_BUFSZ) { + un->un_rx_bufsz = UGETDW(sc->sc_ntb_param.dwNtbInMaxSize); + un->un_tx_bufsz = UGETDW(sc->sc_ntb_param.dwNtbOutMaxSize); + if (un->un_tx_bufsz < NCM_MIN_TX_BUFSZ || + un->un_tx_bufsz - NCM_MIN_TX_BUFSZ < + UGETW(sc->sc_ntb_param.wNdpOutDivisor)) { aprint_error_dev(un->un_dev, "dwNtbOutMaxSize %u too small\n", un->un_tx_bufsz); return; @@ -300,7 +345,7 @@ ncm_uno_rx_loop(struct usbnet *un, struc return; } - const unsigned max_datagrams = (UGETW(ptr->wLength) - + const unsigned max_datagrams = (UGETW(ptr->wLength) - offsetof(struct ncm_dptab16, datagram))/sizeof(ptr->datagram[0]); for (i = 0; i < max_datagrams; i++) { uint16_t data_start = UGETW(ptr->datagram[i].wDatagramIndex); @@ -320,46 +365,236 @@ ncm_uno_rx_loop(struct usbnet *un, struc } } +/* + * The layout of a tx xfer buffer is: + * + * +--------------+-------------+---------------+---------------+-----+ + * | ncm_header16 | ncm_dptab16 | ncm_pointer16 | ncm_pointer16 | ... | + * +--------------+-------------+---------------+---------------+-----+ + * +-----+----------------+-------+-----------+------------+-------------- + * | pad | payload 0 | pad | payload 1 | pad | payload 2 ... + * +-----+----------------+-------+-----------+------------+-------------- + * | | | + * k*d + r (k + 1)*d + r (k + 2)*d + r + * + * The ncm_header16 gives the offset of the datagram pointer table, + * headed by ncm_dptab16. There is one more ncm_pointer16 than packet, + * with zero offset/length to mark the end of the transfer. + * + * The NTB parameters require the packet payloads to start at specific + * offsets modulo a divisor: + * + * offset % wNdpOutDivisor = wNdpOutPayloadRemainder + * + * In other words, each offset is k*d + r for some k, where d is + * wNdpOutDivisor and r is wNdpOutPayloadRemainder. + */ + +/* + * ncm_out_align(sc, off0) + * + * Return the smallest offset off >= off0 such that + * + * off % wNdpOutDivisor = wNdpOutPayloadRemainder. + */ +static unsigned +ncm_out_align(struct ncm_softc *sc, unsigned off0) +{ + unsigned off; + + KASSERTMSG(off0 <= sc->ncm_un.un_tx_bufsz, "off0=%u bufsz=%u", + off0, sc->ncm_un.un_tx_bufsz); + + off = rounddown(off0, UGETW(sc->sc_ntb_param.wNdpOutDivisor)); + off += UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder); + if (off < off0) + off += UGETW(sc->sc_ntb_param.wNdpOutDivisor); + + KASSERTMSG(((off % UGETW(sc->sc_ntb_param.wNdpOutDivisor)) == + UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder)), + "off=%u divisor=%u remainder=%u", + off, + UGETW(sc->sc_ntb_param.wNdpOutDivisor), + UGETW(sc->sc_ntb_param.wNdpOutPayloadRemainder)); + return off; +} + static unsigned ncm_uno_tx_prepare(struct usbnet *un, struct mbuf *m, struct usbnet_chain *c) { struct ncm_softc *sc = un->un_sc; struct ncm_dptab16 *ptr; struct ncm_header16 *hdr; - unsigned hdr_len, len; + unsigned hdr_len, off, len; - hdr_len = sizeof(*hdr) + sizeof(*ptr); + hdr_len = sizeof(*hdr) + offsetof(struct ncm_dptab16, datagram[2]); + off = ncm_out_align(sc, hdr_len); len = m->m_pkthdr.len; - KASSERT(hdr_len <= un->un_tx_bufsz); - if (len > un->un_tx_bufsz - hdr_len) + KASSERTMSG(hdr_len <= un->un_tx_bufsz, + "hdr_len=%u bufsz=%u", hdr_len, un->un_tx_bufsz); + if (off > un->un_tx_bufsz || len > un->un_tx_bufsz - off) return 0; hdr = (struct ncm_header16 *)c->unc_buf; ptr = (struct ncm_dptab16 *)(hdr + 1); memset(hdr, 0, sizeof(*hdr)); + memset(ptr, 0, offsetof(struct ncm_dptab16, datagram[2])); + + USETDW(hdr->dwSignature, NCM_HDR16_SIG); + USETW(hdr->wHeaderLength, sizeof(*hdr)); + USETW(hdr->wSequence, sc->sc_tx_seq); + sc->sc_tx_seq++; + USETW(hdr->wBlockLength, off + len); + USETW(hdr->wNdpIndex, sizeof(*hdr)); + + USETDW(ptr->dwSignature, NCM_PTR16_SIG_NO_CRC); + USETW(ptr->wLength, offsetof(struct ncm_dptab16, datagram[2])); + USETW(ptr->wNextNdpIndex, 0); + + USETW(ptr->datagram[0].wDatagramIndex, off); + USETW(ptr->datagram[0].wDatagramLength, len); + + USETW(ptr->datagram[1].wDatagramIndex, 0); + USETW(ptr->datagram[1].wDatagramLength, 0); + + m_copydata(m, 0, len, c->unc_buf + off); + + return off + len; +} + +/* + * ncm_uno_tx_begin(un) + * + * Called when starting a batch of packets for tx in a single + * transfer. Returns the number of bytes of overhead in the + * transfer buffer, no matter how many packets are being + * transmitted. + */ +static int +ncm_uno_tx_begin(struct usbnet *un) +{ + struct ncm_softc *sc = un->un_sc; + + /* + * Find how many bytes of overhead we need for: + * - a transfer header + * - a datagram pointer table header + * - padding to align the first payload. + */ + return ncm_out_align(sc, sizeof(struct ncm_header16) + + offsetof(struct ncm_dptab16, datagram[1])); +} + +/* + * ncm_uno_tx_add(un, m, pktno, offset) + * + * Called for each candidate packet in a batch for tx in a single + * transfer. Returns the number of bytes of overhead, on top of + * m->m_pkthdr.len, to store this packet in the transfer buffer, + * given that pktno packets are already included. + * + * offset is the sum of ncm_uno_tx_begin(un) and + * ncm_uno_tx_add(un, m0, ...) for each previous packet m0 already + * included in the batch. + * + * Caller guarantees the transfer buffer already has enough space + * for offset + m->m_pkthdr.len bytes, i.e., that + * + * offset + m->m_pkthdr.len <= un->un_tx_bufsz. + */ +static int +ncm_uno_tx_add(struct usbnet *un, struct mbuf *m, + unsigned pktno, unsigned off) +{ + struct ncm_softc *sc = un->un_sc; + unsigned old_start, new_start; + + KASSERTMSG(off <= un->un_tx_bufsz, "off=%u bufsz=%u", + off, un->un_tx_bufsz); + KASSERTMSG((unsigned)m->m_pkthdr.len <= un->un_tx_bufsz - off, + "off=%u m->m_pkthdr.len=%u bufsz=%u", + off, (unsigned)m->m_pkthdr.len, un->un_tx_bufsz); + + if (UGETW(sc->sc_ntb_param.wNtbOutMaxDatagrams) != 0 && + pktno >= UGETW(sc->sc_ntb_param.wNtbOutMaxDatagrams)) + return -1; + + /* + * Find how many bytes we would need for header and padding to + * align the first payload with the previous number of packets, + * so we can subtract that from the number of bytes for the + * next number of packets. + * + * Note that we always need one extra entry in the datagram + * pointer table with zero index/length to terminate the list. + */ + old_start = ncm_out_align(sc, sizeof(struct ncm_header16) + + offsetof(struct ncm_dptab16, datagram[pktno + 1])); + new_start = ncm_out_align(sc, sizeof(struct ncm_header16) + + offsetof(struct ncm_dptab16, datagram[pktno + 2])); + + return (new_start - old_start) + (ncm_out_align(sc, off) - off); +} + +/* + * ncm_uno_tx_commit(un, m0, c, npkt, nbyte) + * + * Called when a batch of npkt packets, totalling nbyte bytes of + * payload and overhead (as returned by ncm_uno_tx_begin and + * ncm_uno_tx_add), has been gathered for tx in a single transfer. + * Formats the packets m0, m1 := m0->m_nextpkt, m2 := + * m1->m_nextpkt, ..., with appropriate headers into the buffer c. + */ +static void +ncm_uno_tx_commit(struct usbnet *un, struct mbuf *m0, struct usbnet_chain *c, + unsigned npkt, unsigned nbyte) +{ + struct ncm_softc *sc = un->un_sc; + struct ncm_header16 *hdr; + struct ncm_dptab16 *ptr; + unsigned i, off, len; + struct mbuf *m; + + KASSERTMSG(nbyte <= un->un_tx_bufsz, "nbyte=%u bufsz=%u", + nbyte, un->un_tx_bufsz); + + hdr = (struct ncm_header16 *)c->unc_buf; + ptr = (struct ncm_dptab16 *)(hdr + 1); + memset(hdr, 0, sizeof(*hdr)); memset(ptr, 0, sizeof(*ptr)); USETDW(hdr->dwSignature, NCM_HDR16_SIG); USETW(hdr->wHeaderLength, sizeof(*hdr)); USETW(hdr->wSequence, sc->sc_tx_seq); sc->sc_tx_seq++; - - USETW(hdr->wBlockLength, hdr_len + len); + USETW(hdr->wBlockLength, nbyte); USETW(hdr->wNdpIndex, sizeof(*hdr)); USETDW(ptr->dwSignature, NCM_PTR16_SIG_NO_CRC); - USETW(ptr->wLength, sizeof(*ptr)); + USETW(ptr->wLength, offsetof(struct ncm_dptab16, datagram[npkt + 1])); USETW(ptr->wNextNdpIndex, 0); - USETW(ptr->datagram[0].wDatagramIndex, hdr_len); - USETW(ptr->datagram[0].wDatagramLength, len); + off = (unsigned)((const uint8_t *)&ptr->datagram[npkt + 1] - + c->unc_buf); + for (i = 0, m = m0; i < npkt; i++, m = m->m_nextpkt) { + KASSERTMSG(m != NULL, "i=%u npkt=%u", i, npkt); + KASSERTMSG(off <= nbyte, "off=%u nbyte=%u", off, nbyte); + off = ncm_out_align(sc, off); + KASSERTMSG(off <= nbyte, "off=%u nbyte=%u", off, nbyte); + len = (unsigned)m->m_pkthdr.len; + KASSERTMSG(len <= nbyte - off, "off=%u len=%u nbyte=%u", + off, len, nbyte); + memset(&ptr->datagram[i], 0, sizeof(ptr->datagram[0])); + USETW(ptr->datagram[i].wDatagramIndex, off); + USETW(ptr->datagram[i].wDatagramLength, len); + m_copydata(m, 0, len, &c->unc_buf[off]); + off += len; + } + KASSERTMSG(i == npkt, "i=%u npkt=%u", i, npkt); + KASSERTMSG(off == nbyte, "off=%u nbyte=%u", off, nbyte); - USETW(ptr->datagram[1].wDatagramIndex, 0); - USETW(ptr->datagram[1].wDatagramLength, 0); - - m_copydata(m, 0, len, c->unc_buf + hdr_len); - - return len + hdr_len; + USETW(ptr->datagram[npkt].wDatagramIndex, 0); + USETW(ptr->datagram[npkt].wDatagramLength, 0); } #ifdef _MODULE diff -r 34f6e8366a6c -r 0f00feeede5f sys/dev/usb/if_ncmreg.h --- a/sys/dev/usb/if_ncmreg.h Sat Jan 24 21:38:32 2026 +0000 +++ b/sys/dev/usb/if_ncmreg.h Sat Jan 24 23:23:28 2026 +0000 @@ -42,7 +42,7 @@ #define NCM_MIN_TX_BUFSZ \ (ETHERMTU + ETHER_HDR_LEN \ + sizeof(struct ncm_header16) \ - + sizeof(struct ncm_pointer16)) + + offsetof(struct ncm_dptab16, datagram[2])) struct ncm_header16 { #define NCM_HDR16_SIG 0x484d434e @@ -73,7 +73,7 @@ struct ncm_dptab16 { uDWord dwSignature; uWord wLength; uWord wNextNdpIndex; - struct ncm_pointer16 datagram[2]; + struct ncm_pointer16 datagram[0]; } UPACKED; struct ncm_pointer32 { @@ -89,7 +89,7 @@ struct ncm_dptab32 { uWord wReserved6; uWord wNextNdpIndex; uDWord dwReserved12; - struct ncm_pointer32 datagram[2]; + struct ncm_pointer32 datagram[0]; } UPACKED; #define NCM_GET_NTB_PARAMETERS 0x80