[ovs-dev] [PATCH] datapath: simplify sample action implementation

Andy Zhou azhou at nicira.com
Fri Aug 29 20:22:42 UTC 2014


Signed-off-by: Andy Zhou <azhou at nicira.com>
---
 datapath/actions.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index 7f25553..c8951f0 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -688,7 +688,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;
@@ -708,33 +707,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 useage of sample action is having a single user-space
+	 * action. Treat this 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)
-- 
1.9.1




More information about the dev mailing list