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 <az...@nicira.com> --- v6->v7: Always clone the packet key 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 | 167 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 153 insertions(+), 14 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 0a22e55..d19f3ef 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -40,12 +40,80 @@ #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 void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key) { *new_key = *OVS_CB(skb)->pkt_key; OVS_CB(skb)->pkt_key = new_key; } +/* Return True iff fifo is not full */ +static bool 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; + flow_key_clone(skb, &da->pkt_key); + } + + return (da != NULL); +} + static void flow_key_set_recirc_id(struct sk_buff *skb, u32 recirc_id) { OVS_CB(skb)->pkt_key->recirc_id = recirc_id; @@ -689,7 +757,6 @@ 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; int rem; @@ -728,10 +795,15 @@ 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); + if (!add_deferred_actions(skb, a)) { + if (net_ratelimit()) + pr_warn("%s: deferred actions limit reached, dropping sample action\n", + ovs_dp_name(dp)); + + kfree_skb(skb); + } - /* do_execute_actions() will consume the cloned skb. */ - return do_execute_actions(dp, skb, a, rem); + return 0; } static void execute_hash(struct sk_buff *skb, const struct nlattr *attr) @@ -750,7 +822,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 +873,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; + bool skb_cloned; if (!is_skb_flow_key_valid(skb)) { int err; @@ -817,27 +888,37 @@ static int execute_recirc(struct datapath *dp, struct sk_buff *skb, } BUG_ON(!is_skb_flow_key_valid(skb)); + skb_cloned = false; if (!last_action(a, rem)) { /* Recirc action is the not the last action - * of the action list. */ + * of the action list, need to clone the skb. */ skb = skb_clone(skb, GFP_ATOMIC); + skb_cloned = true; /* Skip the recirc action when out of memory, but * continue on with the rest of the action list. */ if (!skb) return 0; + } + + if (add_deferred_actions(skb, NULL)) { + flow_key_set_recirc_id(skb, nla_get_u32(a)); + } else { + /* Fifo is full */ + if (skb_cloned) + kfree_skb(skb); - flow_key_clone(skb, &recirc_key); + if (net_ratelimit()) + pr_warn("%s: deferred action limit reached, drop recirc action\n", + ovs_dp_name(dp)); } - flow_key_set_recirc_id(skb, nla_get_u32(a)); - ovs_dp_process_packet(skb); 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 +1005,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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev