[PATCH net-next v3 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1

[PATCH net-next v3 00/12] net: mctp: usb: Add support for MCTP-over-USB v1.1

[Jeremy Kerr]

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.

To: Matt Johnston <matt@codeconstruct.com.au>
To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: netdev@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

Changes in v3:
- split rx urb status handling to a separate change, handle stalls
- merge tx anchor usage into initial usblib implementation
- add skb drop reasons
- prevent unnecessary skb reallocation
- Link to v2: https://patch.msgid.link/20260703-dev-mctp-usb-1-1-v2-0-60367b861b33@codeconstruct.com.au

Changes in v2:
- address sashiko reviews:
  - rx mac_header correction
  - rx/tx stats fixes
  - limit non-spanned packets (where we have no ZLP) to max size
  - handle transient rx urb error status
  - set skb_drop_reasons
  - kunit early-exit cleanups, take rntl lock where needed
- rework tx_qmem locking and anchor handling (we don't need to unanchor
  on completion)
- Link to v1: https://patch.msgid.link/20260630-dev-mctp-usb-1-1-v1-0-86a311fc67b7@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: usb: Improve IN endpoint status handling
      net: mctp: usblib: Move TX transfer processing to mctp-usblib
      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         | 315 +++++++++---------
 drivers/net/mctp/mctp-usblib-test.c | 410 ++++++++++++++++++++++++
 drivers/net/mctp/mctp-usblib.c      | 616 ++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h        |  88 +++++-
 6 files changed, 1297 insertions(+), 149 deletions(-)
---
base-commit: b85966adbf5de0668a815c6e3527f87e0c387fb4
change-id: 20260604-dev-mctp-usb-1-1-6fd854ad13e8

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

[PATCH net-next v3 01/12] net: mctp: usb: Include version indicator in max packet size defines

[Jeremy Kerr]

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

[PATCH net-next v3 02/12] net: mctp: usb: Use packet-length max for maximum packet-size check

[Jeremy Kerr]

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

[PATCH net-next v3 03/12] net: mctp: usblib: Move RX transfer processing to a new mctp-usblib

[Jeremy Kerr]

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>

---
v3:
 - split improved urb status handling to a separate change
 - add module.h include
 - account for rx drops due to failed skb_clone()
v2:
 - drop unneeded skb_reset_mac_header
 - don't count transport header in rx bytes stats
 - disallow >512 bytes on RX URBs; the non-spanning protocol does not
   specify ZLPs, so we cannot allow an over-length transfer
 - add requeue on transient urb errors
---
 drivers/net/mctp/Kconfig       |  11 +++
 drivers/net/mctp/Makefile      |   1 +
 drivers/net/mctp/mctp-usb.c    | 102 +++++------------------
 drivers/net/mctp/mctp-usblib.c | 179 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h   |  26 ++++++
 5 files changed, 238 insertions(+), 81 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..9e5c64e76e4a 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,93 +160,28 @@ 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:
+		mctp_usblib_rx_complete(netdev, &mctp_usb->rx,
+					urb->actual_length);
 		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_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..4140998c30fd
--- /dev/null
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -0,0 +1,179 @@
+// 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/module.h>
+#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 = len;
+
+	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);
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+
+	skb->protocol = htons(ETH_P_MCTP);
+	skb_reset_network_header(skb);
+	cb = __mctp_cb(skb);
+	cb->halen = 0;
+	netif_rx(skb);
+}
+
+static void mctp_usblib_rx_stats_single_drop(struct net_device *dev)
+{
+	struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats);
+	unsigned long flags;
+
+	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+	u64_stats_inc(&dstats->rx_drops);
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+}
+
+/*
+ * 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;
+				}
+			} else {
+				mctp_usblib_rx_stats_single_drop(netdev);
+			}
+			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

[PATCH net-next v3 04/12] net: mctp: usb: Improve IN endpoint status handling

[Jeremy Kerr]

Currently, we give-up on all non-zero status values on our IN/rx urb,
and do not re-queue the urb. This will stall the driver, and prevent
any further receive.

Instead, attempt a re-queue on transient errors, with a max of ten
successive failures. Handle EPIPE specially, by scheduling a
usb_clear_halt() in non-atomic context.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
v3:
 - improved IN urb status handling was introduced in v2, but now split
   to a separate change, via sashiko feedback
 - now with usb_clear_halt on EPIPE.
---
 drivers/net/mctp/mctp-usb.c | 70 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 9e5c64e76e4a..82de4d5967db 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -32,6 +32,9 @@ struct mctp_usb {
 
 	struct urb *tx_urb;
 	struct urb *rx_urb;
+	int in_err_count;
+	int in_err_orig;
+	bool clear_halt;
 
 	/* enforces atomic access to rx_stopped and requeuing the retry work */
 	spinlock_t rx_lock;
