Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1
@ 2026-06-30  3:21 Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 01/12] net: mctp: usb: Include version indicator in max packet size defines Jeremy Kerr
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Version 1.1.0 of DSP0283 (MCTP over USB transport binding) has been
released, this patch series updates our current v1.0.1 support for the
changes in v1.1.x.

The major change in v1.1 is the introduction of "packet spanning" mode,
where a single MCTP packet may be split over multiple USB packets
(themselves forming a single USB bulk transfer). This relaxes the
requirement for USB high-speed mode, as we can now send MCTP packets
contained over multiple 64-byte full-speed USB bulk transfers, and gives
us an increase in the maximum MCTP packet size - we now have 13 bits of
packet length (previously 8) in the transport header.

Handling packet spanning introduces some complexity in the transmit and
receive paths, as we lose some constraints on where packet boundaries
may correspond to USB transfer boundaries, and may need to retain state
across separate transfers. To contain this complexity, we introduce a
new library for the transfer packing- and unpacking implementations,
"mctp-usblib". The host driver is a consumer of this library, and a
future gadget driver can use the same implementations. We can now also
implement tests on the API boundary of the library.

The series implements an incremental shift to mctp-usblib, then
implements packet spanning mode in the new library. We have a few
changes to prepare for this, in altering a few constants and
behaviours as v1.0-specific. Once packet spanning is implemented in
mctp-usblib, we enable it in the host-side driver.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
Jeremy Kerr (12):
      net: mctp: usb: Include version indicator in max packet size defines
      net: mctp: usb: Use packet-length max for maximum packet-size check
      net: mctp: usblib: Move RX transfer processing to a new mctp-usblib
      net: mctp: usblib: Move TX transfer processing to mctp-usblib
      net: mctp: usb: Allow for multiple urb submissions from a packet tx
      net: mctp: usblib: Add support for multi-packet transmit
      net: mctp: usb: Accommodate DSP0283 v1.1 header format
      net: mctp: usblib: Implement receive-side packet spanning
      net: mctp: usblib: Implement transmit-side packet spanning
      net: mctp: usblib: Add initial kunit tests
      net: mctp: usb: enable v1.1 packet spanning
      net: mctp: usb: Allow multiple urbs in flight

 drivers/net/mctp/Kconfig            |  16 ++
 drivers/net/mctp/Makefile           |   1 +
 drivers/net/mctp/mctp-usb.c         | 273 ++++++++----------
 drivers/net/mctp/mctp-usblib-test.c | 330 +++++++++++++++++++++
 drivers/net/mctp/mctp-usblib.c      | 554 ++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h        |  90 +++++-
 6 files changed, 1112 insertions(+), 152 deletions(-)
---
base-commit: b85966adbf5de0668a815c6e3527f87e0c387fb4
change-id: 20260604-dev-mctp-usb-1-1-6fd854ad13e8

Best regards,
--  
Jeremy Kerr <jk@codeconstruct.com.au>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next 01/12] net: mctp: usb: Include version indicator in max packet size defines
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 02/12] net: mctp: usb: Use packet-length max for maximum packet-size check Jeremy Kerr
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

DSP0283 v1.1.0 will introduce larger maximum packet sizes. In
preparation, indicate that the current maxima are specific to v1.0.x.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c  | 8 ++++----
 include/linux/usb/mctp-usb.h | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index fade65f2f269..545eff06322c 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -76,7 +76,7 @@ static netdev_tx_t mctp_usb_start_xmit(struct sk_buff *skb,
 
 	plen = skb->len;
 
-	if (plen + sizeof(*hdr) > MCTP_USB_XFER_SIZE)
+	if (plen + sizeof(*hdr) > MCTP_USB_1_0_XFER_SIZE)
 		goto err_drop;
 
 	rc = skb_cow_head(skb, sizeof(*hdr));
@@ -128,7 +128,7 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
 	struct sk_buff *skb;
 	int rc;
 
-	skb = __netdev_alloc_skb(mctp_usb->netdev, MCTP_USB_XFER_SIZE, gfp);
+	skb = __netdev_alloc_skb(mctp_usb->netdev, MCTP_USB_1_0_XFER_SIZE, gfp);
 	if (!skb) {
 		rc = -ENOMEM;
 		goto err_retry;
@@ -136,7 +136,7 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
 
 	usb_fill_bulk_urb(mctp_usb->rx_urb, mctp_usb->usbdev,
 			  usb_rcvbulkpipe(mctp_usb->usbdev, mctp_usb->ep_in),
-			  skb->data, MCTP_USB_XFER_SIZE,
+			  skb->data, MCTP_USB_1_0_XFER_SIZE,
 			  mctp_usb_in_complete, skb);
 
 	rc = usb_submit_urb(mctp_usb->rx_urb, gfp);
@@ -301,7 +301,7 @@ static void mctp_usb_netdev_setup(struct net_device *dev)
 
 	dev->mtu = MCTP_USB_MTU_MIN;
 	dev->min_mtu = MCTP_USB_MTU_MIN;
-	dev->max_mtu = MCTP_USB_MTU_MAX;
+	dev->max_mtu = MCTP_USB_1_0_MTU_MAX;
 
 	dev->hard_header_len = sizeof(struct mctp_usb_hdr);
 	dev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index a2f6f1e04efb..47e2e3931d63 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -21,10 +21,11 @@ struct mctp_usb_hdr {
 	u8	len;
 } __packed;
 
-#define MCTP_USB_XFER_SIZE	512
+/* max transfer size for DSP0283 v1.0 */
+#define MCTP_USB_1_0_XFER_SIZE	512
 #define MCTP_USB_BTU		68
 #define MCTP_USB_MTU_MIN	MCTP_USB_BTU
-#define MCTP_USB_MTU_MAX	(U8_MAX - sizeof(struct mctp_usb_hdr))
+#define MCTP_USB_1_0_MTU_MAX	(U8_MAX - sizeof(struct mctp_usb_hdr))
 #define MCTP_USB_DMTF_ID	0x1ab4
 
 #endif /*  __LINUX_USB_MCTP_USB_H */

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 02/12] net: mctp: usb: Use packet-length max for maximum packet-size check
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 01/12] net: mctp: usb: Include version indicator in max packet size defines Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib Jeremy Kerr
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

The max packet size is smaller than the max transfer size, as we only
have a u8 length field in the transport header.

Add a define for the maximum representable length, and use that for our
check. Use this for the MTU maximum calculation too.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c  | 2 +-
 include/linux/usb/mctp-usb.h | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 545eff06322c..c6e36b63e87a 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -76,7 +76,7 @@ static netdev_tx_t mctp_usb_start_xmit(struct sk_buff *skb,
 
 	plen = skb->len;
 
-	if (plen + sizeof(*hdr) > MCTP_USB_1_0_XFER_SIZE)
+	if (plen + sizeof(*hdr) > MCTP_USB_1_0_PKTLEN_MAX)
 		goto err_drop;
 
 	rc = skb_cow_head(skb, sizeof(*hdr));
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 47e2e3931d63..2bece8afd1c7 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -25,7 +25,8 @@ struct mctp_usb_hdr {
 #define MCTP_USB_1_0_XFER_SIZE	512
 #define MCTP_USB_BTU		68
 #define MCTP_USB_MTU_MIN	MCTP_USB_BTU
-#define MCTP_USB_1_0_MTU_MAX	(U8_MAX - sizeof(struct mctp_usb_hdr))
+#define MCTP_USB_1_0_PKTLEN_MAX	U8_MAX
+#define MCTP_USB_1_0_MTU_MAX	(MCTP_USB_1_0_PKTLEN_MAX - sizeof(struct mctp_usb_hdr))
 #define MCTP_USB_DMTF_ID	0x1ab4
 
 #endif /*  __LINUX_USB_MCTP_USB_H */

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 01/12] net: mctp: usb: Include version indicator in max packet size defines Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 02/12] net: mctp: usb: Use packet-length max for maximum packet-size check Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-07-02 10:09   ` Paolo Abeni
  2026-06-30  3:21 ` [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib Jeremy Kerr
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

The processing of USB receive transfers is common to both sides of a
MCTP over USB transport. In order to support a future gadget driver,
move the current host-side driver into a new common file, mctp-usblib.

This currently handles the submit-complete-packetise process of the
receive path of the USB transport. We'll add transmit handling in an
upcoming change.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/Kconfig       |  11 +++
 drivers/net/mctp/Makefile      |   1 +
 drivers/net/mctp/mctp-usb.c    | 100 +++++-------------------
 drivers/net/mctp/mctp-usblib.c | 167 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h   |  26 +++++++
 5 files changed, 225 insertions(+), 80 deletions(-)

diff --git a/drivers/net/mctp/Kconfig b/drivers/net/mctp/Kconfig
index cf325ab0b1ef..a564a792801d 100644
--- a/drivers/net/mctp/Kconfig
+++ b/drivers/net/mctp/Kconfig
@@ -47,9 +47,20 @@ config MCTP_TRANSPORT_I3C
 	  A MCTP protocol network device is created for each I3C bus
 	  having a "mctp-controller" devicetree property.
 
+config MCTP_TRANSPORT_USBLIB
+	tristate "MCTP over USB common library"
+	depends on USB
+	help
+	  Common protocol handling functions for MCTP-over-USB transport
+	  implementations, suitable for use in either host- or gadget-side
+	  transport driver
+
+	  This will be automatically enabled by the transport driver.
+
 config MCTP_TRANSPORT_USB
 	tristate "MCTP USB transport"
 	depends on USB
+	select MCTP_TRANSPORT_USBLIB
 	help
 	  Provides a driver to access MCTP devices over USB transport,
 	  defined by DMTF specification DSP0283.
diff --git a/drivers/net/mctp/Makefile b/drivers/net/mctp/Makefile
index c36006849a1e..c870b62d3f1c 100644
--- a/drivers/net/mctp/Makefile
+++ b/drivers/net/mctp/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_MCTP_SERIAL) += mctp-serial.o
 obj-$(CONFIG_MCTP_TRANSPORT_I2C) += mctp-i2c.o
 obj-$(CONFIG_MCTP_TRANSPORT_I3C) += mctp-i3c.o
 obj-$(CONFIG_MCTP_TRANSPORT_USB) += mctp-usb.o
+obj-$(CONFIG_MCTP_TRANSPORT_USBLIB) += mctp-usblib.o
diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index c6e36b63e87a..531b7c994afb 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -28,6 +28,8 @@ struct mctp_usb {
 	u8 ep_in;
 	u8 ep_out;
 
+	struct mctp_usblib_rx rx;
+
 	struct urb *tx_urb;
 	struct urb *rx_urb;
 
@@ -125,24 +127,23 @@ static const unsigned long RX_RETRY_DELAY = HZ / 4;
 static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
 {
 	unsigned long flags;
-	struct sk_buff *skb;
+	size_t len;
+	void *buf;
 	int rc;
 
-	skb = __netdev_alloc_skb(mctp_usb->netdev, MCTP_USB_1_0_XFER_SIZE, gfp);
-	if (!skb) {
-		rc = -ENOMEM;
+	rc = mctp_usblib_rx_prepare(mctp_usb->netdev, &mctp_usb->rx,
+				    &buf, &len, gfp);
+	if (rc)
 		goto err_retry;
-	}
 
 	usb_fill_bulk_urb(mctp_usb->rx_urb, mctp_usb->usbdev,
 			  usb_rcvbulkpipe(mctp_usb->usbdev, mctp_usb->ep_in),
-			  skb->data, MCTP_USB_1_0_XFER_SIZE,
-			  mctp_usb_in_complete, skb);
+			  buf, len, mctp_usb_in_complete, mctp_usb);
 
 	rc = usb_submit_urb(mctp_usb->rx_urb, gfp);
 	if (rc) {
 		netdev_dbg(mctp_usb->netdev, "rx urb submit failure: %d\n", rc);
-		kfree_skb(skb);
+		mctp_usblib_rx_cancel(&mctp_usb->rx);
 		if (rc == -ENOMEM)
 			goto err_retry;
 	}
@@ -159,92 +160,27 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
 
 static void mctp_usb_in_complete(struct urb *urb)
 {
-	struct sk_buff *skb = urb->context;
-	struct net_device *netdev = skb->dev;
-	struct mctp_usb *mctp_usb = netdev_priv(netdev);
-	struct mctp_skb_cb *cb;
-	unsigned int len;
+	struct mctp_usb *mctp_usb = urb->context;
+	struct net_device *netdev = mctp_usb->netdev;
 	int status;
 
 	status = urb->status;
 
 	switch (status) {
+	default:
+		netdev_dbg(netdev, "unexpected rx urb status: %d\n", status);
+		fallthrough;
 	case -ENOENT:
 	case -ECONNRESET:
 	case -ESHUTDOWN:
 	case -EPROTO:
-		kfree_skb(skb);
+		mctp_usblib_rx_cancel(&mctp_usb->rx);
 		return;
 	case 0:
 		break;
-	default:
-		netdev_dbg(netdev, "unexpected rx urb status: %d\n", status);
-		kfree_skb(skb);
-		return;
 	}
 
-	len = urb->actual_length;
-	__skb_put(skb, len);
-
-	while (skb) {
-		struct sk_buff *skb2 = NULL;
-		struct mctp_usb_hdr *hdr;
-		u8 pkt_len; /* length of MCTP packet, no USB header */
-
-		skb_reset_mac_header(skb);
-		hdr = skb_pull_data(skb, sizeof(*hdr));
-		if (!hdr)
-			break;
-
-		if (be16_to_cpu(hdr->id) != MCTP_USB_DMTF_ID) {
-			netdev_dbg(netdev, "rx: invalid id %04x\n",
-				   be16_to_cpu(hdr->id));
-			break;
-		}
-
-		if (hdr->len <
-		    sizeof(struct mctp_hdr) + sizeof(struct mctp_usb_hdr)) {
-			netdev_dbg(netdev, "rx: short packet (hdr) %d\n",
-				   hdr->len);
-			break;
-		}
-
-		/* we know we have at least sizeof(struct mctp_usb_hdr) here */
-		pkt_len = hdr->len - sizeof(struct mctp_usb_hdr);
-		if (pkt_len > skb->len) {
-			netdev_dbg(netdev,
-				   "rx: short packet (xfer) %d, actual %d\n",
-				   hdr->len, skb->len);
-			break;
-		}
-
-		if (pkt_len < skb->len) {
-			/* more packets may follow - clone to a new
-			 * skb to use on the next iteration
-			 */
-			skb2 = skb_clone(skb, GFP_ATOMIC);
-			if (skb2) {
-				if (!skb_pull(skb2, pkt_len)) {
-					kfree_skb(skb2);
-					skb2 = NULL;
-				}
-			}
-			skb_trim(skb, pkt_len);
-		}
-
-		dev_dstats_rx_add(netdev, skb->len);
-
-		skb->protocol = htons(ETH_P_MCTP);
-		skb_reset_network_header(skb);
-		cb = __mctp_cb(skb);
-		cb->halen = 0;
-		netif_rx(skb);
-
-		skb = skb2;
-	}
-
-	if (skb)
-		kfree_skb(skb);
+	mctp_usblib_rx_complete(netdev, &mctp_usb->rx, urb->actual_length);
 
 	mctp_usb_rx_queue(mctp_usb, GFP_ATOMIC);
 }
@@ -341,6 +277,8 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	spin_lock_init(&dev->rx_lock);
 	usb_set_intfdata(intf, dev);
 
+	mctp_usblib_rx_init(&dev->rx);
+
 	dev->ep_in = ep_in->bEndpointAddress;
 	dev->ep_out = ep_out->bEndpointAddress;
 
@@ -362,6 +300,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 err_free_urbs:
 	usb_free_urb(dev->tx_urb);
 	usb_free_urb(dev->rx_urb);
+	mctp_usblib_rx_fini(&dev->rx);
 	free_netdev(netdev);
 	return rc;
 }
