Linux PCI Non-Transparent Bridge framework and drivers
 help / color / mirror / Atom feed
* [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting
@ 2026-08-19 17:25 Koichiro Den
  2026-08-19 17:25 ` [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops Koichiro Den
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Koichiro Den @ 2026-08-19 17:25 UTC (permalink / raw)
  To: Jakub Kicinski, Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Greg Kroah-Hartman,
	Nicholas Bellinger
  Cc: ntb, netdev, linux-kernel

Hi,

This series addresses Jakub's comment on ntb_netdev RX statistics
accounting:
https://lore.kernel.org/r/20260818092938.4121c220@kernel.org/

It fixes the double counting and also the related packet/byte accounting
on RX refill failure.

Notes:
- No ordering dependency on the TX completion/error handling series,
  which is currently awaiting review. Either series can go first.
  https://patchwork.kernel.org/project/netdevbpf/cover/20260817053519.4135287-1-den@valinux.co.jp/
- Once both series land, I plan to post one follow-up patch to net for
  the multi-queue stats race introduced by commit 24d9e73c7e00 ("net:
  ntb_netdev: Support ethtool channels for multi-queue"). My plan is to
  move packet/byte/drop counters to core-managed per-CPU stats and use
  DEV_STATS_INC() for the remaining slow-path counters. IMO this is
  lower severity and better done after the TX/RX changes settle.

Best regards,
Koichiro


Koichiro Den (2):
  net: ntb_netdev: Avoid double-accounting netif_rx() drops
  net: ntb_netdev: Count packets dropped on RX refill failure

 drivers/net/ntb_netdev.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

base-commit: 564973a259ec76f2dad0853420e7034cc43994c4
-- 
2.51.0


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

* [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops
  2026-08-19 17:25 [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting Koichiro Den
@ 2026-08-19 17:25 ` Koichiro Den
  2026-08-20 17:25   ` sashiko-bot
  2026-08-19 17:25 ` [PATCH net 2/2] net: ntb_netdev: Count packets dropped on RX refill failure Koichiro Den
  2026-08-22 20:20 ` [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting patchwork-bot+netdevbpf
  2 siblings, 1 reply; 5+ messages in thread
From: Koichiro Den @ 2026-08-19 17:25 UTC (permalink / raw)
  To: Jakub Kicinski, Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Greg Kroah-Hartman,
	Nicholas Bellinger
  Cc: ntb, netdev, linux-kernel

netif_rx() already accounts packets it drops in the core rx_dropped
counter. ntb_netdev counts them again as both errors and drops.

Leave netif_rx() drops to the core. Count the packet and bytes
unconditionally since it was received successfully by the driver.

Fixes: 548c237c0a99 ("net: Add support for NTB virtual ethernet device")
Cc: stable@vger.kernel.org
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/net/ntb_netdev.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 029a4a532a10..4e53b00f016b 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -155,13 +155,9 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 	skb->ip_summed = CHECKSUM_NONE;
 	skb_record_rx_queue(skb, q->qid);
 
-	if (netif_rx(skb) == NET_RX_DROP) {
-		ndev->stats.rx_errors++;
-		ndev->stats.rx_dropped++;
-	} else {
-		ndev->stats.rx_packets++;
-		ndev->stats.rx_bytes += len;
-	}
+	netif_rx(skb);
+	ndev->stats.rx_packets++;
+	ndev->stats.rx_bytes += len;
 
 	skb = new_skb;
 
-- 
2.51.0


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

* [PATCH net 2/2] net: ntb_netdev: Count packets dropped on RX refill failure
  2026-08-19 17:25 [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting Koichiro Den
  2026-08-19 17:25 ` [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops Koichiro Den
@ 2026-08-19 17:25 ` Koichiro Den
  2026-08-22 20:20 ` [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: Koichiro Den @ 2026-08-19 17:25 UTC (permalink / raw)
  To: Jakub Kicinski, Jon Mason, Dave Jiang, Allen Hubbe, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Greg Kroah-Hartman,
	Nicholas Bellinger
  Cc: ntb, netdev, linux-kernel

When replacement skb allocation fails, ntb_netdev drops a packet that
was received successfully and requeues the original buffer. The drop is
counted, but rx_packets and rx_bytes are not.

Count every good packet before allocating its replacement.

Fixes: d2121faf133a ("NTB: ntb_netdev: Preserve RX queue depth on allocation failure")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/net/ntb_netdev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
index 4e53b00f016b..5bb0c8be00f9 100644
--- a/drivers/net/ntb_netdev.c
+++ b/drivers/net/ntb_netdev.c
@@ -144,6 +144,9 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 		goto enqueue_again;
 	}
 
+	ndev->stats.rx_packets++;
+	ndev->stats.rx_bytes += len;
+
 	new_skb = netdev_alloc_skb(ndev, ndev->mtu + ETH_HLEN);
 	if (!new_skb) {
 		ndev->stats.rx_dropped++;
@@ -156,8 +159,6 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
 	skb_record_rx_queue(skb, q->qid);
 
 	netif_rx(skb);
-	ndev->stats.rx_packets++;
-	ndev->stats.rx_bytes += len;
 
 	skb = new_skb;
 
-- 
2.51.0


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

* Re: [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops
  2026-08-19 17:25 ` [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops Koichiro Den
@ 2026-08-20 17:25   ` sashiko-bot
  0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-20 17:25 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:
- [Critical] Time-of-Check to Time-of-Use (TOCTOU) vulnerability in NTB receive processing allows a malicious remote peer to cause a massive kernel heap buffer overflow.
--

--- Patch [1]: [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops ---
commit 002045303db9fc237bf287acbb8f01a9873c837d
Author: Koichiro Den <den@valinux.co.jp>

net: ntb_netdev: Avoid double-accounting netif_rx() drops

netif_rx() already accounts packets it drops in the core rx_dropped
counter. ntb_netdev counts them again as both errors and drops.
Leave netif_rx() drops to the core. Count the packet and bytes
unconditionally since it was received successfully by the driver.

> diff --git a/drivers/net/ntb_netdev.c b/drivers/net/ntb_netdev.c
> index 029a4a532a104..4e53b00f016b0 100644
> --- a/drivers/net/ntb_netdev.c
> +++ b/drivers/net/ntb_netdev.c
> @@ -155,13 +155,9 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
>  	skb->ip_summed = CHECKSUM_NONE;
>  	skb_record_rx_queue(skb, q->qid);
>  
> -	if (netif_rx(skb) == NET_RX_DROP) {
> -		ndev->stats.rx_errors++;
> -		ndev->stats.rx_dropped++;
> -	} else {
> -		ndev->stats.rx_packets++;
> -		ndev->stats.rx_bytes += len;
> -	}
> +	netif_rx(skb);
> +	ndev->stats.rx_packets++;
> +	ndev->stats.rx_bytes += len;

[Severity: Critical]
This is a pre-existing issue, but does this expose a time-of-check to
time-of-use issue during ntb receive processing?

Looking at ntb_process_rxc() in drivers/ntb/ntb_transport.c, hdr directly
accesses the shared memory window which the remote peer can write to via
pcie. A bounds check is performed, but then hdr->len is re-read directly
from the shared memory:

drivers/ntb/ntb_transport.c:ntb_process_rxc() {
	if (hdr->len > entry->len) {
		...
	} else {
		...
		entry->len = hdr->len;

		ntb_async_rx(entry, offset);
	}
}

Can the remote peer modify hdr->len to a massive value after the bounds
check passes but before the second read?

If so, this bypassed length is then passed to ntb_async_rx() and
subsequently to ntb_memcpy_rx():

drivers/ntb/ntb_transport.c:ntb_memcpy_rx() {
	void *buf = entry->buf;
	size_t len = entry->len;

	memcpy(buf, offset, len);
}

Could this cause memcpy() to overflow the skb data buffer? Should the
payload header values be copied to local variables using READ_ONCE() before
validation to prevent this?

>  
>  	skb = new_skb;
>

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

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

* Re: [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting
  2026-08-19 17:25 [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting Koichiro Den
  2026-08-19 17:25 ` [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops Koichiro Den
  2026-08-19 17:25 ` [PATCH net 2/2] net: ntb_netdev: Count packets dropped on RX refill failure Koichiro Den
@ 2026-08-22 20:20 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-22 20:20 UTC (permalink / raw)
  To: Koichiro Den
  Cc: kuba, jdmason, dave.jiang, allenbh, andrew+netdev, davem,
	edumazet, pabeni, gregkh, nab, ntb, netdev, linux-kernel

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 20 Aug 2026 02:25:37 +0900 you wrote:
> Hi,
> 
> This series addresses Jakub's comment on ntb_netdev RX statistics
> accounting:
> https://lore.kernel.org/r/20260818092938.4121c220@kernel.org/
> 
> It fixes the double counting and also the related packet/byte accounting
> on RX refill failure.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops
    https://git.kernel.org/netdev/net/c/82e15be2d8b9
  - [net,2/2] net: ntb_netdev: Count packets dropped on RX refill failure
    https://git.kernel.org/netdev/net/c/31ded341c375

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-08-22 20:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19 17:25 [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting Koichiro Den
2026-08-19 17:25 ` [PATCH net 1/2] net: ntb_netdev: Avoid double-accounting netif_rx() drops Koichiro Den
2026-08-20 17:25   ` sashiko-bot
2026-08-19 17:25 ` [PATCH net 2/2] net: ntb_netdev: Count packets dropped on RX refill failure Koichiro Den
2026-08-22 20:20 ` [PATCH net 0/2] net: ntb_netdev: Fix RX statistics accounting patchwork-bot+netdevbpf

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