[ovs-dev] [RFC PATCH 4/4] datapath: Remove memory allocations from ovs_flow_cmd_execute.

[Jarno Rajahalme]

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 datapath/datapath.c     |   59 +++++++++++++++++++++++++----------------------
 datapath/flow.h         |    2 ++
 datapath/flow_netlink.c |   10 +++++---
 3 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 5b12a5d..298d520 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -488,14 +488,24 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 {
 	struct ovs_header *ovs_header = info->userhdr;
 	struct nlattr **a = info->attrs;
-	struct sw_flow_actions *acts;
 	struct sk_buff *packet;
-	struct sw_flow *flow;
 	struct datapath *dp;
 	struct ethhdr *eth;
+	struct sw_flow flow;
+	struct {
+		struct sw_flow_actions acts;
+		u64 buf[512 / sizeof (u64)];
+	} sw_acts;
+	struct sw_flow_actions *acts;
 	int len;
 	int err;
 
+	/* Start with actions in the stack with enough space for most cases. */
+	acts = &sw_acts.acts;
+	acts->actions_len = 0;
+	acts->alloc_size = sizeof(sw_acts);
+	acts->alloced = false;
+
 	err = -EINVAL;
 	if (!a[OVS_PACKET_ATTR_PACKET] || !a[OVS_PACKET_ATTR_KEY] ||
 	    !a[OVS_PACKET_ATTR_ACTIONS])
@@ -521,34 +531,27 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	else
 		packet->protocol = htons(ETH_P_802_2);
 
-	/* Build an sw_flow for sending this packet. */
-	flow = ovs_flow_alloc(false);
-	err = PTR_ERR(flow);
-	if (IS_ERR(flow))
-		goto err_kfree_skb;
-
-	err = ovs_flow_extract(packet, -1, &flow->key);
+	/* Build an sw_flow for sending this packet.
+	 * We do not initialize mask, nor stats as they are not used
+	 * by a flow that is not in the flow table. */
+	err = ovs_flow_extract(packet, -1, &flow.key);
 	if (err)
-		goto err_flow_free;
+		goto err_free;
 
-	err = ovs_nla_get_flow_metadata(flow, a[OVS_PACKET_ATTR_KEY]);
+	err = ovs_nla_get_flow_metadata(&flow, a[OVS_PACKET_ATTR_KEY]);
 	if (err)
-		goto err_flow_free;
-	acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_PACKET_ATTR_ACTIONS]));
-	err = PTR_ERR(acts);
-	if (IS_ERR(acts))
-		goto err_flow_free;
+		goto err_free;
 
 	err = ovs_nla_copy_actions(a[OVS_PACKET_ATTR_ACTIONS],
-				   &flow->key, 0, &acts);
-	rcu_assign_pointer(flow->sf_acts, acts);
+				   &flow.key, 0, &acts);
+	rcu_assign_pointer(flow.sf_acts, acts);
 	if (err)
-		goto err_flow_free;
+		goto err_free;
 
-	OVS_CB(packet)->flow = flow;
-	OVS_CB(packet)->pkt_key = &flow->key;
-	packet->priority = flow->key.phy.priority;
-	packet->mark = flow->key.phy.skb_mark;
+	OVS_CB(packet)->flow = &flow;
+	OVS_CB(packet)->pkt_key = &flow.key;
+	packet->priority = flow.key.phy.priority;
+	packet->mark = flow.key.phy.skb_mark;
 
 	rcu_read_lock();
 	dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
@@ -561,14 +564,16 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
 	local_bh_enable();
 	rcu_read_unlock();
 
-	ovs_flow_free(flow, false);
+	if (acts != &sw_acts.acts)
+		kfree(acts);
+
 	return err;
 
 err_unlock:
 	rcu_read_unlock();
-err_flow_free:
-	ovs_flow_free(flow, false);
-err_kfree_skb:
+err_free:
+	if (acts != &sw_acts.acts)
+		kfree(acts);
 	kfree_skb(packet);
 err:
 	return err;
diff --git a/datapath/flow.h b/datapath/flow.h
index eafcfd8..f792ab2 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -146,6 +146,8 @@ struct sw_flow_match {
 struct sw_flow_actions {
 	struct rcu_head rcu;
 	u32 actions_len;
+	u32 alloc_size;
+	bool alloced;
 	struct nlattr actions[];
 };
 
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 3106fc9..9594b50 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -1175,6 +1175,9 @@ struct sw_flow_actions *ovs_nla_alloc_flow_actions(int size)
 		return ERR_PTR(-ENOMEM);
 
 	sfa->actions_len = 0;
+	sfa->alloc_size = ksize(sfa);
+	sfa->alloced = true;
+
 	return sfa;
 }
 
@@ -1203,10 +1206,10 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa,
 	int next_offset = offsetof(struct sw_flow_actions, actions) +
 					(*sfa)->actions_len;
 
-	if (req_size <= (ksize(*sfa) - next_offset))
+	if (req_size <= ((*sfa)->alloc_size - next_offset))
 		goto out;
 
-	new_acts_size = ksize(*sfa) * 2;
+	new_acts_size = (*sfa)->alloc_size * 2;
 
 	if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
 		if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
@@ -1220,7 +1223,8 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa,
 
 	memcpy(acts->actions, (*sfa)->actions, (*sfa)->actions_len);
 	acts->actions_len = (*sfa)->actions_len;
-	kfree(*sfa);
+	if ((*sfa)->alloced)
+		kfree(*sfa);
 	*sfa = acts;
 
 out:
-- 
1.7.10.4