[PATCH net 2/3] Revert "openvswitch: Use nested-BH locking for ovs_pcpu_storage"
From: Gal Pressman <hidden>
Date: 2025-06-10 06:27:00
Also in:
linux-rt-devel
Subsystem:
networking [general], openvswitch, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Aaron Conole, Eelco Chaudron, Ilya Maximets, Linus Torvalds
This reverts commit 672318331b44753ab7bd8545558939c38b4c1132. Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Gal Pressman <redacted> --- net/openvswitch/actions.c | 31 +++++++++++++++++++++++++++++-- net/openvswitch/datapath.c | 24 ------------------------ net/openvswitch/datapath.h | 33 --------------------------------- 3 files changed, 29 insertions(+), 59 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 435725c27a55..7e4a8d41b9ed 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c@@ -39,6 +39,15 @@ #include "flow_netlink.h" #include "openvswitch_trace.h" +struct deferred_action { + struct sk_buff *skb; + const struct nlattr *actions; + int actions_len; + + /* Store pkt_key clone when creating deferred action. */ + struct sw_flow_key pkt_key; +}; + #define MAX_L2_LEN (VLAN_ETH_HLEN + 3 * MPLS_HLEN) struct ovs_frag_data { unsigned long dst;
@@ -55,10 +64,28 @@ struct ovs_frag_data { static DEFINE_PER_CPU(struct ovs_frag_data, ovs_frag_data_storage); -DEFINE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage) = { - .bh_lock = INIT_LOCAL_LOCK(bh_lock), +#define DEFERRED_ACTION_FIFO_SIZE 10 +#define OVS_RECURSION_LIMIT 5 +#define OVS_DEFERRED_ACTION_THRESHOLD (OVS_RECURSION_LIMIT - 2) +struct action_fifo { + int head; + int tail; + /* Deferred action fifo queue storage. */ + struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE]; }; +struct action_flow_keys { + struct sw_flow_key key[OVS_DEFERRED_ACTION_THRESHOLD]; +}; + +struct ovs_pcpu_storage { + struct action_fifo action_fifos; + struct action_flow_keys flow_keys; + int exec_level; +}; + +static DEFINE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage); + /* Make a clone of the 'key', using the pre-allocated percpu 'flow_keys' * space. Return NULL if out of key spaces. */
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 6a304ae2d959..aaa6277bb49c 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c@@ -244,13 +244,11 @@ void ovs_dp_detach_port(struct vport *p) /* Must be called with rcu_read_lock. */ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key) { - struct ovs_pcpu_storage *ovs_pcpu = this_cpu_ptr(&ovs_pcpu_storage); const struct vport *p = OVS_CB(skb)->input_vport; struct datapath *dp = p->dp; struct sw_flow *flow; struct sw_flow_actions *sf_acts; struct dp_stats_percpu *stats; - bool ovs_pcpu_locked = false; u64 *stats_counter; u32 n_mask_hit; u32 n_cache_hit;
@@ -292,26 +290,10 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct sw_flow_key *key) ovs_flow_stats_update(flow, key->tp.flags, skb); sf_acts = rcu_dereference(flow->sf_acts); - /* This path can be invoked recursively: Use the current task to - * identify recursive invocation - the lock must be acquired only once. - * Even with disabled bottom halves this can be preempted on PREEMPT_RT. - * Limit the locking to RT to avoid assigning `owner' if it can be - * avoided. - */ - if (IS_ENABLED(CONFIG_PREEMPT_RT) && ovs_pcpu->owner != current) { - local_lock_nested_bh(&ovs_pcpu_storage.bh_lock); - ovs_pcpu->owner = current; - ovs_pcpu_locked = true; - } - error = ovs_execute_actions(dp, skb, sf_acts, key); if (unlikely(error)) net_dbg_ratelimited("ovs: action execution error on datapath %s: %d\n", ovs_dp_name(dp), error); - if (ovs_pcpu_locked) { - ovs_pcpu->owner = NULL; - local_unlock_nested_bh(&ovs_pcpu_storage.bh_lock); - } stats_counter = &stats->n_hit;
@@ -689,13 +671,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info) sf_acts = rcu_dereference(flow->sf_acts); local_bh_disable(); - local_lock_nested_bh(&ovs_pcpu_storage.bh_lock); - if (IS_ENABLED(CONFIG_PREEMPT_RT)) - this_cpu_write(ovs_pcpu_storage.owner, current); err = ovs_execute_actions(dp, packet, sf_acts, &flow->key); - if (IS_ENABLED(CONFIG_PREEMPT_RT)) - this_cpu_write(ovs_pcpu_storage.owner, NULL); - local_unlock_nested_bh(&ovs_pcpu_storage.bh_lock); local_bh_enable(); rcu_read_unlock();
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 4a665c3cfa90..a12640792605 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h@@ -173,39 +173,6 @@ struct ovs_net { bool xt_label; }; -struct deferred_action { - struct sk_buff *skb; - const struct nlattr *actions; - int actions_len; - - /* Store pkt_key clone when creating deferred action. */ - struct sw_flow_key pkt_key; -}; - -#define DEFERRED_ACTION_FIFO_SIZE 10 -#define OVS_RECURSION_LIMIT 5 -#define OVS_DEFERRED_ACTION_THRESHOLD (OVS_RECURSION_LIMIT - 2) - -struct action_fifo { - int head; - int tail; - /* Deferred action fifo queue storage. */ - struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE]; -}; - -struct action_flow_keys { - struct sw_flow_key key[OVS_DEFERRED_ACTION_THRESHOLD]; -}; - -struct ovs_pcpu_storage { - struct action_fifo action_fifos; - struct action_flow_keys flow_keys; - int exec_level; - struct task_struct *owner; - local_lock_t bh_lock; -}; -DECLARE_PER_CPU(struct ovs_pcpu_storage, ovs_pcpu_storage); - /** * enum ovs_pkt_hash_types - hash info to include with a packet * to send to userspace.
--
2.40.1