@@ -158,27 +161,66 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
 	return 0;
 }
 
+static const unsigned int rx_err_max = 10;
+
 static void mctp_usb_in_complete(struct urb *urb)
 {
 	struct mctp_usb *mctp_usb = urb->context;
 	struct net_device *netdev = mctp_usb->netdev;
+	unsigned long flags;
 	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:
+		/* device shutdown, don't resubmit */
 		mctp_usblib_rx_cancel(&mctp_usb->rx);
 		return;
+
+	case -EPIPE:
+		/* endpoint stall: clear halt, which will cause a resubmit */
+
+		if (!mctp_usb->in_err_count++)
+			mctp_usb->in_err_orig = status;
+		if (mctp_usb->in_err_count >= rx_err_max) {
+			netdev_err(netdev, "excessive stalls from IN EP\n");
+			return;
+		}
+
+		mctp_usb->clear_halt = true;
+		spin_lock_irqsave(&mctp_usb->rx_lock, flags);
+		if (!mctp_usb->rx_stopped)
+			schedule_delayed_work(&mctp_usb->rx_retry_work,
+					      RX_RETRY_DELAY);
+		spin_unlock_irqrestore(&mctp_usb->rx_lock, flags);
+		mctp_usblib_rx_cancel(&mctp_usb->rx);
+		return;
+
+	default:
+		netdev_dbg(netdev, "unexpected rx urb status: %d\n", status);
+		fallthrough;
+	case -ETIME:
+	case -EPROTO:
+	case -EILSEQ:
+	case -EOVERFLOW:
+		/* possibly transient; record first failure, resubmit */
+		mctp_usblib_rx_cancel(&mctp_usb->rx);
+		if (!mctp_usb->in_err_count++)
+			mctp_usb->in_err_orig = status;
+		if (mctp_usb->in_err_count >= rx_err_max) {
+			netdev_err(netdev,
+				   "excessive errors from IN EP, first: %d\n",
+				   mctp_usb->in_err_orig);
+			return;
+		}
+		break;
+
 	case 0:
-		mctp_usblib_rx_complete(netdev, &mctp_usb->rx,
-					urb->actual_length);
+		mctp_usblib_rx_complete(netdev, &mctp_usb->rx, urb->actual_length);
+		mctp_usb->in_err_count = 0;
 		break;
 	}
 
@@ -189,6 +231,20 @@ static void mctp_usb_rx_retry_work(struct work_struct *work)
 {
 	struct mctp_usb *mctp_usb = container_of(work, struct mctp_usb,
 						 rx_retry_work.work);
+	int rc;
+
+	/* We are only called when rx completions are suspended */
+	if (mctp_usb->clear_halt) {
+		int pipe = usb_rcvbulkpipe(mctp_usb->usbdev, mctp_usb->ep_in);
+
+		rc = usb_clear_halt(mctp_usb->usbdev, pipe);
+		if (rc) {
+			netdev_err(mctp_usb->netdev,
+				   "can't clear IN EP halt: %d\n", rc);
+			return;
+		}
+		mctp_usb->clear_halt = false;
+	}
 
 	mctp_usb_rx_queue(mctp_usb, GFP_KERNEL);
 }
@@ -198,6 +254,8 @@ static int mctp_usb_open(struct net_device *dev)
 	struct mctp_usb *mctp_usb = netdev_priv(dev);
 
 	WRITE_ONCE(mctp_usb->rx_stopped, false);
+	mctp_usb->clear_halt = false;
+	mctp_usb->in_err_count = 0;
 
 	netif_start_queue(dev);
 

-- 
2.47.3

[PATCH net-next v3 05/12] net: mctp: usblib: Move TX transfer processing to mctp-usblib

[Jeremy Kerr]

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.
However, we use an anchor in the host driver implementation to track the
submitted TX urb when necessary.

While we're here, fix an inconsistency between tx and rx stats: both
should not include the transport header.

Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

---
v3:
  - consolidate with tx_anchor introduction in the host-side driver;
    the single-urb approach (with racy free) was being introduced then
    immediately removed. Incorporating the anchor-based approach
    resolves this.
v2:
  - make tx stats consistent with rx stats
  - reinstate missing tx_stats_update in tx_send_complete
  - don't usb_kill_urb() with the tx lock held
  - implement skb_drop_reasons
  - [squashed] adjust tx_anchor handling; the urb is unanchored before
    completion, so we don't need to unanchor explicitly. We can now rely
    on the anchor's own lock for serialisation
