net/openvswitch/vport-internal_dev.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
Open vSwitch internal ports currently use RTNL as their ops compatibility
lock. Opt them in to netdev instance locking so eligible control operations
can be serialized per device instead of contending on the global RTNL lock.
The internal port ethtool operations implement get_drvinfo and get_link.
The latter uses ethtool_op_get_link(), which synchronizes linkwatch state
and requires RTNL. Allow get_drvinfo to run under the instance lock, but
set ETHTOOL_OP_NEEDS_RTNL_GLINK to retain RTNL for get_link.
Feature synchronization can acquire a lower device's ops lock while holding
an internal port's lock. Assign these software upper devices a distinct
lockdep class to describe the nesting.
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
---
net/openvswitch/vport-internal_dev.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 125d310871e93..eb2e83a3e16f1 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -11,6 +11,7 @@
#include <linux/skbuff.h>
#include <net/dst.h>
+#include <net/netdev_lock.h>
#include <net/xfrm.h>
#include <net/rtnetlink.h>
@@ -29,6 +30,12 @@ static struct internal_dev *internal_dev_priv(struct net_device *netdev)
return netdev_priv(netdev);
}
+static int internal_dev_init(struct net_device *netdev)
+{
+ netdev_lockdep_set_classes(netdev);
+ return 0;
+}
+
/* Called with rcu_read_lock_bh. */
static netdev_tx_t
internal_dev_xmit(struct sk_buff *skb, struct net_device *netdev)
@@ -69,6 +76,7 @@ static void internal_dev_getinfo(struct net_device *netdev,
}
static const struct ethtool_ops internal_dev_ethtool_ops = {
+ .op_needs_rtnl = ETHTOOL_OP_NEEDS_RTNL_GLINK,
.get_drvinfo = internal_dev_getinfo,
.get_link = ethtool_op_get_link,
};
@@ -81,6 +89,7 @@ static void internal_dev_destructor(struct net_device *dev)
}
static const struct net_device_ops internal_dev_netdev_ops = {
+ .ndo_init = internal_dev_init,
.ndo_open = internal_dev_open,
.ndo_stop = internal_dev_stop,
.ndo_start_xmit = internal_dev_xmit,
@@ -106,6 +115,7 @@ static void do_setup(struct net_device *netdev)
netdev->needs_free_netdev = true;
netdev->priv_destructor = NULL;
netdev->ethtool_ops = &internal_dev_ethtool_ops;
+ netdev->request_ops_lock = true;
netdev->rtnl_link_ops = &internal_dev_link_ops;
netdev->features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA |
base-commit: ac155a26750a595703e7dadff84735456d75a479
--
2.47.3
On Wed, 12 Aug 2026 20:30:03 +0800 Wang Zhan wrote: > Open vSwitch internal ports currently use RTNL as their ops compatibility > lock. Opt them in to netdev instance locking so eligible control operations > can be serialized per device instead of contending on the global RTNL lock. > > The internal port ethtool operations implement get_drvinfo and get_link. > The latter uses ethtool_op_get_link(), which synchronizes linkwatch state > and requires RTNL. Allow get_drvinfo to run under the instance lock, but > set ETHTOOL_OP_NEEDS_RTNL_GLINK to retain RTNL for get_link. > > Feature synchronization can acquire a lower device's ops lock while holding > an internal port's lock. Assign these software upper devices a distinct > lockdep class to describe the nesting. Why are you sending this patch? Do you have an actual real life need for unlocked get_drvinfo??
> Why are you sending this patch? Do you have an actual real life need > for unlocked get_drvinfo?? Yes. The use case is broader than unlocked get_drvinfo. We are trying to reduce cases where ovs-vswitchd gets blocked waiting for the global RTNL lock. In production we saw ovs-vswitchd repeatedly report "Unreasonably long" poll intervals. Tracing showed one OVS operation waiting about 500 ms for RTNL while node-exporter was collecting the speed of an mlx5 interface. The node-exporter process was CPU quota throttled, which stretched that RTNL hold time; mlx5 can make this easier to hit because some queries go through firmware/mailbox paths. netdev_linux_set_ol() is one OVS path that can take RTNL. With userspace-tso-enable, OVS uses this path to read ethtool feature information through ETHTOOL_GSSET_INFO, ETHTOOL_GSTRINGS and ETHTOOL_GFEATURES. For ovs-internal-port, these GET operations do not need RTNL, but currently take it because internal ports do not opt into per-device ops locking.
On 8/13/26 9:40 AM, Wang Zhan wrote: >> Why are you sending this patch? Do you have an actual real life need >> for unlocked get_drvinfo?? > > Yes. The use case is broader than unlocked get_drvinfo. > > We are trying to reduce cases where ovs-vswitchd gets blocked waiting for > the global RTNL lock. In production we saw ovs-vswitchd repeatedly report > "Unreasonably long" poll intervals. Tracing showed one OVS operation waiting > about 500 ms for RTNL while node-exporter was collecting the speed of an > mlx5 interface. The node-exporter process was CPU quota throttled, which > stretched that RTNL hold time; mlx5 can make this easier to hit because some > queries go through firmware/mailbox paths. I'd say this is more indicative of the issues in other places, like mlx5 driver. IIRC, it can call synchronize_net under rtnl many times in a row causing any other application that needs rtnl to stall. We've seen this behavior blocking OVS in ovn-kubernetes setups many times in the past few years and it needs to be addressed in the driver. While we could avoid rtnl here, it won't save us in other operations that do require rtnl. > netdev_linux_set_ol() is one OVS path that can take RTNL. With > userspace-tso-enable, OVS uses this path to read ethtool feature information > through ETHTOOL_GSSET_INFO, ETHTOOL_GSTRINGS and ETHTOOL_GFEATURES. For > ovs-internal-port, these GET operations do not need RTNL, but currently take > it because internal ports do not opt into per-device ops locking. This doesn't sound right. netdev_linux_set_ol() is only called if userspace TSO is enabled, and that doesn't make much sense for the kernel datapath. But also, userspace datapath doesn't create internal type devices, it creates tap devices instead. So, it doesn't look like a real use case, unless I'm missing something. All in all, the change itself may be fine, but I'm not sure it actually helps with anything in a grand scheme of things. Best regards, Ilya Maximets.
On Thu, 13 Aug 2026 19:55:27 +0200 Ilya Maximets wrote: > I'd say this is more indicative of the issues in other places, like mlx5 > driver. IIRC, it can call synchronize_net under rtnl many times in a row > causing any other application that needs rtnl to stall. We've seen this > behavior blocking OVS in ovn-kubernetes setups many times in the past few > years and it needs to be addressed in the driver. > > While we could avoid rtnl here, it won't save us in other operations that > do require rtnl. Right. As Jakub pointed out, mlx5 slowness was one of the reasons for introducing unlocked ethtool operations. However, netdev sysfs operations can still invoke driver callbacks while holding RTNL on current upstream, so contention from slow callbacks has not been fully removed. I agree that long RTNL holds should ultimately be addressed at their source. > This doesn't sound right. netdev_linux_set_ol() is only called if userspace > TSO is enabled, and that doesn't make much sense for the kernel datapath. > But also, userspace datapath doesn't create internal type devices, it creates > tap devices instead. So, it doesn't look like a real use case, unless I'm > missing something. userspace-tso-enable is a global setting. Kernel and userspace datapaths can coexist in the same ovs-vswitchd. In this setup, enabling it for the userspace datapath also causes netdev_linux_construct() to probe ports belonging to the kernel datapath, including internal ports. This is what the current code does and is the path we observed. As a side note, perhaps this probing should be limited to ports used by the userspace datapath? > All in all, the change itself may be fine, but I'm not sure it actually helps > with anything in a grand scheme of things. I agree that it is not the primary fix for long RTNL holds. I had treated this as a straightforward conversion to per-netdev ops locking. If this conversion is not considered worthwhile on its own, I'm fine with dropping the patch.
On Thu, 13 Aug 2026 19:55:27 +0200 Ilya Maximets wrote: > On 8/13/26 9:40 AM, Wang Zhan wrote: > >> Why are you sending this patch? Do you have an actual real life need > >> for unlocked get_drvinfo?? > > > > Yes. The use case is broader than unlocked get_drvinfo. > > > > We are trying to reduce cases where ovs-vswitchd gets blocked waiting for > > the global RTNL lock. In production we saw ovs-vswitchd repeatedly report > > "Unreasonably long" poll intervals. Tracing showed one OVS operation waiting > > about 500 ms for RTNL while node-exporter was collecting the speed of an > > mlx5 interface. The node-exporter process was CPU quota throttled, which > > stretched that RTNL hold time; mlx5 can make this easier to hit because some > > queries go through firmware/mailbox paths. > > I'd say this is more indicative of the issues in other places, like mlx5 > driver. IIRC, it can call synchronize_net under rtnl many times in a row > causing any other application that needs rtnl to stall. We've seen this > behavior blocking OVS in ovn-kubernetes setups many times in the past few > years and it needs to be addressed in the driver. FWIW mlx5 slowness is in fact the reason for unlocked ethtool ops to exist in the first place. So if that was the cause of initial backup the problem may already be solved "at the source", so to speak.
© 2016 - 2026 Red Hat, Inc.