* [PATCH net v9 1/4] octeon_ep: remove firmware stats fetch in ndo_get_stats64
2025-01-17 9:46 [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 Shinas Rasheed
@ 2025-01-17 9:46 ` Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 2/4] octeon_ep: update tx/rx stats locally for persistence Shinas Rasheed
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shinas Rasheed @ 2025-01-17 9:46 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, thaller, wizhao, kheib, konguyen, horms,
einstein.xue, Shinas Rasheed, Veerasenareddy Burru, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Abhijit Ayarekar, Satananda Burla
The firmware stats fetch call that happens in ndo_get_stats64()
is currently not required, and causes a warning to issue.
The warn log is given below:
[ 123.316837] ------------[ cut here ]------------
[ 123.316840] Voluntary context switch within RCU read-side critical section!
[ 123.316917] pc : rcu_note_context_switch+0x2e4/0x300
[ 123.316919] lr : rcu_note_context_switch+0x2e4/0x300
[ 123.316947] Call trace:
[ 123.316949] rcu_note_context_switch+0x2e4/0x300
[ 123.316952] __schedule+0x84/0x584
[ 123.316955] schedule+0x38/0x90
[ 123.316956] schedule_timeout+0xa0/0x1d4
[ 123.316959] octep_send_mbox_req+0x190/0x230 [octeon_ep]
[ 123.316966] octep_ctrl_net_get_if_stats+0x78/0x100 [octeon_ep]
[ 123.316970] octep_get_stats64+0xd4/0xf0 [octeon_ep]
[ 123.316975] dev_get_stats+0x4c/0x114
[ 123.316977] dev_seq_printf_stats+0x3c/0x11c
[ 123.316980] dev_seq_show+0x1c/0x40
[ 123.316982] seq_read_iter+0x3cc/0x4e0
[ 123.316985] seq_read+0xc8/0x110
[ 123.316987] proc_reg_read+0x9c/0xec
[ 123.316990] vfs_read+0xc8/0x2ec
[ 123.316993] ksys_read+0x70/0x100
[ 123.316995] __arm64_sys_read+0x20/0x30
[ 123.316997] invoke_syscall.constprop.0+0x7c/0xd0
[ 123.317000] do_el0_svc+0xb4/0xd0
[ 123.317002] el0_svc+0xe8/0x1f4
[ 123.317005] el0t_64_sync_handler+0x134/0x150
[ 123.317006] el0t_64_sync+0x17c/0x180
[ 123.317008] ---[ end trace 63399811432ab69b ]---
Fixes: 6a610a46bad1 ("octeon_ep: add support for ndo ops")
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V9:
- Update commit messages to reflect reordering.
V8: https://lore.kernel.org/all/20250116083825.2581885-2-srasheed@marvell.com/
- Reordered patch
V7: https://lore.kernel.org/all/20250114125124.2570660-3-srasheed@marvell.com/
- No changes
V6: https://lore.kernel.org/all/20250110122730.2551863-3-srasheed@marvell.com/
- Corrected patch to apply properly
V5: https://lore.kernel.org/all/20250109103221.2544467-3-srasheed@marvell.com/
- No changes
V4: https://lore.kernel.org/all/20250102112246.2494230-3-srasheed@marvell.com/
- No changes
V3: https://lore.kernel.org/all/20241218115111.2407958-3-srasheed@marvell.com/
- Added warn log that happened due to rcu_read_lock in commit message
V2: https://lore.kernel.org/all/20241216075842.2394606-3-srasheed@marvell.com/
- No changes
V1: https://lore.kernel.org/all/20241203072130.2316913-3-srasheed@marvell.com/
drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 549436efc204..730aa5632cce 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -995,12 +995,6 @@ static void octep_get_stats64(struct net_device *netdev,
struct octep_device *oct = netdev_priv(netdev);
int q;
- if (netif_running(netdev))
- octep_ctrl_net_get_if_stats(oct,
- OCTEP_CTRL_NET_INVALID_VFID,
- &oct->iface_rx_stats,
- &oct->iface_tx_stats);
-
tx_packets = 0;
tx_bytes = 0;
rx_packets = 0;
@@ -1018,10 +1012,6 @@ static void octep_get_stats64(struct net_device *netdev,
stats->tx_bytes = tx_bytes;
stats->rx_packets = rx_packets;
stats->rx_bytes = rx_bytes;
- stats->multicast = oct->iface_rx_stats.mcast_pkts;
- stats->rx_errors = oct->iface_rx_stats.err_pkts;
- stats->collisions = oct->iface_tx_stats.xscol;
- stats->tx_fifo_errors = oct->iface_tx_stats.undflw;
}
/**
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net v9 2/4] octeon_ep: update tx/rx stats locally for persistence
2025-01-17 9:46 [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 1/4] octeon_ep: remove firmware stats fetch " Shinas Rasheed
@ 2025-01-17 9:46 ` Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 3/4] octeon_ep_vf: remove firmware stats fetch in ndo_get_stats64 Shinas Rasheed
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Shinas Rasheed @ 2025-01-17 9:46 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, thaller, wizhao, kheib, konguyen, horms,
einstein.xue, Shinas Rasheed, Veerasenareddy Burru, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Update tx/rx stats locally, so that ndo_get_stats64()
can use that and not rely on per queue resources to obtain statistics.
The latter used to cause race conditions when the device stopped.
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V9:
- Iterate over OCTEP_MAX_QUEUES in the ndo_get_stats64() function,
rather than just the active queues.
V8: https://lore.kernel.org/all/20250116083825.2581885-3-srasheed@marvell.com/
- Reordered patch
V7: https://lore.kernel.org/all/20250114125124.2570660-2-srasheed@marvell.com/
- Updated octep_get_stats64() to be reentrant
V6: https://lore.kernel.org/all/20250110122730.2551863-2-srasheed@marvell.com/
- No changes
V5: https://lore.kernel.org/all/20250109103221.2544467-2-srasheed@marvell.com/
- Patch introduced
.../marvell/octeon_ep/octep_ethtool.c | 41 ++++++++-----------
.../ethernet/marvell/octeon_ep/octep_main.c | 19 ++++-----
.../ethernet/marvell/octeon_ep/octep_main.h | 6 +++
.../net/ethernet/marvell/octeon_ep/octep_rx.c | 11 ++---
.../net/ethernet/marvell/octeon_ep/octep_rx.h | 4 +-
.../net/ethernet/marvell/octeon_ep/octep_tx.c | 7 ++--
.../net/ethernet/marvell/octeon_ep/octep_tx.h | 4 +-
7 files changed, 45 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c b/drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c
index 4f4d58189118..a88c006ea65b 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_ethtool.c
@@ -150,17 +150,14 @@ octep_get_ethtool_stats(struct net_device *netdev,
iface_rx_stats,
iface_tx_stats);
- for (q = 0; q < oct->num_oqs; q++) {
- struct octep_iq *iq = oct->iq[q];
- struct octep_oq *oq = oct->oq[q];
-
- tx_packets += iq->stats.instr_completed;
- tx_bytes += iq->stats.bytes_sent;
- tx_busy_errors += iq->stats.tx_busy;
-
- rx_packets += oq->stats.packets;
- rx_bytes += oq->stats.bytes;
- rx_alloc_errors += oq->stats.alloc_failures;
+ for (q = 0; q < OCTEP_MAX_QUEUES; q++) {
+ tx_packets += oct->stats_iq[q].instr_completed;
+ tx_bytes += oct->stats_iq[q].bytes_sent;
+ tx_busy_errors += oct->stats_iq[q].tx_busy;
+
+ rx_packets += oct->stats_oq[q].packets;
+ rx_bytes += oct->stats_oq[q].bytes;
+ rx_alloc_errors += oct->stats_oq[q].alloc_failures;
}
i = 0;
data[i++] = rx_packets;
@@ -198,22 +195,18 @@ octep_get_ethtool_stats(struct net_device *netdev,
data[i++] = iface_rx_stats->err_pkts;
/* Per Tx Queue stats */
- for (q = 0; q < oct->num_iqs; q++) {
- struct octep_iq *iq = oct->iq[q];
-
- data[i++] = iq->stats.instr_posted;
- data[i++] = iq->stats.instr_completed;
- data[i++] = iq->stats.bytes_sent;
- data[i++] = iq->stats.tx_busy;
+ for (q = 0; q < OCTEP_MAX_QUEUES; q++) {
+ data[i++] = oct->stats_iq[q].instr_posted;
+ data[i++] = oct->stats_iq[q].instr_completed;
+ data[i++] = oct->stats_iq[q].bytes_sent;
+ data[i++] = oct->stats_iq[q].tx_busy;
}
/* Per Rx Queue stats */
- for (q = 0; q < oct->num_oqs; q++) {
- struct octep_oq *oq = oct->oq[q];
-
- data[i++] = oq->stats.packets;
- data[i++] = oq->stats.bytes;
- data[i++] = oq->stats.alloc_failures;
+ for (q = 0; q < OCTEP_MAX_QUEUES; q++) {
+ data[i++] = oct->stats_oq[q].packets;
+ data[i++] = oct->stats_oq[q].bytes;
+ data[i++] = oct->stats_oq[q].alloc_failures;
}
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 730aa5632cce..a89f80bac39b 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -822,7 +822,7 @@ static inline int octep_iq_full_check(struct octep_iq *iq)
if (unlikely(IQ_INSTR_SPACE(iq) >
OCTEP_WAKE_QUEUE_THRESHOLD)) {
netif_start_subqueue(iq->netdev, iq->q_no);
- iq->stats.restart_cnt++;
+ iq->stats->restart_cnt++;
return 0;
}
@@ -960,7 +960,7 @@ static netdev_tx_t octep_start_xmit(struct sk_buff *skb,
wmb();
/* Ring Doorbell to notify the NIC of new packets */
writel(iq->fill_cnt, iq->doorbell_reg);
- iq->stats.instr_posted += iq->fill_cnt;
+ iq->stats->instr_posted += iq->fill_cnt;
iq->fill_cnt = 0;
return NETDEV_TX_OK;
@@ -991,22 +991,19 @@ static netdev_tx_t octep_start_xmit(struct sk_buff *skb,
static void octep_get_stats64(struct net_device *netdev,
struct rtnl_link_stats64 *stats)
{
- u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
struct octep_device *oct = netdev_priv(netdev);
+ u64 tx_packets, tx_bytes, rx_packets, rx_bytes;
int q;
tx_packets = 0;
tx_bytes = 0;
rx_packets = 0;
rx_bytes = 0;
- for (q = 0; q < oct->num_oqs; q++) {
- struct octep_iq *iq = oct->iq[q];
- struct octep_oq *oq = oct->oq[q];
-
- tx_packets += iq->stats.instr_completed;
- tx_bytes += iq->stats.bytes_sent;
- rx_packets += oq->stats.packets;
- rx_bytes += oq->stats.bytes;
+ for (q = 0; q < OCTEP_MAX_QUEUES; q++) {
+ tx_packets += oct->stats_iq[q].instr_completed;
+ tx_bytes += oct->stats_iq[q].bytes_sent;
+ rx_packets += oct->stats_oq[q].packets;
+ rx_bytes += oct->stats_oq[q].bytes;
}
stats->tx_packets = tx_packets;
stats->tx_bytes = tx_bytes;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
index fee59e0e0138..936b786f4281 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
@@ -257,11 +257,17 @@ struct octep_device {
/* Pointers to Octeon Tx queues */
struct octep_iq *iq[OCTEP_MAX_IQ];
+ /* Per iq stats */
+ struct octep_iq_stats stats_iq[OCTEP_MAX_IQ];
+
/* Rx queues (OQ: Output Queue) */
u16 num_oqs;
/* Pointers to Octeon Rx queues */
struct octep_oq *oq[OCTEP_MAX_OQ];
+ /* Per oq stats */
+ struct octep_oq_stats stats_oq[OCTEP_MAX_OQ];
+
/* Hardware port number of the PCIe interface */
u16 pcie_port;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
index 8af75cb37c3e..82b6b19e76b4 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
@@ -87,7 +87,7 @@ static int octep_oq_refill(struct octep_device *oct, struct octep_oq *oq)
page = dev_alloc_page();
if (unlikely(!page)) {
dev_err(oq->dev, "refill: rx buffer alloc failed\n");
- oq->stats.alloc_failures++;
+ oq->stats->alloc_failures++;
break;
}
@@ -98,7 +98,7 @@ static int octep_oq_refill(struct octep_device *oct, struct octep_oq *oq)
"OQ-%d buffer refill: DMA mapping error!\n",
oq->q_no);
put_page(page);
- oq->stats.alloc_failures++;
+ oq->stats->alloc_failures++;
break;
}
oq->buff_info[refill_idx].page = page;
@@ -134,6 +134,7 @@ static int octep_setup_oq(struct octep_device *oct, int q_no)
oq->netdev = oct->netdev;
oq->dev = &oct->pdev->dev;
oq->q_no = q_no;
+ oq->stats = &oct->stats_oq[q_no];
oq->max_count = CFG_GET_OQ_NUM_DESC(oct->conf);
oq->ring_size_mask = oq->max_count - 1;
oq->buffer_size = CFG_GET_OQ_BUF_SIZE(oct->conf);
@@ -443,7 +444,7 @@ static int __octep_oq_process_rx(struct octep_device *oct,
if (!skb) {
octep_oq_drop_rx(oq, buff_info,
&read_idx, &desc_used);
- oq->stats.alloc_failures++;
+ oq->stats->alloc_failures++;
continue;
}
skb_reserve(skb, data_offset);
@@ -494,8 +495,8 @@ static int __octep_oq_process_rx(struct octep_device *oct,
oq->host_read_idx = read_idx;
oq->refill_count += desc_used;
- oq->stats.packets += pkt;
- oq->stats.bytes += rx_bytes;
+ oq->stats->packets += pkt;
+ oq->stats->bytes += rx_bytes;
return pkt;
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
index 3b08e2d560dc..b4696c93d0e6 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.h
@@ -186,8 +186,8 @@ struct octep_oq {
*/
u8 __iomem *pkts_sent_reg;
- /* Statistics for this OQ. */
- struct octep_oq_stats stats;
+ /* Pointer to statistics for this OQ. */
+ struct octep_oq_stats *stats;
/* Packets pending to be processed */
u32 pkts_pending;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
index 06851b78aa28..08ee90013fef 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.c
@@ -81,9 +81,9 @@ int octep_iq_process_completions(struct octep_iq *iq, u16 budget)
}
iq->pkts_processed += compl_pkts;
- iq->stats.instr_completed += compl_pkts;
- iq->stats.bytes_sent += compl_bytes;
- iq->stats.sgentry_sent += compl_sg;
+ iq->stats->instr_completed += compl_pkts;
+ iq->stats->bytes_sent += compl_bytes;
+ iq->stats->sgentry_sent += compl_sg;
iq->flush_index = fi;
netdev_tx_completed_queue(iq->netdev_q, compl_pkts, compl_bytes);
@@ -187,6 +187,7 @@ static int octep_setup_iq(struct octep_device *oct, int q_no)
iq->netdev = oct->netdev;
iq->dev = &oct->pdev->dev;
iq->q_no = q_no;
+ iq->stats = &oct->stats_iq[q_no];
iq->max_count = CFG_GET_IQ_NUM_DESC(oct->conf);
iq->ring_size_mask = iq->max_count - 1;
iq->fill_threshold = CFG_GET_IQ_DB_MIN(oct->conf);
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
index 875a2c34091f..58fb39dda977 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_tx.h
@@ -170,8 +170,8 @@ struct octep_iq {
*/
u16 flush_index;
- /* Statistics for this input queue. */
- struct octep_iq_stats stats;
+ /* Pointer to statistics for this input queue. */
+ struct octep_iq_stats *stats;
/* Pointer to the Virtual Base addr of the input ring. */
struct octep_tx_desc_hw *desc_ring;
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net v9 3/4] octeon_ep_vf: remove firmware stats fetch in ndo_get_stats64
2025-01-17 9:46 [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 1/4] octeon_ep: remove firmware stats fetch " Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 2/4] octeon_ep: update tx/rx stats locally for persistence Shinas Rasheed
@ 2025-01-17 9:46 ` Shinas Rasheed
2025-01-17 9:46 ` [PATCH net v9 4/4] octeon_ep_vf: update tx/rx stats locally for persistence Shinas Rasheed
2025-01-19 2:00 ` [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Shinas Rasheed @ 2025-01-17 9:46 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, thaller, wizhao, kheib, konguyen, horms,
einstein.xue, Shinas Rasheed, Veerasenareddy Burru,
Satananda Burla, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
The firmware stats fetch call that happens in ndo_get_stats64()
is currently not required, and causes a warning to issue.
The corresponding warn log for the PF is given below:
[ 123.316837] ------------[ cut here ]------------
[ 123.316840] Voluntary context switch within RCU read-side critical section!
[ 123.316917] pc : rcu_note_context_switch+0x2e4/0x300
[ 123.316919] lr : rcu_note_context_switch+0x2e4/0x300
[ 123.316947] Call trace:
[ 123.316949] rcu_note_context_switch+0x2e4/0x300
[ 123.316952] __schedule+0x84/0x584
[ 123.316955] schedule+0x38/0x90
[ 123.316956] schedule_timeout+0xa0/0x1d4
[ 123.316959] octep_send_mbox_req+0x190/0x230 [octeon_ep]
[ 123.316966] octep_ctrl_net_get_if_stats+0x78/0x100 [octeon_ep]
[ 123.316970] octep_get_stats64+0xd4/0xf0 [octeon_ep]
[ 123.316975] dev_get_stats+0x4c/0x114
[ 123.316977] dev_seq_printf_stats+0x3c/0x11c
[ 123.316980] dev_seq_show+0x1c/0x40
[ 123.316982] seq_read_iter+0x3cc/0x4e0
[ 123.316985] seq_read+0xc8/0x110
[ 123.316987] proc_reg_read+0x9c/0xec
[ 123.316990] vfs_read+0xc8/0x2ec
[ 123.316993] ksys_read+0x70/0x100
[ 123.316995] __arm64_sys_read+0x20/0x30
[ 123.316997] invoke_syscall.constprop.0+0x7c/0xd0
[ 123.317000] do_el0_svc+0xb4/0xd0
[ 123.317002] el0_svc+0xe8/0x1f4
[ 123.317005] el0t_64_sync_handler+0x134/0x150
[ 123.317006] el0t_64_sync+0x17c/0x180
[ 123.317008] ---[ end trace 63399811432ab69b ]---
Fixes: c3fad23cdc06 ("octeon_ep_vf: add support for ndo ops")
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V9:
- Update commit message to reflect reordering. Make note that the warn log is from
the PF driver.
V8: https://lore.kernel.org/all/20250116083825.2581885-4-srasheed@marvell.com/
- Reordered patch
V7: https://lore.kernel.org/all/20250114125124.2570660-5-srasheed@marvell.com/
- No changes
V6: https://lore.kernel.org/all/20250110122730.2551863-5-srasheed@marvell.com/
- No changes
V5: https://lore.kernel.org/all/20250109103221.2544467-5-srasheed@marvell.com/
- No changes
V4: https://lore.kernel.org/all/20250102112246.2494230-5-srasheed@marvell.com/
- No changes
V3: https://lore.kernel.org/all/20241218115111.2407958-5-srasheed@marvell.com/
- Added warn log that happened due to rcu_read_lock in commit message
V2: https://lore.kernel.org/all/20241216075842.2394606-5-srasheed@marvell.com/
- No changes
V1: https://lore.kernel.org/all/20241203072130.2316913-5-srasheed@marvell.com/
drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
index 7e6771c9cdbb..4c699514fd57 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
@@ -799,14 +799,6 @@ static void octep_vf_get_stats64(struct net_device *netdev,
stats->tx_bytes = tx_bytes;
stats->rx_packets = rx_packets;
stats->rx_bytes = rx_bytes;
- if (!octep_vf_get_if_stats(oct)) {
- stats->multicast = oct->iface_rx_stats.mcast_pkts;
- stats->rx_errors = oct->iface_rx_stats.err_pkts;
- stats->rx_dropped = oct->iface_rx_stats.dropped_pkts_fifo_full +
- oct->iface_rx_stats.err_pkts;
- stats->rx_missed_errors = oct->iface_rx_stats.dropped_pkts_fifo_full;
- stats->tx_dropped = oct->iface_tx_stats.dropped;
- }
}
/**
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net v9 4/4] octeon_ep_vf: update tx/rx stats locally for persistence
2025-01-17 9:46 [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 Shinas Rasheed
` (2 preceding siblings ...)
2025-01-17 9:46 ` [PATCH net v9 3/4] octeon_ep_vf: remove firmware stats fetch in ndo_get_stats64 Shinas Rasheed
@ 2025-01-17 9:46 ` Shinas Rasheed
2025-01-19 2:00 ` [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Shinas Rasheed @ 2025-01-17 9:46 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: hgani, sedara, vimleshk, thaller, wizhao, kheib, konguyen, horms,
einstein.xue, Shinas Rasheed, Veerasenareddy Burru,
Satananda Burla, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Update tx/rx stats locally, so that ndo_get_stats64()
can use that and not rely on per queue resources to obtain statistics.
The latter used to cause race conditions when the device stopped.
Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
V9:
- Iterate over OCTEP_VF_MAX_QUEUES in the ndo_get_stats64() function,
rather than just the active queues.
V8: https://lore.kernel.org/all/20250116083825.2581885-5-srasheed@marvell.com/
- Reordered patch
V7: https://lore.kernel.org/all/20250114125124.2570660-4-srasheed@marvell.com/
- Updated octep_get_stats64() to be reentrant
V6: https://lore.kernel.org/all/20250110122730.2551863-4-srasheed@marvell.com/
- No changes
V5: https://lore.kernel.org/all/20250109103221.2544467-4-srasheed@marvell.com/
- Patch introduced
.../marvell/octeon_ep_vf/octep_vf_ethtool.c | 29 +++++++------------
.../marvell/octeon_ep_vf/octep_vf_main.c | 17 +++++------
.../marvell/octeon_ep_vf/octep_vf_main.h | 6 ++++
.../marvell/octeon_ep_vf/octep_vf_rx.c | 9 +++---
.../marvell/octeon_ep_vf/octep_vf_rx.h | 2 +-
.../marvell/octeon_ep_vf/octep_vf_tx.c | 7 +++--
.../marvell/octeon_ep_vf/octep_vf_tx.h | 2 +-
7 files changed, 35 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_ethtool.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_ethtool.c
index 7b21439a315f..d60441928ba9 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_ethtool.c
@@ -114,12 +114,9 @@ static void octep_vf_get_ethtool_stats(struct net_device *netdev,
iface_tx_stats = &oct->iface_tx_stats;
iface_rx_stats = &oct->iface_rx_stats;
- for (q = 0; q < oct->num_oqs; q++) {
- struct octep_vf_iq *iq = oct->iq[q];
- struct octep_vf_oq *oq = oct->oq[q];
-
- tx_busy_errors += iq->stats.tx_busy;
- rx_alloc_errors += oq->stats.alloc_failures;
+ for (q = 0; q < OCTEP_VF_MAX_QUEUES; q++) {
+ tx_busy_errors += oct->stats_iq[q].tx_busy;
+ rx_alloc_errors += oct->stats_oq[q].alloc_failures;
}
i = 0;
data[i++] = rx_alloc_errors;
@@ -134,22 +131,18 @@ static void octep_vf_get_ethtool_stats(struct net_device *netdev,
data[i++] = iface_rx_stats->dropped_octets_fifo_full;
/* Per Tx Queue stats */
- for (q = 0; q < oct->num_iqs; q++) {
- struct octep_vf_iq *iq = oct->iq[q];
-
- data[i++] = iq->stats.instr_posted;
- data[i++] = iq->stats.instr_completed;
- data[i++] = iq->stats.bytes_sent;
- data[i++] = iq->stats.tx_busy;
+ for (q = 0; q < OCTEP_VF_MAX_QUEUES; q++) {
+ data[i++] = oct->stats_iq[q].instr_posted;
+ data[i++] = oct->stats_iq[q].instr_completed;
+ data[i++] = oct->stats_iq[q].bytes_sent;
+ data[i++] = oct->stats_iq[q].tx_busy;
}
/* Per Rx Queue stats */
for (q = 0; q < oct->num_oqs; q++) {
- struct octep_vf_oq *oq = oct->oq[q];
-
- data[i++] = oq->stats.packets;
- data[i++] = oq->stats.bytes;
- data[i++] = oq->stats.alloc_failures;
+ data[i++] = oct->stats_oq[q].packets;
+ data[i++] = oct->stats_oq[q].bytes;
+ data[i++] = oct->stats_oq[q].alloc_failures;
}
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
index 4c699514fd57..18c922dd5fc6 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c
@@ -574,7 +574,7 @@ static int octep_vf_iq_full_check(struct octep_vf_iq *iq)
* caused queues to get re-enabled after
* being stopped
*/
- iq->stats.restart_cnt++;
+ iq->stats->restart_cnt++;
fallthrough;
case 1: /* Queue left enabled, since IQ is not yet full*/
return 0;
@@ -731,7 +731,7 @@ static netdev_tx_t octep_vf_start_xmit(struct sk_buff *skb,
/* Flush the hw descriptors before writing to doorbell */
smp_wmb();
writel(iq->fill_cnt, iq->doorbell_reg);
- iq->stats.instr_posted += iq->fill_cnt;
+ iq->stats->instr_posted += iq->fill_cnt;
iq->fill_cnt = 0;
return NETDEV_TX_OK;
}
@@ -786,14 +786,11 @@ static void octep_vf_get_stats64(struct net_device *netdev,
tx_bytes = 0;
rx_packets = 0;
rx_bytes = 0;
- for (q = 0; q < oct->num_oqs; q++) {
- struct octep_vf_iq *iq = oct->iq[q];
- struct octep_vf_oq *oq = oct->oq[q];
-
- tx_packets += iq->stats.instr_completed;
- tx_bytes += iq->stats.bytes_sent;
- rx_packets += oq->stats.packets;
- rx_bytes += oq->stats.bytes;
+ for (q = 0; q < OCTEP_VF_MAX_QUEUES; q++) {
+ tx_packets += oct->stats_iq[q].instr_completed;
+ tx_bytes += oct->stats_iq[q].bytes_sent;
+ rx_packets += oct->stats_oq[q].packets;
+ rx_bytes += oct->stats_oq[q].bytes;
}
stats->tx_packets = tx_packets;
stats->tx_bytes = tx_bytes;
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.h b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.h
index 5769f62545cd..1a352f41f823 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.h
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.h
@@ -246,11 +246,17 @@ struct octep_vf_device {
/* Pointers to Octeon Tx queues */
struct octep_vf_iq *iq[OCTEP_VF_MAX_IQ];
+ /* Per iq stats */
+ struct octep_vf_iq_stats stats_iq[OCTEP_VF_MAX_IQ];
+
/* Rx queues (OQ: Output Queue) */
u16 num_oqs;
/* Pointers to Octeon Rx queues */
struct octep_vf_oq *oq[OCTEP_VF_MAX_OQ];
+ /* Per oq stats */
+ struct octep_vf_oq_stats stats_oq[OCTEP_VF_MAX_OQ];
+
/* Hardware port number of the PCIe interface */
u16 pcie_port;
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
index 82821bc28634..d70c8be3cfc4 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.c
@@ -87,7 +87,7 @@ static int octep_vf_oq_refill(struct octep_vf_device *oct, struct octep_vf_oq *o
page = dev_alloc_page();
if (unlikely(!page)) {
dev_err(oq->dev, "refill: rx buffer alloc failed\n");
- oq->stats.alloc_failures++;
+ oq->stats->alloc_failures++;
break;
}
@@ -98,7 +98,7 @@ static int octep_vf_oq_refill(struct octep_vf_device *oct, struct octep_vf_oq *o
"OQ-%d buffer refill: DMA mapping error!\n",
oq->q_no);
put_page(page);
- oq->stats.alloc_failures++;
+ oq->stats->alloc_failures++;
break;
}
oq->buff_info[refill_idx].page = page;
@@ -134,6 +134,7 @@ static int octep_vf_setup_oq(struct octep_vf_device *oct, int q_no)
oq->netdev = oct->netdev;
oq->dev = &oct->pdev->dev;
oq->q_no = q_no;
+ oq->stats = &oct->stats_oq[q_no];
oq->max_count = CFG_GET_OQ_NUM_DESC(oct->conf);
oq->ring_size_mask = oq->max_count - 1;
oq->buffer_size = CFG_GET_OQ_BUF_SIZE(oct->conf);
@@ -458,8 +459,8 @@ static int __octep_vf_oq_process_rx(struct octep_vf_device *oct,
oq->host_read_idx = read_idx;
oq->refill_count += desc_used;
- oq->stats.packets += pkt;
- oq->stats.bytes += rx_bytes;
+ oq->stats->packets += pkt;
+ oq->stats->bytes += rx_bytes;
return pkt;
}
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.h b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.h
index fe46838b5200..9e296b7d7e34 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_rx.h
@@ -187,7 +187,7 @@ struct octep_vf_oq {
u8 __iomem *pkts_sent_reg;
/* Statistics for this OQ. */
- struct octep_vf_oq_stats stats;
+ struct octep_vf_oq_stats *stats;
/* Packets pending to be processed */
u32 pkts_pending;
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.c b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.c
index 47a5c054fdb6..8180e5ce3d7e 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.c
@@ -82,9 +82,9 @@ int octep_vf_iq_process_completions(struct octep_vf_iq *iq, u16 budget)
}
iq->pkts_processed += compl_pkts;
- iq->stats.instr_completed += compl_pkts;
- iq->stats.bytes_sent += compl_bytes;
- iq->stats.sgentry_sent += compl_sg;
+ iq->stats->instr_completed += compl_pkts;
+ iq->stats->bytes_sent += compl_bytes;
+ iq->stats->sgentry_sent += compl_sg;
iq->flush_index = fi;
netif_subqueue_completed_wake(iq->netdev, iq->q_no, compl_pkts,
@@ -186,6 +186,7 @@ static int octep_vf_setup_iq(struct octep_vf_device *oct, int q_no)
iq->netdev = oct->netdev;
iq->dev = &oct->pdev->dev;
iq->q_no = q_no;
+ iq->stats = &oct->stats_iq[q_no];
iq->max_count = CFG_GET_IQ_NUM_DESC(oct->conf);
iq->ring_size_mask = iq->max_count - 1;
iq->fill_threshold = CFG_GET_IQ_DB_MIN(oct->conf);
diff --git a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.h b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.h
index f338b975103c..1cede90e3a5f 100644
--- a/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.h
+++ b/drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_tx.h
@@ -129,7 +129,7 @@ struct octep_vf_iq {
u16 flush_index;
/* Statistics for this input queue. */
- struct octep_vf_iq_stats stats;
+ struct octep_vf_iq_stats *stats;
/* Pointer to the Virtual Base addr of the input ring. */
struct octep_vf_tx_desc_hw *desc_ring;
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64
2025-01-17 9:46 [PATCH net v9 0/4] Fix race conditions in ndo_get_stats64 Shinas Rasheed
` (3 preceding siblings ...)
2025-01-17 9:46 ` [PATCH net v9 4/4] octeon_ep_vf: update tx/rx stats locally for persistence Shinas Rasheed
@ 2025-01-19 2:00 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-01-19 2:00 UTC (permalink / raw)
To: Shinas Rasheed
Cc: netdev, linux-kernel, hgani, sedara, vimleshk, thaller, wizhao,
kheib, konguyen, horms, einstein.xue
Hello:
This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Fri, 17 Jan 2025 01:46:49 -0800 you wrote:
> Fix race conditions in ndo_get_stats64 by storing tx/rx stats
> locally and not availing per queue resources which could be torn
> down during interface stop. Also remove stats fetch from
> firmware which is currently unnecessary
>
> Changes:
> V9:
> - Iterate over OCTEP_MAX_QUEUES or OCTEP_VF_MAX_QUEUES in the
> respective ndo_get_stats64() function, rather than just the active
> queues.
> - Update commit messages of 1/4 and 3/4 to reflect reordering.
>
> [...]
Here is the summary with links:
- [net,v9,1/4] octeon_ep: remove firmware stats fetch in ndo_get_stats64
https://git.kernel.org/netdev/net/c/1f64255bb76c
- [net,v9,2/4] octeon_ep: update tx/rx stats locally for persistence
https://git.kernel.org/netdev/net/c/10fad79846e4
- [net,v9,3/4] octeon_ep_vf: remove firmware stats fetch in ndo_get_stats64
https://git.kernel.org/netdev/net/c/cc0e510cc89f
- [net,v9,4/4] octeon_ep_vf: update tx/rx stats locally for persistence
https://git.kernel.org/netdev/net/c/f84039939512
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] 6+ messages in thread