diff mbox

[ovs-dev,06/24] datapath: Sync OVS recursive loop counter with upstream.

Message ID 1468387441-61781-6-git-send-email-pshelar@ovn.org
State Superseded
Headers show

Commit Message

Pravin Shelar July 13, 2016, 5:23 a.m. UTC 
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
 datapath/actions.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

Comments

Jesse Gross July 16, 2016, 11:31 p.m. UTC | #1 
On Tue, Jul 12, 2016 at 10:23 PM, Pravin B Shelar <pshelar@ovn.org> wrote:
> @@ -1196,31 +1194,26 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
>                         const struct sw_flow_actions *acts,
>                         struct sw_flow_key *key)
>  {
[...]
> +       static const int ovs_recursion_limit = 4;

It looks like upstream (at least current net-next) the recursion limit
is set to 5.
Pravin Shelar July 17, 2016, 4:05 p.m. UTC | #2 
On Sat, Jul 16, 2016 at 4:31 PM, Jesse Gross <jesse@kernel.org> wrote:
> On Tue, Jul 12, 2016 at 10:23 PM, Pravin B Shelar <pshelar@ovn.org> wrote:
>> @@ -1196,31 +1194,26 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
>>                         const struct sw_flow_actions *acts,
>>                         struct sw_flow_key *key)
>>  {
> [...]
>> +       static const int ovs_recursion_limit = 4;
>
> It looks like upstream (at least current net-next) the recursion limit
> is set to 5.

Out of tree OVS does store the tunnel key on stack, therefore I did
not wanted to change the limit.
diff mbox

Patch

diff --git a/datapath/actions.c b/datapath/actions.c
index ed44ead..3cd2922 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -78,9 +78,7 @@  struct action_fifo {
 };
 
 static struct action_fifo __percpu *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)
@@ -1196,31 +1194,26 @@  int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
 			const struct sw_flow_actions *acts,
 			struct sw_flow_key *key)
 {
-	int level = this_cpu_read(exec_actions_level);
-	int err;
-
-	if (unlikely(level >= EXEC_ACTIONS_LEVEL_LIMIT)) {
-		if (net_ratelimit())
-			pr_warn("%s: packet loop detected, dropping.\n",
-				ovs_dp_name(dp));
+	static const int ovs_recursion_limit = 4;
+	int err, 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));
 		kfree_skb(skb);
-		return -ELOOP;
+		err = -ENETDOWN;
+		goto out;
 	}
 
-	this_cpu_inc(exec_actions_level);
 	err = do_execute_actions(dp, skb, key,
 				 acts->actions, acts->actions_len);
 
-	if (!level)
+	if (level == 1)
 		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.
-	 */
+out:
+	__this_cpu_dec(exec_actions_level);
 	return err;
 }