@@ -371,6 +310,7 @@ static void mctp_usb_disconnect(struct usb_interface *intf)
 	struct mctp_usb *dev = usb_get_intfdata(intf);
 
 	mctp_unregister_netdev(dev->netdev);
+	mctp_usblib_rx_fini(&dev->rx);
 	usb_free_urb(dev->tx_urb);
 	usb_free_urb(dev->rx_urb);
 	free_netdev(dev->netdev);
diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
new file mode 100644
index 000000000000..9b86eb4310ce
--- /dev/null
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mctp-usblib.c - MCTP-over-USB (DMTF DSP0283) transport helper library
+ *
+ * DSP0283 is available at:
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.1.pdf
+ *
+ * Copyright (C) 2024-2026 Code Construct Pty Ltd
+ */
+
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/usb/mctp-usb.h>
+#include <net/mctp.h>
+
+void mctp_usblib_rx_init(struct mctp_usblib_rx *rx)
+{
+	memset(rx, 0, sizeof(*rx));
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_rx_init);
+
+void mctp_usblib_rx_fini(struct mctp_usblib_rx *rx)
+{
+	kfree_skb(rx->skb);
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_rx_fini);
+
+/*
+ * Prepare a transfer buffer for future completion; *bufp and *lenp will
+ * be populated on success.
+ */
+int mctp_usblib_rx_prepare(struct net_device *netdev,
+			   struct mctp_usblib_rx *rx,
+			   void **bufp, size_t *lenp, gfp_t gfp)
+{
+	const unsigned int len = MCTP_USB_1_0_XFER_SIZE;
+	struct sk_buff *skb;
+
+	skb = __netdev_alloc_skb(netdev, len, gfp);
+	if (!skb)
+		return -ENOMEM;
+
+	rx->skb = skb;
+
+	*bufp = skb_tail_pointer(skb);
+	*lenp = skb_tailroom(skb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_rx_prepare);
+
+static void mctp_usblib_rx(struct net_device *netdev, struct sk_buff *skb)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(netdev->dstats);
+	struct mctp_skb_cb *cb;
+	unsigned long flags;
+
+	/* we're called from an URB completion handler, and cannot assume local
+	 * irqs are always disabled
+	 */
+	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+	u64_stats_inc(&dstats->rx_packets);
+	u64_stats_add(&dstats->rx_bytes, skb->len + sizeof(struct mctp_usb_hdr));
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+
+	skb_reset_mac_header(skb);
+	skb->protocol = htons(ETH_P_MCTP);
+	skb_reset_network_header(skb);
+	cb = __mctp_cb(skb);
+	cb->halen = 0;
+	netif_rx(skb);
+}
+
+/*
+ * Receive a USB completion of @len bytes of incoming data. We will then split
+ * this into packets and netif_rx() each. Intended to be called in atomic
+ * contexts - ie., URB completion.
+ *
+ * Assumes @netdev uses dstats.
+ */
+int mctp_usblib_rx_complete(struct net_device *netdev,
+			    struct mctp_usblib_rx *rx, size_t len)
+{
+	struct sk_buff *skb = rx->skb;
+	int rc = 0;
+
+	__skb_put(skb, len);
+
+	while (skb) {
+		struct sk_buff *skb2 = NULL;
+		struct mctp_usb_hdr *hdr;
+		/* length of MCTP packet, no USB header */
+		u8 pkt_len;
+
+		skb_reset_mac_header(skb);
+		hdr = skb_pull_data(skb, sizeof(*hdr));
+		if (!hdr) {
+			rc = -ENOMSG;
+			break;
+		}
+
+		if (be16_to_cpu(hdr->id) != MCTP_USB_DMTF_ID) {
+			netdev_dbg(netdev, "rx: invalid id %04x\n",
+				   be16_to_cpu(hdr->id));
+			rc = -EPROTO;
+			break;
+		}
+
+		if (hdr->len <
+		    sizeof(struct mctp_hdr) + sizeof(struct mctp_usb_hdr)) {
+			netdev_dbg(netdev, "rx: short packet (hdr) %d\n",
+				   hdr->len);
+			rc = -EPROTO;
+			break;
+		}
+
+		/* we know we have at least sizeof(struct mctp_usb_hdr) here */
+		pkt_len = hdr->len - sizeof(struct mctp_usb_hdr);
+		if (pkt_len > skb->len) {
+			rc = -EPROTO;
+			netdev_dbg(netdev,
+				   "rx: short packet (xfer) %d, actual %d\n",
+				   hdr->len, skb->len);
+			break;
+		}
+
+		if (pkt_len < skb->len) {
+			/* more packets may follow - clone to a new
+			 * skb to use on the next iteration
+			 */
+			skb2 = skb_clone(skb, GFP_ATOMIC);
+			if (skb2) {
+				if (!skb_pull(skb2, pkt_len)) {
+					dev_kfree_skb_any(skb2);
+					skb2 = NULL;
+				}
+			}
+			skb_trim(skb, pkt_len);
+		}
+
+		mctp_usblib_rx(netdev, skb);
+		skb = skb2;
+	}
+
+	if (skb)
+		dev_kfree_skb_any(skb);
+
+	rx->skb = NULL;
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_rx_complete);
+
+/*
+ * Cancel a rx context; subsequent prepare/complete calls will not be a
+ * continuation of any data already received.
+ */
+void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx)
+{
+	dev_kfree_skb_any(rx->skb);
+	rx->skb = NULL;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_rx_cancel);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>");
+MODULE_DESCRIPTION("MCTP USB transport library");
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 2bece8afd1c7..595e6af16dd0 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -13,6 +13,8 @@
 #ifndef __LINUX_USB_MCTP_USB_H
 #define __LINUX_USB_MCTP_USB_H
 
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
 #include <linux/types.h>
 
 struct mctp_usb_hdr {
@@ -29,4 +31,28 @@ struct mctp_usb_hdr {
 #define MCTP_USB_1_0_MTU_MAX	(MCTP_USB_1_0_PKTLEN_MAX - sizeof(struct mctp_usb_hdr))
 #define MCTP_USB_DMTF_ID	0x1ab4
 
+/* mctp-usblib */
+
+/*
+ * RX handle: drivers will typically create one on init, which persists for
+ * the life of the driver. The same handle is used for progressive
+ * prepare -> complete operations (for each incoming USB transfer), which
+ * result in netif_rx()-ing the MCTP packets received
+ */
+struct mctp_usblib_rx {
+	struct sk_buff *skb;
+};
+
+void mctp_usblib_rx_init(struct mctp_usblib_rx *rx);
+void mctp_usblib_rx_fini(struct mctp_usblib_rx *rx);
+
+int mctp_usblib_rx_prepare(struct net_device *netdev,
+			   struct mctp_usblib_rx *rx,
+			   void **bufp, size_t *lenp, gfp_t gfp);
+
+int mctp_usblib_rx_complete(struct net_device *netdev,
+			    struct mctp_usblib_rx *rx, size_t len);
+
+void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx);
+
 #endif /*  __LINUX_USB_MCTP_USB_H */

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (2 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-07-02 10:10   ` Paolo Abeni
  2026-06-30  3:21 ` [PATCH net-next 05/12] net: mctp: usb: Allow for multiple urb submissions from a packet tx Jeremy Kerr
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

With the RX processing in mctp-usblib, add TX processing alongside.

To accommodate packed transfers in DSP0283, where a transfer may contain
multiple MCTP packets, we move to a split process for the transmit API:

 * push: create a new transmit context, and add a skb to it.

 * send: callback to the driver implementation to send the (possibly
   multi-packet) USB transfer

 * complete: update skb accounting and release the tx context

The actual multi-packet transfer implementation will be added in the
next change; no tx context persists beyond the single send at present.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c    | 125 ++++++++++++++--------------
 drivers/net/mctp/mctp-usblib.c | 180 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h   |  38 +++++++++
 3 files changed, 280 insertions(+), 63 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 531b7c994afb..385350792dd4 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -29,90 +29,82 @@ struct mctp_usb {
 	u8 ep_out;
 
 	struct mctp_usblib_rx rx;
-
-	struct urb *tx_urb;
 	struct urb *rx_urb;
 
 	/* enforces atomic access to rx_stopped and requeuing the retry work */
 	spinlock_t rx_lock;
 	bool rx_stopped;
 	struct delayed_work rx_retry_work;
+
+	struct mctp_usblib_tx tx;
+	/* protects tx_urb */
+	spinlock_t tx_lock;
+	struct urb *tx_urb;
 };
 
 static void mctp_usb_out_complete(struct urb *urb)
 {
-	struct sk_buff *skb = urb->context;
-	struct net_device *netdev = skb->dev;
-	int status;
+	struct mctp_usblib_tx_ctx *tx_ctx = urb->context;
+	struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
+	struct net_device *netdev = mctp_usb->netdev;
+	unsigned long flags;
 
-	status = urb->status;
+	mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
 
-	switch (status) {
-	case -ENOENT:
-	case -ECONNRESET:
-	case -ESHUTDOWN:
-	case -EPROTO:
-		dev_dstats_tx_dropped(netdev);
-		break;
-	case 0:
-		dev_dstats_tx_add(netdev, skb->len);
-		netif_wake_queue(netdev);
-		consume_skb(skb);
-		return;
-	default:
-		netdev_dbg(netdev, "unexpected tx urb status: %d\n", status);
-		dev_dstats_tx_dropped(netdev);
-	}
+	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
+	mctp_usb->tx_urb = NULL;
+	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
 
-	kfree_skb(skb);
+	usb_free_urb(urb);
+
+	netif_wake_queue(netdev);
 }
 
-static netdev_tx_t mctp_usb_start_xmit(struct sk_buff *skb,
-				       struct net_device *dev)
+static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
+			    void *data, size_t len)
 {
-	struct mctp_usb *mctp_usb = netdev_priv(dev);
-	struct mctp_usb_hdr *hdr;
-	unsigned int plen;
+	struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
+	unsigned long flags;
 	struct urb *urb;
 	int rc;
 
-	plen = skb->len;
-
-	if (plen + sizeof(*hdr) > MCTP_USB_1_0_PKTLEN_MAX)
-		goto err_drop;
-
-	rc = skb_cow_head(skb, sizeof(*hdr));
-	if (rc)
-		goto err_drop;
-
-	hdr = skb_push(skb, sizeof(*hdr));
-	if (!hdr)
-		goto err_drop;
-
-	hdr->id = cpu_to_be16(MCTP_USB_DMTF_ID);
-	hdr->rsvd = 0;
-	hdr->len = plen + sizeof(*hdr);
-
-	urb = mctp_usb->tx_urb;
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb)
+		return -ENOMEM;
 
 	usb_fill_bulk_urb(urb, mctp_usb->usbdev,
 			  usb_sndbulkpipe(mctp_usb->usbdev, mctp_usb->ep_out),
-			  skb->data, skb->len,
-			  mctp_usb_out_complete, skb);
+			  data, len, mctp_usb_out_complete, tx_ctx);
 
-	/* Stops TX queue first to prevent race condition with URB complete */
-	netif_stop_queue(dev);
+	netif_stop_queue(mctp_usb->netdev);
+
+	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
+	if (!rc)
+		mctp_usb->tx_urb = urb;
+	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
+
 	if (rc) {
-		netif_wake_queue(dev);
-		goto err_drop;
+		netdev_dbg(mctp_usb->netdev, "TX urb submit failed, %d\n", rc);
+		usb_free_urb(urb);
+		netif_start_queue(mctp_usb->netdev);
 	}
 
-	return NETDEV_TX_OK;
+	return rc;
+}
+
+static const struct mctp_usblib_tx_ops tx_ops = {
+	.send = mctp_usb_tx_send,
+};
+
+static netdev_tx_t mctp_usb_start_xmit(struct sk_buff *skb,
+				       struct net_device *dev)
+{
+	struct mctp_usb *mctp_usb = netdev_priv(dev);
+	bool more = netdev_xmit_more();
+
+	mctp_usblib_tx_push(dev, &mctp_usb->tx, skb, more);
 
-err_drop:
-	dev_dstats_tx_dropped(dev);
-	kfree_skb(skb);
 	return NETDEV_TX_OK;
 }
 
@@ -220,7 +212,12 @@ static int mctp_usb_stop(struct net_device *dev)
 	flush_delayed_work(&mctp_usb->rx_retry_work);
 
 	usb_kill_urb(mctp_usb->rx_urb);
+
+	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
 	usb_kill_urb(mctp_usb->tx_urb);
+	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
+
+	mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
 
 	return 0;
 }
@@ -275,31 +272,33 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	dev->usbdev = interface_to_usbdev(intf);
 	dev->intf = intf;
 	spin_lock_init(&dev->rx_lock);
+	spin_lock_init(&dev->tx_lock);
 	usb_set_intfdata(intf, dev);
 
 	mctp_usblib_rx_init(&dev->rx);
+	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev);
 
 	dev->ep_in = ep_in->bEndpointAddress;
 	dev->ep_out = ep_out->bEndpointAddress;
 
-	dev->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
 	dev->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