---
 drivers/net/mctp/mctp-usb.c    | 115 +++++++++++------------
 drivers/net/mctp/mctp-usblib.c | 203 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/mctp-usb.h   |  39 ++++++++
 3 files changed, 293 insertions(+), 64 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 82de4d5967db..ef58703040d6 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -29,8 +29,6 @@ struct mctp_usb {
 	u8 ep_out;
 
 	struct mctp_usblib_rx rx;
-
-	struct urb *tx_urb;
 	struct urb *rx_urb;
 	int in_err_count;
 	int in_err_orig;
@@ -40,82 +38,66 @@ struct mctp_usb {
 	spinlock_t rx_lock;
 	bool rx_stopped;
 	struct delayed_work rx_retry_work;
+
+	struct mctp_usblib_tx tx;
+	struct usb_anchor tx_anchor;
 };
 
 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;
 
-	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);
-	}
+	usb_free_urb(urb);
 
-	kfree_skb(skb);
+	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);
 	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);
+
+	netif_stop_queue(mctp_usb->netdev);
+
+	usb_anchor_urb(urb, &mctp_usb->tx_anchor);
 
-	/* Stops TX queue first to prevent race condition with URB complete */
-	netif_stop_queue(dev);
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
 	if (rc) {
-		netif_wake_queue(dev);
-		goto err_drop;
+		netdev_dbg(mctp_usb->netdev, "TX urb submit failed, %d\n", rc);
+		usb_unanchor_urb(urb);
+		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;
 }
 
@@ -278,7 +260,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);
-	usb_kill_urb(mctp_usb->tx_urb);
+
+	usb_kill_anchored_urbs(&mctp_usb->tx_anchor);
+
+	mctp_usblib_tx_cancel(&mctp_usb->tx, dev, SKB_DROP_REASON_DEV_READY);
 
 	return 0;
 }
@@ -336,28 +321,30 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, dev);
 
 	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;
 
-	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;
@@ -369,7 +356,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 4140998c30fd..3f4295f3145c 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -174,6 +174,209 @@ 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,
+				    enum skb_drop_reason reason)
+{
+	if (ctx)
+		dev_kfree_skb_any_reason(ctx->skb, reason);
+	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) {
+		/* Only include the network-layer data in tx stats; we know
+		 * that there is a 4-byte header pushed to all skbs in
+		 * tx_skb_prepare()
+		 */
+		s64 len = ctx->len - sizeof(struct mctp_usb_hdr);
+
+		u64_stats_inc(&dstats->tx_packets);
+		u64_stats_add(&dstats->tx_bytes, 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)
+{
+	enum skb_drop_reason reason =
+		ok ? SKB_CONSUMED : SKB_DROP_REASON_NOT_SPECIFIED;
+
+	mctp_usblib_tx_stats_update(tx_ctx, dev, ok);
+	mctp_usblib_tx_ctx_free(tx_ctx, reason);
+}
+EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);
+
+/* Prepare a skb for push()
+ *
+ * On error, populates @reason.
+ */
+static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb,
+				      enum skb_drop_reason *reason)
+{
+	struct mctp_usb_hdr *hdr;
+	unsigned long plen;
+	int rc;
+
+	plen = skb->len;
+	if (plen + sizeof(*hdr) > MCTP_USB_1_0_PKTLEN_MAX) {
+		*reason = SKB_DROP_REASON_PKT_TOO_BIG;
+		return -EMSGSIZE;
+	}
+
+	rc = skb_cow_head(skb, sizeof(*hdr));
+	if (rc) {
+		*reason = SKB_DROP_REASON_NOMEM;
+		return rc;
+	}
+
+	hdr = skb_push(skb, sizeof(*hdr));
+	if (!hdr) {
+		*reason = SKB_DROP_REASON_NOMEM;
+		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;
+	enum skb_drop_reason reason;
+	int rc;
+
+	if (!skb)
+		return 0;
+
+	rc = mctp_usblib_tx_skb_prepare(skb, &reason);
+	if (rc)
+		goto err_drop_single;
+
+	ctx = mctp_usblib_tx_ctx_create(tx, skb);
+	if (!ctx) {
+		rc = -ENOMEM;
+		reason = SKB_DROP_REASON_NOMEM;
+		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, SKB_DROP_REASON_NOT_SPECIFIED);
+	}
+
+	return rc;
+
+err_drop_single:
+	mctp_usblib_tx_stats_single_drop(dev);
+	kfree_skb_reason(skb, reason);
+	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,
+			   enum skb_drop_reason reason)
+{
+	/* 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..76f9d8879254 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -55,4 +55,43 @@ 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,
+			   enum skb_drop_reason reason);
+
 #endif /*  __LINUX_USB_MCTP_USB_H */

-- 
2.47.3

[PATCH net-next v3 06/12] net: mctp: usblib: Add support for multi-packet transmit

[Jeremy Kerr]

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>
---
v3:
 - ensure we are setting drop reasons correctly on _prepare() failure
v2:
 - expand on tx_should_send logic, rather than the hardcoded 68 value.
 - don't increment ctx->len (from zero) on ctx_create(), set explicitly
 - add skb_drop_reasons
---
 drivers/net/mctp/mctp-usblib.c | 252 +++++++++++++++++++++++++++++++++--------
 include/linux/usb/mctp-usb.h   |   8 +-
 2 files changed, 210 insertions(+), 50 deletions(-)

diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index 3f4295f3145c..2e464254353e 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -177,11 +177,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,
@@ -191,53 +193,129 @@ 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;
+	/* Use the baseline length (ie, BTU) as an approximate
+	 * "reasonably-sized" packet we could expect. If there is
+	 * insufficient capacity for that, then send.
+	 */
+	const size_t pkt_len = MCTP_USB_BTU + sizeof(struct mctp_usb_hdr);
+
+	return mctp_usblib_tx_avail(ctx) < pkt_len;
 }
