[ovs-dev] [v5 2/2] datapath: Implement recirc action without recursion

Andy Zhou azhou at nicira.com
Wed Sep 3 20:05:44 UTC 2014


Since kernel stack is limited in size, it is not wise to using
recursive function with large stack frames.

This patch provides an alternative implementation of recirc action
without using recursion.

A per CPU fixed sized, 'deferred action FIFO', is used to store either
recirc or sample actions encountered during execution of an action
list. Not executing recirc or sample action in place, but rather execute
them laster as 'deferred actions' avoids recursion.

Deferred actions are only executed after all other actions has been
executed, including the ones triggered by loopback from the kernel
network stack.

The size of the private FIFO, currently set to 20, limits the number
of total 'deferred actions' any one packet can accumulate.

Signed-off-by: Andy Zhou <azhou at nicira.com>

---
v4->v5:
	Reset fifo after processing deferred actions
	move private data structures from actions.h to actions.c
	remove action_fifo init functions, since default percpu data
	   will be zero.
---
 datapath/Modules.mk |   1 +
 datapath/actions.c  | 175 ++++++++++++++++++++++++++++++++++++++++++++++++----
 datapath/actions.h  |  31 ++++++++++
 datapath/datapath.c |   1 +
 datapath/datapath.h |   4 +-
 5 files changed, 197 insertions(+), 15 deletions(-)
 create mode 100644 datapath/actions.h

diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 90e158c..2e74f6e 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -23,6 +23,7 @@ openvswitch_sources = \
 
 openvswitch_headers = \
 	compat.h \
+	actions.h \
 	datapath.h \
 	flow.h \
 	flow_netlink.h \
diff --git a/datapath/actions.c b/datapath/actions.c
index 0a22e55..6ad5bbe 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -39,6 +39,74 @@
 #include "mpls.h"
 #include "vlan.h"
 #include "vport.h"
+#include "actions.h"
+
+struct ovs_deferred_action {
+	struct sk_buff *skb;
+	const struct nlattr *actions;
+
+	/* Store pkt_key clone when creating deferred action. */
+	struct sw_flow_key pkt_key;
+};
+
+#define OVS_DEFERRED_ACTION_FIFO_SIZE 20
+struct ovs_action_fifo {
+	int head;
+	int tail;
+	/* Deferred action fifo queue storage. */
+	struct ovs_deferred_action fifo[OVS_DEFERRED_ACTION_FIFO_SIZE];
+};
+
+static DEFINE_PER_CPU(struct ovs_action_fifo, action_fifos);
+#define OVS_EXEC_ACTIONS_COUNT_LIMIT 4   /* limit used to detect packet
+					    looping by the network stack */
+static DEFINE_PER_CPU(int, ovs_exec_actions_count);
+
+static inline void action_fifo_init(struct ovs_action_fifo *fifo)
+{
+	fifo->head = 0;
+	fifo->tail = 0;
+}
+
+static inline bool action_fifo_is_empty(struct ovs_action_fifo *fifo)
+{
+	return (fifo->head == fifo->tail);
+}
+
+static inline struct ovs_deferred_action *
+action_fifo_get(struct ovs_action_fifo *fifo)
+{
+	if (action_fifo_is_empty(fifo))
+		return NULL;
+
+	return &fifo->fifo[fifo->tail++];
+}
+
+static inline struct ovs_deferred_action *
+action_fifo_put(struct ovs_action_fifo *fifo)
+{
+	if (fifo->head >= OVS_DEFERRED_ACTION_FIFO_SIZE - 1)
+		return NULL;
+
+	return &fifo->fifo[fifo->head++];
+}
+
+static inline struct ovs_deferred_action *
+add_deferred_actions(struct sk_buff *skb, const struct nlattr *attr)
+{
+	struct ovs_action_fifo *fifo;
+	struct ovs_deferred_action *da;
+
+	fifo = this_cpu_ptr(&(action_fifos));
+	da = action_fifo_put(fifo);
+
+	if (da) {
+		da->skb = skb;
+		da->actions = attr;
+	}
+
+	return da;
+}
 
 static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
 {
@@ -689,9 +757,9 @@ static bool last_action(const struct nlattr *a, int rem)
 static int sample(struct datapath *dp, struct sk_buff *skb,
 		  const struct nlattr *attr)
 {
-	struct sw_flow_key sample_key;
 	const struct nlattr *acts_list = NULL;
 	const struct nlattr *a;
+	struct ovs_deferred_action *da;
 	int rem;
 
 	for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
@@ -728,10 +796,19 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
 		/* Skip the sample action when out of memory. */
 		return 0;
 
-	flow_key_clone(skb, &sample_key);
+	da = add_deferred_actions(skb, a);
+	if (!da) {
+		if (net_ratelimit())
+			pr_warn("%s: deferred actions limit reached, dropping sample action\n",
+				ovs_dp_name(dp));
 
-	/* do_execute_actions() will consume the cloned skb. */
-	return do_execute_actions(dp, skb, a, rem);
+		kfree_skb(skb);
+		return 0;
+	}
+
+	flow_key_clone(skb, &da->pkt_key);
+
+	return 0;
 }
 
 static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
@@ -750,7 +827,7 @@ static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
 }
 
 static int execute_set_action(struct sk_buff *skb,
-				 const struct nlattr *nested_attr)
+			      const struct nlattr *nested_attr)
 {
 	int err = 0;
 
@@ -801,11 +878,10 @@ static int execute_set_action(struct sk_buff *skb,
 	return err;
 }
 
-
 static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
 			  const struct nlattr *a, int rem)
 {
-	struct sw_flow_key recirc_key;
+	struct ovs_deferred_action *da;
 
 	if (!is_skb_flow_key_valid(skb)) {
 		int err;
@@ -827,17 +903,33 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
 		if (!skb)
 			return 0;
 
-		flow_key_clone(skb, &recirc_key);
+		/* Add the skb clone to action fifo. */
+		da = add_deferred_actions(skb, NULL);
+		if (!da) {
+			kfree_skb(skb);
+			goto fifo_full;
+		}
+
+		flow_key_clone(skb, &da->pkt_key);
+	} else {
+		if (!add_deferred_actions(skb, NULL))
+			goto fifo_full;
 	}
 
 	flow_key_set_recirc_id(skb, nla_get_u32(a));
-	ovs_dp_process_packet(skb);
+	return 0;
+
+fifo_full:
+	if (net_ratelimit())
+		pr_warn("%s: deferred action limit reached, drop recirc action\n",
+			ovs_dp_name(dp));
+
 	return 0;
 }
 
 /* Execute a list of actions against 'skb'. */
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
-			const struct nlattr *attr, int len)
+			       const struct nlattr *attr, int len)
 {
 	/* Every output action needs a separate clone of 'skb', but the common
 	 * case is just a single output action, so that doing a clone and
@@ -924,8 +1016,67 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 	return 0;
 }
 
+static void ovs_process_deferred_packets(struct datapath *dp)
+{
+	struct ovs_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))
+		return;
+
+	/* Finishing executing all deferred actions. */
+	do {
+		struct ovs_deferred_action *da = action_fifo_get(fifo);
+		struct sk_buff *skb = da->skb;
+		const struct nlattr *actions = da->actions;
+
+		if (actions)
+			do_execute_actions(dp, skb, actions, nla_len(actions));
+		else
+			ovs_dp_process_packet(skb);
+	} while (!action_fifo_is_empty(fifo));
+
+	/* Reset FIFO for the next packet.  */
+	action_fifo_init(fifo);
+}
+
+static bool ovs_exec_actions_limit_exceeded(struct datapath *dp, int count)
+{
+	if (count >= OVS_EXEC_ACTIONS_COUNT_LIMIT) {
+		if (net_ratelimit())
+			pr_warn("%s: packet loop detected, dropping.\n",
+			ovs_dp_name(dp));
+
+		return true;
+	}
+
+	return false;
+}
+
+
 /* Execute a list of actions against 'skb'. */
