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

Harshitha Ramamurthy <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <CAEAWyHd=28DXLT2JoHun6PRWvCRSnmnNbi3_+BjSYx5A93KEKw@mail.gmail.com>
On Fri, Aug 7, 2026 at 5:21 PM Rénich Bon Ćirić <[email protected]> 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.

gve_reset() does not acquire the rtnl_lock(). Also, there is no inner
driver lock context that is acquired at this point. The inner driver
lock context that the AI is complaining about is possibly
priv->adminq_lock but that is already released on error by
gve_adminq_execute_cmd(). gve_reset() is also written in such a way
that the onus is on the caller to acquire the appropriate locks -
either through the ethtool core when calling
gve_user_reset()/gve_set_rxnfc() or explicitly by gve_handle_reset().

Sashiko has also called out a few issues this change could introduce.
Considering this issue hasn't actually been encountered in the wild,
there is no lockdep splat and the commit message is inaccurate, I
don't think this qualifies for the net tree.

For the net-next tree, a more holistic change is in the works for the
reset path that also changes this path and removes the synchronous
reset anyway.

>
> Co-developed with Gemini AI (Google DeepMind) for root cause analysis
> and patch formulation.
>
> Fixes: 57718b60df9b ("gve: Add flow steering adminq commands")
> Reviewed-by: Przemek Kitszel <[email protected]>
> Signed-off-by: Rénich Bon Ćirić <[email protected]>
> ---
> v2: https://lore.kernel.org/netdev/[email protected]/
>   - Update Fixes tag to 57718b60df9b per Przemek Kitszel review.
>   - Add Reviewed-by tag from Przemek Kitszel.
>   - Remove misleading GitHub Issue #93 link (opened for CentOS Stream 10 NAPI bug).
>   - Add explicit AI co-development disclosure in commit body.
> v1: https://lore.kernel.org/netdev/[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;
>         }
> --
> 2.45.2
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.