-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;
+		}
 
-	return tx->ops.send(ctx, buf, ctx->len);
+		buf = ctx->buf;
+	} else {
+		return -EINVAL;
+	}
+
+	return ctx->tx->ops.send(ctx, buf, ctx->len);
 }
 
 static void mctp_usblib_tx_ctx_free(struct mctp_usblib_tx_ctx *ctx,
 				    enum skb_drop_reason reason)
 {
-	if (ctx)
-		dev_kfree_skb_any_reason(ctx->skb, reason);
+	struct sk_buff *skb;
+
+	if (!ctx)
+		return;
+
+	while ((skb = __skb_dequeue(&ctx->skbs)) != NULL)
+		dev_kfree_skb_any_reason(skb, reason);
 	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)
+{
+	mctp_usblib_tx_ctx_free(tx->cur_ctx, SKB_DROP_REASON_NOT_SPECIFIED);
+}
+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)
@@ -251,12 +329,13 @@ static void mctp_usblib_tx_stats_update(struct mctp_usblib_tx_ctx *ctx,
 		 * that there is a 4-byte header pushed to all skbs in
 		 * tx_skb_prepare()
 		 */
-		s64 len = ctx->len - sizeof(struct mctp_usb_hdr);
+		u64 n = ctx->skbs.qlen;
+		s64 len = ctx->len - (n * sizeof(struct mctp_usb_hdr));
 
-		u64_stats_inc(&dstats->tx_packets);
+		u64_stats_add(&dstats->tx_packets, n);
 		u64_stats_add(&dstats->tx_bytes, 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);
@@ -327,8 +406,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.
  */
@@ -336,36 +415,106 @@ 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;
+	struct mctp_usblib_tx_ctx *ctx, *send_ctx = NULL;
 	enum skb_drop_reason reason;
-	int rc;
+	const int max_tries = 3;
+	unsigned long flags;
+	int try = 1, rc;
+
+	rc = mctp_usblib_tx_skb_prepare(skb, &reason);
+	if (rc) {
+		mctp_usblib_tx_stats_single_drop(dev);
+		kfree_skb_reason(skb, reason);
+		/* we may still need to proceed, in case an existing ctx
+		 * is now sendable (ie.: !more).
+		 */
+		skb = NULL;
+	}
+
+	reason = SKB_DROP_REASON_NOT_SPECIFIED;
+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, reason);
+		}
+		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, &reason);
-	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;
-		reason = SKB_DROP_REASON_NOMEM;
-		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, SKB_DROP_REASON_NOT_SPECIFIED);
+	/* 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, reason);
+		}
+		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_reason(skb, reason);
-	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, reason);
+		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);
 
@@ -373,7 +522,18 @@ EXPORT_SYMBOL_GPL(mctp_usblib_tx_push);
 void mctp_usblib_tx_cancel(struct mctp_usblib_tx *tx, struct net_device *dev,
 			   enum skb_drop_reason reason)
 {
-	/* 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, reason);
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_tx_cancel);
 
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index 76f9d8879254..2e1cde6a6745 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -58,10 +58,6 @@ 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;
 
@@ -76,6 +72,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

[PATCH net-next v3 07/12] net: mctp: usb: Accommodate DSP0283 v1.1 header format

[Jeremy Kerr]

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>

---
v2:
 - fix short packet (hdr) debug message: use endian-converted value
---
 drivers/net/mctp/mctp-usblib.c | 14 ++++++++------
 include/linux/usb/mctp-usb.h   | 11 ++++++++---
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mctp/mctp-usblib.c b/drivers/net/mctp/mctp-usblib.c
index 2e464254353e..d58178f47c06 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -99,6 +99,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;
 
@@ -116,21 +117,23 @@ 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);
+				   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);
+		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;
 		}
 
@@ -399,8 +402,7 @@ static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb,
 	}
 
 	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 2e1cde6a6745..1a5e795b4ec1 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

[PATCH net-next v3 08/12] net: mctp: usblib: Implement receive-side packet spanning

[Jeremy Kerr]

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.

One subtle change: the mctp_usblib_rx() helper now handles skbs with the
full transport header, so we shift the skb_pull() for the header data to
the helper, before doing the rx_bytes stats update.

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>

---
v3:
 - only expand skb if cloned or under min xfer size, preventing excessive
   reallocation
 - align rx len to ep pktlen.
v2:
 - note change in semantics for mctp_usblib_rx
 - reject rx packets too short for a MCTP header, rather than deferring
   to the MCTP core do do so
---
 drivers/net/mctp/mctp-usb.c    |   2 +-
 drivers/net/mctp/mctp-usblib.c | 158 ++++++++++++++++++++++++++++-------------
 include/linux/usb/mctp-usb.h   |   6 +-
 3 files changed, 114 insertions(+), 52 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index ef58703040d6..644e7f88ce8b 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -320,7 +320,7 @@ 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);
+	mctp_usblib_rx_init(&dev->rx, le16_to_cpu(ep_in->wMaxPacketSize), 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 d58178f47c06..dad876c4da68 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
  */