-int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb, struct sw_flow_actions *acts)
+int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
+			struct sw_flow_actions *acts)
 {
-	return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
+	int count = this_cpu_read(ovs_exec_actions_count);
+	int err;
+
+	if (ovs_exec_actions_limit_exceeded(dp, count)) {
+		kfree_skb(skb);
+		return -ELOOP;
+	}
+
+	this_cpu_inc(ovs_exec_actions_count);
+
+	err = do_execute_actions(dp, skb, acts->actions, acts->actions_len);
+
+	if (!count)
+		ovs_process_deferred_packets(dp);
+
+	this_cpu_dec(ovs_exec_actions_count);
+
+	/* This return status currently does not reflect the errors
+	 * encounted during deferred actions execution. Probably needs to
+	 * be fixed in the future. */
+	return err;
 }
diff --git a/datapath/actions.h b/datapath/actions.h
new file mode 100644
index 0000000..17749c0
--- /dev/null
+++ b/datapath/actions.h
@@ -0,0 +1,31 @@
+/* Copyright (c) 2014 Nicira, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef ACTIONS_H
+#define ACTIONS_H 1
+
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+
+#include "compat.h"
+#include "flow.h"
+#include "flow_table.h"
+#include "vlan.h"
+#include "vport.h"
+
+int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
+			struct sw_flow_actions *acts);
+void ovs_process_action_fifo(void);
+
+#endif /* actions.h */
diff --git a/datapath/datapath.c b/datapath/datapath.c
index a668222..564bb71 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -61,6 +61,7 @@
 #include "vlan.h"
 #include "vport-internal_dev.h"
 #include "vport-netdev.h"
+#include "actions.h"
 
 int ovs_net_id __read_mostly;
 
diff --git a/datapath/datapath.h b/datapath/datapath.h
index eba2fc4..dbe58d4 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -31,6 +31,7 @@
 #include "flow_table.h"
 #include "vlan.h"
 #include "vport.h"
+#include "actions.h"
 
 #define DP_MAX_PORTS		USHRT_MAX
 #define DP_VPORT_HASH_BUCKETS	1024
@@ -196,9 +197,6 @@ int ovs_dp_upcall(struct datapath *, struct sk_buff *,
 const char *ovs_dp_name(const struct datapath *dp);
 struct sk_buff *ovs_vport_cmd_build_info(struct vport *, u32 portid, u32 seq,
 					 u8 cmd);
-
-int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
-			struct sw_flow_actions *acts);
 void ovs_dp_notify_wq(struct work_struct *work);
 
 #define OVS_NLERR(fmt, ...)					\
-- 
1.9.1




More information about the dev mailing list