Skip to content

Commit

Permalink
dpif: Index flows using unique identifiers.
Browse files Browse the repository at this point in the history 
This patch modifies the dpif interface to allow flows to be manipulated
using a 128-bit identifier. This allows revalidator threads to perform
datapath operations faster, as they do not need to serialise the entire
flow key for operations like flow_get and flow_delete. In conjunction
with a future patch to simplify the dump interface, this provides a
significant performance benefit for revalidation.

When handlers assemble flow_put operations, they specify a unique
identifier (UFID) for each flow as it is passed down to the datapath to
be stored with the flow. The UFID is currently provided to handlers
by the dpif during upcall processing.

When revalidators assemble flow_get or flow_del operations, they may
specify the UFID for the flow along with the key. The dpif will decide
whether to send only the UFID to the datapath, or both the UFID and flow
key. The former is preferred for newer datapaths that support UFID,
while the latter is used for backwards compatibility.

Signed-off-by: Joe Stringer <[email protected]>
Acked-by: Ben Pfaff <[email protected]>
  • Loading branch information
@joestringer
joestringer committed Dec 2, 2014
1 parent d9aa721 commit 70e5ed6
Show file tree
Hide file tree
Showing 12 changed files with 333 additions and 93 deletions.
12 changes: 10 additions & 2 deletions lib/dpctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,14 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)
minimatch_destroy(&minimatch);
}
ds_clear(&ds);
if (dpctl_p->verbosity) {
if (f.ufid_present) {
odp_format_ufid(&f.ufid, &ds);
ds_put_cstr(&ds, ", ");
} else {
ds_put_cstr(&ds, "ufid:<empty>, ");
}
}
odp_flow_format(f.key, f.key_len, f.mask, f.mask_len,
&portno_names, &ds, dpctl_p->verbosity);
ds_put_cstr(&ds, ", ");
Expand Down Expand Up @@ -852,7 +860,7 @@ dpctl_put_flow(int argc, const char *argv[], enum dpif_flow_put_flags flags,
ofpbuf_size(&mask) == 0 ? NULL : ofpbuf_data(&mask),
ofpbuf_size(&mask),
ofpbuf_data(&actions), ofpbuf_size(&actions),
dpctl_p->print_statistics ? &stats : NULL);
NULL, dpctl_p->print_statistics ? &stats : NULL);
if (error) {
dpctl_error(dpctl_p, error, "updating flow table");
goto out_freeactions;
Expand Down Expand Up @@ -938,7 +946,7 @@ dpctl_del_flow(int argc, const char *argv[], struct dpctl_params *dpctl_p)
}