@@ -14,9 +14,11 @@
 #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, u16 ep_pktlen, bool span)
 {
 	memset(rx, 0, sizeof(*rx));
+	rx->span = span;
+	rx->ep_pktlen = ep_pktlen;
 }
 EXPORT_SYMBOL_GPL(mctp_usblib_rx_init);
 
@@ -34,15 +36,51 @@ 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 ? ALIGN(MCTP_USB_1_1_PKTLEN_MAX, rx->ep_pktlen)
+		: MCTP_USB_1_0_XFER_SIZE;
+
+	if (!skb) {
+		skb = __netdev_alloc_skb(netdev, len, gfp);
+		if (!skb)
+			return -ENOMEM;
+
+	} else if (skb->cloned || skb_tailroom(skb) < rx->ep_pktlen) {
+		/* We always need to realloc if ->cloned, as we cannot
+		 * resubmit the (now-shared) skb buffer for possible DMA.
+		 *
+		 * Otherwise (if we have an un-cloned SKB): just ensure we
+		 * have sufficient space to prevent babble. Since we allocated
+		 * for max size in the last prepare (and have not consumed any
+		 * of that space for a prior MCTP packet, because !cloned), we
+		 * have sufficient data to finish the current MCTP packet.
+		 */
+		struct sk_buff *skb2;
+
+		skb2 = skb_copy_expand(skb, 0, len, gfp);
+		if (!skb2)
+			return -ENOMEM;
+		dev_kfree_skb_any(skb);
+		skb = skb2;
+	}
 
 	rx->skb = skb;
 
+	/* Spanning mode allows ZLPs, so we don't require exactly one
+	 * transfer packet. If we have extra tailroom, may as well use it,
+	 * and we have ensured that the tailroom >= ep_pktlen.
+	 */
+	if (rx->span)
+		len = ALIGN_DOWN(skb_tailroom(skb), rx->ep_pktlen);
+
 	*bufp = skb_tail_pointer(skb);
 	*lenp = len;
 
@@ -56,6 +94,9 @@ static void mctp_usblib_rx(struct net_device *netdev, struct sk_buff *skb)
 	struct mctp_skb_cb *cb;
 	unsigned long flags;
 
+	skb_reset_mac_header(skb);
+	skb_pull(skb, sizeof(struct mctp_usb_hdr));
+
 	/* we're called from an URB completion handler, and cannot assume local
 	 * irqs are always disabled
 	 */
