Re: [PATCH net-next] xdp: create locked/unlocked instances of xdp redirect target setters

Martin KaFai Lau
2025-04-22T21:21:01Z

On 4/21/25 6:16 PM, Joshua Washington wrote:
> Commit 03df156dd3a6 ("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.
>
> Cc: bpf@vger.kernel.org
> Fixes: 03df156dd3a6 ("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: Martin KaFai Lau <martin.lau@kernel.org>