Re: [PATCH net v2 7/9] gve: disable NAPI when registering XSK pools in QPL mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] gve: double disable/enable of identical NAPI instance causes deadlock/panic
- [High] gve: missing NAPI schedule for DQO queues causes permanent stall
--

commit fca9a2d42c21eb316a02fe534db2369308d03e28
Author: Joshua Washington <joshwash@google.com>

gve: disable NAPI when registering XSK pools in QPL mode

This commit disables NAPI when registering the XSK memory pool for QPL
interfaces to quiesce traffic and prevent race conditions.

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 49ae2b8c6a270..f2bd4011de23f 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -1626,6 +1626,40 @@ static int gve_xdp_xmit(struct net_device *dev, int n,
>  	return -EOPNOTSUPP;
>  }
>  
> +static void gve_disable_xsk_napis(struct gve_priv *priv, u16 qid)
> +{
> +	struct napi_struct *napi_rx, *napi_tx;
> +	u16 tx_qid;
> +
> +	napi_rx = &priv->ntfy_blocks[priv->rx[qid].ntfy_id].napi;
> +	napi_disable_locked(napi_rx);
> +
> +	tx_qid = gve_xdp_tx_queue_id(priv, qid);
> +	napi_tx = &priv->ntfy_blocks[priv->tx[tx_qid].ntfy_id].napi;
> +	napi_disable_locked(napi_tx);

[Severity: Critical]
Does this double-disable the same NAPI instance in symmetric configurations?

Since XDP TX queues share the same NAPI block as RX queues in the default
symmetric configuration, napi_rx and napi_tx resolve to the exact same pointer.
Calling napi_disable_locked() twice on the same NAPI instance will cause a
deadlock since it will spin forever waiting for the NAPI state to change.

> +}
> +
> +static void gve_enable_xsk_napis(struct gve_priv *priv, u16 qid)
> +{
> +	struct napi_struct *napi_rx, *napi_tx;
> +	u16 tx_qid;
> +
> +	napi_rx = &priv->ntfy_blocks[priv->rx[qid].ntfy_id].napi;
> +	napi_enable_locked(napi_rx);
> +
> +	tx_qid = gve_xdp_tx_queue_id(priv, qid);
> +	napi_tx = &priv->ntfy_blocks[priv->tx[tx_qid].ntfy_id].napi;
> +	napi_enable_locked(napi_tx);

[Severity: Critical]
Similarly, will enabling the same NAPI instance twice trigger a BUG_ON()
panic inside napi_enable_locked()?

> +
> +	if (gve_is_gqi(priv)) {
> +		if (gve_rx_work_pending(&priv->rx[qid]))
> +			napi_schedule(napi_rx);
> +
> +		if (gve_tx_clean_pending(priv, &priv->tx[tx_qid]))
> +			napi_schedule(napi_tx);
> +	}

[Severity: High]
Does this logic leave DQO queues permanently stalled if an interrupt fires
while NAPI is disabled?

This exclusively checks for pending work and schedules NAPI on GQI formats,
completely neglecting DQO formats. If an interrupt fired while NAPI was
disabled, the hardware interrupt remains masked forever.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922194533.631387-1-joshwash@google.com?part=7