[ovs-dev] [PATCH v2] datapath: Increase maximum allocation size of action list.

[Pravin B Shelar]

The switch to flow based tunneling increased the size of each output
action in the flow action list.  In extreme cases, this can result
in the action list exceeding the maximum buffer size.

This doubles the maximum buffer size to compensate for the increase
in action size.  In the common case, most allocations will be
less than a page and those uses kmalloc.  Therefore, for the majority
of situations, this will have no impact.

Bug #15203
Signed-off-by: Pravin B Shelar <pshelar at nicira.com>

---
Fixes some issues noticed in earlier patch.
---
 datapath/datapath.c |    8 ++++----
 datapath/flow.c     |   31 +++++++++++++++++++++++++------
 datapath/flow.h     |    5 ++++-
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index fc5d2de..9801b8a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -434,10 +434,10 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
 	int next_offset = offsetof(struct sw_flow_actions, actions) +
 					(*sfa)->actions_len;
 
-	if (req_size <= (ksize(*sfa) - next_offset))
+	if (req_size <= ((*sfa)->size - next_offset))
 		goto out;
 
-	new_acts_size = ksize(*sfa) * 2;
+	new_acts_size = (*sfa)->size * 2;
 
 	if (new_acts_size > MAX_ACTIONS_BUFSIZE) {
 		if ((MAX_ACTIONS_BUFSIZE - next_offset) < req_size)
@@ -451,7 +451,7 @@ static struct nlattr *reserve_sfa_size(struct sw_flow_actions **sfa, int attr_le
 
 	memcpy(acts->actions, (*sfa)->actions, (*sfa)->actions_len);
 	acts->actions_len = (*sfa)->actions_len;
-	kfree(*sfa);
+	ovs_flow_actions_free(*sfa);
 	*sfa = acts;
 
 out:
@@ -1292,7 +1292,7 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
 	return 0;
 
 err_kfree:
-	kfree(acts);
+	ovs_flow_actions_free(acts);
 error:
 	return error;
 }
diff --git a/datapath/flow.c b/datapath/flow.c
index b14229f..e8e1e14 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -207,14 +207,33 @@ struct sw_flow_actions *ovs_flow_actions_alloc(int size)
 	if (size > MAX_ACTIONS_BUFSIZE)
 		return ERR_PTR(-EINVAL);
 
-	sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
-	if (!sfa)
-		return ERR_PTR(-ENOMEM);
+	size += sizeof(*sfa);
+	if (size <= MAX_ACTIONS_BUFSIZE_KMALLOC) {
+		sfa = kmalloc(size, GFP_KERNEL);
+		if (!sfa)
+			return ERR_PTR(-ENOMEM);
+
+		sfa->actions_len = 0;
+		sfa->size = ksize(sfa);
+	} else {
+		sfa = vmalloc(size);
+		if (!sfa)
+			return ERR_PTR(-ENOMEM);
 
-	sfa->actions_len = 0;
+		sfa->actions_len = 0;
+		sfa->size = size;
+	}
 	return sfa;
 }
 
+void ovs_flow_actions_free(struct sw_flow_actions *sfa)
+{
+	if (sfa->size <= MAX_ACTIONS_BUFSIZE)
+		kfree(sfa);
+	else
+		vfree(sfa);
+}
+
 struct sw_flow *ovs_flow_alloc(void)
 {
 	struct sw_flow *flow;
@@ -437,7 +456,7 @@ static void rcu_free_acts_callback(struct rcu_head *rcu)
 {
 	struct sw_flow_actions *sf_acts = container_of(rcu,
 			struct sw_flow_actions, rcu);
-	kfree(sf_acts);
+	ovs_flow_actions_free(sf_acts);
 }
 
 /* Schedules 'sf_acts' to be freed after the next RCU grace period.
diff --git a/datapath/flow.h b/datapath/flow.h
index 6949640..376e127 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -37,6 +37,7 @@ struct sk_buff;
 struct sw_flow_actions {
 	struct rcu_head rcu;
 	u32 actions_len;
+	int size;
 	struct nlattr actions[];
 };
 
@@ -194,7 +195,8 @@ int ovs_flow_from_nlattrs(struct sw_flow_key *swkey, int *key_lenp,
 int ovs_flow_metadata_from_nlattrs(struct sw_flow *flow, int key_len,
 				   const struct nlattr *attr);
 
-#define MAX_ACTIONS_BUFSIZE	(16 * 1024)
+#define MAX_ACTIONS_BUFSIZE		(32 * 1024)
+#define MAX_ACTIONS_BUFSIZE_KMALLOC	PAGE_SIZE
 #define TBL_MIN_BUCKETS		1024
 
 struct flow_table {
@@ -221,6 +223,7 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *table,
 void ovs_flow_tbl_destroy(struct flow_table *table);
 void ovs_flow_tbl_deferred_destroy(struct flow_table *table);
 struct flow_table *ovs_flow_tbl_alloc(int new_size);
+void ovs_flow_actions_free(struct sw_flow_actions *sfa);
 struct flow_table *ovs_flow_tbl_expand(struct flow_table *table);
 struct flow_table *ovs_flow_tbl_rehash(struct flow_table *table);
 void ovs_flow_tbl_insert(struct flow_table *table, struct sw_flow *flow,
-- 
1.7.1