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

Andy Zhou azhou at nicira.com
Thu Sep 4 01:10:10 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>

---
v5->v6:
	Remove ovs_ prefix for internal symbols.
	Remove actions.h
	Rename ovs_exec_actions_count to exec_actions_limit
	Rename ovs_process_deferred_packets() to
	    process_deferred_actions()

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/actions.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 161 insertions(+), 12 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 0a22e55..38aab64 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -40,6 +40,73 @@
 #include "vlan.h"
 #include "vport.h"
 
+struct 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 DEFERRED_ACTION_FIFO_SIZE 20
+struct action_fifo {
+	int head;
+	int tail;
+	/* Deferred action fifo queue storage. */
+	struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
+};
+
+static DEFINE_PER_CPU(struct action_fifo, action_fifos);
+#define EXEC_ACTIONS_LEVEL_LIMIT 4   /* limit used to detect packet
+					looping by the network stack */
+static DEFINE_PER_CPU(int, exec_actions_level);
+
+static void action_fifo_init(struct action_fifo *fifo)
+{
+	fifo->head = 0;
+	fifo->tail = 0;
+}
+
+static bool action_fifo_is_empty(struct action_fifo *fifo)
+{
+	return (fifo->head == fifo->tail);
+}
+
+static struct deferred_action *
+action_fifo_get(struct action_fifo *fifo)
+{
+	if (action_fifo_is_empty(fifo))
+		return NULL;
+
+	return &fifo->fifo[fifo->tail++];
+}
+
+static struct deferred_action *
+action_fifo_put(struct action_fifo *fifo)
+{
+	if (fifo->head >= DEFERRED_ACTION_FIFO_SIZE - 1)
+		return NULL;
+
+	return &fifo->fifo[fifo->head++];
+}
+
+static inline struct deferred_action *
+add_deferred_actions(struct sk_buff *skb, const struct nlattr *attr)
+{
+	struct action_fifo *fifo;
+	struct 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)
 {
 	*new_key = *OVS_CB(skb)->pkt_key;
@@ -689,9 +756,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 deferred_action *da;
 	int rem;
 
 	for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
@@ -728,10 +795,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 +826,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 +877,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 deferred_action *da;
 
 	if (!is_skb_flow_key_valid(skb)) {
 		int err;
@@ -827,17 +902,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 +1015,66 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 	return 0;
 }
 
+static void process_deferred_actions(struct datapath *dp)
+{
+	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))
+		return;
+
+	/* Finishing executing all deferred actions. */
+	do {
+		struct 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 level)
+{
+	if (unlikely(level >= EXEC_ACTIONS_LEVEL_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 level = this_cpu_read(exec_actions_level);
+	int err;
+
+	if (ovs_exec_actions_limit_exceeded(dp, level)) {
+		kfree_skb(skb);
+		return -ELOOP;
+	}
+
+	this_cpu_inc(exec_actions_level);
+
+	err = do_execute_actions(dp, skb, acts->actions, acts->actions_len);
+
+	if (!level)
+		process_deferred_actions(dp);
+
+	this_cpu_dec(exec_actions_level);
+
+	/* This return status currently does not reflect the errors
+	 * encounted during deferred actions execution. Probably needs to
+	 * be fixed in the future. */
+	return err;
 }
-- 
1.9.1




More information about the dev mailing list