NetBSD-Bugs archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/59939: usbnet(9): support multi-packet tx transfers
The following reply was made to PR kern/59939; it has been noted by GNATS.
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: netbsd-bugs%NetBSD.org@localhost, gnats-bugs%NetBSD.org@localhost
Cc: Jared McNeill <jmcneill%invisible.ca@localhost>
Subject: Re: kern/59939: usbnet(9): support multi-packet tx transfers
Date: Sat, 24 Jan 2026 21:44:35 +0000
This is a multi-part message in MIME format.
--=_rFLsIlDMAmZS54PhaM3eE2ii5a8qA3Zz
Compile-tested only so far. Doesn't address the ABI compatibility
issue, just does this in a way to maintain API compatibility so I
don't have to patch all the existing usbnet(9) drivers in tree.
Handles up to 17 packets in a batch. Could maybe eliminate that
arbitrary constant by adapting it to use its own linked list of mbufs
for the unwind path if the transfer fails, since the mbufs are no
longer on the ifq. TBD; let's see if this is worth doing first.
--=_rFLsIlDMAmZS54PhaM3eE2ii5a8qA3Zz
Content-Type: text/plain; charset="ISO-8859-1"; name="pr59939-multipkttxxfer"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="pr59939-multipkttxxfer.patch"
# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1769290484 0
# Sat Jan 24 21:34:44 2026 +0000
# Branch trunk
# Node ID 0c1b80d41c95e437ff64c85b0e72466bebf8dd60
# Parent 2abb22668d716977f0c5b6d599d23691f2845a30
# EXP-Topic riastradh-pr59939-usbnetmultipkttxxfer
WIP: usbnet(9): Support multi-packet tx transfers.
New usbnet_ops operation uno_tx_prepare1(un, c, m, offset) asks the
driver to encode packet m at the given byte offset in the tx chain
buffer c, and returns the number of bytes consumed by that encoding
with any device-specific header included, or returns zero if it
wouldn't fit.
uno_tx_prepare(un, c, m) should be equivalent to uno_tx_prepare1(un,
c, m, 0), but the original operation remains intact in this draft in
order to avoid having to update all the drivers that don't support
multi-packet tx transfers: usbnet(9) will only call uno_tx_prepare1
with a nonzero offset if it is provided by the driver.
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 2abb22668d71 -r 0c1b80d41c95 sys/dev/usb/usbnet.c
--- a/sys/dev/usb/usbnet.c Thu Jan 22 03:22:43 2026 +0000
+++ b/sys/dev/usb/usbnet.c Sat Jan 24 21:34:44 2026 +0000
@@ -229,6 +229,14 @@ uno_tx_prepare(struct usbnet *un, struct
return (*un->un_ops->uno_tx_prepare)(un, m, c);
}
=20
+static unsigned
+uno_tx_prepare1(struct usbnet *un, struct mbuf *m, struct usbnet_chain *c,
+ unsigned offset)
+{
+ usbnet_isowned_tx(un);
+ return (*un->un_ops->uno_tx_prepare1)(un, m, c, offset);
+}
+
static void
uno_rx_loop(struct usbnet *un, struct usbnet_chain *c, uint32_t total_len)
{
@@ -521,26 +529,72 @@ usbnet_start_locked(struct ifnet *ifp)
idx =3D cd->uncd_tx_prod;
count =3D 0;
while (cd->uncd_tx_cnt < un->un_tx_list_cnt) {
+ struct mbuf *batch[16]; /* XXX arbitrary limit */
+ unsigned nbatch =3D 0;
+
IFQ_POLL(&ifp->if_snd, m);
if (m =3D=3D NULL) {
DPRINTF("start called, queue empty", 0, 0, 0, 0);
break;
}
+ IFQ_DEQUEUE(&ifp->if_snd, m);
+
+ /*
+ * Obeying the MTU is the sender's responsibility.
+ */
KASSERT(m->m_pkthdr.len <=3D un->un_tx_bufsz);
=20
- struct usbnet_chain *c =3D &cd->uncd_tx_chain[idx];
+ struct usbnet_chain *const c =3D &cd->uncd_tx_chain[idx];
=20
length =3D uno_tx_prepare(un, m, c);
if (length =3D=3D 0) {
DPRINTF("uno_tx_prepare gave zero length", 0, 0, 0, 0);
if_statinc(ifp, if_oerrors);
+ m_freem(m);
+ break;
+ }
+ KASSERTMSG(length <=3D un->un_tx_bufsz,
+ "length=3D%u bufsz=3D%u", length, un->un_tx_bufsz);
+
+ /* XXX KASSERT */
+ if (__predict_false(c->unc_xfer =3D=3D NULL)) {
+ DPRINTF("unc_xfer is NULL", 0, 0, 0, 0);
+ if_statinc(ifp, if_oerrors);
+ m_freem(m);
break;
}
=20
- if (__predict_false(c->unc_xfer =3D=3D NULL)) {
- DPRINTF("unc_xfer is NULL", 0, 0, 0, 0);
- if_statinc(ifp, if_oerrors);
- break;
+ /*
+ * If the driver supports it, cram a batch of as many
+ * packets we can fit into a single transfer at once.
+ */
+ if (un->un_ops->uno_tx_prepare1) {
+ unsigned offset =3D length;
+
+ while (nbatch < __arraycount(batch)) {
+ batch[nbatch++] =3D m;
+ IFQ_POLL(&ifp->if_snd, m);
+ if (m =3D=3D NULL) {
+ m =3D batch[--nbatch];
+ break;
+ }
+ KASSERT(m->m_pkthdr.len <=3D un->un_tx_bufsz);
+ length =3D uno_tx_prepare1(un, m, c, offset);
+ KASSERTMSG(length <=3D un->un_tx_bufsz - offset,
+ "length=3D%u bufsz=3D%u offset=3D%u",
+ length, un->un_tx_bufsz, offset);
+ if (length =3D=3D 0) {
+ /*
+ * Wouldn't fit in the buffer;
+ * leave the last packet queued
+ * in ifp->if_snd.
+ */
+ m =3D batch[--nbatch];
+ break;
+ }
+ offset +=3D length;
+ IFQ_DEQUEUE(&ifp->if_snd, m);
+ }
}
=20
usbd_setup_xfer(c->unc_xfer, c, c->unc_buf, length,
@@ -551,19 +605,22 @@ usbnet_start_locked(struct ifnet *ifp)
if (err !=3D 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);
+ if_statadd(ifp, if_oerrors, nbatch + 1);
+ m_freem(m);
+ while (nbatch --> 0)
+ m_freem(batch[nbatch]);
break;
}
done_transmit =3D true;
=20
- IFQ_DEQUEUE(&ifp->if_snd, m);
-
/*
* If there's a BPF listener, bounce a copy of this frame
* to him.
*/
bpf_mtap(ifp, m, BPF_D_OUT);
m_freem(m);
+ while (nbatch --> 0)
+ m_freem(batch[nbatch]);
=20
idx =3D (idx + 1) % un->un_tx_list_cnt;
cd->uncd_tx_cnt++;
diff -r 2abb22668d71 -r 0c1b80d41c95 sys/dev/usb/usbnet.h
--- a/sys/dev/usb/usbnet.h Thu Jan 22 03:22:43 2026 +0000
+++ b/sys/dev/usb/usbnet.h Sat Jan 24 21:34:44 2026 +0000
@@ -141,6 +141,9 @@ 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 *);
+/* Prepare subsequent packet to send callback, returns length. */
+typedef unsigned (*usbnet_tx_prepare1_cb)(struct usbnet *, struct mbuf *,
+ struct usbnet_chain *, unsigned);
/* Receive some packets callback. */
typedef void (*usbnet_rx_loop_cb)(struct usbnet *, struct usbnet_chain *,
uint32_t);
@@ -181,6 +184,7 @@ 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_prepare1_cb uno_tx_prepare1; /* T */
};
=20
/*
# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1769290712 0
# Sat Jan 24 21:38:32 2026 +0000
# Branch trunk
# Node ID 0aa5dacc886a083d3e52de8738c4fdf5d258cbba
# Parent 0c1b80d41c95e437ff64c85b0e72466bebf8dd60
# 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 0c1b80d41c95 -r 0aa5dacc886a 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
@@ -91,6 +91,8 @@ static void ure_uno_mcast(struct ifnet *
static int ure_uno_mii_read_reg(struct usbnet *, int, int, uint16_t *);
static int ure_uno_mii_write_reg(struct usbnet *, int, int, uint16_t);
static void ure_uno_miibus_statchg(struct ifnet *);
+static unsigned ure_uno_tx_prepare1(struct usbnet *, struct mbuf *,
+ struct usbnet_chain *, unsigned);
static unsigned ure_uno_tx_prepare(struct usbnet *, struct mbuf *,
struct usbnet_chain *);
static void ure_uno_rx_loop(struct usbnet *, struct usbnet_chain *,
@@ -110,6 +112,7 @@ static const struct usbnet_ops ure_ops =3D
.uno_write_reg =3D ure_uno_mii_write_reg,
.uno_statchg =3D ure_uno_miibus_statchg,
.uno_tx_prepare =3D ure_uno_tx_prepare,
+ .uno_tx_prepare1 =3D ure_uno_tx_prepare1,
.uno_rx_loop =3D ure_uno_rx_loop,
.uno_init =3D ure_uno_init,
};
@@ -1024,14 +1027,18 @@ ure_rxcsum(struct ifnet *ifp, struct ure
}
=20
static unsigned
-ure_uno_tx_prepare(struct usbnet *un, struct mbuf *m, struct usbnet_chain =
*c)
+ure_uno_tx_prepare1(struct usbnet *un, struct mbuf *m, struct usbnet_chain=
*c,
+ unsigned offset)
{
struct ure_txpkt txhdr;
uint32_t frm_len =3D 0;
uint8_t *buf =3D c->unc_buf;
=20
- if ((unsigned)m->m_pkthdr.len > un->un_tx_bufsz - sizeof(txhdr))
+ if (sizeof(txhdr) > un->un_tx_bufsz - offset ||
+ (unsigned)m->m_pkthdr.len > (un->un_tx_bufsz -
+ - offset - sizeof(txhdr)))
return 0;
+ buf +=3D offset;
=20
/* header */
txhdr.ure_pktlen =3D htole32(m->m_pkthdr.len | URE_TXPKT_TX_FS |
@@ -1050,6 +1057,13 @@ ure_uno_tx_prepare(struct usbnet *un, st
return frm_len;
}
=20
+static unsigned
+ure_uno_tx_prepare(struct usbnet *un, struct mbuf *m, struct usbnet_chain =
*c)
+{
+
+ return ure_uno_tx_prepare1(un, m, c, /*offset*/0);
+}
+
/*
* We need to calculate L4 checksum in software, if the offset of
* L4 header is larger than 0x7ff =3D 2047.
--=_rFLsIlDMAmZS54PhaM3eE2ii5a8qA3Zz--
Home |
Main Index |
Thread Index |
Old Index