Due to the race condition in userspace, there is chance that two
overlapping megaflows could be installed in datapath. And this
causes userspace unable to delete the less inclusive megaflow flow
even after it timeout, since the flow_del logic will stop at the
first match of masked flow.
This commit fixes the bug by making the kernel flow_del and flow_get
logic check all masks in that case.
Signed-off-by: Alex Wang <al...@nicira.com>
Acked-by: Andy Zhou <az...@nicira.com>
---
PATCH -> V2:
- use exact lookup in flow_new and flow_set.
- remove the initialization to unused variable.
- change mask array iteration range to (0, count].
- use ovsl_dereference().
---
datapath/datapath.c | 24 ++++++++++++------------
datapath/flow_table.c | 26 ++++++++++++++++++++++++--
datapath/flow_table.h | 2 ++
3 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/datapath/datapath.c b/datapath/datapath.c
index a4d6473..ab63947 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -923,8 +923,12 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct
genl_info *info)
}
/* The unmasked key has to be the same for flow updates. */
if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) {
- error = -EEXIST;
- goto err_unlock_ovs;
+ /* Look for any overlapping flow. */
+ flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
+ if (!flow) {
+ error = -ENOENT;
+ goto err_unlock_ovs;
+ }
}
/* Update actions. */
old_acts = ovsl_dereference(flow->sf_acts);
@@ -1015,16 +1019,12 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct
genl_info *info)
goto err_unlock_ovs;
}
/* Check that the flow exists. */
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
+ flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
if (unlikely(!flow)) {
error = -ENOENT;
goto err_unlock_ovs;
}
- /* The unmasked key has to be the same for flow updates. */
- if (unlikely(!ovs_flow_cmp_unmasked_key(flow, &match))) {
- error = -EEXIST;
- goto err_unlock_ovs;
- }
+
/* Update actions, if present. */
if (likely(acts)) {
old_acts = ovsl_dereference(flow->sf_acts);
@@ -1096,8 +1096,8 @@ static int ovs_flow_cmd_get(struct sk_buff *skb, struct
genl_info *info)
goto unlock;
}
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
- if (!flow || !ovs_flow_cmp_unmasked_key(flow, &match)) {
+ flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
+ if (!flow) {
err = -ENOENT;
goto unlock;
}
@@ -1144,8 +1144,8 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct
genl_info *info)
err = ovs_flow_tbl_flush(&dp->table);
goto unlock;
}
- flow = ovs_flow_tbl_lookup(&dp->table, &key);
- if (unlikely(!flow || !ovs_flow_cmp_unmasked_key(flow, &match))) {
+ flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
+ if (unlikely(!flow)) {
err = -ENOENT;
goto unlock;
}
diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 58a25c7..f54cb8e 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -513,7 +513,6 @@ static struct sw_flow *masked_flow_lookup(struct
table_instance *ti,
return NULL;
}
-
static struct sw_flow *flow_lookup(struct flow_table *tbl,
struct table_instance *ti,
struct mask_array *ma,
@@ -614,10 +613,33 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table
*tbl,
u32 __always_unused n_mask_hit;
u32 __always_unused index;
- n_mask_hit = 0;
return flow_lookup(tbl, ti, ma, key, &n_mask_hit, &index);
}
+struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
+ struct sw_flow_match *match)
+{
+ struct table_instance *ti = rcu_dereference_ovsl(tbl->ti);
+ struct mask_array *ma = rcu_dereference_ovsl(tbl->mask_array);
+ struct sw_flow *flow;
+ u32 __always_unused n_mask_hit;
+ int i;
+
+ /* Always called under ovs-mutex. */
+ for (i = 0; i <= ma->max; i++) {
+ struct sw_flow_mask *mask;
+
+ mask = ovsl_dereference(ma->masks[i]);
+ if (mask) {
+ flow = masked_flow_lookup(ti, match->key, mask,
&n_mask_hit);
+ if (flow && ovs_flow_cmp_unmasked_key(flow, match)) {
/* Found */
+ return flow;
+ }
+ }
+ }
+ return NULL;
+}
+
int ovs_flow_tbl_num_masks(const struct flow_table *table)
{
struct mask_array *ma;
diff --git a/datapath/flow_table.h b/datapath/flow_table.h
index ee86953..a05d36a 100644
--- a/datapath/flow_table.h
+++ b/datapath/flow_table.h
@@ -89,6 +89,8 @@ struct sw_flow *ovs_flow_tbl_lookup_stats(struct flow_table *,
u32 *n_mask_hit);
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
const struct sw_flow_key *);
+struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *,
+ struct sw_flow_match *match);
bool ovs_flow_cmp_unmasked_key(const struct sw_flow *flow,
struct sw_flow_match *match);
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
