On Wed, Dec 18, 2024 at 05:34:13AM -0800, Praveen Kaligineedi wrote:
> From: Joshua Washington <joshwash@xxxxxxxxxx>
>
> This patch predicates the enabling and disabling of XSK pools on the
> existence of queues. As it stands, if the interface is down, disabling
> or enabling XSK pools would result in a crash, as the RX queue pointer
> would be NULL. XSK pool registration will occur as part of the next
> interface up.
>
> Similarly, xsk_wakeup needs be guarded against queues disappearing
> while the function is executing, so a check against the
> GVE_PRIV_FLAGS_NAPI_ENABLED flag is added to synchronize with the
> disabling of the bit and the synchronize_net() in gve_turndown.
>
> Fixes: fd8e40321a12 ("gve: Add AF_XDP zero-copy support for GQI-QPL format")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Joshua Washington <joshwash@xxxxxxxxxx>
> Signed-off-by: Praveen Kaligineedi <pkaligineedi@xxxxxxxxxx>
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@xxxxxxxxxx>
> Reviewed-by: Shailend Chand <shailend@xxxxxxxxxx>
> Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>
I think the sender's SoB still should be last, but otherwise looks good.
Reviewed-by: Larysa Zaremba <larysa.zaremba@xxxxxxxxx>
> ---
> drivers/net/ethernet/google/gve/gve_main.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 5d7b0cc59959..e4e8ff4f9f80 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -1623,8 +1623,8 @@ static int gve_xsk_pool_enable(struct net_device *dev,
> if (err)
> return err;
>
> - /* If XDP prog is not installed, return */
> - if (!priv->xdp_prog)
> + /* If XDP prog is not installed or interface is down, return. */
> + if (!priv->xdp_prog || !netif_running(dev))
> return 0;
>
> rx = &priv->rx[qid];
> @@ -1669,21 +1669,16 @@ static int gve_xsk_pool_disable(struct net_device *dev,
> if (qid >= priv->rx_cfg.num_queues)
> return -EINVAL;
>
> - /* If XDP prog is not installed, unmap DMA and return */
> - if (!priv->xdp_prog)
> + /* If XDP prog is not installed or interface is down, unmap DMA and
> + * return.
> + */
> + if (!priv->xdp_prog || !netif_running(dev))
> goto done;
>
> - tx_qid = gve_xdp_tx_queue_id(priv, qid);
> - if (!netif_running(dev)) {
> - priv->rx[qid].xsk_pool = NULL;
> - xdp_rxq_info_unreg(&priv->rx[qid].xsk_rxq);
> - priv->tx[tx_qid].xsk_pool = NULL;
> - goto done;
> - }
> -
> napi_rx = &priv->ntfy_blocks[priv->rx[qid].ntfy_id].napi;
> napi_disable(napi_rx); /* make sure current rx poll is done */
>
> + tx_qid = gve_xdp_tx_queue_id(priv, qid);
> napi_tx = &priv->ntfy_blocks[priv->tx[tx_qid].ntfy_id].napi;
> napi_disable(napi_tx); /* make sure current tx poll is done */
>
> @@ -1711,6 +1706,9 @@ static int gve_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
> struct gve_priv *priv = netdev_priv(dev);
> int tx_queue_id = gve_xdp_tx_queue_id(priv, queue_id);
>
> + if (!gve_get_napi_enabled(priv))
> + return -ENETDOWN;
> +
> if (queue_id >= priv->rx_cfg.num_queues || !priv->xdp_prog)
> return -EINVAL;
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
>
