Skip to content

Commit 0e0a7e3

Browse files
josh8551021kuba-moo
authored andcommitted
xdp: create locked/unlocked instances of xdp redirect target setters
Commit 03df156 ("xdp: double protect netdev->xdp_flags with netdev->lock") introduces the netdev lock to xdp_set_features_flag(). The change includes a _locked version of the method, as it is possible for a driver to have already acquired the netdev lock before calling this helper. However, the same applies to xdp_features_(set|clear)_redirect_flags(), which ends up calling the unlocked version of xdp_set_features_flags() leading to deadlocks in GVE, which grabs the netdev lock as part of its suspend, reset, and shutdown processes: [ 833.265543] WARNING: possible recursive locking detected [ 833.270949] 6.15.0-rc1 #6 Tainted: G E [ 833.276271] -------------------------------------------- [ 833.281681] systemd-shutdow/1 is trying to acquire lock: [ 833.287090] ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: xdp_set_features_flag+0x29/0x90 [ 833.295470] [ 833.295470] but task is already holding lock: [ 833.301400] ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: gve_shutdown+0x44/0x90 [gve] [ 833.309508] [ 833.309508] other info that might help us debug this: [ 833.316130] Possible unsafe locking scenario: [ 833.316130] [ 833.322142] CPU0 [ 833.324681] ---- [ 833.327220] lock(&dev->lock); [ 833.330455] lock(&dev->lock); [ 833.333689] [ 833.333689] *** DEADLOCK *** [ 833.333689] [ 833.339701] May be due to missing lock nesting notation [ 833.339701] [ 833.346582] 5 locks held by systemd-shutdow/1: [ 833.351205] #0: ffffffffa9c89130 (system_transition_mutex){+.+.}-{4:4}, at: __se_sys_reboot+0xe6/0x210 [ 833.360695] #1: ffff93b399e5c1b8 (&dev->mutex){....}-{4:4}, at: device_shutdown+0xb4/0x1f0 [ 833.369144] #2: ffff949d19a471b8 (&dev->mutex){....}-{4:4}, at: device_shutdown+0xc2/0x1f0 [ 833.377603] #3: ffffffffa9eca050 (rtnl_mutex){+.+.}-{4:4}, at: gve_shutdown+0x33/0x90 [gve] [ 833.386138] #4: ffff949d2b148c68 (&dev->lock){+.+.}-{4:4}, at: gve_shutdown+0x44/0x90 [gve] Introduce xdp_features_(set|clear)_redirect_target_locked() versions which assume that the netdev lock has already been acquired before setting the XDP feature flag and update GVE to use the locked version. Fixes: 03df156 ("xdp: double protect netdev->xdp_flags with netdev->lock") Tested-by: Mina Almasry <almasrymina@google.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com> Signed-off-by: Joshua Washington <joshwash@google.com> Acked-by: Stanislav Fomichev <sdf@fomichev.me> Acked-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20250422011643.3509287-1-joshwash@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent a1980cc commit 0e0a7e3

File tree

3 files changed

+26
-6
lines changed

3 files changed

+26
-6
lines changed

drivers/net/ethernet/google/gve/gve_main.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1830,7 +1830,7 @@ static void gve_turndown(struct gve_priv *priv)
18301830
/* Stop tx queues */
18311831
netif_tx_disable(priv->dev);
18321832

1833-
xdp_features_clear_redirect_target(priv->dev);
1833+
xdp_features_clear_redirect_target_locked(priv->dev);
18341834

18351835
gve_clear_napi_enabled(priv);
18361836
gve_clear_report_stats(priv);
@@ -1902,7 +1902,7 @@ static void gve_turnup(struct gve_priv *priv)
19021902
}
19031903

19041904
if (priv->tx_cfg.num_xdp_queues && gve_supports_xdp_xmit(priv))
1905-
xdp_features_set_redirect_target(priv->dev, false);
1905+
xdp_features_set_redirect_target_locked(priv->dev, false);
19061906

19071907
gve_set_napi_enabled(priv);
19081908
}

include/net/xdp.h

+3
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,10 @@ bool bpf_dev_bound_kfunc_id(u32 btf_id);
618618
void xdp_set_features_flag(struct net_device *dev, xdp_features_t val);
619619
void xdp_set_features_flag_locked(struct net_device *dev, xdp_features_t val);
620620
void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg);
621+
void xdp_features_set_redirect_target_locked(struct net_device *dev,
622+
bool support_sg);
621623
void xdp_features_clear_redirect_target(struct net_device *dev);
624+
void xdp_features_clear_redirect_target_locked(struct net_device *dev);
622625
#else
623626
static inline u32 bpf_xdp_metadata_kfunc_id(int id) { return 0; }
624627
static inline bool bpf_dev_bound_kfunc_id(u32 btf_id) { return false; }

net/core/xdp.c

+21-4
Original file line numberDiff line numberDiff line change
@@ -1014,21 +1014,38 @@ void xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
10141014
}
10151015
EXPORT_SYMBOL_GPL(xdp_set_features_flag);
10161016

1017-
void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg)
1017+
void xdp_features_set_redirect_target_locked(struct net_device *dev,
1018+
bool support_sg)
10181019
{
10191020
xdp_features_t val = (dev->xdp_features | NETDEV_XDP_ACT_NDO_XMIT);
10201021

10211022
if (support_sg)
10221023
val |= NETDEV_XDP_ACT_NDO_XMIT_SG;
1023-
xdp_set_features_flag(dev, val);
1024+
xdp_set_features_flag_locked(dev, val);
1025+
}
1026+
EXPORT_SYMBOL_GPL(xdp_features_set_redirect_target_locked);
1027+
1028+
void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg)
1029+
{
1030+
netdev_lock(dev);
1031+
xdp_features_set_redirect_target_locked(dev, support_sg);
1032+
netdev_unlock(dev);
10241033
}
10251034
EXPORT_SYMBOL_GPL(xdp_features_set_redirect_target);
10261035

1027-
void xdp_features_clear_redirect_target(struct net_device *dev)
1036+
void xdp_features_clear_redirect_target_locked(struct net_device *dev)
10281037
{
10291038
xdp_features_t val = dev->xdp_features;
10301039

10311040
val &= ~(NETDEV_XDP_ACT_NDO_XMIT | NETDEV_XDP_ACT_NDO_XMIT_SG);
1032-
xdp_set_features_flag(dev, val);
1041+
xdp_set_features_flag_locked(dev, val);
1042+
}
1043+
EXPORT_SYMBOL_GPL(xdp_features_clear_redirect_target_locked);
1044+
1045+
void xdp_features_clear_redirect_target(struct net_device *dev)
1046+
{
1047+
netdev_lock(dev);
1048+
xdp_features_clear_redirect_target_locked(dev);
1049+
netdev_unlock(dev);
10331050
}
10341051
EXPORT_SYMBOL_GPL(xdp_features_clear_redirect_target);

0 commit comments

Comments
 (0)