sFlow: Genericize/simplify kernel sFlow implementation

Following patch adds sampling action which takes probability and set
of actions as arguments. When probability is hit, actions are executed for
given packet.
USERSPACE action's userdata (u64) is used to store struct
user_action_cookie as cookie. CONTROLLER action is fixed accordingly.

Now we can remove sFlow code from kernel and implement sFlow generically
as SAMPLE action. sFlow is defined as SAMPLE Action with probability (sFlow
sampling rate) and USERSPACE action as argument. USERSPACE action's data
is used as cookie. sFlow uses this cookie to store output-port, number of
output ports and vlan-id. sample-pool is calculated by using vport
stats.

Signed-off-by: Pravin Shelar <[email protected]>
Acked-by: Jesse Gross <[email protected]>
Acked-by: Ben Pfaff <[email protected]>

datapath/actions.c

@@ -28,8 +28,8 @@
 #include "vlan.h"
 #include "vport.h"
 
-static int do_execute_actions(struct datapath *, struct sk_buff *,
-			      struct sw_flow_actions *acts);
+static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
+			const struct nlattr *attr, int len, bool keep_skb);
 
 static int make_writable(struct sk_buff *skb, int write_len)
 {
@@ -255,15 +255,37 @@ static int output_userspace(struct datapath *dp, struct sk_buff *skb, u64 arg)
 	upcall.cmd = OVS_PACKET_CMD_ACTION;
 	upcall.key = &OVS_CB(skb)->flow->key;
 	upcall.userdata = arg;
-	upcall.sample_pool = 0;
-	upcall.actions = NULL;
-	upcall.actions_len = 0;
 	return dp_upcall(dp, skb, &upcall);
 }
 
+static int sample(struct datapath *dp, struct sk_buff *skb,
+		  const struct nlattr *attr)
+{
+	const struct nlattr *acts_list = NULL;
+	const struct nlattr *a;
+	int rem;
+
+	for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
+		 a = nla_next(a, &rem)) {
+		switch (nla_type(a)) {
+		case OVS_SAMPLE_ATTR_PROBABILITY:
+			if (net_random() >= nla_get_u32(a))
+				return 0;
+			break;
+
+		case OVS_SAMPLE_ATTR_ACTIONS:
+			acts_list = a;
+			break;
+		}
+	}
+
+	return do_execute_actions(dp, skb, nla_data(acts_list),
+						 nla_len(acts_list), true);
+}
+
 /* Execute a list of actions against 'skb'. */
 static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
-			      struct sw_flow_actions *acts)
+			const struct nlattr *attr, int len, bool keep_skb)
 {
 	/* 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
@@ -274,7 +296,7 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 	const struct nlattr *a;
 	int rem;
 
-	for (a = acts->actions, rem = acts->actions_len; rem > 0;
+	for (a = attr, rem = len; rem > 0;
 	     a = nla_next(a, &rem)) {
 		int err = 0;
 
@@ -339,49 +361,29 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
 		case OVS_ACTION_ATTR_POP_PRIORITY:
 			skb->priority = priority;
 			break;
-		}
 
+		case OVS_ACTION_ATTR_SAMPLE:
+			err = sample(dp, skb, a);
+			break;
+
+		}
 		if (unlikely(err)) {
 			kfree_skb(skb);
 			return err;
 		}
 	}
 
-	if (prev_port != -1)
+	if (prev_port != -1) {
+		if (keep_skb)
+			skb = skb_clone(skb, GFP_ATOMIC);
+
 		do_output(dp, skb, prev_port);
-	else
+	} else if (!keep_skb)
 		consume_skb(skb);
 
 	return 0;
 }
 
-static void sflow_sample(struct datapath *dp, struct sk_buff *skb,
-			 struct sw_flow_actions *acts)
-{
-	struct sk_buff *nskb;
-	struct vport *p = OVS_CB(skb)->vport;
-	struct dp_upcall_info upcall;
-
-	if (unlikely(!p))
-		return;
-
-	atomic_inc(&p->sflow_pool);
-	if (net_random() >= dp->sflow_probability)
-		return;
-
-	nskb = skb_clone(skb, GFP_ATOMIC);
-	if (unlikely(!nskb))
-		return;
-
-	upcall.cmd = OVS_PACKET_CMD_SAMPLE;
-	upcall.key = &OVS_CB(skb)->flow->key;
-	upcall.userdata = 0;
-	upcall.sample_pool = atomic_read(&p->sflow_pool);
-	upcall.actions = acts->actions;
-	upcall.actions_len = acts->actions_len;
-	dp_upcall(dp, nskb, &upcall);
-}
-
 /* Execute a list of actions against 'skb'. */
 int execute_actions(struct datapath *dp, struct sk_buff *skb)
 {
@@ -399,11 +401,9 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb)
 		goto out_loop;
 	}
 