@@ -96,72 +137,89 @@ 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) + sizeof(struct mctp_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;
-				}
-			} else {
-				mctp_usblib_rx_stats_single_drop(netdev);
-			}
-			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);
+		} else {
+			mctp_usblib_rx_stats_single_drop(netdev);
+		}
+		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 1a5e795b4ec1..c8fe22d7f68e 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,11 @@ struct mctp_usb_hdr {
  */
 struct mctp_usblib_rx {
 	struct sk_buff *skb;
+	u16 ep_pktlen;
+	bool span;
 };
 
-void mctp_usblib_rx_init(struct mctp_usblib_rx *rx);
+void mctp_usblib_rx_init(struct mctp_usblib_rx *rx, u16 ep_pktlen, bool span);
 void mctp_usblib_rx_fini(struct mctp_usblib_rx *rx);
 
 int mctp_usblib_rx_prepare(struct net_device *netdev,

-- 
2.47.3

[PATCH net-next v3 09/12] net: mctp: usblib: Implement transmit-side packet spanning

[Jeremy Kerr]

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 644e7f88ce8b..0507307c875f 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -321,7 +321,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, dev);
 
 	mctp_usblib_rx_init(&dev->rx, le16_to_cpu(ep_in->wMaxPacketSize), 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 dad876c4da68..4131be31db1a 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -239,7 +239,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,
@@ -249,18 +249,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)
@@ -349,6 +350,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)
@@ -357,11 +364,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);
@@ -370,6 +377,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);
@@ -434,15 +442,17 @@ EXPORT_SYMBOL_GPL(mctp_usblib_tx_send_complete);
  *
  * On error, populates @reason.
  */
-static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb,
+static int mctp_usblib_tx_skb_prepare(struct sk_buff *skb, bool span,
 				      enum skb_drop_reason *reason)
 {
+	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) {
 		*reason = SKB_DROP_REASON_PKT_TOO_BIG;
 		return -EMSGSIZE;
 	}
