[PATCH net-next] net: wangxun: protect ring accesses with READ- and WRITE_ONCE

[PATCH net-next] net: wangxun: protect ring accesses with READ- and WRITE_ONCE

[Mengyuan Lou]

In wx_update_stats(), the ring pointers wx->rx_ring[i] and wx->tx_ring[i]
can become NULL during queue teardown or reconfiguration (e.g., via
wx_free_q_vector()).

READ_ONCE should be used when reading rings prior to accessing the
statistics pointer to ensure protected access. As well as the
corresponding WRITE_ONCE usage when allocating and freeing the rings.

Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_hw.c  | 20 ++++++++++++++++----
 drivers/net/ethernet/wangxun/libwx/wx_lib.c |  8 ++++----
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
index 122c4952d203..096fe7f3d84f 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
@@ -2893,10 +2893,13 @@ void wx_update_stats(struct wx *wx)
 
 	spin_lock(&wx->hw_stats_lock);
 
+	rcu_read_lock();
 	/* gather some stats to the wx struct that are per queue */
 	for (i = 0; i < wx->num_rx_queues; i++) {
-		struct wx_ring *rx_ring = wx->rx_ring[i];
+		struct wx_ring *rx_ring = READ_ONCE(wx->rx_ring[i]);
 
+		if (!rx_ring)
+			continue;
 		non_eop_descs += rx_ring->rx_stats.non_eop_descs;
 		alloc_rx_buff_failed += rx_ring->rx_stats.alloc_rx_buff_failed;
 		hw_csum_rx_good += rx_ring->rx_stats.csum_good_cnt;
@@ -2912,19 +2915,28 @@ void wx_update_stats(struct wx *wx)
 		u64 rsc_flush = 0;
 
 		for (i = 0; i < wx->num_rx_queues; i++) {
-			rsc_count += wx->rx_ring[i]->rx_stats.rsc_count;
-			rsc_flush += wx->rx_ring[i]->rx_stats.rsc_flush;
+			struct wx_ring *rx_ring = READ_ONCE(wx->rx_ring[i]);
+
+			if (!rx_ring)
+				continue;
+
+			rsc_count += rx_ring->rx_stats.rsc_count;
+			rsc_flush += rx_ring->rx_stats.rsc_flush;
 		}
 		wx->rsc_count = rsc_count;
 		wx->rsc_flush = rsc_flush;
 	}
 
 	for (i = 0; i < wx->num_tx_queues; i++) {
-		struct wx_ring *tx_ring = wx->tx_ring[i];
+		struct wx_ring *tx_ring = READ_ONCE(wx->tx_ring[i]);
+
+		if (!tx_ring)
+			continue;
 
 		restart_queue += tx_ring->tx_stats.restart_queue;
 		tx_busy += tx_ring->tx_stats.tx_busy;
 	}
