From f9e2511d80c2eaeb940c5f8280d7eea9d7946ece Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Wed, 18 Jun 2025 01:32:42 -0700 Subject: [PATCH 1/4] netdevsim: migrate to dstats stats collection Replace custom statistics tracking with the kernel's dstats infrastructure to simplify code and improve consistency with other network drivers. This change: - Sets dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS for automatic automatic allocation and deallocation. - Removes manual stats fields and their update - Replaces custom nsim_get_stats64() with dev_get_stats() - Uses dev_dstats_tx_add() and dev_dstats_tx_dropped() helpers - Eliminates the need for manual synchronization primitives The dstats framework provides the same functionality with less code. Suggested-by: Jakub Kicinski Reviewed-by: Joe Damato Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20250618-netdevsim_stat-v4-1-19fe0d35e28e@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netdevsim/netdev.c | 33 ++++++------------------------- drivers/net/netdevsim/netdevsim.h | 5 ----- 2 files changed, 6 insertions(+), 32 deletions(-) diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index fa5fbd97ad69..5010d8eefc85 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -93,19 +93,14 @@ static netdev_tx_t nsim_start_xmit(struct sk_buff *skb, struct net_device *dev) hrtimer_start(&rq->napi_timer, us_to_ktime(5), HRTIMER_MODE_REL); rcu_read_unlock(); - u64_stats_update_begin(&ns->syncp); - ns->tx_packets++; - ns->tx_bytes += len; - u64_stats_update_end(&ns->syncp); + dev_dstats_tx_add(dev, skb->len); return NETDEV_TX_OK; out_drop_free: dev_kfree_skb(skb); out_drop_cnt: rcu_read_unlock(); - u64_stats_update_begin(&ns->syncp); - ns->tx_dropped++; - u64_stats_update_end(&ns->syncp); + dev_dstats_tx_dropped(dev); return NETDEV_TX_OK; } @@ -126,20 +121,6 @@ static int nsim_change_mtu(struct net_device *dev, int new_mtu) return 0; } -static void -nsim_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats) -{ - struct netdevsim *ns = netdev_priv(dev); - unsigned int start; - - do { - start = u64_stats_fetch_begin(&ns->syncp); - stats->tx_bytes = ns->tx_bytes; - stats->tx_packets = ns->tx_packets; - stats->tx_dropped = ns->tx_dropped; - } while (u64_stats_fetch_retry(&ns->syncp, start)); -} - static int nsim_setup_tc_block_cb(enum tc_setup_type type, void *type_data, void *cb_priv) { @@ -556,7 +537,6 @@ static const struct net_device_ops nsim_netdev_ops = { .ndo_set_mac_address = eth_mac_addr, .ndo_validate_addr = eth_validate_addr, .ndo_change_mtu = nsim_change_mtu, - .ndo_get_stats64 = nsim_get_stats64, .ndo_set_vf_mac = nsim_set_vf_mac, .ndo_set_vf_vlan = nsim_set_vf_vlan, .ndo_set_vf_rate = nsim_set_vf_rate, @@ -580,7 +560,6 @@ static const struct net_device_ops nsim_vf_netdev_ops = { .ndo_set_mac_address = eth_mac_addr, .ndo_validate_addr = eth_validate_addr, .ndo_change_mtu = nsim_change_mtu, - .ndo_get_stats64 = nsim_get_stats64, .ndo_setup_tc = nsim_setup_tc, .ndo_set_features = nsim_set_features, }; @@ -594,7 +573,7 @@ static void nsim_get_queue_stats_rx(struct net_device *dev, int idx, struct rtnl_link_stats64 rtstats = {}; if (!idx) - nsim_get_stats64(dev, &rtstats); + dev_get_stats(dev, &rtstats); stats->packets = rtstats.rx_packets - !!rtstats.rx_packets; stats->bytes = rtstats.rx_bytes; @@ -606,7 +585,7 @@ static void nsim_get_queue_stats_tx(struct net_device *dev, int idx, struct rtnl_link_stats64 rtstats = {}; if (!idx) - nsim_get_stats64(dev, &rtstats); + dev_get_stats(dev, &rtstats); stats->packets = rtstats.tx_packets - !!rtstats.tx_packets; stats->bytes = rtstats.tx_bytes; @@ -618,7 +597,7 @@ static void nsim_get_base_stats(struct net_device *dev, { struct rtnl_link_stats64 rtstats = {}; - nsim_get_stats64(dev, &rtstats); + dev_get_stats(dev, &rtstats); rx->packets = !!rtstats.rx_packets; rx->bytes = 0; @@ -890,6 +869,7 @@ static void nsim_setup(struct net_device *dev) NETIF_F_HW_CSUM | NETIF_F_LRO | NETIF_F_TSO; + dev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS; dev->max_mtu = ETH_MAX_MTU; dev->xdp_features = NETDEV_XDP_ACT_HW_OFFLOAD; } @@ -1022,7 +1002,6 @@ nsim_create(struct nsim_dev *nsim_dev, struct nsim_dev_port *nsim_dev_port) dev_net_set(dev, nsim_dev_net(nsim_dev)); ns = netdev_priv(dev); ns->netdev = dev; - u64_stats_init(&ns->syncp); ns->nsim_dev = nsim_dev; ns->nsim_dev_port = nsim_dev_port; ns->nsim_bus_dev = nsim_dev->nsim_bus_dev; diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h index 511ed72a93ce..4a0c48c7a384 100644 --- a/drivers/net/netdevsim/netdevsim.h +++ b/drivers/net/netdevsim/netdevsim.h @@ -108,11 +108,6 @@ struct netdevsim { int rq_reset_mode; - u64 tx_packets; - u64 tx_bytes; - u64 tx_dropped; - struct u64_stats_sync syncp; - struct nsim_bus_dev *nsim_bus_dev; struct bpf_prog *bpf_offloaded; From 788eb4de608bbebad237674e1057305340d653ba Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Wed, 18 Jun 2025 01:32:43 -0700 Subject: [PATCH 2/4] netdevsim: collect statistics at RX side When the RX side of netdevsim was added, the RX statistics were missing, making the driver unusable for GenerateTraffic() test framework. This patch adds proper statistics tracking on RX side, complementing the TX path. Reviewed-by: Joe Damato Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20250618-netdevsim_stat-v4-2-19fe0d35e28e@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netdevsim/netdev.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index 5010d8eefc85..3ea14f5c5e0c 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -331,16 +331,24 @@ static int nsim_get_iflink(const struct net_device *dev) static int nsim_rcv(struct nsim_rq *rq, int budget) { + struct net_device *dev = rq->napi.dev; struct sk_buff *skb; - int i; + unsigned int skblen; + int i, ret; for (i = 0; i < budget; i++) { if (skb_queue_empty(&rq->skb_queue)) break; skb = skb_dequeue(&rq->skb_queue); + /* skb might be discard at netif_receive_skb, save the len */ + skblen = skb->len; skb_mark_napi_id(skb, &rq->napi); - netif_receive_skb(skb); + ret = netif_receive_skb(skb); + if (ret == NET_RX_SUCCESS) + dev_dstats_rx_add(dev, skblen); + else + dev_dstats_rx_dropped(dev); } return i; From 27480a7c8f0274f8f2fc6c40e4522f38e52bd05f Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Wed, 18 Jun 2025 01:32:44 -0700 Subject: [PATCH 3/4] net: add dev_dstats_rx_dropped_add() helper Introduce the dev_dstats_rx_dropped_add() helper to allow incrementing the rx_drops per-CPU statistic by an arbitrary value, rather than just one. This is useful for drivers or code paths that need to account for multiple dropped packets at once, such as when dropping entire queues. Reviewed-by: Joe Damato Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20250618-netdevsim_stat-v4-3-19fe0d35e28e@debian.org Signed-off-by: Jakub Kicinski --- include/linux/netdevice.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 9cbc4e54b7e4..03c26bb0fbbe 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -3016,6 +3016,16 @@ static inline void dev_dstats_rx_dropped(struct net_device *dev) u64_stats_update_end(&dstats->syncp); } +static inline void dev_dstats_rx_dropped_add(struct net_device *dev, + unsigned int packets) +{ + struct pcpu_dstats *dstats = this_cpu_ptr(dev->dstats); + + u64_stats_update_begin(&dstats->syncp); + u64_stats_add(&dstats->rx_drops, packets); + u64_stats_update_end(&dstats->syncp); +} + static inline void dev_dstats_tx_add(struct net_device *dev, unsigned int len) { From 2a68a22304f90e5ee960cccd86895a50cf154723 Mon Sep 17 00:00:00 2001 From: Breno Leitao Date: Wed, 18 Jun 2025 01:32:45 -0700 Subject: [PATCH 4/4] netdevsim: account dropped packet length in stats on queue free Add a call to dev_dstats_rx_dropped_add() in nsim_queue_free() to account for the number of packets dropped when purging the skb queue. This improves the accuracy of RX drop statistics reported by netdevsim. local_bh_{disable, enable}() protection is used to disable preemption, which is necessary given that dev_dstats_rx_dropped_add() access this_cpu_ptr(). See discussion in [1]. Link: https://lore.kernel.org/all/20250617055934.3fd9d322@kernel.org/ [1] Suggested-by: Jakub Kicinski Signed-off-by: Breno Leitao Link: https://patch.msgid.link/20250618-netdevsim_stat-v4-4-19fe0d35e28e@debian.org Signed-off-by: Jakub Kicinski --- drivers/net/netdevsim/netdev.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c index 3ea14f5c5e0c..7f2809be5d48 100644 --- a/drivers/net/netdevsim/netdev.c +++ b/drivers/net/netdevsim/netdev.c @@ -632,9 +632,12 @@ static struct nsim_rq *nsim_queue_alloc(void) return rq; } -static void nsim_queue_free(struct nsim_rq *rq) +static void nsim_queue_free(struct net_device *dev, struct nsim_rq *rq) { hrtimer_cancel(&rq->napi_timer); + local_bh_disable(); + dev_dstats_rx_dropped_add(dev, rq->skb_queue.qlen); + local_bh_enable(); skb_queue_purge_reason(&rq->skb_queue, SKB_DROP_REASON_QUEUE_PURGE); kfree(rq); } @@ -681,7 +684,7 @@ nsim_queue_mem_alloc(struct net_device *dev, void *per_queue_mem, int idx) return 0; err_free: - nsim_queue_free(qmem->rq); + nsim_queue_free(dev, qmem->rq); return err; } @@ -695,7 +698,7 @@ static void nsim_queue_mem_free(struct net_device *dev, void *per_queue_mem) if (!ns->rq_reset_mode) netif_napi_del_locked(&qmem->rq->napi); page_pool_destroy(qmem->rq->page_pool); - nsim_queue_free(qmem->rq); + nsim_queue_free(dev, qmem->rq); } } @@ -913,7 +916,7 @@ static void nsim_queue_uninit(struct netdevsim *ns) int i; for (i = 0; i < dev->num_rx_queues; i++) - nsim_queue_free(ns->rq[i]); + nsim_queue_free(dev, ns->rq[i]); kfree(ns->rq); ns->rq = NULL;