| Message ID | 20241218133415.3759501-3-pkaligineedi@google.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | gve: various XDP fixes | expand |
From: Praveen Kaligineedi <pkaligineedi@google.com> Date: Wed, 18 Dec 2024 05:34:12 -0800 > From: Joshua Washington <joshwash@google.com> > > 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@vger.kernel.org > Signed-off-by: Joshua Washington <joshwash@google.com> > Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com> > Reviewed-by: Praveen Kaligineedi <pkaligineedi@google.com> > Reviewed-by: Shailend Chand <shailend@google.com> > Reviewed-by: Willem de Bruijn <willemb@google.com> > --- > 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
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(); } 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; + if (!gve_get_napi_enabled(priv)) + return -ENETDOWN; + qid = gve_xdp_tx_queue_id(priv, smp_processor_id() % priv->num_xdp_queues);