Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
datapath: Use exact lookup for flow_get and flow_del.
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 <alexw@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
  • Loading branch information
yew011 committed Jul 1, 2014
1 parent bc253fd commit 3601bd8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 6 deletions.
17 changes: 11 additions & 6 deletions datapath/datapath.c
Expand Up @@ -866,8 +866,13 @@ static int ovs_flow_cmd_new_or_set(struct sk_buff *skb, struct genl_info *info)
goto err_unlock_ovs;

/* The unmasked key has to be the same for flow updates. */
if (!ovs_flow_cmp_unmasked_key(flow, &match))
goto err_unlock_ovs;
if (!ovs_flow_cmp_unmasked_key(flow, &match)) {
/* Look for any overlapping flow. */
flow = ovs_flow_tbl_lookup_exact(&dp->table, &match);
if (!flow) {
goto err_unlock_ovs;
}
}

/* Update actions. */
old_acts = ovsl_dereference(flow->sf_acts);
Expand Down Expand Up @@ -927,8 +932,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;
}
Expand Down Expand Up @@ -974,8 +979,8 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
if (err)
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;
}
Expand Down
16 changes: 16 additions & 0 deletions datapath/flow_table.c
Expand Up @@ -482,6 +482,22 @@ struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *tbl,
return ovs_flow_tbl_lookup_stats(tbl, key, &n_mask_hit);
}

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 sw_flow_mask *mask;
struct sw_flow *flow;

/* Always called under ovs-mutex. */
list_for_each_entry(mask, &tbl->mask_list, list) {
flow = masked_flow_lookup(ti, match->key, mask);
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 sw_flow_mask *mask;
Expand Down
2 changes: 2 additions & 0 deletions datapath/flow_table.h
Expand Up @@ -72,6 +72,8 @@ struct sw_flow *ovs_flow_tbl_dump_next(struct table_instance *table,
struct sw_flow *ovs_flow_tbl_lookup_stats(struct flow_table *,
const struct sw_flow_key *,
u32 *n_mask_hit);
struct sw_flow *ovs_flow_tbl_lookup_exact(struct flow_table *tbl,
struct sw_flow_match *match);
struct sw_flow *ovs_flow_tbl_lookup(struct flow_table *,
const struct sw_flow_key *);

Expand Down

0 comments on commit 3601bd8

Please sign in to comment.