Re: [PATCH v2] openvswitch: fix soft lockup in the netlink flow dump

On 9/15/26 2:24 PM, Denis V. Lunev wrote:

From: Denis V. Lunev <redacted>

A production compute node carrying a few thousand datapath flows hit a
soft lockup inside a single netlink flow dump and panicked.

ovs_flow_cmd_dump() calls ovs_flow_stats_get() for every flow it
emits, and that releases stats->lock with spin_unlock_bh() once per
CPU that has touched the flow. Every release is a local_bh_enable(),
and each one runs the pending softirq backlog in the dumping thread's
own context.

The skb bounds how many flows one callback emits, and a large but
sparse table adds only a walk over empty buckets, so no dumper carries
a budget of its own. Neither bounds the softirq work the callback
absorbs. On a CPU that carries the box's packet load the backlog
refills as fast as it drains, so the dumping thread becomes that CPU's
softirq engine. It never sleeps and it has no reschedule point, so
under voluntary preemption nothing can take the CPU away from it:
neither the ksoftirqd the kernel woke to take the work over, nor the
stopper thread the softlockup detector dispatches to refresh its
timestamp.

Hold BH off across the whole callback instead, the way
ctnetlink_dump_table() does, so the nested spin_unlock_bh() stop
draining softirqs. The loop already runs under rcu_read_lock() and
cannot sleep. What it gives up is preemption under CONFIG_PREEMPT,
since a BH-off region is not preemptible outside PREEMPT_RT. That
region is bounded by the skb and the table size, where the softirq
backlog it used to absorb is not.

Sashiko argues that the table size is not really bounded, which is
fair, so it may be good to try and reword the argument a bit.  Maybe
point again to the fact that these buckets are empty and the walk
should be fast enough.  Not very important, only asking because there
is a couple of things to change below anyway.

Signed-off-by: Denis V. Lunev <redacted>
---
v2:
- leave ovs_vport_cmd_dump() alone: nsid_lock has not been BH-safe
  since commit aed4969f2bdf ("net: net->nsid_lock does not need BH
  safety"), so the vport dump never drained softirqs
- disable BH before the table dereference and say in a comment that
  the region is not there for safety
- drop the ovs_flow_stats_get() history, note the empty-bucket walk
  and the lost CONFIG_PREEMPT preemption in the message
- move the Cc list out of the commit message, add the net prefix

You say here that the net prefix was added, but it wasn't.  It should
be '[PATCH net v2]'.  But also, if you're targeting the net tree then,
as also noted by the sashiko, you need a Fixes tag (63e7959c4b9b would
work, I suppose) and the Cc for stable in the commit message tag section.

Also, please, add links to previous versions of the patch here in the
changelog section.  Especially if you're renaming the patch between
versions.

The word 'here' reads strange, I'd suggest removing it.

+	 */
+	local_bh_disable();

Since the comment only applies to the local_bh_disable(), I'd suggest
having an empty line here.

And an empty line here for symmetry.