-	if (!dev->tx_urb || !dev->rx_urb) {
+	if (!dev->rx_urb) {
 		rc = -ENOMEM;
-		goto err_free_urbs;
+		goto err_fini_rxtx;
 	}
 
 	INIT_DELAYED_WORK(&dev->rx_retry_work, mctp_usb_rx_retry_work);
 
 	rc = mctp_register_netdev(netdev, NULL, MCTP_PHYS_BINDING_USB);
 	if (rc)
-		goto err_free_urbs;
+		goto err_free_urb;
 
 	return 0;
 
-err_free_urbs:
-	usb_free_urb(dev->tx_urb);
+err_free_urb:
 	usb_free_urb(dev->rx_urb);
+err_fini_rxtx:
+	mctp_usblib_tx_fini(&dev->tx);
 	mctp_usblib_rx_fini(&dev->rx);
 	free_netdev(netdev);
 	return rc;
@@ -311,7 +310,7 @@ static void mctp_usb_disconnect(struct usb_interface *intf)
 
 	mctp_unregister_netdev(dev->netdev);
 	mctp_usblib_rx_fini(&dev->rx);
-	usb_free_urb(dev->tx_urb);
+	mctp_usblib_tx_fini(&dev->tx);
 	usb_free_urb(dev->rx_urb);
 	free_netdev(dev->netdev);
 }
diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index 9b86eb4310ce..56eca496bbe4 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -162,6 +162,186 @@ void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx)
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_rx_cancel);
 
+/* transmit context: encapsulates one transfer */
+struct mctp_usblib_tx_ctx {
+	struct mctp_usblib_tx *tx;
+	struct sk_buff *skb;
+	unsigned int len;
+	enum mctp_usblib_tx_buf_type {
+		TX_SINGLE,
+	} buf_type;
+};
+
+void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
+			 const struct mctp_usblib_tx_ops *ops,
+			 void *priv)
+{
+	memset(tx, 0, sizeof(*tx));
+	tx->ops = *ops;
+	tx->priv = priv;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_init);
+
+void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx)
+{
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_fini);
+
+void *mctp_usblib_tx_ctx_priv(struct mctp_usblib_tx_ctx *tx_ctx)
+{
+	return tx_ctx->tx->priv;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_ctx_priv);
+
+static struct mctp_usblib_tx_ctx *
+mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb)
+{
+	struct mctp_usblib_tx_ctx *ctx;
+
+	ctx = kzalloc_obj(*ctx, GFP_ATOMIC);
+	if (!ctx)
+		return NULL;
+
+	ctx->tx = tx;
+	ctx->buf_type = TX_SINGLE;
+	ctx->skb = skb;
+	ctx->len += skb->len;
+
+	return ctx;
+}
+
+static int mctp_usblib_tx_send(struct mctp_usblib_tx_ctx *ctx)
+{
+	struct mctp_usblib_tx *tx = ctx->tx;
+	void *buf = ctx->skb->data;
+
+	return tx->ops.send(ctx, buf, ctx->len);
+}
+
+static void mctp_usblib_tx_ctx_free(struct mctp_usblib_tx_ctx *ctx)
+{
+	if (ctx)
+		dev_kfree_skb_any(ctx->skb);
+	kfree(ctx);
+}
+
+static void mctp_usblib_tx_stats_update(struct mctp_usblib_tx_ctx *ctx,
+					struct net_device *dev,
+					bool ok)
+{
+	struct pcpu_dstats *dstats = get_cpu_ptr(dev->dstats);
+	unsigned long flags;
+
+	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+	if (ok) {
+		u64_stats_inc(&dstats->tx_packets);
+		u64_stats_add(&dstats->tx_bytes, ctx->len);
+	} else {
+		u64_stats_inc(&dstats->tx_drops);
+	}
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+	put_cpu_ptr(dev->dstats);
+}
+
+static void mctp_usblib_tx_stats_single_drop(struct net_device *dev)
+{
+	struct pcpu_dstats *dstats = get_cpu_ptr(dev->dstats);
+	unsigned long flags;
+
+	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+	u64_stats_inc(&dstats->tx_drops);
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+	put_cpu_ptr(dev->dstats);
+}
+
+/*
+ * Completion for the ->send() op. This will update netdev stats and
+ * free the tx context.
+ *
+ * Likely called from (atomic) URB completion context.
+ */
+void mctp_usblib_tx_send_complete(struct mctp_usblib_tx_ctx *tx_ctx,
+				  struct net_device *dev, bool ok)
+{
+	mctp_usblib_tx_ctx_free(tx_ctx);
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);
+
+/* Prepare a skb for push() */
+static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)
+{
+	struct mctp_usb_hdr *hdr;
+	unsigned long plen;
+	int rc;
+
+	plen = skb->len;
+	if (plen + sizeof(*hdr) > MCTP_USB_1_0_PKTLEN_MAX)
+		return -EMSGSIZE;
+
+	rc = skb_cow_head(skb, sizeof(*hdr));
+	if (rc)
+		return rc;
+
+	hdr = skb_push(skb, sizeof(*hdr));
+	if (!hdr)
+		return -ENOMEM;
+
+	hdr->id = cpu_to_be16(MCTP_USB_DMTF_ID);
+	hdr->rsvd = 0;
+	hdr->len = plen + sizeof(*hdr);
+
+	return 0;
+}
+
+/*
+ * Push a new skb to the transfer. At present, no send must be in progress,
+ * as we only handle single-packet USB transfers.
+ *
+ * Takes ownership of @skb, including on error.
+ */
+int mctp_usblib_tx_push(struct net_device *dev,
+			struct mctp_usblib_tx *tx,
+			struct sk_buff *skb, bool more)
+{
+	struct mctp_usblib_tx_ctx *ctx;
+	int rc;
+
+	if (!skb)
+		return 0;
+
+	rc = mctp_usblib_tx_skb_prepare(skb);
+	if (rc)
+		goto err_drop_single;
+
+	ctx = mctp_usblib_tx_ctx_create(tx, skb);
+	if (!ctx) {
+		rc = -ENOMEM;
+		goto err_drop_single;
+	}
+
+	rc = mctp_usblib_tx_send(ctx);
+	if (rc) {
+		mctp_usblib_tx_stats_update(ctx, dev, false);
+		mctp_usblib_tx_ctx_free(ctx);
+	}
+
+	return rc;
+
+err_drop_single:
+	mctp_usblib_tx_stats_single_drop(dev);
+	kfree_skb(skb);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_push);
+
+/* Cancel a tx: any un-sent context is released. */
+void mctp_usblib_tx_cancel(struct mctp_usblib_tx *tx,
+			   struct net_device *dev)
+{
+	/* nothing to do at present, no ctx is persistent */
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_cancel);
+
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>");
 MODULE_DESCRIPTION("MCTP USB transport library");
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 595e6af16dd0..9fe314c2551e 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -55,4 +55,42 @@ int mctp_usblib_rx_complete(struct net_device *netdev,
 
 void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx);
 
+/*
+ * TX handle: created by mctp_usblib_tx_push() during the tx path, and
+ * may persist across multiple packet transmits.
+ *
+ * Currently though, there is a 1:1 mapping between packets and transfers, so
+ * the tx context will be cleared over each transmit. This will change in
+ * future.
+ */
+struct mctp_usblib_tx_ctx;
+
+struct mctp_usblib_tx_ops {
+	/* Start a USB TX for @data. On returning success, the implementation
+	 * must arrange for mctp_usblib_tx_send_complete() to be called at some
+	 * later point (eg., on urb completion).
+	 */
+	int (*send)(struct mctp_usblib_tx_ctx *tx_ctx, void *data, size_t len);
+};
+
+struct mctp_usblib_tx {
+	struct mctp_usblib_tx_ops ops;
+	void *priv;
+};
+
+void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
+			 const struct mctp_usblib_tx_ops *ops, void *priv);
+void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx);
+
+void *mctp_usblib_tx_ctx_priv(struct mctp_usblib_tx_ctx *tx_ctx);
+
+int mctp_usblib_tx_push(struct net_device *dev,
+			struct mctp_usblib_tx *tx,
+			struct sk_buff *skb, bool more);
+
+void mctp_usblib_tx_send_complete(struct mctp_usblib_tx_ctx *tx_ctx,
+				  struct net_device *dev, bool ok);
+
+void mctp_usblib_tx_cancel(struct mctp_usblib_tx *tx, struct net_device *dev);
+
 #endif /*  __LINUX_USB_MCTP_USB_H */

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 05/12] net: mctp: usb: Allow for multiple urb submissions from a packet tx
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (3 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 06/12] net: mctp: usblib: Add support for multi-packet transmit Jeremy Kerr
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

We currently assume that one packet tx (ie., a mctp_usblib_tx_push())
will result in zero or one calls to the ->send() op, and so zero or one
urb submissions.

However, in order to support multi-packet transfers in future (and later,
packet-spanning mode), we may have up to two: one flushing an existing
transmit (if we could not append to that), and one for the new packet
(if we are not expecting to add more packets to the new transfer).

Remove the assumption that we have a single tx urb in flight, by
tracking the tx urb with a usb_anchor rather than a single urb pointer.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 385350792dd4..b31599dfaa7e 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -37,9 +37,9 @@ struct mctp_usb {
 	struct delayed_work rx_retry_work;
 
 	struct mctp_usblib_tx tx;
-	/* protects tx_urb */
+	/* protects tx_anchor across submission / completion / cancellation */
 	spinlock_t tx_lock;
-	struct urb *tx_urb;
+	struct usb_anchor tx_anchor;
 };
 
 static void mctp_usb_out_complete(struct urb *urb)
@@ -52,7 +52,7 @@ static void mctp_usb_out_complete(struct urb *urb)
 	mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
 
 	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
-	mctp_usb->tx_urb = NULL;
+	usb_unanchor_urb(urb);
 	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
 
 	usb_free_urb(urb);
@@ -81,7 +81,7 @@ static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
 	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
 	if (!rc)
-		mctp_usb->tx_urb = urb;
+		usb_anchor_urb(urb, &mctp_usb->tx_anchor);
 	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
 
 	if (rc) {
@@ -212,10 +212,10 @@ static int mctp_usb_stop(struct net_device *dev)
 	flush_delayed_work(&mctp_usb->rx_retry_work);
 
 	usb_kill_urb(mctp_usb->rx_urb);
-
-	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
-	usb_kill_urb(mctp_usb->tx_urb);
-	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
+	/* we have stopped queues, the anchor's own lock will serialise
+	 * access from the urb completion.
+	 */
+	usb_kill_anchored_urbs(&mctp_usb->tx_anchor);
 
 	mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
 
@@ -277,6 +277,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 
 	mctp_usblib_rx_init(&dev->rx);
 	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev);
+	init_usb_anchor(&dev->tx_anchor);
 
 	dev->ep_in = ep_in->bEndpointAddress;
 	dev->ep_out = ep_out->bEndpointAddress;

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 06/12] net: mctp: usblib: Add support for multi-packet transmit
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (4 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 05/12] net: mctp: usb: Allow for multiple urb submissions from a packet tx Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 07/12] net: mctp: usb: Accommodate DSP0283 v1.1 header format Jeremy Kerr
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

The MCTP over USB spec allows us to pack multiple packets in one
transfer. Given the packet max length is 255, and the transfer max
length is 512, we can typically include two full-size packets per
urb submission.

To do this, we allow a struct mctp_usb_tx to persist a tx_ctx,
representing the ongoing context for a transmit. If possible, a TX skb
will be queued to the context and the send deferred until the context is
full, or the device queue reports no more packets.

This typically requires a linear buffer for the 512-byte TX, which we
allocate along with the TX context.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usblib.c | 242 +++++++++++++++++++++++++++++++++--------
 include/linux/usb/mctp-usb.h   |   4 +
 2 files changed, 202 insertions(+), 44 deletions(-)

diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index 56eca496bbe4..a8491ec04df3 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -165,11 +165,13 @@ EXPORT_SYMBOL_GPL(mctp_usblib_rx_cancel);
 /* transmit context: encapsulates one transfer */
 struct mctp_usblib_tx_ctx {
 	struct mctp_usblib_tx *tx;
-	struct sk_buff *skb;
+	struct sk_buff_head skbs;
 	unsigned int len;
 	enum mctp_usblib_tx_buf_type {
 		TX_SINGLE,
+		TX_FLAT,
 	} buf_type;
+	u8 buf[] ____cacheline_aligned;
 };
 
 void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
@@ -179,52 +181,123 @@ void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
 	memset(tx, 0, sizeof(*tx));
 	tx->ops = *ops;
 	tx->priv = priv;
+	spin_lock_init(&tx->lock);
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_init);
 
-void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx)
+static int mctp_usblib_tx_avail(struct mctp_usblib_tx_ctx *ctx)
 {
+	return ctx->buf_type == TX_SINGLE ? 0 : MCTP_USB_1_0_XFER_SIZE - ctx->len;
 }
-EXPORT_SYMBOL_GPL(mctp_usblib_tx_fini);
 
-void *mctp_usblib_tx_ctx_priv(struct mctp_usblib_tx_ctx *tx_ctx)
+static bool mctp_usblib_tx_should_send(struct mctp_usblib_tx_ctx *ctx)
 {
-	return tx_ctx->tx->priv;
+	return mctp_usblib_tx_avail(ctx) < (68 + sizeof(struct mctp_usb_hdr));
 }
-EXPORT_SYMBOL_GPL(mctp_usblib_tx_ctx_priv);
 
