netdev - Re: [PATCH v2] net: openvswitch: Avoid needlessly taking the RTNL on vport destroy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <198C2570-F384-4385-8A6B-84DCC38BB5F5@redhat.com>
Date: Mon, 15 Dec 2025 09:41:10 +0100
From: Eelco Chaudron <echaudro@...hat.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>,
 Adrian Moreno <amorenoz@...hat.com>
Cc: Aaron Conole <aconole@...hat.com>, Ilya Maximets <i.maximets@....org>,
 Alexei Starovoitov <ast@...nel.org>, Jesse Gross <jesse@...ira.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, netdev@...r.kernel.org, dev@...nvswitch.org
Subject: Re: [PATCH v2] net: openvswitch: Avoid needlessly taking the RTNL on
 vport destroy



On 11 Dec 2025, at 12:50, Toke Høiland-Jørgensen wrote:

> The openvswitch teardown code will immediately call
> ovs_netdev_detach_dev() in response to a NETDEV_UNREGISTER notification.
> It will then start the dp_notify_work workqueue, which will later end up
> calling the vport destroy() callback. This callback takes the RTNL to do
> another ovs_netdev_detach_port(), which in this case is unnecessary.
> This causes extra pressure on the RTNL, in some cases leading to
> "unregister_netdevice: waiting for XX to become free" warnings on
> teardown.
>
> We can straight-forwardly avoid the extra RTNL lock acquisition by
> checking the device flags before taking the lock, and skip the locking
> altogether if the IFF_OVS_DATAPATH flag has already been unset.
>
> Fixes: b07c26511e94 ("openvswitch: fix vport-netdev unregister")
> Tested-by: Adrian Moreno <amorenoz@...hat.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>

Guess the change looks good, but I’m waiting for some feedback from Adrian to see if this change makes sense.
Any luck reproducing the issue it’s supposed to fix?

Cheers,

Eelco

> ---
> v2:
> - Expand comments explaining the logic
>
>  net/openvswitch/vport-netdev.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 91a11067e458..6574f9bcdc02 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -160,10 +160,19 @@ void ovs_netdev_detach_dev(struct vport *vport)
>
>  static void netdev_destroy(struct vport *vport)
>  {
> -	rtnl_lock();
> -	if (netif_is_ovs_port(vport->dev))
> -		ovs_netdev_detach_dev(vport);
> -	rtnl_unlock();
> +	/* When called from ovs_db_notify_wq() after a dp_device_event(), the
> +	 * port has already been detached, so we can avoid taking the RTNL by
> +	 * checking this first.
> +	 */
> +	if (netif_is_ovs_port(vport->dev)) {
> +		rtnl_lock();
> +		/* Check again while holding the lock to ensure we don't race
> +		 * with the netdev notifier and detach twice.
> +		 */
> +		if (netif_is_ovs_port(vport->dev))
> +			ovs_netdev_detach_dev(vport);
> +		rtnl_unlock();
> +	}
>
>  	call_rcu(&vport->rcu, vport_netdev_free);
>  }
> -- 
> 2.52.0