From d7ff93d7532717ea9d610a7181f24c773170be80 Mon Sep 17 00:00:00 2001 From: Andy Zhou Date: Fri, 29 Aug 2014 13:20:23 -0700 Subject: [PATCH] datapath: simplify sample action implementation The current sample() function implementation is more complicated than necessary in handling single user space action optimization and skb reference counting. There is no functional changes. Signed-off-by: Andy Zhou Acked-by: Pravin B Shelar --- datapath/actions.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 8fb4ad739..43ca2a095 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -692,7 +692,6 @@ static int sample(struct datapath *dp, struct sk_buff *skb, struct sw_flow_key sample_key; const struct nlattr *acts_list = NULL; const struct nlattr *a; - struct sk_buff *sample_skb; int rem; for (a = nla_data(attr), rem = nla_len(attr); rem > 0; @@ -712,33 +711,27 @@ static int sample(struct datapath *dp, struct sk_buff *skb, rem = nla_len(acts_list); a = nla_data(acts_list); - /* Actions list is either empty or only contains a single user-space - * action, the latter being a special case as it is the only known - * usage of the sample action. - * In these special cases don't clone the skb as there are no - * side-effects in the nested actions. - * Otherwise, clone in case the nested actions have side effects. */ - if (likely(rem == 0 || - (nla_type(a) == OVS_ACTION_ATTR_USERSPACE && - last_action(a, rem)))) { - sample_skb = skb; - skb_get(skb); - } else { - sample_skb = skb_clone(skb, GFP_ATOMIC); - if (!sample_skb) - /* Skip the sample action when out of memory. */ - return 0; + /* Actions list is empty, do nothing */ + if (unlikely(!rem)) + return 0; - flow_key_clone(skb, &sample_key); - } + /* The only known usage of sample action is having a single user-space + * action. Treat this usage as a special case. + * The output_userspace() should clone the skb to be sent to the + * user space. This skb will be consumed by its caller. */ + if (likely(nla_type(a) == OVS_ACTION_ATTR_USERSPACE && + last_action(a, rem))) + return output_userspace(dp, skb, a); + + skb = skb_clone(skb, GFP_ATOMIC); + if (!skb) + /* Skip the sample action when out of memory. */ + return 0; + + flow_key_clone(skb, &sample_key); - /* Note that do_execute_actions() never consumes skb. - * In the case where skb has been cloned above it is the clone that - * is consumed. Otherwise the skb_get(skb) call prevents - * consumption by do_execute_actions(). Thus, it is safe to simply - * return the error code and let the caller (also - * do_execute_actions()) free skb on error. */ - return do_execute_actions(dp, sample_skb, a, rem); + /* do_execute_actions() will consume the cloned skb. */ + return do_execute_actions(dp, skb, a, rem); } static void execute_hash(struct sk_buff *skb, const struct nlattr *attr) -- 2.20.1