Re: [PATCH net] gve: use gve_schedule_reset() on AdminQ flow rule timeout

Przemek Kitszel <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <[email protected]>
On 8/6/26 10:51, Rénich Bon Ćirić wrote:
> When an AdminQ command to configure a flow rule times out in
> gve_adminq_configure_flow_rule(), the driver currently calls
> gve_reset(priv, true) synchronously.
> 
> When flow rule configuration is invoked under netlink or ethtool
> callbacks, gve_reset() attempts to acquire rtnl_lock while already
> executing within an inner driver lock context. This violates the driver's
> lock hierarchy and causes an AB-BA circular lock inversion deadlock.
> 
> Resolve this lock inversion by replacing synchronous gve_reset() with
> gve_schedule_reset(). This defers reset execution safely to
> gve_service_task out-of-band under the proper lock ordering.
> 
> Fixes: 8ffade77b633 ("gve: Flow steering trigger reset only for timeout error")

this is not the commit that introduced the bug

initial implementation was in commit 57718b60df9b ("gve: Add flow 
steering adminq commands"), so this could be used as "wrong code intro",
but likely it was the addition of netdev_lock() around ethtool call,
that exposed the issue, and should be the actual Fixes tag

> Link: https://github.com/GoogleCloudPlatform/compute-virtual-ethernet-linux/issues/93
> Signed-off-by: Rénich Bon Ćirić <[email protected]>
> ---
>   drivers/net/ethernet/google/gve/gve_adminq.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 08587bf12345..2037b1767890 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -1322,7 +1322,7 @@ gve_adminq_configure_flow_rule(struct gve_priv *priv,
>   
>   	if (err == -ETIME) {
>   		dev_err(&priv->pdev->dev, "Timeout to configure the flow rule, trigger reset");
> -		gve_reset(priv, true);
> +		gve_schedule_reset(priv);
>   	} else if (!err) {
>   		priv->flow_rules_cache.rules_cache_synced = false;
>   	}

code looks good, after updating Fixes tag, feel free to add my:
Reviewed-by: Przemek Kitszel <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.