All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB
@ 2026-08-17  6:49 Koichiro Den
  2026-08-17  6:49 ` [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion Koichiro Den
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Koichiro Den @ 2026-08-17  6:49 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: ntb, netdev, linux-kernel

Hi,

ntb_netdev may be used on embedded systems, where CPU resources are
often limited. L4 checksum calculation can therefore become a bottleneck
even when traffic stays within a trusted PCIe fabric.

This small series makes it possible to carry CHECKSUM_PARTIAL state
across the NTB link using opaque per-payload metadata in ntb_transport.
Existing peers continue to use software checksumming. The feature
remains disabled by default and must be enabled explicitly for trusted
links.

Note: the first two fixes came from Sashiko's review of v1. They touch
the same ntb_transport path as the metadata patch, so keeping them here
avoids a cross-tree dependency. With review from the NTB side, I hope
the whole series can go through net-next.

Best regards,
Koichiro
---
Changes in v2:
  - Reset peer checksum capability on every link event (Sashiko)
  - Add prerequisite fixes for RX ordering and shared field endianness
    (Sashiko)

v1: https://lore.kernel.org/r/20260814032913.3558500-1-den@valinux.co.jp/

Koichiro Den (4):
  NTB: ntb_transport: Order RX descriptor reads after completion
  NTB: ntb_transport: Use little-endian shared fields
  NTB: ntb_transport: Add per-payload client metadata
  net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB

 drivers/net/ntb_netdev.c      | 77 +++++++++++++++++++++++++++++++++--
 drivers/ntb/ntb_transport.c   | 73 +++++++++++++++++++++------------
 include/linux/ntb_transport.h |  6 ++-
 3 files changed, 123 insertions(+), 33 deletions(-)

base-commit: e6a5d573d24cd375e09d24f136523cb3cc85c9d3
-- 
2.51.0


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

* [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion
  2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
@ 2026-08-17  6:49 ` Koichiro Den
  2026-08-18  6:49   ` sashiko-bot
  2026-08-17  6:49 ` [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields Koichiro Den
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Koichiro Den @ 2026-08-17  6:49 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: ntb, netdev, linux-kernel

The peer writes payloads and descriptors into a DMA-coherent memory
window. ntb_process_rxc() checks DESC_DONE_FLAG before consuming the
descriptor and payload, but coherent memory alone does not order those
reads on weakly ordered CPUs.

Read the completion word once and issue dma_rmb() after DONE is observed.
Use the saved word for subsequent transport flag checks.

Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - New patch. (Sashiko)

 drivers/ntb/ntb_transport.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index f59f926d4bfa..d458a8b1de11 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1609,21 +1609,25 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 {
 	struct ntb_payload_header *hdr;
 	struct ntb_queue_entry *entry;
+	unsigned int flags;
 	void *offset;
 
 	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
 	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
 
-	dev_dbg(&qp->ndev->pdev->dev, "qp %d: RX ver %u len %d flags %x\n",
-		qp->qp_num, hdr->ver, hdr->len, hdr->flags);
-
-	if (!(hdr->flags & DESC_DONE_FLAG)) {
+	flags = READ_ONCE(hdr->flags);
+	if (!(flags & DESC_DONE_FLAG)) {
 		dev_dbg(&qp->ndev->pdev->dev, "done flag not set\n");
 		qp->rx_ring_empty++;
 		return -EAGAIN;
 	}
 
-	if (hdr->flags & LINK_DOWN_FLAG) {
+	dma_rmb();
+
+	dev_dbg(&qp->ndev->pdev->dev, "qp %d: RX ver %u len %d flags %x\n",
+		qp->qp_num, hdr->ver, hdr->len, flags);
+
+	if (flags & LINK_DOWN_FLAG) {
 		dev_dbg(&qp->ndev->pdev->dev, "link down flag set\n");
 		ntb_qp_link_down(qp);
 		hdr->flags = 0;
-- 
2.51.0


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

* [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields
  2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
  2026-08-17  6:49 ` [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion Koichiro Den
@ 2026-08-17  6:49 ` Koichiro Den
  2026-08-18  6:49   ` sashiko-bot
  2026-08-17  6:49 ` [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata Koichiro Den
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Koichiro Den @ 2026-08-17  6:49 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: ntb, netdev, linux-kernel

ntb_transport writes payload headers and the RX ring tail with
iowrite32(), but reads peer-written copies from coherent memory as native
integers. The values are therefore byte-swapped when read on a big-endian
system.

Mark the shared fields as __le32 and convert coherent-memory accesses
accordingly.

Fixes: 74465645cdb4 ("NTB: Fix Sparse Warnings")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - New patch. (Sashiko)

 drivers/ntb/ntb_transport.c | 47 +++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index d458a8b1de11..967a5ad38164 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -132,7 +132,7 @@ struct ntb_queue_entry {
 };
 
 struct ntb_rx_info {
-	unsigned int entry;
+	__le32 entry;
 };
 
 struct ntb_transport_qp {
@@ -265,9 +265,9 @@ enum {
 };
 
 struct ntb_payload_header {
-	unsigned int ver;
-	unsigned int len;
-	unsigned int flags;
+	__le32 ver;
+	__le32 len;
+	__le32 flags;
 };
 
 enum {
@@ -514,7 +514,8 @@ static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
 	seq_printf(s, "tx_err_no_buf - %llu\n", qp->tx_err_no_buf);
 	seq_printf(s, "tx_mw - \t0x%p\n", qp->tx_mw);
 	seq_printf(s, "tx_index (H) - \t%u\n", qp->tx_index);
-	seq_printf(s, "RRI (T) - \t%u\n", qp->remote_rx_info->entry);
+	seq_printf(s, "RRI (T) - \t%u\n",
+		   le32_to_cpu(qp->remote_rx_info->entry));
 	seq_printf(s, "tx_max_entry - \t%u\n", qp->tx_max_entry);
 	seq_printf(s, "free tx - \t%u\n", ntb_transport_tx_free_entry(qp));
 	seq_putc(s, '\n');
@@ -633,7 +634,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
 		qp->rx_alloc_entry++;
 	}
 
-	qp->remote_rx_info->entry = qp->rx_max_entry - 1;
+	qp->remote_rx_info->entry = cpu_to_le32(qp->rx_max_entry - 1);
 
 	/* setup the hdr offsets with 0's */
 	for (i = 0; i < qp->rx_max_entry; i++) {
@@ -919,7 +920,7 @@ static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
 {
 	ntb_qp_link_context_reset(qp);
 	if (qp->remote_rx_info)
-		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
+		qp->remote_rx_info->entry = cpu_to_le32(qp->rx_max_entry - 1);
 }
 
 static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
@@ -1445,7 +1446,7 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
 		if (!(entry->flags & DESC_DONE_FLAG))
 			break;
 
-		entry->rx_hdr->flags = 0;
+		entry->rx_hdr->flags = cpu_to_le32(0);
 		iowrite32(entry->rx_index, &qp->rx_info->entry);
 
 		cb_data = entry->cb_data;
@@ -1609,13 +1610,15 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 {
 	struct ntb_payload_header *hdr;
 	struct ntb_queue_entry *entry;
-	unsigned int flags;
 	void *offset;
+	u32 flags;
+	u32 len;
+	u32 ver;
 
 	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
 	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
 
-	flags = READ_ONCE(hdr->flags);
+	flags = le32_to_cpu(READ_ONCE(hdr->flags));
 	if (!(flags & DESC_DONE_FLAG)) {
 		dev_dbg(&qp->ndev->pdev->dev, "done flag not set\n");
 		qp->rx_ring_empty++;
@@ -1623,21 +1626,23 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	}
 
 	dma_rmb();
+	ver = le32_to_cpu(hdr->ver);
+	len = le32_to_cpu(hdr->len);
 
 	dev_dbg(&qp->ndev->pdev->dev, "qp %d: RX ver %u len %d flags %x\n",
-		qp->qp_num, hdr->ver, hdr->len, flags);
+		qp->qp_num, ver, len, flags);
 
 	if (flags & LINK_DOWN_FLAG) {
 		dev_dbg(&qp->ndev->pdev->dev, "link down flag set\n");
 		ntb_qp_link_down(qp);
-		hdr->flags = 0;
+		hdr->flags = cpu_to_le32(0);
 		return -EAGAIN;
 	}
 
-	if (hdr->ver != (u32)qp->rx_pkts) {
+	if (ver != (u32)qp->rx_pkts) {
 		dev_dbg(&qp->ndev->pdev->dev,
 			"version mismatch, expected %llu - got %u\n",
-			qp->rx_pkts, hdr->ver);
+			qp->rx_pkts, ver);
 		qp->rx_err_ver++;
 		return -EIO;
 	}
@@ -1652,10 +1657,10 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	entry->rx_hdr = hdr;
 	entry->rx_index = qp->rx_index;
 
-	if (hdr->len > entry->len) {
+	if (len > entry->len) {
 		dev_dbg(&qp->ndev->pdev->dev,
 			"receive buffer overflow! Wanted %d got %d\n",
-			hdr->len, entry->len);
+			len, entry->len);
 		qp->rx_err_oflow++;
 
 		entry->len = -EIO;
@@ -1665,12 +1670,12 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 	} else {
 		dev_dbg(&qp->ndev->pdev->dev,
 			"RX OK index %u ver %u size %d into buf size %d\n",
-			qp->rx_index, hdr->ver, hdr->len, entry->len);
+			qp->rx_index, ver, len, entry->len);
 
-		qp->rx_bytes += hdr->len;
+		qp->rx_bytes += len;
 		qp->rx_pkts++;
 
-		entry->len = hdr->len;
+		entry->len = len;
 
 		ntb_async_rx(entry, offset);
 	}
@@ -2492,7 +2497,9 @@ EXPORT_SYMBOL_GPL(ntb_transport_max_size);
 unsigned int ntb_transport_tx_free_entry(struct ntb_transport_qp *qp)
 {
 	unsigned int head = qp->tx_index;
-	unsigned int tail = qp->remote_rx_info->entry;
+	unsigned int tail;
+
+	tail = le32_to_cpu(READ_ONCE(qp->remote_rx_info->entry));
 
 	return tail >= head ? tail - head : qp->tx_max_entry + tail - head;
 }
-- 
2.51.0


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

* [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata
  2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
  2026-08-17  6:49 ` [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion Koichiro Den
  2026-08-17  6:49 ` [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields Koichiro Den
@ 2026-08-17  6:49 ` Koichiro Den
  2026-08-18  6:49   ` sashiko-bot
  2026-08-17  6:49 ` [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB Koichiro Den
  2026-08-17 15:39 ` [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload " Jakub Kicinski
  4 siblings, 1 reply; 10+ messages in thread
From: Koichiro Den @ 2026-08-17  6:49 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: ntb, netdev, linux-kernel

ntb_transport currently carries only payload bytes, with no way for clients
to associate metadata with an individual payload.

The payload header has a 32-bit flags field, with only BIT(0) and BIT(1) in
use. Carry opaque client metadata in the upper 24 bits. Expose it through
the transmit enqueue interface and receive callback. Reject values that do
not fit. Keep the low byte for transport flags so future flags can continue
from BIT(2).

No protocol version bump is needed. Existing Linux version 4 peers ignore
the upper bits on receive and always transmit them as zero.

Adapt ntb_netdev to the new interfaces without using metadata.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - No changes.

 drivers/net/ntb_netdev.c      |  4 ++--
 drivers/ntb/ntb_transport.c   | 18 +++++++++++++-----
 include/linux/ntb_transport.h |  6 ++++--
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 029a4a532a10..5c7fe6883cb9 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -123,7 +123,7 @@ static void ntb_netdev_event_handler(void *data, int link_is_up)
 }
 
 static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
-				  void *data, int len)
+				  void *data, int len, unsigned int meta)
 {
 	struct ntb_netdev_queue *q = qp_data;
 	struct ntb_netdev *dev = q->ntdev;
@@ -258,7 +258,7 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 
 	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
 
-	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len);
+	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, 0);
 	if (rc)
 		goto err;
 
diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 967a5ad38164..70be06a42201 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -167,7 +167,7 @@ struct ntb_transport_qp {
 	unsigned int tx_max_frame;
 
 	void (*rx_handler)(struct ntb_transport_qp *qp, void *qp_data,
-			   void *data, int len);
+			   void *data, int len, unsigned int meta);
 	struct list_head rx_post_q;
 	struct list_head rx_pend_q;
 	struct list_head rx_free_q;
@@ -264,6 +264,10 @@ enum {
 	LINK_DOWN_FLAG = BIT(1),
 };
 
+/* Reserve the low byte for transport flags. */
+#define DESC_META_SHIFT		8
+#define DESC_META_MASK		(~0U << DESC_META_SHIFT)
+
 struct ntb_payload_header {
 	__le32 ver;
 	__le32 len;
@@ -1436,6 +1440,7 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
 	struct ntb_queue_entry *entry;
 	void *cb_data;
 	unsigned int len;
+	unsigned int meta;
 	unsigned long irqflags;
 
 	spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
@@ -1451,13 +1456,14 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
 
 		cb_data = entry->cb_data;
 		len = entry->len;
+		meta = entry->flags >> DESC_META_SHIFT;
 
 		list_move_tail(&entry->entry, &qp->rx_free_q);
 
 		spin_unlock_irqrestore(&qp->ntb_rx_q_lock, irqflags);
 
 		if (qp->rx_handler && qp->client_ready)
-			qp->rx_handler(qp, qp->cb_data, cb_data, len);
+			qp->rx_handler(qp, qp->cb_data, cb_data, len, meta);
 
 		spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
 	}
@@ -1656,6 +1662,7 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
 
 	entry->rx_hdr = hdr;
 	entry->rx_index = qp->rx_index;
+	entry->flags = flags & DESC_META_MASK;
 
 	if (len > entry->len) {
 		dev_dbg(&qp->ndev->pdev->dev,
@@ -2341,6 +2348,7 @@ EXPORT_SYMBOL_GPL(ntb_transport_rx_enqueue);
  * @cb: per buffer pointer for callback function to use
  * @data: pointer to data buffer that will be sent
  * @len: length of the data buffer
+ * @meta: client metadata to send with the buffer
  *
  * Enqueue a new transmit buffer onto the transport queue from which a NTB
  * payload will be transmitted.  This assumes that a lock is being held to
@@ -2349,12 +2357,12 @@ EXPORT_SYMBOL_GPL(ntb_transport_rx_enqueue);
  * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
  */
 int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
-			     unsigned int len)
+			     unsigned int len, unsigned int meta)
 {
 	struct ntb_queue_entry *entry;
 	int rc;
 
-	if (!qp || !len)
+	if (!qp || !len || meta > NTB_TRANSPORT_MAX_META)
 		return -EINVAL;
 
 	/* If the qp link is down already, just ignore. */
@@ -2370,7 +2378,7 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 	entry->cb_data = cb;
 	entry->buf = data;
 	entry->len = len;
-	entry->flags = 0;
+	entry->flags = meta << DESC_META_SHIFT;
 	entry->errors = 0;
 	entry->tx_index = 0;
 
diff --git a/include/linux/ntb_transport.h b/include/linux/ntb_transport.h
index 7243eb98a722..9e807542b6c4 100644
--- a/include/linux/ntb_transport.h
+++ b/include/linux/ntb_transport.h
@@ -50,6 +50,8 @@
 
 struct ntb_transport_qp;
 
+#define NTB_TRANSPORT_MAX_META	0x00ffffffU
+
 struct ntb_transport_client {
 	struct device_driver driver;
 	int (*probe)(struct device *client_dev);
@@ -63,7 +65,7 @@ void ntb_transport_unregister_client_dev(char *device_name);
 
 struct ntb_queue_handlers {
 	void (*rx_handler)(struct ntb_transport_qp *qp, void *qp_data,
-			   void *data, int len);
+			   void *data, int len, unsigned int meta);
 	void (*tx_handler)(struct ntb_transport_qp *qp, void *qp_data,
 			   void *data, int len);
 	void (*event_handler)(void *data, int status);
@@ -78,7 +80,7 @@ void ntb_transport_free_queue(struct ntb_transport_qp *qp);
 int ntb_transport_rx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
 			     unsigned int len);
 int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
-			     unsigned int len);
+			     unsigned int len, unsigned int meta);
 void *ntb_transport_rx_remove(struct ntb_transport_qp *qp, unsigned int *len);
 void ntb_transport_link_up(struct ntb_transport_qp *qp);
 void ntb_transport_link_down(struct ntb_transport_qp *qp);
-- 
2.51.0


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

* [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB
  2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
                   ` (2 preceding siblings ...)
  2026-08-17  6:49 ` [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata Koichiro Den
@ 2026-08-17  6:49 ` Koichiro Den
  2026-08-18  6:49   ` sashiko-bot
  2026-08-17 15:39 ` [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload " Jakub Kicinski
  4 siblings, 1 reply; 10+ messages in thread
From: Koichiro Den @ 2026-08-17  6:49 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: ntb, netdev, linux-kernel

Calculating L4 checksums can limit ntb_netdev throughput especially on
embedded systems, where CPU resources are often limited. A trusted PCIe
fabric can avoid that work.

Carry CHECKSUM_PARTIAL with csum_start and csum_offset across the NTB link.
Advertise support in every frame and fall back to software until the peer
capability is seen. This preserves netdev checksum semantics and
interoperability with existing transport version 4 peers.

Leave the TX and RX checksum features disabled by default. Users can
just enable them explicitly for links they trust for lower CPU usage
and/or higher throughput.

Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v2:
  - Reset peer checksum capability on every link event (Sashiko)
  - Keep local variable declarations in reverse Christmas tree order

 drivers/net/ntb_netdev.c | 75 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 5c7fe6883cb9..9cfb463e472c 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -4,6 +4,7 @@
  */
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
+#include <linux/if_vlan.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/ntb.h>
@@ -29,6 +30,21 @@ static unsigned int tx_stop = 5;
 #define NTB_NETDEV_MAX_QUEUES		64
 #define NTB_NETDEV_DEFAULT_QUEUES	1
 
+/*
+ * Checksum metadata layout:
+ *   bit 23     capability, advertised on every packet
+ *   bit 22     per-packet CHECKSUM_PARTIAL flag
+ *   bit 21..6  skb_checksum_start_offset() (16 bits)
+ *   bit 5..0   skb->csum_offset (6 bits)
+ *
+ * Until the capability is observed, complete partial checksums in software.
+ * Six offset bits cover TCP/UDP. Larger offsets use software checksumming.
+ */
+#define NTB_NETDEV_META_CAP_CSUM		BIT(23)
+#define NTB_NETDEV_META_CSUM			BIT(22)
+#define NTB_NETDEV_META_CSUM_START_SHIFT	6
+#define NTB_NETDEV_META_CSUM_OFFSET_MASK	GENMASK(5, 0)
+
 struct ntb_netdev;
 
 struct ntb_netdev_queue {
@@ -44,6 +60,7 @@ struct ntb_netdev {
 	struct net_device *ndev;
 	unsigned int num_queues;
 	struct ntb_netdev_queue *queues;
+	bool peer_csum;
 };
 
 #define	NTB_TX_TIMEOUT_MS	1000
@@ -108,6 +125,7 @@ static void ntb_netdev_event_handler(void *data, int link_is_up)
 	struct net_device *ndev;
 
 	ndev = dev->ndev;
+	WRITE_ONCE(dev->peer_csum, false);
 
 	netdev_dbg(ndev, "Event %x, Link %x, qp %u\n", link_is_up,
 		   ntb_transport_link_query(q->qp), q->qid);
@@ -151,8 +169,21 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 	}
 
 	skb_put(skb, len);
+	if (meta & NTB_NETDEV_META_CAP_CSUM)
+		WRITE_ONCE(dev->peer_csum, true);
+
+	if (meta & NTB_NETDEV_META_CSUM) {
+		u16 csum_start = (meta >> NTB_NETDEV_META_CSUM_START_SHIFT) & U16_MAX;
+		u16 csum_offset = meta & NTB_NETDEV_META_CSUM_OFFSET_MASK;
+
+		if (!skb_partial_csum_set(skb, csum_start, csum_offset))
+			goto rx_drop;
+
+		if (!(ndev->features & NETIF_F_RXCSUM) &&
+		    skb_checksum_help(skb))
+			goto rx_drop;
+	}
 	skb->protocol = eth_type_trans(skb, ndev);
-	skb->ip_summed = CHECKSUM_NONE;
 	skb_record_rx_queue(skb, q->qid);
 
 	if (netif_rx(skb) == NET_RX_DROP) {
@@ -172,6 +203,14 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 		ndev->stats.rx_errors++;
 		ndev->stats.rx_fifo_errors++;
 	}
+	return;
+
+rx_drop:
+	ndev->stats.rx_errors++;
+	ndev->stats.rx_dropped++;
+	dev_kfree_skb_any(skb);
+	skb = new_skb;
+	goto enqueue_again;
 }
 
 static int __ntb_netdev_maybe_stop_tx(struct net_device *netdev,
@@ -249,6 +288,7 @@ static const struct ntb_queue_handlers ntb_netdev_handlers = {
 static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 					 struct net_device *ndev)
 {
+	unsigned int meta = NTB_NETDEV_META_CAP_CSUM;
 	struct ntb_netdev *dev = netdev_priv(ndev);
 	u16 qid = skb_get_queue_mapping(skb);
 	struct ntb_netdev_queue *q;
@@ -258,7 +298,17 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 
 	ntb_netdev_maybe_stop_tx(ndev, q, tx_stop);
 
-	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, 0);
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		if (READ_ONCE(dev->peer_csum))
+			meta |= NTB_NETDEV_META_CSUM |
+				(skb_checksum_start_offset(skb) <<
+				 NTB_NETDEV_META_CSUM_START_SHIFT) |
+				skb->csum_offset;
+		else if (skb_checksum_help(skb))
+			goto drop;
+	}
+
+	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, meta);
 	if (rc)
 		goto err;
 
@@ -267,12 +317,29 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
 
 	return NETDEV_TX_OK;
 
+drop:
+	dev_kfree_skb_any(skb);
+	ndev->stats.tx_dropped++;
+	ndev->stats.tx_errors++;
+	return NETDEV_TX_OK;
+
 err:
 	ndev->stats.tx_dropped++;
 	ndev->stats.tx_errors++;
 	return NETDEV_TX_BUSY;
 }
 
+static netdev_features_t ntb_netdev_features_check(struct sk_buff *skb,
+						   struct net_device *ndev,
+						   netdev_features_t features)
+{
+	if (skb->ip_summed == CHECKSUM_PARTIAL &&
+	    skb->csum_offset > NTB_NETDEV_META_CSUM_OFFSET_MASK)
+		features &= ~NETIF_F_CSUM_MASK;
+
+	return vlan_features_check(skb, features);
+}
+
 static void ntb_netdev_tx_timer(struct timer_list *t)
 {
 	struct ntb_netdev_queue *q = timer_container_of(q, t, tx_timer);
@@ -423,6 +490,7 @@ static const struct net_device_ops ntb_netdev_ops = {
 	.ndo_open = ntb_netdev_open,
 	.ndo_stop = ntb_netdev_close,
 	.ndo_start_xmit = ntb_netdev_start_xmit,
+	.ndo_features_check = ntb_netdev_features_check,
 	.ndo_change_mtu = ntb_netdev_change_mtu,
 	.ndo_set_mac_address = eth_mac_addr,
 };
@@ -642,7 +710,8 @@ static int ntb_netdev_probe(struct device *client_dev)
 
 	ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 
-	ndev->hw_features = ndev->features;
+	/* Checksum bypass assumes a trusted NTB link, so keep it opt-in. */
+	ndev->hw_features = ndev->features | NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
 	ndev->watchdog_timeo = msecs_to_jiffies(NTB_TX_TIMEOUT_MS);
 
 	eth_random_addr(ndev->perm_addr);
-- 
2.51.0


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

* Re: [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB
  2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
                   ` (3 preceding siblings ...)
  2026-08-17  6:49 ` [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB Koichiro Den
@ 2026-08-17 15:39 ` Jakub Kicinski
  4 siblings, 0 replies; 10+ messages in thread
From: Jakub Kicinski @ 2026-08-17 15:39 UTC (permalink / raw)
  To: Koichiro Den
  Cc: Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni, ntb, netdev, linux-kernel

On Mon, 17 Aug 2026 15:49:12 +0900 Koichiro Den wrote:
> Subject: [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB

## Form letter - net-next-closed

The merge window for v7.3 has started, and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We will only consider applying net-next patches which were posted
before the announcement:

https://lore.kernel.org/20260816155953.072d73da@kernel.org

Fixes are obviously welcome at any time. net-next patches may be sent
for review and discussion only with an RFC tag.

Please repost when net-next reopens.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
-- 
pw-bot: defer
pv-bot: closed


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

* Re: [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata
  2026-08-17  6:49 ` [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata Koichiro Den
@ 2026-08-18  6:49   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-18  6:49 UTC (permalink / raw)
  To: Koichiro Den; +Cc: ntb

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak of SKB when NTB queue link is down during transmit.
- [High] Memory leak of the transmitted SKB when the payload exceeds the maximum frame size.
- [High] Memory leak of receive buffers (SKBs) if completed when the client is not ready.
--

--- Patch [3]: [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata ---
commit 21a42de29f1891a17d65f9eb283c9a3b65134a03
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Add per-payload client metadata

ntb_transport currently carries only payload bytes, with no way for clients
to associate metadata with an individual payload. This patch adapts the
interface to pass metadata in the payload header.

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 967a5ad381646..70be06a42201f 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c

[ ... ]

> @@ -1436,6 +1440,7 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
>  	struct ntb_queue_entry *entry;
>  	void *cb_data;
>  	unsigned int len;
> +	unsigned int meta;
>  	unsigned long irqflags;
>  
>  	spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
> @@ -1451,13 +1456,14 @@ static void ntb_complete_rxc(struct ntb_transport_qp *qp)
>  
>  		cb_data = entry->cb_data;
>  		len = entry->len;
> +		meta = entry->flags >> DESC_META_SHIFT;
>  
>  		list_move_tail(&entry->entry, &qp->rx_free_q);
>  
>  		spin_unlock_irqrestore(&qp->ntb_rx_q_lock, irqflags);
>  
>  		if (qp->rx_handler && qp->client_ready)
> -			qp->rx_handler(qp, qp->cb_data, cb_data, len);
> +			qp->rx_handler(qp, qp->cb_data, cb_data, len, meta);

[Severity: High]
This is a pre-existing issue, but does this lead to a memory leak of receive
buffers (SKBs) if an RX packet completes while the client is not ready?

If ntb_complete_rxc() runs during a link down event (!qp->client_ready), the
function skips invoking the client's rx_handler. However, it moves the entry
to qp->rx_free_q without releasing the client-provided buffer stored in
cb_data.

During teardown, the client uses ntb_transport_rx_remove() to recover unused
buffers, but that function only checks qp->rx_pend_q. Are the buffers left
stranded in rx_free_q permanently lost?

>  
>  		spin_lock_irqsave(&qp->ntb_rx_q_lock, irqflags);
>  	}

[ ... ]

> @@ -2349,12 +2357,12 @@ EXPORT_SYMBOL_GPL(ntb_transport_rx_enqueue);
>   * RETURNS: An appropriate -ERRNO error value on error, or zero for success.
>   */
>  int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
> -			     unsigned int len)
> +			     unsigned int len, unsigned int meta)
>  {
>  	struct ntb_queue_entry *entry;
>  	int rc;
>  
> -	if (!qp || !len)
> +	if (!qp || !len || meta > NTB_TRANSPORT_MAX_META)
>  		return -EINVAL;
>  
>  	/* If the qp link is down already, just ignore. */
>  	if (!qp->link_is_up)
>  		return 0;

[Severity: High]
This is a pre-existing issue, but does returning 0 instead of an error code
cause a memory leak of the SKB when the link is down?

If ntb_transport_tx_enqueue() returns 0, the caller (such as
ntb_netdev_start_xmit()) receives a success code and assumes the buffer
was successfully enqueued, returning NETDEV_TX_OK without calling
dev_kfree_skb_any().

Should this return an error like -ENETDOWN so the caller knows to drop the
packet?

>  
>  	entry = ntb_list_rm(&qp->ntb_tx_free_q_lock, &qp->tx_free_q);
>  	if (!entry) {
> @@ -2370,7 +2378,7 @@ int ntb_transport_tx_enqueue(struct ntb_transport_qp *qp, void *cb, void *data,
>  	entry->cb_data = cb;
>  	entry->buf = data;
>  	entry->len = len;
> -	entry->flags = 0;
> +	entry->flags = meta << DESC_META_SHIFT;
>  	entry->errors = 0;
>  	entry->tx_index = 0;
>  
>  	rc = ntb_process_tx(qp, entry);

[Severity: High]
This is a pre-existing issue, but can this lead to an SKB memory leak when
the payload exceeds the maximum frame size?

When a packet is larger than the NTB transport MTU, ntb_process_tx() handles
the overflow like this:

drivers/ntb/ntb_transport.c:ntb_process_tx() {
	if (entry->len > qp->tx_max_frame - sizeof(struct ntb_payload_header)) {
		if (qp->tx_handler)
			qp->tx_handler(qp, qp->cb_data, NULL, -EIO);

		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
			     &qp->tx_free_q);
		return 0;
	}
	...
}

Because ntb_process_tx() passes NULL instead of entry->cb_data to the 
tx_handler, the client (ntb_netdev_tx_handler()) cannot recover and free 
the SKB. ntb_process_tx() then returns 0, so the original caller also thinks 
the packet was enqueued and does not free it.

>  	if (rc)
>  		ntb_list_add(&qp->ntb_tx_free_q_lock, &entry->entry,
>  			     &qp->tx_free_q);
>  
>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817064916.13278-1-den@valinux.co.jp?part=3

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

* Re: [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields
  2026-08-17  6:49 ` [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields Koichiro Den
@ 2026-08-18  6:49   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-18  6:49 UTC (permalink / raw)
  To: Koichiro Den; +Cc: ntb

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Infinite error loop on packet version mismatch in ntb_process_rxc.
- [High] Missing READ_ONCE() for `hdr->len` can lead to a double-fetch vulnerability, allowing a malicious peer to bypass length checks.
--

--- Patch [2]: [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields ---
commit 1e93f678e1d03757c62cfd1f9dcbe502878847d8
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Use little-endian shared fields

This commit marks the shared payload headers and RX ring tail as __le32 and
converts the accesses to coherent memory using cpu_to_le32() and le32_to_cpu()
respectively. This fixes byte-swapping issues when the shared fields are read
on big-endian systems.

Link: https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index d458a8b1de114..967a5ad381646 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
[ ... ]
> @@ -1609,13 +1610,15 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  {
>  	struct ntb_payload_header *hdr;
>  	struct ntb_queue_entry *entry;
> -	unsigned int flags;
>  	void *offset;
> +	u32 flags;
> +	u32 len;
> +	u32 ver;
>  
>  	offset = qp->rx_buff + qp->rx_max_frame * qp->rx_index;
>  	hdr = offset + qp->rx_max_frame - sizeof(struct ntb_payload_header);
>  
> -	flags = READ_ONCE(hdr->flags);
> +	flags = le32_to_cpu(READ_ONCE(hdr->flags));
>  	if (!(flags & DESC_DONE_FLAG)) {
>  		dev_dbg(&qp->ndev->pdev->dev, "done flag not set\n");
>  		qp->rx_ring_empty++;
> @@ -1623,21 +1626,23 @@ static int ntb_process_rxc(struct ntb_transport_qp *qp)
>  	}
>  
>  	dma_rmb();
> +	ver = le32_to_cpu(hdr->ver);
> +	len = le32_to_cpu(hdr->len);

[Severity: High]
This is a pre-existing issue, but missing READ_ONCE() for hdr->len could
lead to a double-fetch vulnerability.

Since hdr->len is located in DMA-coherent memory shared with the peer,
reading it without READ_ONCE() allows the compiler to rematerialize the read
later in the function (e.g., when setting entry->len = len). 

Could a malicious peer exploit this TOCTOU race by increasing hdr->len after
the bounds check passes, causing an out-of-bounds copy into the host's RX
buffer? Should we use READ_ONCE() for hdr->ver and hdr->len as we do for
flags?

>  
>  	dev_dbg(&qp->ndev->pdev->dev, "qp %d: RX ver %u len %d flags %x\n",
> -		qp->qp_num, hdr->ver, hdr->len, flags);
> +		qp->qp_num, ver, len, flags);
>  
>  	if (flags & LINK_DOWN_FLAG) {
>  		dev_dbg(&qp->ndev->pdev->dev, "link down flag set\n");
>  		ntb_qp_link_down(qp);
> -		hdr->flags = 0;
> +		hdr->flags = cpu_to_le32(0);
>  		return -EAGAIN;
>  	}
>  
> -	if (hdr->ver != (u32)qp->rx_pkts) {
> +	if (ver != (u32)qp->rx_pkts) {
>  		dev_dbg(&qp->ndev->pdev->dev,
>  			"version mismatch, expected %llu - got %u\n",
> -			qp->rx_pkts, hdr->ver);
> +			qp->rx_pkts, ver);
>  		qp->rx_err_ver++;
>  		return -EIO;

[Severity: High]
This is a pre-existing issue, but does returning here cause an infinite
error loop?

If a remote peer sends a packet with a wrong sequence number (so
ver != qp->rx_pkts), ntb_process_rxc() returns -EIO without clearing
DESC_DONE_FLAG or advancing qp->rx_index.

Would this cause the host to repeatedly process the same stalled descriptor
on every subsequent tasklet execution, permanently stalling the RX queue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817064916.13278-1-den@valinux.co.jp?part=2

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

* Re: [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion
  2026-08-17  6:49 ` [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion Koichiro Den
@ 2026-08-18  6:49   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-18  6:49 UTC (permalink / raw)
  To: Koichiro Den; +Cc: ntb

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Double-fetch vulnerability (TOCTOU) on `hdr->len` allowing kernel heap buffer overflow.
- [Critical] Use-After-Free of memory windows during link teardown due to missing tasklet and DMA synchronization.
- [Critical] Invalid use of `virt_to_phys()` on DMA coherent memory corrupts DMA mappings.
- [High] Missing endianness conversion on memory-mapped payload headers breaks big-endian systems.
- [High] RX DMA abort fallback copies data from the wrong ring index, causing data corruption.
- [High] Concurrent modification of `qp->rx_index` causes ring state corruption.
- [High] Missing bounds validation of `hdr->len` against frame size causes out-of-bounds read and information leak.
--

--- Patch [1]: [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion ---
(Note: The format of this report has been altered due to recitation
restrictions. The original patch diff is not quoted, and findings are
provided as a free-form summary.)

Commit: 27d90886c93f5cd8fe8f9c93d5acf5cd83c90d55
Author: Koichiro Den <den@valinux.co.jp>
Subject: NTB: ntb_transport: Order RX descriptor reads after completion

This commit addresses an ordering issue on weakly ordered CPUs where the
descriptor and payload could be consumed before DESC_DONE_FLAG is fully
observed. It ensures the completion word is read once, followed by a
dma_rmb(), and uses the saved word for subsequent checks.

Link: https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/

[Severity: Critical]
This is a pre-existing issue, but in ntb_process_rxc(), could the double
fetch of hdr->len lead to a TOCTOU vulnerability?

The code reads hdr->len to validate it against entry->len. After this check,
operations like qp->rx_bytes += hdr->len and entry->len = hdr->len read the
length directly from DMA-coherent memory again.

Could a malicious peer concurrently swap a valid small length with a huge
value after the bounds check passes, causing the fallback CPU memcpy to
copy out-of-bounds into the local kernel buffer?

[Severity: High]
This isn't a bug introduced by this patch, but does the lack of endianness
conversion on memory-mapped payload headers break big-endian systems?

The TX side writes header fields using iowrite32(), which implicitly converts
to little-endian. However, the RX side in ntb_process_rxc() reads the
memory directly via READ_ONCE(hdr->flags) and evaluates flags against
DESC_DONE_FLAG without using le32_to_cpu().

Would this cause the receiver on big-endian hosts to misinterpret the bytes
and drop all received packets?

[Severity: Critical]
This is a pre-existing issue, but does ntb_transport_link_cleanup() safely
free memory windows if tasklets or DMA engines are still active?

When the link drops, it calls ntb_free_mw(), invoking dma_free_attrs().
However, it does not call tasklet_kill(&qp->rxc_db_work) or
dmaengine_terminate_all().

If the RX tasklet is actively processing doorbells or the DMA engine is
transferring data, will this result in a use-after-free or IOMMU DMA fault?

[Severity: High]
This isn't a bug introduced by this patch, but does ntb_rx_copy_callback()
use the correct index when falling back to a CPU copy?

If a DMA transaction aborts, it calculates the offset using the shared
qp->rx_index via qp->rx_buff + qp->rx_max_frame * qp->rx_index. Because the
callback is asynchronous, the tasklet may have already advanced
qp->rx_index.

Should it use the saved entry->rx_index instead to avoid copying data from
the wrong ring slot and corrupting the stream?

[Severity: High]
This is a pre-existing issue, but can ntb_qp_link_down_reset() safely reset
indices while traffic is flowing?

It forcefully zeroes qp->rx_index and qp->tx_index from a workqueue without
synchronizing with the rxc_db_work tasklet.

If the tasklet is running concurrently in ntb_process_rxc(), could the
unsynchronized writes corrupt the ring buffer state?

[Severity: High]
This isn't a bug introduced by this patch, but does ntb_process_rxc()
adequately validate hdr->len against the physical frame size?

The code verifies hdr->len against entry->len (the client buffer size) but
never against qp->rx_max_frame (the hardware memory window slice size).

If a client configures a large buffer, could a malicious remote peer set
hdr->len to a value exceeding the frame size, allowing an out-of-bounds
read that leaks adjacent kernel memory?

[Severity: Critical]
This is a pre-existing issue, but is it valid to use virt_to_phys() on
DMA coherent memory in ntb_async_rx_submit()?

The offset pointer is derived from qp->rx_buff, which is allocated via
dma_alloc_attrs(). On architectures where this returns a non-linear virtual
address in the vmalloc area, virt_to_phys() will return an incorrect
physical address.

Will this cause the DMA engine to write received data to arbitrary physical
memory locations, leading to silent memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817064916.13278-1-den@valinux.co.jp?part=1

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

* Re: [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB
  2026-08-17  6:49 ` [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB Koichiro Den
@ 2026-08-18  6:49   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-18  6:49 UTC (permalink / raw)
  To: Koichiro Den; +Cc: ntb

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing memory leak of `skb` when transmitting while the NTB link is down.
--

--- Patch [4]: [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB ---
commit 52574632d1af60a032a3fbef27bc73eb5ad8f90c
Author: Koichiro Den <den@valinux.co.jp>

net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB

This commit carries CHECKSUM_PARTIAL with csum_start and csum_offset across
the NTB link to offload checksum calculations to the peer. It advertises
support in every frame and falls back to software until the peer capability
is seen.

> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 5c7fe6883cb93..9cfb463e472c3 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c

[ ... ]

> @@ -249,6 +288,7 @@ static const struct ntb_queue_handlers ntb_netdev_handlers = {
>  static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
>  					 struct net_device *ndev)
>  {
> +	unsigned int meta = NTB_NETDEV_META_CAP_CSUM;

[ ... ]

> -	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, 0);
> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		if (READ_ONCE(dev->peer_csum))
> +			meta |= NTB_NETDEV_META_CSUM |
> +				(skb_checksum_start_offset(skb) <<
> +				 NTB_NETDEV_META_CSUM_START_SHIFT) |
> +				skb->csum_offset;
> +		else if (skb_checksum_help(skb))
> +			goto drop;
> +	}
> +
> +	rc = ntb_transport_tx_enqueue(q->qp, skb, skb->data, skb->len, meta);

[Severity: High]
This is a pre-existing issue, but does this code leak the skb when the NTB
link is down?

Looking at ntb_transport_tx_enqueue() in drivers/ntb/ntb_transport.c, if the
link is down, it returns 0 without taking ownership of the buffer:

drivers/ntb/ntb_transport.c:ntb_transport_tx_enqueue() {
    ...
    /* If the qp link is down already, just ignore. */
    if (!qp->link_is_up)
        return 0;
    ...
}

Since ntb_netdev_start_xmit() sees a return code of 0, it assumes successful
queuing and returns NETDEV_TX_OK without freeing the skb. Because the driver
accepts packets from the networking stack regardless of the physical link
state, sending packets while the link is disconnected appears to cause an
unbounded memory leak.

>  	if (rc)
>  		goto err;
>  
> @@ -267,12 +317,29 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
>  
>  	return NETDEV_TX_OK;
>  
> +drop:
> +	dev_kfree_skb_any(skb);
> +	ndev->stats.tx_dropped++;
> +	ndev->stats.tx_errors++;
> +	return NETDEV_TX_OK;
> +
>  err:
>  	ndev->stats.tx_dropped++;
>  	ndev->stats.tx_errors++;
>  	return NETDEV_TX_BUSY;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817064916.13278-1-den@valinux.co.jp?part=4

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

end of thread, other threads:[~2026-08-18  6:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17  6:49 [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload across NTB Koichiro Den
2026-08-17  6:49 ` [PATCH net-next v2 1/4] NTB: ntb_transport: Order RX descriptor reads after completion Koichiro Den
2026-08-18  6:49   ` sashiko-bot
2026-08-17  6:49 ` [PATCH net-next v2 2/4] NTB: ntb_transport: Use little-endian shared fields Koichiro Den
2026-08-18  6:49   ` sashiko-bot
2026-08-17  6:49 ` [PATCH net-next v2 3/4] NTB: ntb_transport: Add per-payload client metadata Koichiro Den
2026-08-18  6:49   ` sashiko-bot
2026-08-17  6:49 ` [PATCH net-next v2 4/4] net: ntb_netdev: Preserve CHECKSUM_PARTIAL across NTB Koichiro Den
2026-08-18  6:49   ` sashiko-bot
2026-08-17 15:39 ` [PATCH net-next v2 0/4] net: ntb_netdev: Preserve checksum offload " Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.