RE: [PATCH net-next v2 1/6] net: ngbe: implement libwx reset ops

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Sun, May 3, 2026 10:15 AM, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net: ngbe: implement libwx reset ops
> 
> This commit implements the wx->do_reset() operation for the ngbe driver,
> allowing the library module to trigger a reset by bringing the interface
> down and back up.
> 
> > diff --git a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > index d8e3827a8b1f2..bd905e2675758 100644
> > --- a/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> > +++ b/drivers/net/ethernet/wangxun/ngbe/ngbe_main.c
> 
> [ ... ]
> 
> > +static void ngbe_reinit_locked(struct wx *wx)
> > +{
> > +	netif_trans_update(wx->netdev);
> > +
> > +	mutex_lock(&wx->reset_lock);
> > +	set_bit(WX_STATE_RESETTING, wx->state);
> > +
> > +	ngbe_down(wx);
> > +	ngbe_up(wx);
> > +
> > +	clear_bit(WX_STATE_RESETTING, wx->state);
> 
> Does this introduce a regression by using WX_STATE_RESETTING as an ad-hoc
> lock here?
> 
> While this pattern appears to exist elsewhere in the driver, adding it
> to ngbe_reinit_locked() relies on a design where background readers might
> check test_bit(WX_STATE_RESETTING, wx->state) without holding reset_lock.
> 
> Since the flag check and subsequent accesses are not atomic, a reader could
> test the flag just before it is set and then proceed to access shared state
> concurrently with the ngbe_down() and ngbe_up() teardown path.
> 
> Could this result in torn or corrupted reads, and would it be safer to use a
> proper synchronization mechanism like RCU or a shared lock for readers?

I think we have come up with a good way to deal with this in the previous
discussion:
https://lore.kernel.org/all/990c010b-f716-499d-b3ca-e8296003c37e@intel.com

Currently, only wx_update_stats() in watchdog that needs to test bit
WX_STATE_RESETTING, because the hardware statistics cannot be counted at the
reset time. Even if wx_update_stats() test the flag before ngbe_reinit_locked()
set it, the work task will be canceled before the reset.

> 
> > +	mutex_unlock(&wx->reset_lock);
> > +}
> > +
> > +void ngbe_do_reset(struct net_device *netdev)
> 
> [ ... ]
>