+	rcu_read_unlock();
 	wx->restart_queue = restart_queue;
 	wx->tx_busy = tx_busy;
 
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
index ed5aad7857bd..c227e9ab8a4a 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c
@@ -2191,7 +2191,7 @@ static int wx_alloc_q_vector(struct wx *wx,
 		ring->queue_index = txr_idx;
 
 		/* assign ring to wx */
-		wx->tx_ring[txr_idx] = ring;
+		WRITE_ONCE(wx->tx_ring[txr_idx], ring);
 
 		/* update count and index */
 		txr_count--;
@@ -2217,7 +2217,7 @@ static int wx_alloc_q_vector(struct wx *wx,
 		ring->queue_index = rxr_idx;
 
 		/* assign ring to wx */
-		wx->rx_ring[rxr_idx] = ring;
+		WRITE_ONCE(wx->rx_ring[rxr_idx], ring);
 
 		/* update count and index */
 		rxr_count--;
@@ -2245,10 +2245,10 @@ static void wx_free_q_vector(struct wx *wx, int v_idx)
 	struct wx_ring *ring;
 
 	wx_for_each_ring(ring, q_vector->tx)
-		wx->tx_ring[ring->queue_index] = NULL;
+		WRITE_ONCE(wx->tx_ring[ring->queue_index], NULL);
 
 	wx_for_each_ring(ring, q_vector->rx)
-		wx->rx_ring[ring->queue_index] = NULL;
+		WRITE_ONCE(wx->rx_ring[ring->queue_index], NULL);
 
 	wx->q_vector[v_idx] = NULL;
 	netif_napi_del(&q_vector->napi);
-- 
2.30.1

Re: [PATCH net-next] net: wangxun: protect ring accesses with READ- and WRITE_ONCE

[Simon Horman]

On Thu, Aug 13, 2026 at 05:51:41PM +0800, Mengyuan Lou wrote:
> In wx_update_stats(), the ring pointers wx->rx_ring[i] and wx->tx_ring[i]
> can become NULL during queue teardown or reconfiguration (e.g., via
> wx_free_q_vector()).
> 
> READ_ONCE should be used when reading rings prior to accessing the
> statistics pointer to ensure protected access. As well as the
> corresponding WRITE_ONCE usage when allocating and freeing the rings.

Hi Mengyuan,

As observed in an AI-generated review of this patch [1], this patch is
doing a bit more than adding READ_ONCE/WRITE_ONCE.  It also adds an RCU
read-side critical section (the most important part of the change, IMHO),
and some NULL checks. It might be nice to extend this description somehow.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260813095141.88227-1-mengyuanlou%40net-swift.com

> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
> ---
>  drivers/net/ethernet/wangxun/libwx/wx_hw.c  | 20 ++++++++++++++++----
>  drivers/net/ethernet/wangxun/libwx/wx_lib.c |  8 ++++----
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> index 122c4952d203..096fe7f3d84f 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
> @@ -2893,10 +2893,13 @@ void wx_update_stats(struct wx *wx)
>  
>  	spin_lock(&wx->hw_stats_lock);
>  
> +	rcu_read_lock();
>  	/* gather some stats to the wx struct that are per queue */
>  	for (i = 0; i < wx->num_rx_queues; i++) {
> -		struct wx_ring *rx_ring = wx->rx_ring[i];
> +		struct wx_ring *rx_ring = READ_ONCE(wx->rx_ring[i]);
>  
> +		if (!rx_ring)
> +			continue;
>  		non_eop_descs += rx_ring->rx_stats.non_eop_descs;
>  		alloc_rx_buff_failed += rx_ring->rx_stats.alloc_rx_buff_failed;
>  		hw_csum_rx_good += rx_ring->rx_stats.csum_good_cnt;
> @@ -2912,19 +2915,28 @@ void wx_update_stats(struct wx *wx)
>  		u64 rsc_flush = 0;
>  
>  		for (i = 0; i < wx->num_rx_queues; i++) {
> -			rsc_count += wx->rx_ring[i]->rx_stats.rsc_count;
> -			rsc_flush += wx->rx_ring[i]->rx_stats.rsc_flush;
> +			struct wx_ring *rx_ring = READ_ONCE(wx->rx_ring[i]);
> +
> +			if (!rx_ring)
> +				continue;
> +
> +			rsc_count += rx_ring->rx_stats.rsc_count;
> +			rsc_flush += rx_ring->rx_stats.rsc_flush;
>  		}
>  		wx->rsc_count = rsc_count;
>  		wx->rsc_flush = rsc_flush;
>  	}
>  
>  	for (i = 0; i < wx->num_tx_queues; i++) {
> -		struct wx_ring *tx_ring = wx->tx_ring[i];
> +		struct wx_ring *tx_ring = READ_ONCE(wx->tx_ring[i]);
> +
> +		if (!tx_ring)
> +			continue;
>  
>  		restart_queue += tx_ring->tx_stats.restart_queue;
>  		tx_busy += tx_ring->tx_stats.tx_busy;
>  	}
> +	rcu_read_unlock();
>  	wx->restart_queue = restart_queue;
>  	wx->tx_busy = tx_busy;
>  

A littler further down in this function there is a call to
wx_update_xoff_rx_lfc().

And since commit 22d95e93c05b ("net: wangxun: add Tx timeout process")
that function includes the following loop:


	for (i = 0; i < wx->num_tx_queues; i++)
		clear_bit(WX_HANG_CHECK_ARMED, wx->tx_ring[i]->state);

So I am wondering of this access to wx->tx_ring should be protected.
E.g. by extending the RCU read-side critical section to also cover
the call to wx_update_xoff_rx_lfc(), and by using READ_ONCE in
the loop above.

Observed with some assistance from Goose:gemini-3.1-flash-lite

...