-	/* Really execute actions. */
-	if (dp->sflow_probability)
-		sflow_sample(dp, skb, acts);
 	OVS_CB(skb)->tun_id = 0;
-	error = do_execute_actions(dp, skb, acts);
+	error = do_execute_actions(dp, skb, acts->actions,
+					 acts->actions_len, false);
 
 	/* Check whether sub-actions looped too much. */
 	if (unlikely(loop->looping))

datapath/datapath.c

@@ -310,10 +310,6 @@ void dp_process_received_packet(struct vport *p, struct sk_buff *skb)
 
 			upcall.cmd = OVS_PACKET_CMD_MISS;
 			upcall.key = &key;
-			upcall.userdata = 0;
-			upcall.sample_pool = 0;
-			upcall.actions = NULL;
-			upcall.actions_len = 0;
 			dp_upcall(dp, skb, &upcall);
 			stats_counter_off = offsetof(struct dp_stats_percpu, n_missed);
 			goto out;
@@ -453,12 +449,8 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid,
 		len = sizeof(struct ovs_header);
 		len += nla_total_size(skb->len);
 		len += nla_total_size(FLOW_BUFSIZE);
-		if (upcall_info->userdata)
+		if (upcall_info->cmd == OVS_PACKET_CMD_ACTION)
 			len += nla_total_size(8);