@@ -481,7 +491,7 @@ int mctp_usblib_tx_push(struct net_device *dev,
 	unsigned long flags;
 	int try = 1, rc;
 
-	rc = mctp_usblib_tx_skb_prepare(skb, &reason);
+	rc = mctp_usblib_tx_skb_prepare(skb, tx->span, &reason);
 	if (rc) {
 		mctp_usblib_tx_stats_single_drop(dev);
 		kfree_skb_reason(skb, reason);
diff --git a/include/linux/usb/mctp-usb.h b/include/linux/usb/mctp-usb.h
index c8fe22d7f68e..35cc2cc15f9a 100644
--- a/include/linux/usb/mctp-usb.h
+++ b/include/linux/usb/mctp-usb.h
@@ -81,6 +81,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 */
@@ -88,7 +89,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

[PATCH net-next v3 10/12] net: mctp: usblib: Add initial kunit tests

[Jeremy Kerr]

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>
---
v2:
 - account for KUNIT_ASSERT-based exits; do cleanup through kunit_action
   facilities.
 - fix off-by-one in packet count duing skb length checks, in case we
   ended up with more skbs than expected
 - perform route updates under rntl lock
---
 drivers/net/mctp/Kconfig            |   5 +
 drivers/net/mctp/mctp-usblib-test.c | 410 ++++++++++++++++++++++++++++++++++++
 drivers/net/mctp/mctp-usblib.c      |   4 +
 3 files changed, 419 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..2a22be999fa0
--- /dev/null
+++ b/drivers/net/mctp/mctp-usblib-test.c
@@ -0,0 +1,410 @@
+// 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 u16 ep_maxpacket = 512;
+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 void mctp_usblib_test_dev_action(void *data)
+{
+	struct mctp_usblib_test_dev *dev = data;
+
+	skb_queue_purge(&dev->rx_pkts);
+	if (dev->mdev)
+		mctp_dev_put(dev->mdev);
+	unregister_netdev(dev->ndev);
+}
+
+static struct mctp_usblib_test_dev *
+mctp_usblib_test_create_dev(struct kunit *test)
+{
+	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;
+	}
+
+	rc = kunit_add_action_or_reset(test, mctp_usblib_test_dev_action, dev);
+	if (rc)
+		return NULL;
+
+	rcu_read_lock();
+	dev->mdev = __mctp_dev_get(ndev);
+	if (dev->mdev)
+		dev->mdev->net = mctp_default_net(dev_net(ndev));
+	rcu_read_unlock();
+
+	if (!dev->mdev)
+		return NULL;
+
+	rtnl_lock();
+	rc = dev_open(ndev, NULL);
+	rtnl_unlock();
+	if (rc)
+		return NULL;
+
+	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_fini_action(void *data)
+{
+	struct mctp_usblib_test_ctx *ctx = data;
+
+	/* The device will have been destroyed, so ->rt will be unlinked.
+	 * Just ensure that the refcount is as expected.
+	 */
+	KUNIT_ASSERT_TRUE(current->kunit_test,
+			  refcount_dec_and_test(&ctx->rt.refs));
+
+	kfree(ctx);
+}
+
+static struct mctp_usblib_test_ctx *mctp_usblib_test_init(struct kunit *test)
+{
+	struct mctp_usblib_test_ctx *ctx;
+	struct mctp_route *rt;
+	int rc;
+
+	ctx = kzalloc_obj(*ctx);
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+
+	INIT_LIST_HEAD(&ctx->rt.list);
+	rt = &ctx->rt;
+	refcount_set(&rt->refs, 1);
+
+	rc = kunit_add_action_or_reset(test, mctp_usblib_test_fini_action, ctx);
+	KUNIT_ASSERT_EQ(test, rc, 0);
+
+	ctx->dev = mctp_usblib_test_create_dev(test);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->dev);
+
+	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;
+
+	rtnl_lock();
+	list_add_rcu(&ctx->rt.list, &init_net.mctp.routes);
+	refcount_inc(&rt->refs);
+	rtnl_unlock();
+
+	return ctx;
+}
+
+/* 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));
+}
+
+static void action_rx_fini(void *data)
+{
+	struct mctp_usblib_rx *rx = data;
+
+	mctp_usblib_rx_fini(rx);
+	kfree(rx);
+}
+
+static struct mctp_usblib_rx *
+mctp_usblib_test_rx_init(struct kunit *test, bool span)
+{
+	struct mctp_usblib_rx *rx;
+	int rc;
+
+	rx = kzalloc_obj(*rx);
+	if (rx) {
+		rc = kunit_add_action_or_reset(test, action_rx_fini, rx);
+		KUNIT_ASSERT_EQ(test, rc, 0);
+	}
+	KUNIT_ASSERT_NOT_NULL(test, rx);
+	mctp_usblib_rx_init(rx, ep_maxpacket, span);
+
+	return rx;
+}
+
+/* Wrappers for usblib's rx_complete callback, which is intended to be called
+ * from atomic context
+ */
+static int mctp_usblib_test_rx_complete(struct net_device *netdev,
+					struct mctp_usblib_rx *rx, size_t len)
+{
+	int rc;
+
+	local_bh_disable();
+	rc = mctp_usblib_rx_complete(netdev, rx, len);
+	local_bh_enable();
+
+	return rc;
+}
+
+/* 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;
+
+	ctx = mctp_usblib_test_init(test);
+	dev = ctx->dev;
+
+	rx = mctp_usblib_test_rx_init(test, true);
+
+	rc = mctp_usblib_rx_prepare(dev->ndev, rx,
+				    &buf, &len, GFP_KERNEL);
+	KUNIT_ASSERT_EQ(test, rc, 0);
+
+	/* we should always have a maxpacket of transfer available */
+	KUNIT_ASSERT_GE(test, len, ep_maxpacket);
+
+	mctp_usblib_test_init_pkt(buf, 8, 8);
+
+	rc = mctp_usblib_test_rx_complete(dev->ndev, rx, 8);
+	KUNIT_ASSERT_EQ(test, rc, 0);
+
+	skb = __skb_dequeue(&dev->rx_pkts);
+	KUNIT_EXPECT_NOT_NULL(test, skb);
+	if (skb)
+		KUNIT_EXPECT_EQ(test, skb->len, 4);
+	kfree_skb(skb);
+}
+
+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 = kunit_kmalloc_array(test, 1, len, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, pktbuf);
+
+	/* 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;
+	}
+
+	ctx = mctp_usblib_test_init(test);
+	dev = ctx->dev;
+
+	rx = mctp_usblib_test_rx_init(test, 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);
+
+		KUNIT_ASSERT_GE(test, len, ep_maxpacket);
+
+		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_test_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);
+	}
+}
+
+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 4131be31db1a..fc0282152e6e 100644
--- a/drivers/net/mctp/mctp-usblib.c
+++ b/drivers/net/mctp/mctp-usblib.c
@@ -610,3 +610,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

[PATCH net-next v3 11/12] net: mctp: usb: enable v1.1 packet spanning

[Jeremy Kerr]

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 | 27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 0507307c875f..8e29be9ebd1d 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;
 
@@ -43,6 +44,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;
@@ -71,6 +77,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);
 
 	usb_anchor_urb(urb, &mctp_usb->tx_anchor);
@@ -296,6 +305,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 */
@@ -307,6 +317,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)
@@ -314,14 +326,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;
 	spin_lock_init(&dev->rx_lock);
+	if (dev->span)
+		netdev->max_mtu = MCTP_USB_1_1_MTU_MAX;
 	usb_set_intfdata(intf, dev);
 
