* [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection
@ 2026-05-29 13:15 Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 1/2] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Zahka @ 2026-05-29 13:15 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn
Cc: netdev, linux-kernel
It has come to my attention via a sashiko review of my net-next series
for aes-gcm in netdevsim [1] that there were preexisting issues with
netdevsim's implementation of psp statistics.
API usage issues:
1. not calling u64_stats_init() on the u64_stats_sync object during
init
2. not serializing usage of the writer side API during stats update
Logical Bugs:
1. We were incrementing rx stats on the sending devices stats
counters.
Fix the first set of issues by removing the u64_stats_t api entirely,
and keep track of stats with atomics. Fix the second issue by charging
events to the right netdevsim object.
[1]: https://sashiko.dev/#/patchset/20260508-nsim-psp-crypto-v1-0-4b50ed09b794%40gmail.com
TAP version 13
1..28
ok 1 psp.data_basic_send_v0_ip4
ok 2 psp.data_basic_send_v0_ip6
ok 3 psp.data_basic_send_v1_ip4
ok 4 psp.data_basic_send_v1_ip6
ok 5 psp.data_basic_send_v2_ip4
ok 6 psp.data_basic_send_v2_ip6
ok 7 psp.data_basic_send_v3_ip4
ok 8 psp.data_basic_send_v3_ip6
ok 9 psp.data_mss_adjust_ip4
ok 10 psp.data_mss_adjust_ip6
ok 11 psp.dev_list_devices
ok 12 psp.dev_get_device
ok 13 psp.dev_get_device_bad
ok 14 psp.dev_rotate
ok 15 psp.dev_rotate_spi
ok 16 psp.assoc_basic
ok 17 psp.assoc_bad_dev
ok 18 psp.assoc_sk_only_conn
ok 19 psp.assoc_sk_only_mismatch
ok 20 psp.assoc_sk_only_mismatch_tx
ok 21 psp.assoc_sk_only_unconn
ok 22 psp.assoc_version_mismatch
ok 23 psp.assoc_twice
ok 24 psp.data_send_bad_key
ok 25 psp.data_send_disconnect
ok 26 psp.data_stale_key
ok 27 psp.removal_device_rx
ok 28 psp.removal_device_bi
# Totals: pass:28 fail:0 xfail:0 xpass:0 skip:0 error:0
Dump stats on both devs tx on one should match rx on other:
local dev:
id=5 ifindex=2 stats={'dev-id': 5, 'key-rotations': 0,
'stale-events': 0, 'rx-packets': 1226, 'rx-bytes': 39244,
'rx-auth-fail': 0, 'rx-error': 0, 'rx-bad': 0, 'tx-packets': 1931,
'tx-bytes': 2478908, 'tx-error': 0}
remote dev:
id=3 ifindex=2 stats={'dev-id': 3, 'key-rotations': 0, 'stale-events':
0, 'rx-packets': 1931, 'rx-bytes': 2478908, 'rx-auth-fail': 0,
'rx-error': 0, 'rx-bad': 0, 'tx-packets': 1226, 'tx-bytes': 39244,
'tx-error': 0}
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Changes in v2:
- get rid of u64_stats_t and use atomics instead
- Link to v1: https://lore.kernel.org/r/20260513-fix-psp-stats-v1-0-cdb3174f252f@gmail.com
---
Daniel Zahka (2):
netdevsim: psp: update rx stats on the peer netdevsim
netdevsim: psp: use atomic64 for psp stats counters
drivers/net/netdevsim/netdevsim.h | 10 ++++------
drivers/net/netdevsim/psp.c | 27 +++++++++++----------------
2 files changed, 15 insertions(+), 22 deletions(-)
---
base-commit: 422b5233b607476ac7176bfa2a101b9a103d7653
change-id: 20260512-fix-psp-stats-e96c6d069d01
Best regards,
--
Daniel Zahka <daniel.zahka@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v2 1/2] netdevsim: psp: update rx stats on the peer netdevsim
2026-05-29 13:15 [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection Daniel Zahka
@ 2026-05-29 13:15 ` Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 2/2] netdevsim: psp: use atomic64 for psp stats counters Daniel Zahka
2026-06-02 20:00 ` [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Zahka @ 2026-05-29 13:15 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn
Cc: netdev, linux-kernel
nsim_do_psp() handles both tx and rx psp processing in the sending
device's nsim_start_xmit() path. The existing code has a logical bug,
where we erroneously increment rx_bytes and rx_packets on the sending
devices stats, instead of the peer device.
Additionally, compute psp_len after psp_dev_encapsulate() and before
psp_dev_rcv(), which modifies the header region of the skb. The
existing calculation was actually correct, because psp_dev_rcv()
leaves skb_inner_transport_header pointing at the tcp header, but this
is fragile and confusing as there is no actual inner transport header
after psp_dev_rcv has removed udp encapsulation.
Fixes: 178f0763c5f3 ("netdevsim: implement psp device stats")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
drivers/net/netdevsim/psp.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index 6936ecb8173e..afe58b21041a 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -22,6 +22,7 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
struct psp_dev *peer_psd;
struct psp_assoc *pas;
struct net *net;
+ int psp_len;
void **ptr;
rcu_read_lock();
@@ -48,6 +49,12 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
goto out_unlock;
}
+ psp_len = skb->len - skb_inner_transport_offset(skb);
+ u64_stats_update_begin(&ns->psp.syncp);
+ u64_stats_inc(&ns->psp.tx_packets);
+ u64_stats_add(&ns->psp.tx_bytes, psp_len);
+ u64_stats_update_end(&ns->psp.syncp);
+
/* Now pretend we just received this frame */
peer_psd = rcu_dereference(peer_ns->psp.dev);
if (peer_psd && peer_psd->config.versions & (1 << pas->version)) {
@@ -72,14 +79,10 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
refcount_inc(&(*psp_ext)->refcnt);
skb->decrypted = 1;
- u64_stats_update_begin(&ns->psp.syncp);
- u64_stats_inc(&ns->psp.tx_packets);
- u64_stats_inc(&ns->psp.rx_packets);
- u64_stats_add(&ns->psp.tx_bytes,
- skb->len - skb_inner_transport_offset(skb));
- u64_stats_add(&ns->psp.rx_bytes,
- skb->len - skb_inner_transport_offset(skb));
- u64_stats_update_end(&ns->psp.syncp);
+ u64_stats_update_begin(&peer_ns->psp.syncp);
+ u64_stats_inc(&peer_ns->psp.rx_packets);
+ u64_stats_add(&peer_ns->psp.rx_bytes, psp_len);
+ u64_stats_update_end(&peer_ns->psp.syncp);
} else {
struct ipv6hdr *ip6h __maybe_unused;
struct iphdr *iph;
--
2.52.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v2 2/2] netdevsim: psp: use atomic64 for psp stats counters
2026-05-29 13:15 [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 1/2] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
@ 2026-05-29 13:15 ` Daniel Zahka
2026-06-02 20:00 ` [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Zahka @ 2026-05-29 13:15 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn
Cc: netdev, linux-kernel
The existing u64_stats_t-based psp counters had two preexisting api
usage bugs: u64_stats_init() was never called on the syncp object, and
the writer side of the u64_stats_update_begin()/end() api was not
serialized. Switch the counters to atomic64_t instead. Atomics need
no initialization and are inherently safe against concurrent writers,
eliminating both bugs at once.
Use atomic64_t rather than atomic_long_t so byte counters don't wrap
at 4 GiB on 32-bit builds.
Fixes: 178f0763c5f3 ("netdevsim: implement psp device stats")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
drivers/net/netdevsim/netdevsim.h | 10 ++++------
drivers/net/netdevsim/psp.c | 24 ++++++++----------------
2 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index d909c4160ea1..4c9cc96dcec3 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -22,7 +22,6 @@
#include <linux/list.h>
#include <linux/netdevice.h>
#include <linux/ptp_mock.h>
-#include <linux/u64_stats_sync.h>
#include <net/devlink.h>
#include <net/udp_tunnel.h>
#include <net/xdp.h>
@@ -115,11 +114,10 @@ struct netdevsim {
int rq_reset_mode;
struct {
- u64_stats_t rx_packets;
- u64_stats_t rx_bytes;
- u64_stats_t tx_packets;
- u64_stats_t tx_bytes;
- struct u64_stats_sync syncp;
+ atomic64_t rx_packets;
+ atomic64_t rx_bytes;
+ atomic64_t tx_packets;
+ atomic64_t tx_bytes;
struct psp_dev __rcu *dev;
struct dentry *rereg;
struct mutex rereg_lock;
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index afe58b21041a..1c415b3d6f1a 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -50,10 +50,8 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
}
psp_len = skb->len - skb_inner_transport_offset(skb);
- u64_stats_update_begin(&ns->psp.syncp);
- u64_stats_inc(&ns->psp.tx_packets);
- u64_stats_add(&ns->psp.tx_bytes, psp_len);
- u64_stats_update_end(&ns->psp.syncp);
+ atomic64_inc(&ns->psp.tx_packets);
+ atomic64_add(psp_len, &ns->psp.tx_bytes);
/* Now pretend we just received this frame */
peer_psd = rcu_dereference(peer_ns->psp.dev);
@@ -79,10 +77,8 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
refcount_inc(&(*psp_ext)->refcnt);
skb->decrypted = 1;
- u64_stats_update_begin(&peer_ns->psp.syncp);
- u64_stats_inc(&peer_ns->psp.rx_packets);
- u64_stats_add(&peer_ns->psp.rx_bytes, psp_len);
- u64_stats_update_end(&peer_ns->psp.syncp);
+ atomic64_inc(&peer_ns->psp.rx_packets);
+ atomic64_add(psp_len, &peer_ns->psp.rx_bytes);
} else {
struct ipv6hdr *ip6h __maybe_unused;
struct iphdr *iph;
@@ -180,20 +176,16 @@ static void nsim_assoc_del(struct psp_dev *psd, struct psp_assoc *pas)
static void nsim_get_stats(struct psp_dev *psd, struct psp_dev_stats *stats)
{
struct netdevsim *ns = psd->drv_priv;
- unsigned int start;
/* WARNING: do *not* blindly zero stats in real drivers!
* All required stats must be reported by the device!
*/
memset(stats, 0, sizeof(struct psp_dev_stats));
- do {
- start = u64_stats_fetch_begin(&ns->psp.syncp);
- stats->rx_bytes = u64_stats_read(&ns->psp.rx_bytes);
- stats->rx_packets = u64_stats_read(&ns->psp.rx_packets);
- stats->tx_bytes = u64_stats_read(&ns->psp.tx_bytes);
- stats->tx_packets = u64_stats_read(&ns->psp.tx_packets);
- } while (u64_stats_fetch_retry(&ns->psp.syncp, start));
+ stats->rx_bytes = atomic64_read(&ns->psp.rx_bytes);
+ stats->rx_packets = atomic64_read(&ns->psp.rx_packets);
+ stats->tx_bytes = atomic64_read(&ns->psp.tx_bytes);
+ stats->tx_packets = atomic64_read(&ns->psp.tx_packets);
}
static struct psp_dev_ops nsim_psp_ops = {
--
2.52.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection
2026-05-29 13:15 [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 1/2] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 2/2] netdevsim: psp: use atomic64 for psp stats counters Daniel Zahka
@ 2026-06-02 20:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-06-02 20:00 UTC (permalink / raw)
To: Daniel Zahka
Cc: kuba, andrew+netdev, davem, edumazet, pabeni, willemb, netdev,
linux-kernel
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 29 May 2026 06:15:26 -0700 you wrote:
> It has come to my attention via a sashiko review of my net-next series
> for aes-gcm in netdevsim [1] that there were preexisting issues with
> netdevsim's implementation of psp statistics.
>
> API usage issues:
> 1. not calling u64_stats_init() on the u64_stats_sync object during
> init
> 2. not serializing usage of the writer side API during stats update
>
> [...]
Here is the summary with links:
- [net,v2,1/2] netdevsim: psp: update rx stats on the peer netdevsim
https://git.kernel.org/netdev/net-next/c/abaef7e966fd
- [net,v2,2/2] netdevsim: psp: use atomic64 for psp stats counters
https://git.kernel.org/netdev/net-next/c/163bea8010bd
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] 4+ messages in thread
end of thread, other threads:[~2026-06-02 20:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-29 13:15 [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 1/2] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
2026-05-29 13:15 ` [PATCH net v2 2/2] netdevsim: psp: use atomic64 for psp stats counters Daniel Zahka
2026-06-02 20:00 ` [PATCH net v2 0/2] netdevsim: psp: fix issues with stats collection 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
all inboxes | Powered by JetHome®