-		if (upcall_info->sample_pool)
-			len += nla_total_size(4);
-		if (upcall_info->actions_len)
-			len += nla_total_size(upcall_info->actions_len);
 
 		user_skb = genlmsg_new(len, GFP_ATOMIC);
 		if (!user_skb) {
@@ -473,18 +465,9 @@ static int queue_userspace_packets(struct datapath *dp, u32 pid,
 		flow_to_nlattrs(upcall_info->key, user_skb);
 		nla_nest_end(user_skb, nla);
 
-		if (upcall_info->userdata)
-			nla_put_u64(user_skb, OVS_PACKET_ATTR_USERDATA, upcall_info->userdata);
-		if (upcall_info->sample_pool)
-			nla_put_u32(user_skb, OVS_PACKET_ATTR_SAMPLE_POOL, upcall_info->sample_pool);
-		if (upcall_info->actions_len) {
-			const struct nlattr *actions = upcall_info->actions;
-			u32 actions_len = upcall_info->actions_len;
-
-			nla = nla_nest_start(user_skb, OVS_PACKET_ATTR_ACTIONS);
-			memcpy(__skb_put(user_skb, actions_len), actions, actions_len);
-			nla_nest_end(user_skb, nla);
-		}
+		if (upcall_info->cmd == OVS_PACKET_CMD_ACTION)
+			nla_put_u64(user_skb, OVS_PACKET_ATTR_USERDATA,
+						upcall_info->userdata);
 
 		nla = __nla_reserve(user_skb, OVS_PACKET_ATTR_PACKET, skb->len);
 		if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -532,10 +515,37 @@ static int flush_flows(int dp_ifindex)
 	return 0;
 }
 
-static int validate_actions(const struct nlattr *attr)
+static int validate_actions(const struct nlattr *attr, int depth);
+
+static int validate_sample(const struct nlattr *attr, int depth)
+{
+	static const struct nla_policy sample_policy[OVS_SAMPLE_ATTR_MAX + 1] =
+	{
+		[OVS_SAMPLE_ATTR_PROBABILITY] = {.type = NLA_U32 },
+		[OVS_SAMPLE_ATTR_ACTIONS] = {.type = NLA_UNSPEC },
+	};
+	struct nlattr *a[OVS_SAMPLE_ATTR_MAX + 1];
+	int error;
+
+	error = nla_parse_nested(a, OVS_SAMPLE_ATTR_MAX, attr, sample_policy);
+	if (error)
+		return error;
+
+	if (!a[OVS_SAMPLE_ATTR_PROBABILITY])
+		return -EINVAL;
+	if (!a[OVS_SAMPLE_ATTR_ACTIONS])
+		return -EINVAL;
+
+	return validate_actions(a[OVS_SAMPLE_ATTR_ACTIONS], (depth + 1));
+}
+
+static int validate_actions(const struct nlattr *attr, int depth)
 {
 	const struct nlattr *a;
-	int rem;
+	int rem, err;
+
+	if (depth >= SAMPLE_ACTION_DEPTH)
+		return -EOVERFLOW;
 
 	nla_for_each_nested(a, attr, rem) {
 		static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
@@ -556,7 +566,12 @@ static int validate_actions(const struct nlattr *attr)
 		};
 		int type = nla_type(a);
 
-		if (type > OVS_ACTION_ATTR_MAX || nla_len(a) != action_lens[type])
+		/* Match expected attr len for given attr len except for
+		 * OVS_ACTION_ATTR_SAMPLE, as it has nested actions list which
+		 * is variable size. */
+		if (type > OVS_ACTION_ATTR_MAX ||
+		   (nla_len(a) != action_lens[type] &&
+			  type != OVS_ACTION_ATTR_SAMPLE))
 			return -EINVAL;
 
 		switch (type) {
@@ -592,6 +607,12 @@ static int validate_actions(const struct nlattr *attr)
 				return -EINVAL;
 			break;
 
+		case OVS_ACTION_ATTR_SAMPLE:
+			err = validate_sample(a, depth);
+			if (err)
+				return err;
+			break;
+
 		default:
 			return -EOPNOTSUPP;
 		}
@@ -630,7 +651,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	    nla_len(a[OVS_PACKET_ATTR_PACKET]) < ETH_HLEN)
 		goto err;
 
-	err = validate_actions(a[OVS_PACKET_ATTR_ACTIONS]);
+	err = validate_actions(a[OVS_PACKET_ATTR_ACTIONS], 0);
 	if (err)
 		goto err;
 
@@ -897,7 +918,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 
 	/* Validate actions. */
 	if (a[OVS_FLOW_ATTR_ACTIONS]) {
-		error = validate_actions(a[OVS_FLOW_ATTR_ACTIONS]);
+		error = validate_actions(a[OVS_FLOW_ATTR_ACTIONS], 0);
 		if (error)
 			goto error;
 	} else if (info->genlhdr->cmd == OVS_FLOW_CMD_NEW) {
@@ -1158,7 +1179,6 @@ static const struct nla_policy datapath_policy[OVS_DP_ATTR_MAX + 1] = {
 #endif
 	[OVS_DP_ATTR_UPCALL_PID] = { .type = NLA_U32 },
 	[OVS_DP_ATTR_IPV4_FRAGS] = { .type = NLA_U32 },
-	[OVS_DP_ATTR_SAMPLING] = { .type = NLA_U32 },
 };
 
 static struct genl_family dp_datapath_genl_family = {
@@ -1201,9 +1221,6 @@ static int ovs_dp_cmd_fill_info(struct datapath *dp, struct sk_buff *skb,
 	NLA_PUT_U32(skb, OVS_DP_ATTR_IPV4_FRAGS,
 		    dp->drop_frags ? OVS_DP_FRAG_DROP : OVS_DP_FRAG_ZERO);
 
-	if (dp->sflow_probability)
-		NLA_PUT_U32(skb, OVS_DP_ATTR_SAMPLING, dp->sflow_probability);
-
 	return genlmsg_end(skb, ovs_header);
 
 nla_put_failure:
@@ -1265,8 +1282,6 @@ static void change_datapath(struct datapath *dp, struct nlattr *a[OVS_DP_ATTR_MA
 {
 	if (a[OVS_DP_ATTR_IPV4_FRAGS])
 		dp->drop_frags = nla_get_u32(a[OVS_DP_ATTR_IPV4_FRAGS]) == OVS_DP_FRAG_DROP;
-	if (a[OVS_DP_ATTR_SAMPLING])
-		dp->sflow_probability = nla_get_u32(a[OVS_DP_ATTR_SAMPLING]);
 }
 
 static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)

datapath/datapath.h

@@ -34,6 +34,8 @@ struct vport;
 
 #define DP_MAX_PORTS 1024
 
+#define SAMPLE_ACTION_DEPTH 3
+
 /**
  * struct dp_stats_percpu - per-cpu packet processing statistics for a given
  * datapath.
@@ -68,9 +70,6 @@ struct dp_stats_percpu {
  * @port_list: List of all ports in @ports in arbitrary order.  RTNL required
  * to iterate or modify.
  * @stats_percpu: Per-CPU datapath statistics.
- * @sflow_probability: Number of packets out of UINT_MAX to sample to the
- * %OVS_PACKET_CMD_SAMPLE upcall, e.g. (@sflow_probability/UINT_MAX)
- * is the probability of sampling a given packet.
  *
  * Context: See the comment on locking at the top of datapath.c for additional
  * locking information.
@@ -91,9 +90,6 @@ struct datapath {
 
 	/* Stats. */
 	struct dp_stats_percpu __percpu *stats_percpu;
-
-	/* sFlow Sampling */
-	unsigned int sflow_probability;
 };
 
 /**
@@ -127,18 +123,13 @@ struct ovs_skb_cb {
  * struct dp_upcall - metadata to include with a packet to send to userspace
  * @cmd: One of %OVS_PACKET_CMD_*.
  * @key: Becomes %OVS_PACKET_ATTR_KEY.  Must be nonnull.
- * @userdata: Becomes %OVS_PACKET_ATTR_USERDATA if nonzero.
- * @sample_pool: Becomes %OVS_PACKET_ATTR_SAMPLE_POOL if nonzero.
- * @actions: Becomes %OVS_PACKET_ATTR_ACTIONS if nonnull.
- * @actions_len: Number of bytes in @actions.
-*/
+ * @userdata: Is passed to user-space as %OVS_PACKET_ATTR_USERDATA if @cmd is
+ * %OVS_PACKET_CMD_ACTION.
+ */
 struct dp_upcall_info {
 	u8 cmd;
 	const struct sw_flow_key *key;
 	u64 userdata;
-	u32 sample_pool;
-	const struct nlattr *actions;
-	u32 actions_len;
 };
 
 extern struct notifier_block dp_device_notifier;

datapath/vport.c

@@ -177,7 +177,6 @@ struct vport *vport_alloc(int priv_size, const struct vport_ops *ops, const stru
 	vport->dp = parms->dp;
 	vport->port_no = parms->port_no;
 	vport->upcall_pid = parms->upcall_pid;
-	atomic_set(&vport->sflow_pool, 0);
 	vport->ops = ops;
 
 	/* Initialize kobject for bridge.  This will be added as

datapath/vport.h

@@ -80,8 +80,6 @@ struct vport_err_stats {
  * &struct vport.  (We keep this around so that we can delete it if the
  * device gets renamed.)  Set to the null string when no link exists.
  * @node: Element in @dp's @port_list.
- * @sflow_pool: Number of packets that were candidates for sFlow sampling,
- * regardless of whether they were actually chosen and sent down to userspace.
  * @upcall_pid: The Netlink port to use for packets received on this port that
  * miss the flow table.
  * @hash_node: Element in @dev_table hash table in vport.c.
@@ -99,7 +97,6 @@ struct vport {
 	struct kobject kobj;
 	char linkname[IFNAMSIZ];
 	struct list_head node;
-	atomic_t sflow_pool;
 	u32 upcall_pid;
 
 	struct hlist_node hash_node;