-static struct mctp_usblib_tx_ctx *
-mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb)
+/*
+ * Returns zero on success, non-zero on failure - indicating that the new skb
+ * could not be appended. So, errors reported here to the TX path will result
+ * in the TX being transmitted.
+ */
+static int mctp_usblib_tx_append(struct mctp_usblib_tx_ctx *ctx,
+				 struct sk_buff *skb)
 {
-	struct mctp_usblib_tx_ctx *ctx;
+	if (ctx->buf_type == TX_SINGLE)
+		return -EINVAL;
 
-	ctx = kzalloc_obj(*ctx, GFP_ATOMIC);
-	if (!ctx)
-		return NULL;
+	if (mctp_usblib_tx_avail(ctx) < skb->len)
+		return -ENOBUFS;
+
+	__skb_queue_tail(&ctx->skbs, skb);
 
-	ctx->tx = tx;
-	ctx->buf_type = TX_SINGLE;
-	ctx->skb = skb;
 	ctx->len += skb->len;
 
-	return ctx;
+	return 0;
 }
 
 static int mctp_usblib_tx_send(struct mctp_usblib_tx_ctx *ctx)
 {
-	struct mctp_usblib_tx *tx = ctx->tx;
-	void *buf = ctx->skb->data;
+	void *buf;
+
+	/* If we have a qlen of 1, we only ended up packing a single skb,
+	 * despite allocating for multiple. Skip the copy and send directly
+	 * from the skb data.
+	 */
+	if (ctx->buf_type == TX_SINGLE || ctx->skbs.qlen == 1) {
+		buf = ctx->skbs.next->data;
+
+	} else if (ctx->buf_type == TX_FLAT) {
+		struct sk_buff *skb;
+		size_t pos = 0;
+
+		skb_queue_walk(&ctx->skbs, skb) {
+			skb_copy_bits(skb, 0, ctx->buf + pos, skb->len);
+			pos += skb->len;
+		}
+
+		buf = ctx->buf;
+	} else {
+		return -EINVAL;
+	}
 
-	return tx->ops.send(ctx, buf, ctx->len);
+	return ctx->tx->ops.send(ctx, buf, ctx->len);
 }
 
 static void mctp_usblib_tx_ctx_free(struct mctp_usblib_tx_ctx *ctx)
 {
-	if (ctx)
-		dev_kfree_skb_any(ctx->skb);
+	struct sk_buff *skb;
+
+	if (!ctx)
+		return;
+
+	while ((skb = __skb_dequeue(&ctx->skbs)) != NULL)
+		dev_kfree_skb_any(skb);
 	kfree(ctx);
 }
 
+void *mctp_usblib_tx_ctx_priv(struct mctp_usblib_tx_ctx *tx_ctx)
+{
+	return tx_ctx->tx->priv;
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_ctx_priv);
+
+/* caller must ensure the tx & completion path is quiesced */
+void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx)
+{
+	if (tx->cur_ctx)
+		mctp_usblib_tx_ctx_free(tx->cur_ctx);
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_fini);
+
+static struct mctp_usblib_tx_ctx *
+mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb,
+			  bool single)
+{
+	enum mctp_usblib_tx_buf_type type;
+	struct mctp_usblib_tx_ctx *ctx;
+	size_t sz = 0;
+
+	if (single) {
+		type = TX_SINGLE;
+	} else {
+		type = TX_FLAT;
+		sz = MCTP_USB_1_0_XFER_SIZE;
+	}
+
+	ctx = kzalloc_flex(*ctx, buf, sz, GFP_ATOMIC);
+	if (!ctx)
+		return NULL;
+
+	ctx->tx = tx;
+	ctx->buf_type = type;
+	ctx->len += skb->len;
+	skb_queue_head_init(&ctx->skbs);
+	__skb_queue_tail(&ctx->skbs, skb);
+
+	return ctx;
+}
+
 static void mctp_usblib_tx_stats_update(struct mctp_usblib_tx_ctx *ctx,
 					struct net_device *dev,
 					bool ok)
@@ -234,10 +307,10 @@ static void mctp_usblib_tx_stats_update(struct mctp_usblib_tx_ctx *ctx,
 
 	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
 	if (ok) {
-		u64_stats_inc(&dstats->tx_packets);
+		u64_stats_add(&dstats->tx_packets, ctx->skbs.qlen);
 		u64_stats_add(&dstats->tx_bytes, ctx->len);
 	} else {
-		u64_stats_inc(&dstats->tx_drops);
+		u64_stats_add(&dstats->tx_drops, ctx->skbs.qlen);
 	}
 	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
 	put_cpu_ptr(dev->dstats);
@@ -294,8 +367,8 @@ static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)
 }
 
 /*
- * Push a new skb to the transfer. At present, no send must be in progress,
- * as we only handle single-packet USB transfers.
+ * Push a new skb to the transfer. May result in zero or more calls to
+ * ops->send().
  *
  * Takes ownership of @skb, including on error.
  */
@@ -303,34 +376,104 @@ int mctp_usblib_tx_push(struct net_device *dev,
 			struct mctp_usblib_tx *tx,
 			struct sk_buff *skb, bool more)
 {
-	struct mctp_usblib_tx_ctx *ctx;
-	int rc;
+	struct mctp_usblib_tx_ctx *ctx, *send_ctx = NULL;
+	const int max_tries = 3;
+	unsigned long flags;
+	int try = 1, rc;
+
+	rc = mctp_usblib_tx_skb_prepare(skb);
+	if (rc) {
+		mctp_usblib_tx_stats_single_drop(dev);
+		kfree_skb(skb);
+		/* we may still need to proceed, in case an existing ctx
+		 * is now sendable (ie.: !more).
+		 */
+		skb = NULL;
+	}
+
+retry:
+	/* Try and queue to the current context. We exit this critical section
+	 * with a few bits of state:
+	 *  - send_ctx: indicating a prior context that needs to be sent
+	 *  - skb: indicating that a skb still needs to be queued/sent
+	 */
+	spin_lock_irqsave(&tx->lock, flags);
+	ctx = tx->cur_ctx;
+	if (ctx) {
+		if (skb) {
+			rc = mctp_usblib_tx_append(ctx, skb);
+			if (rc) {
+				/* can't append to the pending tx - detach for
+				 * sending, and we'll create a new tx below.
+				 */
+				swap(tx->cur_ctx, send_ctx);
+			} else {
+				/* we have queued */
+				skb = NULL;
+				if (!more || mctp_usblib_tx_should_send(ctx))
+					swap(tx->cur_ctx, send_ctx);
+			}
+		} else if (!more) {
+			swap(tx->cur_ctx, send_ctx);
+		}
+	}
+	spin_unlock_irqrestore(&tx->lock, flags);
+
+	if (send_ctx) {
+		rc = mctp_usblib_tx_send(send_ctx);
+		if (rc) {
+			mctp_usblib_tx_stats_update(send_ctx, dev, false);
+			mctp_usblib_tx_ctx_free(send_ctx);
+		}
+		send_ctx = NULL;
+	}
 
+	/* we have either queued, or the prepare failed; nothing more to do */
 	if (!skb)
 		return 0;
 
-	rc = mctp_usblib_tx_skb_prepare(skb);
-	if (rc)
-		goto err_drop_single;
-
-	ctx = mctp_usblib_tx_ctx_create(tx, skb);
+	ctx = mctp_usblib_tx_ctx_create(tx, skb, !more);
 	if (!ctx) {
-		rc = -ENOMEM;
-		goto err_drop_single;
+		netdev_dbg(dev, "TX context create failed\n");
+		mctp_usblib_tx_stats_single_drop(dev);
+		kfree_skb(skb);
+		return -ENOMEM;
 	}
 
-	rc = mctp_usblib_tx_send(ctx);
-	if (rc) {
-		mctp_usblib_tx_stats_update(ctx, dev, false);
-		mctp_usblib_tx_ctx_free(ctx);
+	/* if we're ready to send now, no need to enqueue */
+	if (!more || mctp_usblib_tx_should_send(ctx)) {
+		rc = mctp_usblib_tx_send(ctx);
+		if (rc) {
+			mctp_usblib_tx_stats_update(ctx, dev, false);
+			mctp_usblib_tx_ctx_free(ctx);
+		}
+		return 0;
 	}
 
-	return rc;
+	spin_lock_irqsave(&tx->lock, flags);
+	if (!tx->cur_ctx) {
+		tx->cur_ctx = ctx;
+		ctx = NULL;
+	}
+	spin_unlock_irqrestore(&tx->lock, flags);
 
-err_drop_single:
-	mctp_usblib_tx_stats_single_drop(dev);
-	kfree_skb(skb);
-	return rc;
+	/* we may have lost the race with a concurrent tx; shouldn't happen, as
+	 * ndo_start_xmit should be serialised over one queue, but try again
+	 * from the top, as we may be able to queue the skb to that context.
+	 */
+	if (ctx) {
+		/* unlink the new (sole) skb, we don't want it freed with ctx */
+		__skb_queue_head_init(&ctx->skbs);
+		mctp_usblib_tx_ctx_free(ctx);
+		if (++try > max_tries) {
+			kfree_skb(skb);
+			mctp_usblib_tx_stats_single_drop(dev);
+			return -EBUSY;
+		}
+		goto retry;
+	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_push);
 
@@ -338,7 +481,18 @@ EXPORT_SYMBOL_GPL(mctp_usblib_tx_push);
 void mctp_usblib_tx_cancel(struct mctp_usblib_tx *tx,
 			   struct net_device *dev)
 {
-	/* nothing to do at present, no ctx is persistent */
+	struct mctp_usblib_tx_ctx *ctx = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tx->lock, flags);
+	swap(tx->cur_ctx, ctx);
+	spin_unlock_irqrestore(&tx->lock, flags);
+
+	if (!ctx)
+		return;
+
+	mctp_usblib_tx_stats_update(ctx, dev, false);
+	mctp_usblib_tx_ctx_free(ctx);
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_cancel);
 
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 9fe314c2551e..56a4aef1f7d0 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -76,6 +76,10 @@ struct mctp_usblib_tx_ops {
 struct mctp_usblib_tx {
 	struct mctp_usblib_tx_ops ops;
 	void *priv;
+	/* protects access to cur_ctx */
+	spinlock_t lock;
+	/* context to which we are adding packets, cleared on send */
+	struct mctp_usblib_tx_ctx *cur_ctx;
 };
 
 void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 07/12] net: mctp: usb: Accommodate DSP0283 v1.1 header format
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (5 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 06/12] net: mctp: usblib: Add support for multi-packet transmit Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 08/12] net: mctp: usblib: Implement receive-side packet spanning Jeremy Kerr
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

In the v1.1 update to DSP0283, we have a larger header field, of 13 bits
rather than 8.

In order to accommodate this, in preparation for proper v1.1 support,
expand our struct mctp_usb_hdr's len field to a u16, and endian-convert
when necessary. Because we don't yet support spanning mode, we will
never receive or transmit with the top 5 bits set, so we always mask
out anyway.

This allows for a future change where we allow spanning mode with
>512-byte transfers.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usblib.c | 12 +++++++-----
 include/linux/usb/mctp-usb.h   | 11 ++++++++---
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index a8491ec04df3..acfae6d32390 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -89,6 +89,7 @@ int mctp_usblib_rx_complete(struct net_device *netdev,
 	while (skb) {
 		struct sk_buff *skb2 = NULL;
 		struct mctp_usb_hdr *hdr;
+		u16 hdr_len;
 		/* length of MCTP packet, no USB header */
 		u8 pkt_len;
 
@@ -106,7 +107,9 @@ int mctp_usblib_rx_complete(struct net_device *netdev,
 			break;
 		}
 
-		if (hdr->len <
+		hdr_len = be16_to_cpu(hdr->len) & MCTP_USB_1_0_PKTLEN_MAX;
+
+		if (hdr_len <
 		    sizeof(struct mctp_hdr) + sizeof(struct mctp_usb_hdr)) {
 			netdev_dbg(netdev, "rx: short packet (hdr) %d\n",
 				   hdr->len);
@@ -115,12 +118,12 @@ int mctp_usblib_rx_complete(struct net_device *netdev,
 		}
 
 		/* we know we have at least sizeof(struct mctp_usb_hdr) here */
-		pkt_len = hdr->len - sizeof(struct mctp_usb_hdr);
+		pkt_len = hdr_len - sizeof(struct mctp_usb_hdr);
 		if (pkt_len > skb->len) {
 			rc = -EPROTO;
 			netdev_dbg(netdev,
 				   "rx: short packet (xfer) %d, actual %d\n",
-				   hdr->len, skb->len);
+				   hdr_len, skb->len);
 			break;
 		}
 
@@ -360,8 +363,7 @@ static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)
 		return -ENOMEM;
 
 	hdr->id = cpu_to_be16(MCTP_USB_DMTF_ID);
-	hdr->rsvd = 0;
-	hdr->len = plen + sizeof(*hdr);
+	hdr->len = cpu_to_be16(plen + sizeof(*hdr));
 
 	return 0;
 }
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 56a4aef1f7d0..47561f2471e5 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -2,7 +2,7 @@
 /*
  * mctp-usb.h - MCTP USB transport binding: common definitions,
  * based on DMTF0283 specification:
- * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.1.pdf
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.1.0.pdf
  *
  * These are protocol-level definitions, that may be shared between host
  * and gadget drivers.
@@ -17,10 +17,15 @@
 #include <linux/skbuff.h>
 #include <linux/types.h>
 
+/*
+ * MCTP-over-USB transport header. DSP0283 v1.0 has an 8-bit length field
+ * (preceded by 8 reserved bits), v1.1 has a 13-bit length field (preceded by
+ * 3 reserved bits). We use a be16 for our length to handle the larger v1.1
+ * representation, and mask as appropriate.
+ */
 struct mctp_usb_hdr {
 	__be16	id;
-	u8	rsvd;
-	u8	len;
+	__be16	len;
 } __packed;
 
 /* max transfer size for DSP0283 v1.0 */

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 08/12] net: mctp: usblib: Implement receive-side packet spanning
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (6 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 07/12] net: mctp: usb: Accommodate DSP0283 v1.1 header format Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 09/12] net: mctp: usblib: Implement transmit-side " Jeremy Kerr
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Using the existing prepare/complete API, we can persist the rx skb
across receives to implement v1.1 packet spanning.

Alter the packet-extraction loop to allow truncated packets, returning
early with the skb persisted for the next IN urb completion. When we see
we have a complete packet, netif_rx() that. If the packet boundary
aligns with the urb completion, we can netif_rx() the whole thing.

We still need to handle non-spanning mode, so error out on
truncated-packet cases there.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c    |   2 +-
 drivers/net/mctp/mctp-usblib.c | 135 ++++++++++++++++++++++++++---------------
 include/linux/usb/mctp-usb.h   |   5 +-
 3 files changed, 91 insertions(+), 51 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index b31599dfaa7e..c89588741855 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -275,7 +275,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	spin_lock_init(&dev->tx_lock);
 	usb_set_intfdata(intf, dev);
 
-	mctp_usblib_rx_init(&dev->rx);
+	mctp_usblib_rx_init(&dev->rx, false);
 	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev);
 	init_usb_anchor(&dev->tx_anchor);
 
diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index acfae6d32390..a1649f24d937 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -3,7 +3,7 @@
  * mctp-usblib.c - MCTP-over-USB (DMTF DSP0283) transport helper library
  *
  * DSP0283 is available at:
- * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.1.pdf
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.1.0.pdf
  *
  * Copyright (C) 2024-2026 Code Construct Pty Ltd
  */
