[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250610062631.1645885-4-gal@nvidia.com>
Date: Tue, 10 Jun 2025 09:26:31 +0300
From: Gal Pressman <gal@...dia.com>
To: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Andrew Lunn <andrew+netdev@...n.ch>,
<netdev@...r.kernel.org>
CC: Aaron Conole <aconole@...hat.com>, Eelco Chaudron <echaudro@...hat.com>,
Ilya Maximets <i.maximets@....org>, Simon Horman <horms@...nel.org>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams
<clrkwllms@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
<dev@...nvswitch.org>, <linux-rt-devel@...ts.linux.dev>, Gal Pressman
<gal@...dia.com>
Subject: [PATCH net 3/3] Revert "openvswitch: Merge three per-CPU structures into one"
This reverts commit 035fcdc4d240c873c89b76b752dd9921bc88c1ba.
The cited commit changed openvswitch to use static percpu allocations,
and exhausted the reserved chunk on module init.
Allocation of struct ovs_pcpu_storage (6488 bytes) fails on ARM:
percpu: allocation failed, size=6488 align=8 atomic=0, alloc from reserved chunk failed
CPU: 2 UID: 0 PID: 4571 Comm: modprobe Not tainted 6.15.0-for-upstream-bluefield-2025-05-28-15-45 #1 NONE
Hardware name: https://www.mellanox.com BlueField SoC/BlueField SoC, BIOS 4.9.0.13378 Oct 30 2024
Call trace:
show_stack+0x20/0x38 (C)
dump_stack_lvl+0x80/0xf8
dump_stack+0x18/0x28
pcpu_alloc_noprof+0x860/0xa48
load_module+0xc68/0x23f0
init_module_from_file+0x90/0xe0
__arm64_sys_finit_module+0x21c/0x3d8
invoke_syscall+0x50/0x120
el0_svc_common.constprop.0+0x48/0xf0
do_el0_svc+0x24/0x38
el0_svc+0x34/0xf0
el0t_64_sync_handler+0x10c/0x138
el0t_64_sync+0x1ac/0x1b0
openvswitch: Could not allocate 6488 bytes percpu data
For large buffers, dynamic allocations are preferred, revert the patch.
Cc: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Signed-off-by: Gal Pressman <gal@...dia.com>
---
net/openvswitch/actions.c | 49 +++++++++++++++++++++++++-------------
net/openvswitch/datapath.c | 9 ++++++-
net/openvswitch/datapath.h | 3 +++
3 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 7e4a8d41b9ed..2f22ca59586f 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -78,22 +78,17 @@ 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);
+static struct action_fifo __percpu *action_fifos;
+static struct action_flow_keys __percpu *flow_keys;
+static DEFINE_PER_CPU(int, exec_actions_level);
/* Make a clone of the 'key', using the pre-allocated percpu 'flow_keys'
* space. Return NULL if out of key spaces.
*/
static struct sw_flow_key *clone_key(const struct sw_flow_key *key_)
{
- struct ovs_pcpu_storage *ovs_pcpu = this_cpu_ptr(&ovs_pcpu_storage);
- struct action_flow_keys *keys = &ovs_pcpu->flow_keys;
- int level = ovs_pcpu->exec_level;
+ struct action_flow_keys *keys = this_cpu_ptr(flow_keys);
+ int level = this_cpu_read(exec_actions_level);
struct sw_flow_key *key = NULL;
if (level <= OVS_DEFERRED_ACTION_THRESHOLD) {
@@ -137,9 +132,10 @@ static struct deferred_action *add_deferred_actions(struct sk_buff *skb,
const struct nlattr *actions,
const int actions_len)
{
- struct action_fifo *fifo = this_cpu_ptr(&ovs_pcpu_storage.action_fifos);
+ struct action_fifo *fifo;
struct deferred_action *da;
+ fifo = this_cpu_ptr(action_fifos);
da = action_fifo_put(fifo);
if (da) {
da->skb = skb;
@@ -1612,13 +1608,13 @@ static int clone_execute(struct datapath *dp, struct sk_buff *skb,
if (actions) { /* Sample action */
if (clone_flow_key)
- __this_cpu_inc(ovs_pcpu_storage.exec_level);
+ __this_cpu_inc(exec_actions_level);
err = do_execute_actions(dp, skb, clone,
actions, len);
if (clone_flow_key)
- __this_cpu_dec(ovs_pcpu_storage.exec_level);
+ __this_cpu_dec(exec_actions_level);
} else { /* Recirc action */
clone->recirc_id = recirc_id;
ovs_dp_process_packet(skb, clone);
@@ -1654,7 +1650,7 @@ static int clone_execute(struct datapath *dp, struct sk_buff *skb,
static void process_deferred_actions(struct datapath *dp)
{
- struct action_fifo *fifo = this_cpu_ptr(&ovs_pcpu_storage.action_fifos);
+ struct action_fifo *fifo = this_cpu_ptr(action_fifos);
/* Do not touch the FIFO in case there is no deferred actions. */
if (action_fifo_is_empty(fifo))
@@ -1685,7 +1681,7 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
{
int err, level;
- level = __this_cpu_inc_return(ovs_pcpu_storage.exec_level);
+ level = __this_cpu_inc_return(exec_actions_level);
if (unlikely(level > OVS_RECURSION_LIMIT)) {
net_crit_ratelimited("ovs: recursion limit reached on datapath %s, probable configuration error\n",
ovs_dp_name(dp));
@@ -1702,6 +1698,27 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
process_deferred_actions(dp);
out:
- __this_cpu_dec(ovs_pcpu_storage.exec_level);
+ __this_cpu_dec(exec_actions_level);
return err;
}
+
+int action_fifos_init(void)
+{
+ action_fifos = alloc_percpu(struct action_fifo);
+ if (!action_fifos)
+ return -ENOMEM;
+
+ flow_keys = alloc_percpu(struct action_flow_keys);
+ if (!flow_keys) {
+ free_percpu(action_fifos);
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+void action_fifos_exit(void)
+{
+ free_percpu(action_fifos);
+ free_percpu(flow_keys);
+}
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index aaa6277bb49c..5d548eda742d 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -2729,10 +2729,14 @@ static int __init dp_init(void)
pr_info("Open vSwitch switching datapath\n");
- err = ovs_internal_dev_rtnl_link_register();
+ err = action_fifos_init();
if (err)
goto error;
+ err = ovs_internal_dev_rtnl_link_register();
+ if (err)
+ goto error_action_fifos_exit;
+
err = ovs_flow_init();
if (err)
goto error_unreg_rtnl_link;
@@ -2774,6 +2778,8 @@ static int __init dp_init(void)
ovs_flow_exit();
error_unreg_rtnl_link:
ovs_internal_dev_rtnl_link_unregister();
+error_action_fifos_exit:
+ action_fifos_exit();
error:
return err;
}
@@ -2789,6 +2795,7 @@ static void dp_cleanup(void)
ovs_vport_exit();
ovs_flow_exit();
ovs_internal_dev_rtnl_link_unregister();
+ action_fifos_exit();
}
module_init(dp_init);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index a12640792605..384ca77f4e79 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -281,6 +281,9 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
void ovs_dp_notify_wq(struct work_struct *work);
+int action_fifos_init(void);
+void action_fifos_exit(void);
+
/* 'KEY' must not have any bits set outside of the 'MASK' */
#define OVS_MASKED(OLD, KEY, MASK) ((KEY) | ((OLD) & ~(MASK)))
#define OVS_SET_MASKED(OLD, KEY, MASK) ((OLD) = OVS_MASKED(OLD, KEY, MASK))
--
2.40.1
Powered by blists - more mailing lists