[ovs-dev] [PATCH 1/2] Revert "datapath: Increase maximum allocation size of action list."

Pravin B Shelar pshelar at nicira.com
Thu Feb 28 19:00:48 UTC 2013


This reverts commit 82b0d755094ec675ea1a49b4ae58bc1c5e8e51c2.

This patch introduced bug by calling vfree() in interrupt context.

Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
---
 datapath/datapath.c |    8 ++++----
 datapath/flow.c     |   19 ++-----------------
 datapath/flow.h     |    5 +----
 3 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index bd2d57b..fc5d2de 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 <= ((*sfa)->buf_size - next_offset))
+	if (req_size <= (ksize(*sfa) - next_offset))
 		goto out;
 
-	new_acts_size = (*sfa)->buf_size * 2;
+	new_acts_size = ksize(*sfa) * 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;
-	ovs_flow_actions_free(*sfa);
+	kfree(*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:
-	ovs_flow_actions_free(acts);
+	kfree(acts);
 error:
 	return error;
 }
diff --git a/datapath/flow.c b/datapath/flow.c
index b6bb7a7..b14229f 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -207,29 +207,14 @@ struct sw_flow_actions *ovs_flow_actions_alloc(int size)
 	if (size > MAX_ACTIONS_BUFSIZE)
 		return ERR_PTR(-EINVAL);
 
-	size += sizeof(*sfa);
-	if (size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
-		sfa = kmalloc(size, GFP_KERNEL);
-	else
-		sfa = vmalloc(size);
-
+	sfa = kmalloc(sizeof(*sfa) + size, GFP_KERNEL);
 	if (!sfa)
 		return ERR_PTR(-ENOMEM);
 
 	sfa->actions_len = 0;
-	sfa->buf_size = size;
-
 	return sfa;
 }
 
-void ovs_flow_actions_free(struct sw_flow_actions *sfa)
-{
-	if (sfa->buf_size <= MAX_ACTIONS_BUFSIZE_KMALLOC)
-		kfree(sfa);
-	else
-		vfree(sfa);
-}
-
 struct sw_flow *ovs_flow_alloc(void)
 {
 	struct sw_flow *flow;
@@ -452,7 +437,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);
-	ovs_flow_actions_free(sf_acts);
+	kfree(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 3b08ea6..6949640 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -37,7 +37,6 @@ struct sk_buff;
 struct sw_flow_actions {
 	struct rcu_head rcu;
 	u32 actions_len;
-	int buf_size;
 	struct nlattr actions[];
 };
 
@@ -152,7 +151,6 @@ void ovs_flow_deferred_free(struct sw_flow *);
 void ovs_flow_free(struct sw_flow *);
 
 struct sw_flow_actions *ovs_flow_actions_alloc(int actions_len);
-void ovs_flow_actions_free(struct sw_flow_actions *sfa);
 void ovs_flow_deferred_free_acts(struct sw_flow_actions *);
 
 int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *,
@@ -196,8 +194,7 @@ 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		(32 * 1024)
-#define MAX_ACTIONS_BUFSIZE_KMALLOC	PAGE_SIZE
+#define MAX_ACTIONS_BUFSIZE	(16 * 1024)
 #define TBL_MIN_BUCKETS		1024
 
 struct flow_table {
-- 
1.7.1




More information about the dev mailing list