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 for Android: free password hash cracker in your pocket
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