* [PATCH net 0/3] netdevsim: psp: fix issues with stats collection
@ 2026-05-13 12:59 Daniel Zahka
2026-05-13 12:59 ` [PATCH net 1/3] netdevsim: psp: initialize stats syncp before use Daniel Zahka
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daniel Zahka @ 2026-05-13 12:59 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.
[1]: https://sashiko.dev/#/patchset/20260508-nsim-psp-crypto-v1-0-4b50ed09b794%40gmail.com
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Daniel Zahka (3):
netdevsim: psp: initialize stats syncp before use
netdevsim: psp: update rx stats on the peer netdevsim
netdevsim: psp: serialize psp stats writers
drivers/net/netdevsim/netdevsim.h | 1 +
drivers/net/netdevsim/psp.c | 26 ++++++++++++++++++--------
2 files changed, 19 insertions(+), 8 deletions(-)
---
base-commit: f5b2772d14884f4be9e718644f1203d4d0e6f0d6
change-id: 20260512-fix-psp-stats-e96c6d069d01
Best regards,
--
Daniel Zahka <daniel.zahka@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/3] netdevsim: psp: initialize stats syncp before use
2026-05-13 12:59 [PATCH net 0/3] netdevsim: psp: fix issues with stats collection Daniel Zahka
@ 2026-05-13 12:59 ` Daniel Zahka
2026-05-13 12:59 ` [PATCH net 2/3] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
2026-05-13 12:59 ` [PATCH net 3/3] netdevsim: psp: serialize psp stats writers Daniel Zahka
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Zahka @ 2026-05-13 12:59 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn
Cc: netdev, linux-kernel
The reader and writer stats paths access this object, which was not
being initialized. Initialize in nsim_psp_init() before registering
the psp device.
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 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index 6936ecb8173e..92a2ed770a36 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -268,6 +268,8 @@ int nsim_psp_init(struct netdevsim *ns)
struct dentry *ddir = ns->nsim_dev_port->ddir;
struct psp_dev *psd;
+ u64_stats_init(&ns->psp.syncp);
+
psd = psp_dev_create(ns->netdev, &nsim_psp_ops, &nsim_psp_caps, ns);
if (IS_ERR(psd))
return PTR_ERR(psd);
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/3] netdevsim: psp: update rx stats on the peer netdevsim
2026-05-13 12:59 [PATCH net 0/3] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-13 12:59 ` [PATCH net 1/3] netdevsim: psp: initialize stats syncp before use Daniel Zahka
@ 2026-05-13 12:59 ` Daniel Zahka
2026-05-13 12:59 ` [PATCH net 3/3] netdevsim: psp: serialize psp stats writers Daniel Zahka
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Zahka @ 2026-05-13 12:59 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 92a2ed770a36..5ff91da24539 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 related [flat|nested] 6+ messages in thread
* [PATCH net 3/3] netdevsim: psp: serialize psp stats writers
2026-05-13 12:59 [PATCH net 0/3] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-13 12:59 ` [PATCH net 1/3] netdevsim: psp: initialize stats syncp before use Daniel Zahka
2026-05-13 12:59 ` [PATCH net 2/3] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
@ 2026-05-13 12:59 ` Daniel Zahka
2026-05-13 18:03 ` Jakub Kicinski
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Zahka @ 2026-05-13 12:59 UTC (permalink / raw)
To: Jakub Kicinski, Andrew Lunn, David S. Miller, Eric Dumazet,
Paolo Abeni, Willem de Bruijn
Cc: netdev, linux-kernel
The u64_stats_* api requires mutual exclusion on writers. The simplest
way to do this is just to add a spinlock to the writer path, rather
than making the stats per queue.
Synchronization of the reader and writer paths use the
u64_stats_update_begin() api and does not require any fixes.
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 | 1 +
drivers/net/netdevsim/psp.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index d909c4160ea1..bc36033b511b 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -115,6 +115,7 @@ struct netdevsim {
int rq_reset_mode;
struct {
+ spinlock_t stats_lock;
u64_stats_t rx_packets;
u64_stats_t rx_bytes;
u64_stats_t tx_packets;
diff --git a/drivers/net/netdevsim/psp.c b/drivers/net/netdevsim/psp.c
index 5ff91da24539..56ddbf146489 100644
--- a/drivers/net/netdevsim/psp.c
+++ b/drivers/net/netdevsim/psp.c
@@ -50,10 +50,12 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
}
psp_len = skb->len - skb_inner_transport_offset(skb);
+ spin_lock_bh(&ns->psp.stats_lock);
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);
+ spin_unlock_bh(&ns->psp.stats_lock);
/* Now pretend we just received this frame */
peer_psd = rcu_dereference(peer_ns->psp.dev);
@@ -79,10 +81,12 @@ nsim_do_psp(struct sk_buff *skb, struct netdevsim *ns,
refcount_inc(&(*psp_ext)->refcnt);
skb->decrypted = 1;
+ spin_lock_bh(&peer_ns->psp.stats_lock);
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);
+ spin_unlock_bh(&peer_ns->psp.stats_lock);
} else {
struct ipv6hdr *ip6h __maybe_unused;
struct iphdr *iph;
@@ -271,6 +275,7 @@ int nsim_psp_init(struct netdevsim *ns)
struct dentry *ddir = ns->nsim_dev_port->ddir;
struct psp_dev *psd;
+ spin_lock_init(&ns->psp.stats_lock);
u64_stats_init(&ns->psp.syncp);
psd = psp_dev_create(ns->netdev, &nsim_psp_ops, &nsim_psp_caps, ns);
--
2.52.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 3/3] netdevsim: psp: serialize psp stats writers
2026-05-13 12:59 ` [PATCH net 3/3] netdevsim: psp: serialize psp stats writers Daniel Zahka
@ 2026-05-13 18:03 ` Jakub Kicinski
2026-05-13 18:05 ` Daniel Zahka
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2026-05-13 18:03 UTC (permalink / raw)
To: Daniel Zahka
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, netdev, linux-kernel
On Wed, 13 May 2026 05:59:38 -0700 Daniel Zahka wrote:
> The u64_stats_* api requires mutual exclusion on writers. The simplest
> way to do this is just to add a spinlock to the writer path, rather
> than making the stats per queue.
>
> Synchronization of the reader and writer paths use the
> u64_stats_update_begin() api and does not require any fixes.
The spin lock makes the use of u64_stats_t a bit pointless.
Let's switch to atomic_long instead?
--
pw-bot: cr
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 3/3] netdevsim: psp: serialize psp stats writers
2026-05-13 18:03 ` Jakub Kicinski
@ 2026-05-13 18:05 ` Daniel Zahka
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Zahka @ 2026-05-13 18:05 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Willem de Bruijn, netdev, linux-kernel
On 5/13/26 2:03 PM, Jakub Kicinski wrote:
> On Wed, 13 May 2026 05:59:38 -0700 Daniel Zahka wrote:
>> The u64_stats_* api requires mutual exclusion on writers. The simplest
>> way to do this is just to add a spinlock to the writer path, rather
>> than making the stats per queue.
>>
>> Synchronization of the reader and writer paths use the
>> u64_stats_update_begin() api and does not require any fixes.
> The spin lock makes the use of u64_stats_t a bit pointless.
> Let's switch to atomic_long instead?
Makes sense. I'll repost the series.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-05-13 18:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 12:59 [PATCH net 0/3] netdevsim: psp: fix issues with stats collection Daniel Zahka
2026-05-13 12:59 ` [PATCH net 1/3] netdevsim: psp: initialize stats syncp before use Daniel Zahka
2026-05-13 12:59 ` [PATCH net 2/3] netdevsim: psp: update rx stats on the peer netdevsim Daniel Zahka
2026-05-13 12:59 ` [PATCH net 3/3] netdevsim: psp: serialize psp stats writers Daniel Zahka
2026-05-13 18:03 ` Jakub Kicinski
2026-05-13 18:05 ` Daniel Zahka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox