From: Praveen Kaligineedi <pkaligineedi@xxxxxxxxxx>
Date: Wed, 18 Dec 2024 05:34:12 -0800
> From: Joshua Washington <joshwash@xxxxxxxxxx>
>
> In GVE, dedicated XDP queues only exist when an XDP program is installed
> and the interface is up. As such, the NDO XDP XMIT callback should
> return early if either of these conditions are false.
>
> In the case of no loaded XDP program, priv->num_xdp_queues=0 which can
> cause a divide-by-zero error, and in the case of interface down,
> num_xdp_queues remains untouched to persist XDP queue count for the next
> interface up, but the TX pointer itself would be NULL.
>
> The XDP xmit callback also needs to synchronize with a device
> transitioning from open to close. This synchronization will happen via
> the GVE_PRIV_FLAGS_NAPI_ENABLED bit along with a synchronize_net() call,
> which waits for any RCU critical sections at call-time to complete.
>
> Fixes: 39a7f4aa3e4a ("gve: Add XDP REDIRECT 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>
> ---
> drivers/net/ethernet/google/gve/gve_main.c | 3 +++
> drivers/net/ethernet/google/gve/gve_tx.c | 5 ++++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index e171ca248f9a..5d7b0cc59959 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -1899,6 +1899,9 @@ static void gve_turndown(struct gve_priv *priv)
>
> gve_clear_napi_enabled(priv);
> gve_clear_report_stats(priv);
> +
> + /* Make sure that all traffic is finished processing. */
> + synchronize_net();
Wouldn't synchronize_rcu() be enough, have you checked?
> }
>
> static void gve_turnup(struct gve_priv *priv)
> diff --git a/drivers/net/ethernet/google/gve/gve_tx.c b/drivers/net/ethernet/google/gve/gve_tx.c
> index 83ad278ec91f..852f8c7e39d2 100644
> --- a/drivers/net/ethernet/google/gve/gve_tx.c
> +++ b/drivers/net/ethernet/google/gve/gve_tx.c
> @@ -837,9 +837,12 @@ int gve_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
> struct gve_tx_ring *tx;
> int i, err = 0, qid;
>
> - if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
> + if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK) || !priv->xdp_prog)
> return -EINVAL;
The first condition (weird xmit flags) is certainly EINVAL.
Lack of installed XDP prog is *not*.
You need to use xdp_features_{set,clear}_redirect_target() when you
install/remove XDP prog to notify the kernel that ndo_start_xmit is now
available / not available anymore.
If you want to leave this check, I'd suggest changing it to
if (unlikely(!priv->num_xdp_queues))
return -ENXIO;
if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
return -EINVAL;
>
> + if (!gve_get_napi_enabled(priv))
> + return -ENETDOWN;
> +
> qid = gve_xdp_tx_queue_id(priv,
> smp_processor_id() % priv->num_xdp_queues);
Thanks,
Olek