error = dpif_flow_del(dpif,
ofpbuf_data(&key), ofpbuf_size(&key),
ofpbuf_data(&key), ofpbuf_size(&key), NULL,
dpctl_p->print_statistics ? &stats : NULL);
if (error) {
dpctl_error(dpctl_p, error, "deleting flow");
Expand Down
87 changes: 53 additions & 34 deletions lib/dpif-netdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ struct dp_netdev_flow {

/* Hash table index by unmasked flow. */
const struct cmap_node node; /* In owning dp_netdev's 'flow_table'. */
const ovs_u128 ufid; /* Unique flow identifier. */
const struct flow flow; /* Unmasked flow that created this entry. */

/* Number of references.
Expand All @@ -327,6 +328,8 @@ struct dp_netdev_flow {

static void dp_netdev_flow_unref(struct dp_netdev_flow *);
static bool dp_netdev_flow_ref(struct dp_netdev_flow *);
static int dpif_netdev_flow_from_nlattrs(const struct nlattr *, uint32_t,
struct flow *);

/* Contained by struct dp_netdev_flow's 'stats' member. */
struct dp_netdev_flow_stats {
Expand Down Expand Up @@ -1156,14 +1159,20 @@ static void dp_netdev_flow_unref(struct dp_netdev_flow *flow)
}
}

static uint32_t
dp_netdev_flow_hash(const ovs_u128 *ufid)
{
return ufid->u32[0];
}

static void
dp_netdev_remove_flow(struct dp_netdev *dp, struct dp_netdev_flow *flow)
OVS_REQUIRES(dp->flow_mutex)
{
struct cmap_node *node = CONST_CAST(struct cmap_node *, &flow->node);

dpcls_remove(&dp->cls, &flow->cr);
cmap_remove(&dp->flow_table, node, flow_hash(&flow->flow, 0));
cmap_remove(&dp->flow_table, node, dp_netdev_flow_hash(&flow->ufid));
flow->dead = true;

dp_netdev_flow_unref(flow);
Expand Down Expand Up @@ -1531,14 +1540,26 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp,
}

static struct dp_netdev_flow *
dp_netdev_find_flow(const struct dp_netdev *dp, const struct flow *flow)
dp_netdev_find_flow(const struct dp_netdev *dp, const ovs_u128 *ufidp,
const struct nlattr *key, size_t key_len)
{
struct dp_netdev_flow *netdev_flow;
struct flow flow;
ovs_u128 ufid;

/* If a UFID is not provided, determine one based on the key. */
if (!ufidp && key && key_len
&& !dpif_netdev_flow_from_nlattrs(key, key_len, &flow)) {
dpif_flow_hash(dp->dpif, &flow, sizeof flow, &ufid);
ufidp = &ufid;
}

CMAP_FOR_EACH_WITH_HASH (netdev_flow, node, flow_hash(flow, 0),
&dp->flow_table) {
if (flow_equal(&netdev_flow->flow, flow)) {
return netdev_flow;
if (ufidp) {
CMAP_FOR_EACH_WITH_HASH (netdev_flow, node, dp_netdev_flow_hash(ufidp),
&dp->flow_table) {
if (ovs_u128_equal(&netdev_flow->ufid, ufidp)) {
return netdev_flow;
}
}
}

Expand Down Expand Up @@ -1568,8 +1589,7 @@ get_dpif_flow_stats(const struct dp_netdev_flow *netdev_flow,
* 'mask_buf'. Actions will be returned without copying, by relying on RCU to
* protect them. */
static void
dp_netdev_flow_to_dpif_flow(const struct dpif *dpif,
const struct dp_netdev_flow *netdev_flow,
dp_netdev_flow_to_dpif_flow(const struct dp_netdev_flow *netdev_flow,
struct ofpbuf *key_buf, struct ofpbuf *mask_buf,
struct dpif_flow *flow)
{
Expand Down Expand Up @@ -1599,8 +1619,8 @@ dp_netdev_flow_to_dpif_flow(const struct dpif *dpif,
flow->actions = actions->actions;
flow->actions_len = actions->size;

dpif_flow_hash(dpif, &netdev_flow->flow, sizeof netdev_flow->flow,
&flow->ufid);
flow->ufid = netdev_flow->ufid;
flow->ufid_present = true;
get_dpif_flow_stats(netdev_flow, &flow->stats);
}

Expand Down Expand Up @@ -1701,20 +1721,13 @@ dpif_netdev_flow_get(const struct dpif *dpif, const struct dpif_flow_get *get)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
struct dp_netdev_flow *netdev_flow;
struct flow key;
int error;

error = dpif_netdev_flow_from_nlattrs(get->key, get->key_len, &key);
if (error) {
return error;
}

netdev_flow = dp_netdev_find_flow(dp, &key);
int error = 0;

netdev_flow = dp_netdev_find_flow(dp, get->ufid, get->key, get->key_len);
if (netdev_flow) {
dp_netdev_flow_to_dpif_flow(dpif, netdev_flow, get->buffer,
get->buffer, get->flow);
} else {
dp_netdev_flow_to_dpif_flow(netdev_flow, get->buffer, get->buffer,
get->flow);
} else {
error = ENOENT;
}

Expand All @@ -1723,6 +1736,7 @@ dpif_netdev_flow_get(const struct dpif *dpif, const struct dpif_flow_get *get)

static struct dp_netdev_flow *
dp_netdev_flow_add(struct dp_netdev *dp, struct match *match,
const ovs_u128 *ufid,
const struct nlattr *actions, size_t actions_len)
OVS_REQUIRES(dp->flow_mutex)
{
Expand All @@ -1737,13 +1751,14 @@ dp_netdev_flow_add(struct dp_netdev *dp, struct match *match,
flow = xmalloc(sizeof *flow - sizeof flow->cr.flow.mf + mask.len);
flow->dead = false;
*CONST_CAST(struct flow *, &flow->flow) = match->flow;
*CONST_CAST(ovs_u128 *, &flow->ufid) = *ufid;
ovs_refcount_init(&flow->ref_cnt);
ovsthread_stats_init(&flow->stats);
ovsrcu_set(&flow->actions, dp_netdev_actions_create(actions, actions_len));

cmap_insert(&dp->flow_table,
CONST_CAST(struct cmap_node *, &flow->node),
flow_hash(&flow->flow, 0));
dp_netdev_flow_hash(&flow->ufid));
netdev_flow_key_init_masked(&flow->cr.flow, &match->flow, &mask);
dpcls_insert(&dp->cls, &flow->cr, &mask);

Expand All @@ -1755,6 +1770,8 @@ dp_netdev_flow_add(struct dp_netdev *dp, struct match *match,
miniflow_expand(&flow->cr.mask->mf, &match.wc.masks);

ds_put_cstr(&ds, "flow_add: ");
odp_format_ufid(ufid, &ds);
ds_put_cstr(&ds, " ");
match_format(&match, &ds, OFP_DEFAULT_PRIORITY);
ds_put_cstr(&ds, ", actions:");
format_odp_actions(&ds, actions, actions_len);
Expand Down Expand Up @@ -1790,6 +1807,7 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
struct dp_netdev_flow *netdev_flow;
struct netdev_flow_key key;
struct match match;
ovs_u128 ufid;
int error;

error = dpif_netdev_flow_from_nlattrs(put->key, put->key_len, &match.flow);
Expand All @@ -1808,6 +1826,12 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
* for upcall processing any more. */
netdev_flow_key_from_flow(&key, &match.flow);

if (put->ufid) {
ufid = *put->ufid;
} else {
dpif_flow_hash(dpif, &match.flow, sizeof match.flow, &ufid);
}

ovs_mutex_lock(&dp->flow_mutex);
netdev_flow = dp_netdev_lookup_flow(dp, &key);
if (!netdev_flow) {
Expand All @@ -1816,7 +1840,8 @@ dpif_netdev_flow_put(struct dpif *dpif, const struct dpif_flow_put *put)
if (put->stats) {
memset(put->stats, 0, sizeof *put->stats);
}
dp_netdev_flow_add(dp, &match, put->actions, put->actions_len);
dp_netdev_flow_add(dp, &match, &ufid, put->actions,
put->actions_len);
error = 0;
} else {
error = EFBIG;
Expand Down Expand Up @@ -1861,16 +1886,10 @@ dpif_netdev_flow_del(struct dpif *dpif, const struct dpif_flow_del *del)
{
struct dp_netdev *dp = get_dp_netdev(dpif);
struct dp_netdev_flow *netdev_flow;
struct flow key;
int error;

error = dpif_netdev_flow_from_nlattrs(del->key, del->key_len, &key);
if (error) {
return error;
}
int error = 0;

ovs_mutex_lock(&dp->flow_mutex);
netdev_flow = dp_netdev_find_flow(dp, &key);
netdev_flow = dp_netdev_find_flow(dp, del->ufid, del->key, del->key_len);
if (netdev_flow) {
if (del->stats) {
get_dpif_flow_stats(netdev_flow, del->stats);
Expand Down Expand Up @@ -1994,7 +2013,7 @@ dpif_netdev_flow_dump_next(struct dpif_flow_dump_thread *thread_,

ofpbuf_use_stack(&key, keybuf, sizeof *keybuf);
ofpbuf_use_stack(&mask, maskbuf, sizeof *maskbuf);
dp_netdev_flow_to_dpif_flow(&dpif->dpif, netdev_flow, &key, &mask, f);
dp_netdev_flow_to_dpif_flow(netdev_flow, &key, &mask, f);
}

return n_flows;
Expand Down Expand Up @@ -2900,7 +2919,7 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
ovs_mutex_lock(&dp->flow_mutex);
netdev_flow = dp_netdev_lookup_flow(dp, &keys[i]);
if (OVS_LIKELY(!netdev_flow)) {
netdev_flow = dp_netdev_flow_add(dp, &match,
netdev_flow = dp_netdev_flow_add(dp, &match, &ufid,
ofpbuf_data(add_actions),
ofpbuf_size(add_actions));
}
Expand Down
Loading

0 comments on commit 70e5ed6

Please sign in to comment.