-	mctp_usblib_rx_init(&dev->rx, le16_to_cpu(ep_in->wMaxPacketSize), false);
-	mctp_usblib_tx_init(&dev->tx, &tx_ops, dev, false);
+	mctp_usblib_rx_init(&dev->rx, le16_to_cpu(ep_in->wMaxPacketSize),
+			    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;
@@ -362,7 +378,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

[PATCH net-next v3 12/12] net: mctp: usb: Allow multiple urbs in flight

[Jeremy Kerr]

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>
---
v2:
 - rework tx_qmem locking, prevent race between completion and
   submission for stopping/waking the tx queues; perform the wake while
   locked
 - only wake if netif_running()
---
 drivers/net/mctp/mctp-usb.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 8e29be9ebd1d..2d31075374b4 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -42,6 +42,9 @@ struct mctp_usb {
 
 	struct mctp_usblib_tx tx;
 	struct usb_anchor tx_anchor;
+	/* serialises tx_qmem updates to netdev queue states */
+	spinlock_t tx_qmem_lock;
+	int tx_qmem;
 };
 
 enum {
@@ -49,23 +52,41 @@ 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);
+	unsigned int len = urb->transfer_buffer_length;
 	struct net_device *netdev = mctp_usb->netdev;
+	unsigned long flags;
 
 	mctp_usblib_tx_send_complete(tx_ctx, netdev, urb->status == 0);
 
 	usb_free_urb(urb);
 
-	netif_wake_queue(netdev);
+	spin_lock_irqsave(&mctp_usb->tx_qmem_lock, flags);
+	mctp_usb->tx_qmem -= len;
+	if (mctp_usb->tx_qmem < TX_QMEM_MAX && netif_running(netdev))
+		netif_wake_queue(netdev);
+	spin_unlock_irqrestore(&mctp_usb->tx_qmem_lock, flags);
 }
 
 static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
 			    void *data, size_t len)
 {
 	struct mctp_usb *mctp_usb = mctp_usblib_tx_ctx_priv(tx_ctx);
+	unsigned long flags;
 	struct urb *urb;
 	int rc;
 
@@ -80,8 +101,6 @@ 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);
-
 	usb_anchor_urb(urb, &mctp_usb->tx_anchor);
 
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
@@ -89,7 +108,12 @@ static int mctp_usb_tx_send(struct mctp_usblib_tx_ctx *tx_ctx,
 		netdev_dbg(mctp_usb->netdev, "TX urb submit failed, %d\n", rc);
 		usb_unanchor_urb(urb);
 		usb_free_urb(urb);
-		netif_start_queue(mctp_usb->netdev);
+	} else {
+		spin_lock_irqsave(&mctp_usb->tx_qmem_lock, flags);
+		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_qmem_lock, flags);
 	}
 
 	return rc;
@@ -333,6 +357,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
 	spin_lock_init(&dev->rx_lock);
 	if (dev->span)
 		netdev->max_mtu = MCTP_USB_1_1_MTU_MAX;
+	spin_lock_init(&dev->tx_qmem_lock);
 	usb_set_intfdata(intf, dev);
 
 	mctp_usblib_rx_init(&dev->rx, le16_to_cpu(ep_in->wMaxPacketSize),

-- 
2.47.3

Re: [PATCH net-next v3 04/12] net: mctp: usb: Improve IN endpoint status handling

[Oliver Neukum]

On 08.07.26 11:58, Jeremy Kerr wrote:

Hi,

> @@ -198,6 +254,8 @@ static int mctp_usb_open(struct net_device *dev)
>   	struct mctp_usb *mctp_usb = netdev_priv(dev);
>   
>   	WRITE_ONCE(mctp_usb->rx_stopped, false);
> +	mctp_usb->clear_halt = false;
> +	mctp_usb->in_err_count = 0;

What allows the assumption that mctp_usb_open() does not race
with error handling? What happens if the device is still stalled?
It seems to me that you need to wait for the halt to be cleared.

	Regards
		Oliver

Re: [PATCH net-next v3 04/12] net: mctp: usb: Improve IN endpoint status handling

[Jeremy Kerr]

Hi Oliver,
Thanks for taking a look.



> What allows the assumption that mctp_usb_open() does not race
> with error handling? What happens if the device is still stalled?

When ndo_open is called, we know that the delayed rx work (which
performs the usb_clear_halt) has been quiesced, so aren't racing with
the actual recovery.

The device may still be stalled; that should be okay though, as we'll
detect that through the subsequent IN urb submission, which will re-set
->clear halt and schedule the rx work to do the clear.

Unless I have missed a case there?

Cheers,


Jeremy