@@ -13,9 +13,10 @@
 #include <linux/usb/mctp-usb.h>
 #include <net/mctp.h>
 
-void mctp_usblib_rx_init(struct mctp_usblib_rx *rx)
+void mctp_usblib_rx_init(struct mctp_usblib_rx *rx, bool span)
 {
 	memset(rx, 0, sizeof(*rx));
+	rx->span = span;
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_rx_init);
 
@@ -33,12 +34,30 @@ int mctp_usblib_rx_prepare(struct net_device *netdev,
 			   struct mctp_usblib_rx *rx,
 			   void **bufp, size_t *lenp, gfp_t gfp)
 {
-	const unsigned int len = MCTP_USB_1_0_XFER_SIZE;
-	struct sk_buff *skb;
+	struct sk_buff *skb = rx->skb;
+	unsigned int len = 0;
 
-	skb = __netdev_alloc_skb(netdev, len, gfp);
-	if (!skb)
-		return -ENOMEM;
+	if (skb && skb->len >= MCTP_USB_1_1_PKTLEN_MAX) {
+		/* something must have gone terribly wrong. clear and restart */
+		mctp_usblib_rx_cancel(rx);
+		skb = NULL;
+	}
+
+	len = rx->span ? MCTP_USB_1_1_PKTLEN_MAX : MCTP_USB_1_0_XFER_SIZE;
+
+	if (skb) {
+		struct sk_buff *skb2;
+
+		skb2 = skb_copy_expand(skb, 0, len, gfp);
+		if (!skb2)
+			return -ENOMEM;
+		dev_kfree_skb_any(skb);
+		skb = skb2;
+	} else {
+		skb = __netdev_alloc_skb(netdev, len, gfp);
+		if (!skb)
+			return -ENOMEM;
+	}
 
 	rx->skb = skb;
 
@@ -60,10 +79,11 @@ static void mctp_usblib_rx(struct net_device *netdev, struct sk_buff *skb)
 	 */
 	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
 	u64_stats_inc(&dstats->rx_packets);
-	u64_stats_add(&dstats->rx_bytes, skb->len + sizeof(struct mctp_usb_hdr));
+	u64_stats_add(&dstats->rx_bytes, skb->len);
 	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
 
 	skb_reset_mac_header(skb);
+	skb_pull(skb, sizeof(struct mctp_usb_hdr));
 	skb->protocol = htons(ETH_P_MCTP);
 	skb_reset_network_header(skb);
 	cb = __mctp_cb(skb);
@@ -86,70 +106,87 @@ int mctp_usblib_rx_complete(struct net_device *netdev,
 
 	__skb_put(skb, len);
 
-	while (skb) {
-		struct sk_buff *skb2 = NULL;
+	for (;;) {
 		struct mctp_usb_hdr *hdr;
-		u16 hdr_len;
-		/* length of MCTP packet, no USB header */
-		u8 pkt_len;
-
-		skb_reset_mac_header(skb);
-		hdr = skb_pull_data(skb, sizeof(*hdr));
-		if (!hdr) {
-			rc = -ENOMSG;
+		struct sk_buff *skb2;
+		/* length of MCTP packet, including USB header */
+		u16 pkt_len;
+
+		/* no header yet, resubmit for the rest of the packet */
+		if (skb->len < sizeof(*hdr)) {
+			if (!rx->span) {
+				netdev_dbg(netdev,
+					   "rx: tiny xfer (%d) in non-span mode",
+					   skb->len);
+				rc = -ENOMSG;
+				goto err_reset;
+			}
 			break;
 		}
 
+		hdr = (struct mctp_usb_hdr *)skb->data;
+
 		if (be16_to_cpu(hdr->id) != MCTP_USB_DMTF_ID) {
+			/* By resetting here, will start the next IN transfer
+			 * at the beginning of the new skb. This will mean
+			 * we re-sync when we next see a spanned packet aligned
+			 * with the start of a transfer.
+			 *
+			 * In non-spanning mode, this just means we'll drop
+			 * the current transfer only
+			 */
 			netdev_dbg(netdev, "rx: invalid id %04x\n",
 				   be16_to_cpu(hdr->id));
 			rc = -EPROTO;
-			break;
+			goto err_reset;
 		}
 
-		hdr_len = be16_to_cpu(hdr->len) & MCTP_USB_1_0_PKTLEN_MAX;
-
-		if (hdr_len <
-		    sizeof(struct mctp_hdr) + sizeof(struct mctp_usb_hdr)) {
-			netdev_dbg(netdev, "rx: short packet (hdr) %d\n",
-				   hdr->len);
+		pkt_len = be16_to_cpu(hdr->len);
+		/* v1.1, with span enabled, has a 13-bit length */
+		pkt_len &= rx->span ?
+			MCTP_USB_1_1_PKTLEN_MAX : MCTP_USB_1_0_PKTLEN_MAX;
+		if (pkt_len < sizeof(*hdr)) {
+			netdev_dbg(netdev, "rx: invalid len %d\n", pkt_len);
 			rc = -EPROTO;
-			break;
+			goto err_reset;
 		}
 
-		/* we know we have at least sizeof(struct mctp_usb_hdr) here */
-		pkt_len = hdr_len - sizeof(struct mctp_usb_hdr);
+		/* span continues to the next transfer, resubmit */
 		if (pkt_len > skb->len) {
-			rc = -EPROTO;
-			netdev_dbg(netdev,
-				   "rx: short packet (xfer) %d, actual %d\n",
-				   hdr_len, skb->len);
+			if (!rx->span) {
+				netdev_dbg(netdev,
+					   "rx: short xfer (%d vs %d) in non-span mode",
+					   pkt_len, skb->len);
+				rc = -ENOMSG;
+				goto err_reset;
+			}
 			break;
 		}
 
-		if (pkt_len < skb->len) {
-			/* more packets may follow - clone to a new
-			 * skb to use on the next iteration
-			 */
-			skb2 = skb_clone(skb, GFP_ATOMIC);
-			if (skb2) {
-				if (!skb_pull(skb2, pkt_len)) {
-					dev_kfree_skb_any(skb2);
-					skb2 = NULL;
-				}
-			}
-			skb_trim(skb, pkt_len);
+		/* we have (exactly) a complete packet, RX it directly */
+		if (pkt_len == skb->len) {
+			mctp_usblib_rx(netdev, skb);
+			rx->skb = NULL;
+			break;
 		}
 
-		mctp_usblib_rx(netdev, skb);
-		skb = skb2;
+		/* more packets follow - RX a clone so that we can continue
+		 * processing the current SKB, which may be the start of a
+		 * span.
+		 */
+		skb2 = skb_clone(skb, GFP_ATOMIC);
+		if (skb2) {
+			skb_trim(skb2, pkt_len);
+			mctp_usblib_rx(netdev, skb2);
+		}
+		skb_pull(skb, pkt_len);
 	}
 
-	if (skb)
-		dev_kfree_skb_any(skb);
+	return 0;
 
+err_reset:
+	dev_kfree_skb_any(rx->skb);
 	rx->skb = NULL;
-
 	return rc;
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_rx_complete);
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 47561f2471e5..00c538a8e211 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -34,6 +34,8 @@ struct mctp_usb_hdr {
 #define MCTP_USB_MTU_MIN	MCTP_USB_BTU
 #define MCTP_USB_1_0_PKTLEN_MAX	U8_MAX
 #define MCTP_USB_1_0_MTU_MAX	(MCTP_USB_1_0_PKTLEN_MAX - sizeof(struct mctp_usb_hdr))
+#define MCTP_USB_1_1_PKTLEN_MAX	GENMASK(12, 0)
+#define MCTP_USB_1_1_MTU_MAX	(MCTP_USB_1_1_PKTLEN_MAX - sizeof(struct mctp_usb_hdr))
 #define MCTP_USB_DMTF_ID	0x1ab4
 
 /* mctp-usblib */
@@ -46,9 +48,10 @@ struct mctp_usb_hdr {
  */
 struct mctp_usblib_rx {
 	struct sk_buff *skb;
+	bool span;
 };
 
-void mctp_usblib_rx_init(struct mctp_usblib_rx *rx);
+void mctp_usblib_rx_init(struct mctp_usblib_rx *rx, bool span);
 void mctp_usblib_rx_fini(struct mctp_usblib_rx *rx);
 
 int mctp_usblib_rx_prepare(struct net_device *netdev,

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 09/12] net: mctp: usblib: Implement transmit-side packet spanning
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (7 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 08/12] net: mctp: usblib: Implement receive-side packet spanning Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests Jeremy Kerr
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Add support for packet spanning as defined in DSP0283 v1.1.

With the existing v1.0 implementation of multi-packet transfers, all we
need here is to adjust the buffer sizes to suit v1.1.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c    |  2 +-
 drivers/net/mctp/mctp-usblib.c | 28 +++++++++++++++++++---------
 include/linux/usb/mctp-usb.h   |  4 +++-
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index c89588741855..91109ad17d1c 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -276,7 +276,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, dev);
 
 	mctp_usblib_rx_init(&dev->rx, false);
-	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev);
+	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev, false);
 	init_usb_anchor(&dev->tx_anchor);
 
 	dev->ep_in = ep_in->bEndpointAddress;
diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index a1649f24d937..dbc056bfd627 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -206,7 +206,7 @@ EXPORT_SYMBOL_GPL(mctp_usblib_rx_cancel);
 struct mctp_usblib_tx_ctx {
 	struct mctp_usblib_tx *tx;
 	struct sk_buff_head skbs;
-	unsigned int len;
+	unsigned int buf_len, len;
 	enum mctp_usblib_tx_buf_type {
 		TX_SINGLE,
 		TX_FLAT,
@@ -216,18 +216,19 @@ struct mctp_usblib_tx_ctx {
 
 void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
 			 const struct mctp_usblib_tx_ops *ops,
-			 void *priv)
+			 void *priv, bool span)
 {
 	memset(tx, 0, sizeof(*tx));
 	tx->ops = *ops;
 	tx->priv = priv;
+	tx->span = span;
 	spin_lock_init(&tx->lock);
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_init);
 
 static int mctp_usblib_tx_avail(struct mctp_usblib_tx_ctx *ctx)
 {
-	return ctx->buf_type == TX_SINGLE ? 0 : MCTP_USB_1_0_XFER_SIZE - ctx->len;
+	return ctx->buf_type == TX_SINGLE ? 0 : ctx->buf_len - ctx->len;
 }
 
 static bool mctp_usblib_tx_should_send(struct mctp_usblib_tx_ctx *ctx)
@@ -310,6 +311,12 @@ void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx)
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_fini);
 
+/* Max size of a spanned TX. Since we allocate a separate span buffer, limit
+ * the tx-time allocations to 4k. Larger packets will be sent as single
+ * transfers.
+ */
+static const unsigned int TX_SPAN_MAX = 4096 - sizeof(struct mctp_usblib_tx_ctx);
+
 static struct mctp_usblib_tx_ctx *
 mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb,
 			  bool single)
@@ -318,11 +325,11 @@ mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb,
 	struct mctp_usblib_tx_ctx *ctx;
 	size_t sz = 0;
 
-	if (single) {
+	if (single || skb->len > TX_SPAN_MAX) {
 		type = TX_SINGLE;
 	} else {
 		type = TX_FLAT;
-		sz = MCTP_USB_1_0_XFER_SIZE;
+		sz = tx->span ? TX_SPAN_MAX : MCTP_USB_1_0_XFER_SIZE;
 	}
 
 	ctx = kzalloc_flex(*ctx, buf, sz, GFP_ATOMIC);
@@ -331,6 +338,7 @@ mctp_usblib_tx_ctx_create(struct mctp_usblib_tx *tx, struct sk_buff *skb,
 
 	ctx->tx = tx;
 	ctx->buf_type = type;
+	ctx->buf_len = sz;
 	ctx->len += skb->len;
 	skb_queue_head_init(&ctx->skbs);
 	__skb_queue_tail(&ctx->skbs, skb);
@@ -381,14 +389,16 @@ void mctp_usblib_tx_send_complete(struct mctp_usblib_tx_ctx *tx_ctx,
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);
 
 /* Prepare a skb for push() */
-static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)
+static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb, bool span)
 {
+	unsigned long plen, max_len;
 	struct mctp_usb_hdr *hdr;
-	unsigned long plen;
 	int rc;
 
+	max_len = span ? MCTP_USB_1_1_PKTLEN_MAX : MCTP_USB_1_0_PKTLEN_MAX;
+
 	plen = skb->len;
-	if (plen + sizeof(*hdr) > MCTP_USB_1_0_PKTLEN_MAX)
+	if (plen + sizeof(*hdr) > max_len)
 		return -EMSGSIZE;
 
 	rc = skb_cow_head(skb, sizeof(*hdr));
@@ -420,7 +430,7 @@ int mctp_usblib_tx_push(struct net_device *dev,
 	unsigned long flags;
 	int try = 1, rc;
 
-	rc = mctp_usblib_tx_skb_prepare(skb);
+	rc = mctp_usblib_tx_skb_prepare(skb, tx->span);
 	if (rc) {
 		mctp_usblib_tx_stats_single_drop(dev);
 		kfree_skb(skb);
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 00c538a8e211..61fb36f34525 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -84,6 +84,7 @@ struct mctp_usblib_tx_ops {
 struct mctp_usblib_tx {
 	struct mctp_usblib_tx_ops ops;
 	void *priv;
+	bool span;
 	/* protects access to cur_ctx */
 	spinlock_t lock;
 	/* context to which we are adding packets, cleared on send */
@@ -91,7 +92,8 @@ struct mctp_usblib_tx {
 };
 
 void mctp_usblib_tx_init(struct mctp_usblib_tx *tx,
-			 const struct mctp_usblib_tx_ops *ops, void *priv);
+			 const struct mctp_usblib_tx_ops *ops, void *priv,
+			 bool span);
 void mctp_usblib_tx_fini(struct mctp_usblib_tx *tx);
 
 void *mctp_usblib_tx_ctx_priv(struct mctp_usblib_tx_ctx *tx_ctx);

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (8 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 09/12] net: mctp: usblib: Implement transmit-side " Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-07-02 10:10   ` Paolo Abeni
  2026-06-30  3:21 ` [PATCH net-next 11/12] net: mctp: usb: enable v1.1 packet spanning Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight Jeremy Kerr
  11 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Add some initial tests for the usblib receive path, where we're
extracting MCTP packets from incoming USB transfer data.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/Kconfig            |   5 +
 drivers/net/mctp/mctp-usblib-test.c | 330 ++++++++++++++++++++++++++++++++++++
 drivers/net/mctp/mctp-usblib.c      |   4 +
 3 files changed, 339 insertions(+)

diff --git a/drivers/net/mctp/Kconfig b/drivers/net/mctp/Kconfig
index a564a792801d..c40ac9c665b7 100644
--- a/drivers/net/mctp/Kconfig
+++ b/drivers/net/mctp/Kconfig
@@ -57,6 +57,11 @@ config MCTP_TRANSPORT_USBLIB
 
 	  This will be automatically enabled by the transport driver.
 
+config MCTP_TRANSPORT_USBLIB_TEST
+        bool "MCTP usblib tests" if !KUNIT_ALL_TESTS
+        depends on MCTP_TRANSPORT_USBLIB=y && KUNIT=y
+        default KUNIT_ALL_TESTS
+
 config MCTP_TRANSPORT_USB
 	tristate "MCTP USB transport"
 	depends on USB
diff --git a/drivers/net/mctp/mctp-usblib-test.c b/drivers/net/mctp/mctp-usblib-test.c
new file mode 100644
index 000000000000..ac837cbb4436
--- /dev/null
+++ b/drivers/net/mctp/mctp-usblib-test.c
@@ -0,0 +1,330 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mctp-usblib-test.c - MCTP-over-USB (DMTF DSP0283) transport helper library,
+ * unit test definitions.
+ *
+ * Copyright (C) 2026 Code Construct Pty Ltd
+ */
+
+#include <uapi/linux/netdevice.h>
+#include <linux/netdevice.h>
+#include <kunit/test.h>
+#include <linux/if_arp.h>
+#include <net/mctp.h>
+#include <net/mctpdevice.h>
+#include <linux/usb/mctp-usb.h>
+
+struct mctp_usblib_test_dev {
+	struct net_device *ndev;
+	struct mctp_dev *mdev;
+	struct sk_buff_head rx_pkts;
+};
+
+struct mctp_usblib_test_ctx {
+	struct mctp_usblib_test_dev *dev;
+	struct mctp_route rt;
+};
+
+static netdev_tx_t mctp_usblib_dev_tx(struct sk_buff *skb,
+				      struct net_device *ndev)
+{
+	/* we don't track any TXed packets at present */
+	kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops mctp_test_netdev_ops = {
+	.ndo_start_xmit = mctp_usblib_dev_tx,
+};
+
+static const mctp_eid_t local_eid = 8;
+
+static void mctp_usblib_dev_setup(struct net_device *ndev)
+{
+	ndev->type = ARPHRD_MCTP;
+	ndev->mtu = 8192;
+	ndev->flags = IFF_NOARP;
+	ndev->netdev_ops = &mctp_test_netdev_ops;
+	ndev->needs_free_netdev = true;
+	ndev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
+}
+
+static struct mctp_usblib_test_dev *mctp_usblib_test_create_dev(void)
+{
+	struct mctp_usblib_test_dev *dev;
+	struct net_device *ndev;
+	int rc;
+
+	ndev = alloc_netdev(sizeof(*dev), "mctptest%d", NET_NAME_ENUM,
+			    mctp_usblib_dev_setup);
+	if (!ndev)
+		return NULL;
+
+	dev = netdev_priv(ndev);
+	dev->ndev = ndev;
+	skb_queue_head_init(&dev->rx_pkts);
+
+	rc = register_netdev(ndev);
+	if (rc) {
+		free_netdev(ndev);
+		return NULL;
+	}
+
+	rcu_read_lock();
+	dev->mdev = __mctp_dev_get(ndev);
+	dev->mdev->net = mctp_default_net(dev_net(ndev));
+	rcu_read_unlock();
+
+	rtnl_lock();
+	dev_open(ndev, NULL);
+	rtnl_unlock();
+
+	return dev;
+}
+
+static int mctp_usblib_test_dst_output(struct mctp_dst *dst,
+				       struct sk_buff *skb)
+{
+	struct mctp_usblib_test_dev *dev = netdev_priv(skb->dev);
+
+	skb_queue_tail(&dev->rx_pkts, skb);
+
+	return 0;
+}
+
+static void mctp_usblib_test_init(struct kunit *test,
+				  struct mctp_usblib_test_ctx *ctx)
+{
+	struct mctp_route *rt = &ctx->rt;
+
+	ctx->dev = mctp_usblib_test_create_dev();
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev);
+
+	memset(rt, 0, sizeof(*rt));
+	rt->min = local_eid;
+	rt->max = local_eid;
+	rt->dst_type = MCTP_ROUTE_DIRECT;
+	rt->type = RTN_LOCAL;
+	rt->dev = ctx->dev->mdev;
+	rt->output = mctp_usblib_test_dst_output;
+
+	list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);
+}
+
+static void mctp_usblib_test_fini(struct kunit *test,
+				  struct mctp_usblib_test_ctx *ctx)
+{
+	list_del_rcu(&ctx->rt.list);
+	synchronize_rcu();
+
+	skb_queue_purge(&ctx->dev->rx_pkts);
+	mctp_dev_put(ctx->dev->mdev);
+	unregister_netdev(ctx->dev->ndev);
+}
+
+/* Init a MCTP-over-USB packet within a buffer. @len is the length of the
+ * buffer to write, @payload_len is the reported size of the MCTP-over-USB
+ * packet.
+ */
+static void mctp_usblib_test_init_pkt(void *data, size_t len,
+				      size_t payload_len)
+{
+	struct {
+		struct mctp_usb_hdr usb;
+		struct mctp_hdr mctp;
+	} hdr;
+
+	hdr.usb.id = cpu_to_be16(MCTP_USB_DMTF_ID);
+	hdr.usb.len = cpu_to_be16(payload_len);
+	hdr.mctp.ver = 1;
+	hdr.mctp.dest = local_eid;
+	hdr.mctp.src = 0;
+	hdr.mctp.flags_seq_tag = 0;
+
+	memcpy(data, &hdr, min(len, sizeof(hdr)));
+	if (len > sizeof(hdr))
+		memset(data + sizeof(hdr), 0, len - sizeof(hdr));
+}
+
+/* Single packet, starting on a transfer boundary, contained entirely within
+ * the transfer
+ */
+static void mctp_usblib_test_rx_single(struct kunit *test)
+{
+	struct mctp_usblib_test_dev *dev;
+	struct mctp_usblib_test_ctx ctx;
+	struct mctp_usblib_rx rx;
+	struct sk_buff *skb;
+	size_t len;
+	void *buf;
+	int rc;
+
+	mctp_usblib_test_init(test, &ctx);
+	dev = ctx.dev;
+
+	mctp_usblib_rx_init(&rx, true);
+
+	rc = mctp_usblib_rx_prepare(dev->ndev, &rx, &buf, &len, GFP_KERNEL);
+	KUNIT_ASSERT_EQ(test, rc, 0);
+
+	mctp_usblib_test_init_pkt(buf, 8, 8);
+
+	rc = mctp_usblib_rx_complete(dev->ndev, &rx, 8);
+	KUNIT_ASSERT_EQ(test, rc, 0);
+
+	skb = __skb_dequeue(&dev->rx_pkts);
+	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, skb);
+	KUNIT_EXPECT_EQ(test, skb->len, 4);
+	kfree_skb(skb);
+
+	mctp_usblib_rx_fini(&rx);
+	mctp_usblib_test_fini(test, &ctx);
+}
+
+struct mctp_usblib_test_pkt_span {
+	const char *name;
+	size_t n_pkts;
+	size_t pkts[6];
+	size_t n_xfers;
+	size_t xfers[6];
+};
+
+static void
+mctp_usblib_test_pkt_span_to_desc(const struct mctp_usblib_test_pkt_span *t,
+				  char *desc)
+{
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+static void
+mctp_usblib_test_pkt_span_validate(struct kunit *test,
+				   const struct mctp_usblib_test_pkt_span *span,
+				   size_t *len)
+{
+	size_t pkt_len = 0, xfer_len = 0;
+	unsigned int i;
+
+	for (i = 0; i < span->n_pkts; i++) {
+		KUNIT_ASSERT_GE_MSG(test, span->pkts[i], 8,
+				    "pkt[%d] len too small (%zd) for %s",
+				    i, span->pkts[i], span->name);
+		pkt_len += span->pkts[i];
+	}
+
+	for (i = 0; i < span->n_xfers; i++)
+		xfer_len += span->xfers[i];
+
+	KUNIT_ASSERT_EQ_MSG(test, pkt_len, xfer_len,
+			    "invalid pkt_len (%zd) != xfer_len (%zd) for %s",
+			    pkt_len, xfer_len, span->name);
+
+	*len = pkt_len;
+}
+
+static void mctp_usblib_test_rx_pkt_span(struct kunit *test)
+{
+	const struct mctp_usblib_test_pkt_span *pkt_span = test->param_value;
+	size_t len, xfer_len, off, xfer_off;
+	struct mctp_usblib_test_dev *dev;
+	struct mctp_usblib_test_ctx ctx;
+	struct mctp_usblib_rx rx;
+	unsigned int i;
+	u8 *pktbuf;
+	void *buf;
+	int rc;
+
+	mctp_usblib_test_pkt_span_validate(test, pkt_span, &len);
+	pktbuf = kmalloc_array(1, len, GFP_KERNEL);
+
+	/* lay out packets */
+	for (off = 0, i = 0; i < pkt_span->n_pkts; i++) {
+		len = pkt_span->pkts[i];
+		mctp_usblib_test_init_pkt(pktbuf + off, len, len);
+		off += len;
+	}
+
+	mctp_usblib_test_init(test, &ctx);
+	dev = ctx.dev;
+
+	mctp_usblib_rx_init(&rx, true);
+
+	/* feed transfers */
+	for (off = 0, xfer_off = 0, i = 0; i < pkt_span->n_xfers;) {
+		xfer_len = pkt_span->xfers[i] - xfer_off;
+		rc = mctp_usblib_rx_prepare(dev->ndev, &rx, &buf, &len,
+					    GFP_KERNEL);
+		KUNIT_ASSERT_EQ(test, rc, 0);
+
+		len = min(len, xfer_len);
+		memcpy(buf, pktbuf + off, len);
+
+		if (len == xfer_len) {
+			/* whole/end xfer, proceed to next */
+			xfer_off = 0;
+			i++;
+		} else {
+			/* partial */
+			xfer_off += len;
+		}
+
+		rc = mctp_usblib_rx_complete(dev->ndev, &rx, len);
+		KUNIT_ASSERT_EQ(test, rc, 0);
+		off += len;
+	}
+
+	/* check received packets */
+	KUNIT_EXPECT_EQ(test, dev->rx_pkts.qlen, pkt_span->n_pkts);
+	for (i = 0; ; i++) {
+		struct sk_buff *skb = __skb_dequeue(&dev->rx_pkts);
+
+		if (!skb)
+			break;
+
+		if (i <= pkt_span->n_pkts)
+			KUNIT_EXPECT_EQ(test, skb->len, pkt_span->pkts[i] - 4);
+
+		kfree_skb(skb);
+	}
+
+	kfree(pktbuf);
+	mctp_usblib_rx_fini(&rx);
+	mctp_usblib_test_fini(test, &ctx);
+}
+
+static const struct mctp_usblib_test_pkt_span mctp_usblib_test_pkt_spans[] = {
+	/* One packet completely within a transfer */
+	{ "1p1x-complete", 1, { 8 }, 1, { 8 } },
+	/* Two small packets combined within one transfer */
+	{ "2p1x-combined", 2, { 8, 8 }, 1, { 16 } },
+	/* A packet split over two transfers, at the MCTP payload */
+	{ "1p2x-split-payload", 1, { 16 }, 2, { 8, 8 } },
+	/* A packet split over two transfers, at the USB transport header */
+	{ "1p2x-split-usbhdr", 1, { 16 }, 2, { 2, 14 } },
+	/* A packet split over two transfers, at the MCTP header */
+	{ "1p2x-split-mctphdr", 1, { 16 }, 2, { 6, 10 } },
+	/* Single packet split over 3 transfers, middle entirely continuation */
+	{ "1p3x-split", 1, { 12 }, 3, { 4, 4, 4 } },
+	/* Max-sized single transfer */
+	{ "1p1x-large", 1, { 8191 }, 1, { 8191 } },
+	/* Two large packets, split at the worst-case for allocation, with a
+	 * single byte continuing the span
+	 */
+	{ "2p2x-large-split", 2, { 8190, 8190 }, 2, { 8191, 8189 } },
+};
+
+KUNIT_ARRAY_PARAM(mctp_usblib_test_rx_pkt_span, mctp_usblib_test_pkt_spans,
+		  mctp_usblib_test_pkt_span_to_desc);
+
+static struct kunit_case mctp_usblib_test_cases[] = {
+	KUNIT_CASE(mctp_usblib_test_rx_single),
+	KUNIT_CASE_PARAM(mctp_usblib_test_rx_pkt_span,
+			 mctp_usblib_test_rx_pkt_span_gen_params),
+	{}
+};
+
+static struct kunit_suite mctp_usblib_test_suite = {
+	.name = "mctp-usblib",
+	.test_cases = mctp_usblib_test_cases,
+};
+
+kunit_test_suite(mctp_usblib_test_suite);
diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index dbc056bfd627..d518fa3906e3 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -548,3 +548,7 @@ EXPORT_SYMBOL_GPL(mctp_usblib_tx_cancel);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>");
 MODULE_DESCRIPTION("MCTP USB transport library");
+
+#if IS_ENABLED(CONFIG_MCTP_TRANSPORT_USBLIB_TEST)
+#include "mctp-usblib-test.c"
+#endif

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 11/12] net: mctp: usb: enable v1.1 packet spanning
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (9 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-06-30  3:21 ` [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight Jeremy Kerr
  11 siblings, 0 replies; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Now that mctp-usblib supports DSP0283 v1.1 packet spanning, enable it in
our host-side transport driver.

Add a match for the new device subclass (0x02), and indicating spanning
mode to the usblib rx/tx implementation.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 91109ad17d1c..5739c87da109 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -3,9 +3,9 @@
  * mctp-usb.c - MCTP-over-USB (DMTF DSP0283) transport binding driver.
  *
  * DSP0283 is available at:
- * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.0.1.pdf
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0283_1.1.0.pdf
  *
- * Copyright (C) 2024-2025 Code Construct Pty Ltd
+ * Copyright (C) 2024-2026 Code Construct Pty Ltd
  */
 
 #include <linux/module.h>
@@ -22,6 +22,7 @@
 struct mctp_usb {
 	struct usb_device *usbdev;
 	struct usb_interface *intf;
+	bool span;
 
 	struct net_device *netdev;
 
@@ -42,6 +43,11 @@ struct mctp_usb {
 	struct usb_anchor tx_anchor;
 };
 
+enum {
+	MCTP_USB_SUBCLASS_BASE = 0x00,
+	MCTP_USB_SUBCLASS_SPAN = 0x02,
+};
+
 static void mctp_usb_out_complete(struct urb *urb)
 {
 	struct mctp_usblib_tx_ctx *tx_ctx = urb->context;
@@ -76,6 +82,9 @@ static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
 			  usb_sndbulkpipe(mctp_usb->usbdev, mctp_usb->ep_out),
 			  data, len, mctp_usb_out_complete, tx_ctx);
 
+	if (mctp_usb->span)
+		urb->transfer_flags |= URB_ZERO_PACKET;
+
 	netif_stop_queue(mctp_usb->netdev);
 
 	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
@@ -250,6 +259,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	struct usb_host_interface *iface_desc;
 	struct net_device *netdev;
 	struct mctp_usb *dev;
+	bool span;
 	int rc;
 
 	/* only one alternate */
@@ -261,6 +271,8 @@ static int mctp_usb_probe(struct usb_interface *intf,
 		return rc;
 	}
 
+	span = iface_desc->desc.bInterfaceSubClass == MCTP_USB_SUBCLASS_SPAN;
+
 	netdev = alloc_netdev(sizeof(*dev), "mctpusb%d", NET_NAME_ENUM,
 			      mctp_usb_netdev_setup);
 	if (!netdev)
@@ -268,15 +280,18 @@ static int mctp_usb_probe(struct usb_interface *intf,
 
 	SET_NETDEV_DEV(netdev, &intf->dev);
 	dev = netdev_priv(netdev);
+	dev->span = span;
 	dev->netdev = netdev;
 	dev->usbdev = interface_to_usbdev(intf);
 	dev->intf = intf;
+	if (dev->span)
+		netdev->max_mtu = MCTP_USB_1_1_MTU_MAX;
 	spin_lock_init(&dev->rx_lock);
 	spin_lock_init(&dev->tx_lock);
 	usb_set_intfdata(intf, dev);
 
-	mctp_usblib_rx_init(&dev->rx, false);
-	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev, false);
+	mctp_usblib_rx_init(&dev->rx, dev->span);
+	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev, dev->span);
 	init_usb_anchor(&dev->tx_anchor);
 
 	dev->ep_in = ep_in->bEndpointAddress;
@@ -317,7 +332,8 @@ static void mctp_usb_disconnect(struct usb_interface *intf)
 }
 
 static const struct usb_device_id mctp_usb_devices[] = {
-	{ USB_INTERFACE_INFO(USB_CLASS_MCTP, 0x0, 0x1) },
+	{ USB_INTERFACE_INFO(USB_CLASS_MCTP, MCTP_USB_SUBCLASS_BASE, 0x1) },
+	{ USB_INTERFACE_INFO(USB_CLASS_MCTP, MCTP_USB_SUBCLASS_SPAN, 0x1) },
 	{ 0 },
 };
 

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight
  2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
                   ` (10 preceding siblings ...)
  2026-06-30  3:21 ` [PATCH net-next 11/12] net: mctp: usb: enable v1.1 packet spanning Jeremy Kerr
@ 2026-06-30  3:21 ` Jeremy Kerr
  2026-07-02 10:09   ` Paolo Abeni
  11 siblings, 1 reply; 17+ messages in thread
From: Jeremy Kerr @ 2026-06-30  3:21 UTC (permalink / raw)
  To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Greg Kroah-Hartman
  Cc: netdev, linux-usb

Currently, we stop tx queues when we have one urb submitted. This means
we will immediately hit dev_hard_start_xmit's tx-queues-off ->
NETDEV_TX_BUSY case, and revert to the requeue -> gso_skb single-dequeue
path, and no longer be able to pack skbs without an xmit_more
indication.

Instead, allow a few urbs to be in-flight, with a limit of 16kB of data
outstanding (after which we will disable queues). With this, the tx path
will cause fewer requeues (and therefore non-packed transfers) under
normal loads.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 drivers/net/mctp/mctp-usb.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 5739c87da109..28aeb5b25872 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -38,9 +38,12 @@ struct mctp_usb {
 	struct delayed_work rx_retry_work;
 
 	struct mctp_usblib_tx tx;
-	/* protects tx_anchor across submission / completion / cancellation */
+	/* protects tx_anchor & tx_qmem across submission / completion /
+	 * cancellation
+	 */
 	spinlock_t tx_lock;
 	struct usb_anchor tx_anchor;
+	unsigned int tx_qmem;
 };
 
 enum {
@@ -48,22 +51,38 @@ enum {
 	MCTP_USB_SUBCLASS_SPAN = 0x02,
 };
 
+/* We use a total-size limit for outstanding URBs, as the transfer counts
+ * may vary a lot between spanning- and non-spanning modes. In spanning mode,
+ * this will allow for a couple of max-sized transfers to be in flight. In
+ * non-spanning mode, 32.
+ *
+ * We want to avoid disabling the tx queue if possible; doing so will end up
+ * requeueing to gso_skb, and we only dequeue from that one skb at a time,
+ * so can no longer perform transfer packing.
+ */
+static const unsigned int TX_QMEM_MAX = 16384;
+
 static void mctp_usb_out_complete(struct urb *urb)
 {
 	struct mctp_usblib_tx_ctx *tx_ctx = urb->context;
 	struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
 	struct net_device *netdev = mctp_usb->netdev;
 	unsigned long flags;
+	bool wake = false;
 
 	mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
 
 	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
+	mctp_usb->tx_qmem -= urb->transfer_buffer_length;
+	if (mctp_usb->tx_qmem < TX_QMEM_MAX)
+		wake = true;
 	usb_unanchor_urb(urb);
 	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
 
 	usb_free_urb(urb);
 
-	netif_wake_queue(netdev);
+	if (wake)
+		netif_wake_queue(netdev);
 }
 
 static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
@@ -85,18 +104,19 @@ static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
 	if (mctp_usb->span)
 		urb->transfer_flags |= URB_ZERO_PACKET;
 
-	netif_stop_queue(mctp_usb->netdev);
-
 	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
-	if (!rc)
+	if (!rc) {
 		usb_anchor_urb(urb, &mctp_usb->tx_anchor);
+		mctp_usb->tx_qmem += len;
+		if (mctp_usb->tx_qmem >= TX_QMEM_MAX)
+			netif_stop_queue(mctp_usb->netdev);
+	}
 	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
 
 	if (rc) {
 		netdev_dbg(mctp_usb->netdev, "TX urb submit failed, %d\n", rc);
 		usb_free_urb(urb);
-		netif_start_queue(mctp_usb->netdev);
 	}
 
 	return rc;
@@ -221,12 +241,15 @@ static int mctp_usb_stop(struct net_device *dev)
 	flush_delayed_work(&mctp_usb->rx_retry_work);
 
 	usb_kill_urb(mctp_usb->rx_urb);
-	/* we have stopped queues, the anchor's own lock will serialise
-	 * access from the urb completion.
+
+	/* We have stopped queues, the anchor's own lock will serialise
+	 * access from the urb completion. We are then guaranteed that no
+	 * further completions can occur, so can clear tx_qmem without locking.
 	 */
 	usb_kill_anchored_urbs(&mctp_usb->tx_anchor);
 
 	mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
+	mctp_usb->tx_qmem = 0;
 
 	return 0;
 }

-- 
2.47.3


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib
  2026-06-30  3:21 ` [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib Jeremy Kerr
@ 2026-07-02 10:09   ` Paolo Abeni
  0 siblings, 0 replies; 17+ messages in thread
From: Paolo Abeni @ 2026-07-02 10:09 UTC (permalink / raw)
  To: jk
  Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
	netdev, linux-usb

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net: mctp: usblib: Move RX transfer processing to a new mctp-usblib

This commit extracts common USB receive transfer processing for MCTP into a
new library file (mctp-usblib.c). This is intended to support both the
current host-side driver and a future gadget driver.

> diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
> new file mode 100644
> index 0000000000000..9b86eb4310ce3
> --- /dev/null
> +++ b/drivers/net/mctp/mctp-usblib.c
[ ... ]
> +/*
> + * Prepare a transfer buffer for future completion; *bufp and *lenp will
> + * be populated on success.
> + */
> +int mctp_usblib_rx_prepare(struct net_device *netdev,
> +			   struct mctp_usblib_rx *rx,
> +			   void **bufp, size_t *lenp, gfp_t gfp)
> +{
> +	const unsigned int len = MCTP_USB_1_0_XFER_SIZE;
> +	struct sk_buff *skb;
> +
> +	skb = __netdev_alloc_skb(netdev, len, gfp);
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	rx->skb = skb;
> +
> +	*bufp = skb_tail_pointer(skb);
> +	*lenp = skb_tailroom(skb);

[Severity: High]
Does assigning the full skb_tailroom() here create a risk of a USB host
controller hang?

Because of slab alignment and padding, skb_tailroom() often exceeds the
exact requested allocation length (e.g., returning 640 bytes instead of
512). If the MCTP device sends exactly a full USB High-Speed packet
(512 bytes) and does not append a Zero Length Packet (ZLP), the USB
host controller might not terminate the transfer since 512 is a multiple
of wMaxPacketSize and the requested 640 bytes haven't been met.

Could this instead use the exact protocol max transfer size (e.g., passing
the len variable instead of skb_tailroom(skb)) to ensure the controller
completes the transfer correctly?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(mctp_usblib_rx_prepare);
-- 
This is an AI-generated review.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight
  2026-06-30  3:21 ` [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight Jeremy Kerr
@ 2026-07-02 10:09   ` Paolo Abeni
  0 siblings, 0 replies; 17+ messages in thread
From: Paolo Abeni @ 2026-07-02 10:09 UTC (permalink / raw)
  To: jk
  Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
	netdev, linux-usb

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net: mctp: usb: Allow multiple urbs in flight

Currently, we stop tx queues when we have one urb submitted. This means
we will immediately hit dev_hard_start_xmit's tx-queues-off ->
NETDEV_TX_BUSY case, and revert to the requeue -> gso_skb single-dequeue
path, and no longer be able to pack skbs without an xmit_more
indication.

Instead, allow a few urbs to be in-flight, with a limit of 16kB of data
outstanding (after which we will disable queues). With this, the tx path
will cause fewer requeues (and therefore non-packed transfers) under
normal loads.

> diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
> index 5739c87da1095..28aeb5b258723 100644
> --- a/drivers/net/mctp/mctp-usb.c
> +++ b/drivers/net/mctp/mctp-usb.c
[ ... ]
> @@ -48,22 +51,38 @@ enum {
>  	MCTP_USB_SUBCLASS_SPAN = 0x02,
>  };
>  
> +/* We use a total-size limit for outstanding URBs, as the transfer counts
> + * may vary a lot between spanning- and non-spanning modes. In spanning mode,
> + * this will allow for a couple of max-sized transfers to be in flight. In
> + * non-spanning mode, 32.
> + *
> + * We want to avoid disabling the tx queue if possible; doing so will end up
> + * requeueing to gso_skb, and we only dequeue from that one skb at a time,
> + * so can no longer perform transfer packing.
> + */
> +static const unsigned int TX_QMEM_MAX = 16384;
> +
>  static void mctp_usb_out_complete(struct urb *urb)
>  {
>  	struct mctp_usblib_tx_ctx *tx_ctx = urb->context;
>  	struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
>  	struct net_device *netdev = mctp_usb->netdev;
>  	unsigned long flags;
> +	bool wake = false;
>  
>  	mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
>  
>  	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
> +	mctp_usb->tx_qmem -= urb->transfer_buffer_length;

[Severity: High]
Does this code underflow tx_qmem if called concurrently with or after
mctp_usb_stop() zeroes it?

If a user downs the interface while TX URBs are in flight, the USB core
(__usb_hcd_giveback_urb) unanchors the URB before invoking this completion
handler.

Because the URB is unanchored, usb_kill_anchored_urbs() in mctp_usb_stop()
can see an empty anchor list and return immediately, without waiting for
the completion handler.

> +	if (mctp_usb->tx_qmem < TX_QMEM_MAX)
> +		wake = true;
>  	usb_unanchor_urb(urb);
>  	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
>  
>  	usb_free_urb(urb);
>  
> -	netif_wake_queue(netdev);
> +	if (wake)
> +		netif_wake_queue(netdev);
>  }
[ ... ]
> @@ -221,12 +241,15 @@ static int mctp_usb_stop(struct net_device *dev)
>  	flush_delayed_work(&mctp_usb->rx_retry_work);
>  
>  	usb_kill_urb(mctp_usb->rx_urb);
> -	/* we have stopped queues, the anchor's own lock will serialise
> -	 * access from the urb completion.
> +
> +	/* We have stopped queues, the anchor's own lock will serialise
> +	 * access from the urb completion. We are then guaranteed that no
> +	 * further completions can occur, so can clear tx_qmem without locking.
>  	 */
>  	usb_kill_anchored_urbs(&mctp_usb->tx_anchor);
>  
>  	mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
> +	mctp_usb->tx_qmem = 0;

[Severity: High]
Is the assumption in the comment correct?

Since __usb_hcd_giveback_urb unanchors the URB before calling the completion
handler, usb_kill_anchored_urbs() doesn't guarantee that all completion
handlers have finished.

If tx_qmem is zeroed here before the completion handler finishes, the
subsequent subtraction in mctp_usb_out_complete() will underflow tx_qmem to a
huge unsigned value, causing the TX queue to be permanently stalled on the
next device open.

>  
>  	return 0;
>  }
-- 
This is an AI-generated review.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib
  2026-06-30  3:21 ` [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib Jeremy Kerr
@ 2026-07-02 10:10   ` Paolo Abeni
  0 siblings, 0 replies; 17+ messages in thread
From: Paolo Abeni @ 2026-07-02 10:10 UTC (permalink / raw)
  To: jk
  Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
	netdev, linux-usb

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: mctp: usblib: Move TX transfer processing to mctp-usblib

This change moves the TX path from mctp-usb into the mctp-usblib helper
by splitting it into push, send, and complete steps so future packed
transfers can carry multiple MCTP packets per USB transfer. The current
implementation keeps a 1:1 mapping between packets and transfers, with no
tx context persisting beyond a single send.

> diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
> index 531b7c994afb..385350792dd4 100644
> --- a/drivers/net/mctp/mctp-usb.c
> +++ b/drivers/net/mctp/mctp-usb.c
> @@ -29,90 +29,82 @@ struct mctp_usb {

[ ... ]

> @@ -220,7 +212,12 @@ static int mctp_usb_stop(struct net_device *dev)
>  	flush_delayed_work(&mctp_usb->rx_retry_work);
>  
>  	usb_kill_urb(mctp_usb->rx_urb);
> +
> +	spin_lock_irqsave(&mctp_usb->tx_lock, flags);
>  	usb_kill_urb(mctp_usb->tx_urb);
> +	spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);

[Critical]
Can usb_kill_urb() safely be called from under spin_lock_irqsave()?

usb_kill_urb() starts with might_sleep() and then blocks on
wait_event(usb_kill_urb_queue, urb->use_count == 0):

drivers/usb/core/urb.c:usb_kill_urb() {
    might_sleep();
    ...
    wait_event(usb_kill_urb_queue, atomic_read(&urb->use_count) == 0);
    ...
}

With CONFIG_DEBUG_ATOMIC_SLEEP, would this trigger a
"sleeping function called from invalid context" splat because tx_lock
is held with IRQs disabled?

There also appears to be a nested-lock concern with the URB completion
handler taking the same tx_lock:

mctp_usb_out_complete() {
    ...
    spin_lock_irqsave(&mctp_usb->tx_lock, flags);
    mctp_usb->tx_urb = NULL;
    spin_unlock_irqrestore(&mctp_usb->tx_lock, flags);
    ...
}

The HCD giveback path needs to run the completion before urb->use_count
drops to zero. If the completion is waiting for tx_lock held by the
caller of usb_kill_urb(), and usb_kill_urb() is itself waiting for
use_count to drop, can this deadlock?

The next patch in the series ("net: mctp: usb: Allow for multiple urb
submissions from a packet tx", 5cf1f88ce49a) appears to remove this
lock-wrapped usb_kill_urb() in favour of usb_kill_anchored_urbs()
called outside the lock — should the fix be folded into this patch
instead?

> +
> +	mctp_usblib_tx_cancel(&mctp_usb->tx, dev);
>  
>  	return 0;
>  }

[ ... ]

> diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
> index 9b86eb4310ce..56eca496bbe4 100644
> --- a/drivers/net/mctp/mctp-usblib.c
> +++ b/drivers/net/mctp/mctp-usblib.c
> @@ -162,6 +162,186 @@ void mctp_usblib_rx_cancel(struct mctp_usblib_rx *rx)

[ ... ]

> +/*
> + * Completion for the ->send() op. This will update netdev stats and
> + * free the tx context.
> + *
> + * Likely called from (atomic) URB completion context.
> + */
> +void mctp_usblib_tx_send_complete(struct mctp_usblib_tx_ctx *tx_ctx,
> +				  struct net_device *dev, bool ok)
> +{
> +	mctp_usblib_tx_ctx_free(tx_ctx);
> +}
> +EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);

[High]
Does the body of mctp_usblib_tx_send_complete() match the comment
above it?

The comment says "This will update netdev stats and free the tx
context", and the function takes dev and ok parameters, but the body
only calls mctp_usblib_tx_ctx_free(tx_ctx) and never invokes
mctp_usblib_tx_stats_update().

Prior to this patch, mctp_usb_out_complete() updated counters for both
success and failure:

    case 0:
        dev_dstats_tx_add(netdev, skb->len);
        ...
    case -ENOENT:
    case -ECONNRESET:
    case -ESHUTDOWN:
    case -EPROTO:
        dev_dstats_tx_dropped(netdev);
        ...

After this patch, mctp_usb_out_complete() defers stats to
mctp_usblib_tx_send_complete(), which does not touch them. The only
caller of mctp_usblib_tx_stats_update() is the synchronous push()
failure path. Does this mean tx_packets, tx_bytes and the URB-failure
tx_drops counters are no longer incremented for any successful or
asynchronously-failed transmit?

Was the intent to call mctp_usblib_tx_stats_update(tx_ctx, dev, ok)
here before freeing the context?

> +
> +/* Prepare a skb for push() */
> +static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb)

[ ... ]
-- 
This is an AI-generated review.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests
  2026-06-30  3:21 ` [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests Jeremy Kerr
@ 2026-07-02 10:10   ` Paolo Abeni
  0 siblings, 0 replies; 17+ messages in thread
From: Paolo Abeni @ 2026-07-02 10:10 UTC (permalink / raw)
  To: jk
  Cc: matt, andrew+netdev, davem, edumazet, kuba, pabeni, gregkh,
	netdev, linux-usb

From: AI Reviewer <ai@example.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: mctp: usblib: Add initial kunit tests

This commit introduces an initial set of kunit tests covering the
mctp-usblib receive path, exercising packet extraction from incoming
USB transfer data.

> diff --git a/drivers/net/mctp/mctp-usblib-test.c b/drivers/net/mctp/mctp-usblib-test.c
> new file mode 100644
> index 000000000000..ac837cbb4436
> --- /dev/null
> +++ b/drivers/net/mctp/mctp-usblib-test.c
> @@ -0,0 +1,330 @@

[ ... ]

> +static struct mctp_usblib_test_dev *mctp_usblib_test_create_dev(void)
> +{
> +	struct mctp_usblib_test_dev *dev;
> +	struct net_device *ndev;
> +	int rc;
> +
> +	ndev = alloc_netdev(sizeof(*dev), "mctptest%d", NET_NAME_ENUM,
> +			    mctp_usblib_dev_setup);
> +	if (!ndev)
> +		return NULL;
> +
> +	dev = netdev_priv(ndev);
> +	dev->ndev = ndev;
> +	skb_queue_head_init(&dev->rx_pkts);
> +
> +	rc = register_netdev(ndev);
> +	if (rc) {
> +		free_netdev(ndev);
> +		return NULL;
> +	}
> +
> +	rcu_read_lock();
> +	dev->mdev = __mctp_dev_get(ndev);
> +	dev->mdev->net = mctp_default_net(dev_net(ndev));
> +	rcu_read_unlock();

[Low]
Can __mctp_dev_get() return NULL here?  Looking at net/mctp/device.c:

    struct mctp_dev *__mctp_dev_get(const struct net_device *dev)
    {
        struct mctp_dev *mdev = rcu_dereference(dev->mctp_ptr);

        if (mdev)
            if (!refcount_inc_not_zero(&mdev->refs))
                return NULL;
        return mdev;
    }

If mctp_add_dev() fails inside the NETDEV_REGISTER notifier path, mctp_ptr
will not be installed and __mctp_dev_get() returns NULL, making the
subsequent dev->mdev->net = ... a NULL dereference in the test kthread.

> +
> +	rtnl_lock();
> +	dev_open(ndev, NULL);
> +	rtnl_unlock();

[Low]
Is it intentional to ignore the return value of dev_open() here?  If
dev_open() fails the netdev is registered but never brought up, and the
test then proceeds against an inconsistently-initialized device.

> +
> +	return dev;
> +}
> +
> +static int mctp_usblib_test_dst_output(struct mctp_dst *dst,
> +				       struct sk_buff *skb)
> +{
> +	struct mctp_usblib_test_dev *dev = netdev_priv(skb->dev);
> +
> +	skb_queue_tail(&dev->rx_pkts, skb);
> +
> +	return 0;
> +}
> +
> +static void mctp_usblib_test_init(struct kunit *test,
> +				  struct mctp_usblib_test_ctx *ctx)
> +{
> +	struct mctp_route *rt = &ctx->rt;
> +
> +	ctx->dev = mctp_usblib_test_create_dev();
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev);
> +
> +	memset(rt, 0, sizeof(*rt));
> +	rt->min = local_eid;
> +	rt->max = local_eid;
> +	rt->dst_type = MCTP_ROUTE_DIRECT;
> +	rt->type = RTN_LOCAL;
> +	rt->dev = ctx->dev->mdev;
> +	rt->output = mctp_usblib_test_dst_output;
> +
> +	list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);
> +}

[High]
What happens to ctx.rt on a KUNIT_ASSERT_* failure later in the test?

The ctx variable is a stack local in mctp_usblib_test_rx_single() and
mctp_usblib_test_rx_pkt_span(), and mctp_usblib_test_init() inserts
&ctx->rt.list into the global RCU-protected list init_net.mctp.routes:

    list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);

KUNIT_ASSERT_* macros invoke __kunit_abort() which calls
kunit_try_catch_throw() and ultimately kthread_exit(0), so control never
returns to the test function:

    lib/kunit/test.c: __kunit_abort()
      -> kunit_try_catch_throw()
        -> kthread_exit(0)

When the test kthread exits, its stack is gone, leaving a list_head in
init_net.mctp.routes that points at freed/reused stack memory.

The next RCU traversal of init_net.mctp.routes (subsequent mctp test,
mctp routing code, another kunit run) would then dereference that
dangling pointer.

The same abort path also bypasses unregister_netdev() and the
mctp_dev_put() done by mctp_usblib_test_fini().

Would using kunit_add_action() / kunit_add_action_or_reset() (or suite
.init/.exit hooks together with kunit_kmalloc()) ensure list_del_rcu(),
synchronize_rcu() and the netdev teardown always run, regardless of
assertion outcome?

> +
[ ... ]

> +static void mctp_usblib_test_rx_single(struct kunit *test)
> +{
> +	struct mctp_usblib_test_dev *dev;
> +	struct mctp_usblib_test_ctx ctx;
> +	struct mctp_usblib_rx rx;
> +	struct sk_buff *skb;
> +	size_t len;
> +	void *buf;
> +	int rc;
> +
> +	mctp_usblib_test_init(test, &ctx);
> +	dev = ctx.dev;
> +
> +	mctp_usblib_rx_init(&rx, true);
> +
> +	rc = mctp_usblib_rx_prepare(dev->ndev, &rx, &buf, &len, GFP_KERNEL);
> +	KUNIT_ASSERT_EQ(test, rc, 0);
> +
> +	mctp_usblib_test_init_pkt(buf, 8, 8);
> +
> +	rc = mctp_usblib_rx_complete(dev->ndev, &rx, 8);
> +	KUNIT_ASSERT_EQ(test, rc, 0);
> +
> +	skb = __skb_dequeue(&dev->rx_pkts);
> +	KUNIT_EXPECT_NOT_ERR_OR_NULL(test, skb);
> +	KUNIT_EXPECT_EQ(test, skb->len, 4);
> +	kfree_skb(skb);
> +
> +	mctp_usblib_rx_fini(&rx);
> +	mctp_usblib_test_fini(test, &ctx);
> +}
> +
[ ... ]

> +static void mctp_usblib_test_rx_pkt_span(struct kunit *test)
> +{
> +	const struct mctp_usblib_test_pkt_span *pkt_span = test->param_value;
> +	size_t len, xfer_len, off, xfer_off;
> +	struct mctp_usblib_test_dev *dev;
> +	struct mctp_usblib_test_ctx ctx;
> +	struct mctp_usblib_rx rx;
> +	unsigned int i;
> +	u8 *pktbuf;
> +	void *buf;
> +	int rc;
> +
> +	mctp_usblib_test_pkt_span_validate(test, pkt_span, &len);
> +	pktbuf = kmalloc_array(1, len, GFP_KERNEL);
> +
> +	/* lay out packets */
> +	for (off = 0, i = 0; i < pkt_span->n_pkts; i++) {
> +		len = pkt_span->pkts[i];
> +		mctp_usblib_test_init_pkt(pktbuf + off, len, len);
> +		off += len;
> +	}

[Low]
Is the kmalloc_array() return value missing a NULL check?  pktbuf is
dereferenced immediately via mctp_usblib_test_init_pkt(pktbuf + off, ...),
so an allocation failure (including under kunit fault injection) becomes
a NULL deref.  Would kunit_kmalloc_array() or an explicit
KUNIT_ASSERT_NOT_NULL(test, pktbuf) be more appropriate here?

> +
[ ... ]

> +	/* check received packets */
> +	KUNIT_EXPECT_EQ(test, dev->rx_pkts.qlen, pkt_span->n_pkts);
> +	for (i = 0; ; i++) {
> +		struct sk_buff *skb = __skb_dequeue(&dev->rx_pkts);
> +
> +		if (!skb)
> +			break;
> +
> +		if (i <= pkt_span->n_pkts)
> +			KUNIT_EXPECT_EQ(test, skb->len, pkt_span->pkts[i] - 4);
> +
> +		kfree_skb(skb);
> +	}

[Low]
Should this comparison be i < pkt_span->n_pkts rather than
i <= pkt_span->n_pkts?

With <=, when i == n_pkts the test reads pkts[n_pkts], which is a
zero-initialized tail slot of pkts[6]; the expected length then
underflows to (size_t)0 - 4 and produces a confusing diagnostic instead
of a clean failure if the queue ever holds more SKBs than expected.

> +
[ ... ]
-- 
This is an AI-generated review.


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-07-02 10:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-30  3:21 [PATCH net-next 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1 Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 01/12] net: mctp: usb: Include version indicator in max packet size defines Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 02/12] net: mctp: usb: Use packet-length max for maximum packet-size check Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib Jeremy Kerr
2026-07-02 10:09   ` Paolo Abeni
2026-06-30  3:21 ` [PATCH net-next 04/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib Jeremy Kerr
2026-07-02 10:10   ` Paolo Abeni
2026-06-30  3:21 ` [PATCH net-next 05/12] net: mctp: usb: Allow for multiple urb submissions from a packet tx Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 06/12] net: mctp: usblib: Add support for multi-packet transmit Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 07/12] net: mctp: usb: Accommodate DSP0283 v1.1 header format Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 08/12] net: mctp: usblib: Implement receive-side packet spanning Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 09/12] net: mctp: usblib: Implement transmit-side " Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 10/12] net: mctp: usblib: Add initial kunit tests Jeremy Kerr
2026-07-02 10:10   ` Paolo Abeni
2026-06-30  3:21 ` [PATCH net-next 11/12] net: mctp: usb: enable v1.1 packet spanning Jeremy Kerr
2026-06-30  3:21 ` [PATCH net-next 12/12] net: mctp: usb: Allow multiple urbs in flight Jeremy Kerr
2026-07-02 10:09   ` Paolo Abeni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox