Skip to content

Commit

Permalink
odp-util: Return exact mask if netlink mask attribute is missing.
Browse files Browse the repository at this point in the history
In the ODP context an empty mask netlink attribute usually means that
the flow should be an exact match.

odp_flow_key_to_mask{,_udpif}() instead return a struct flow_wildcards
with matches only on recirc_id and vlan_tci.

A more appropriate behavior is to handle a missing (zero length) netlink
mask specially (like we do in userspace and Linux datapath) and create
an exact match flow_wildcards from the original flow.

This fixes a bug in revalidate_ukey(): every flow created with
megaflows disabled would be revalidated away, because the mask would
seem too generic. (Another possible fix would be to handle the special
case of a missing mask in revalidate_ukey(), but this seems a more
generic solution).
  • Loading branch information
ddiproietto committed Dec 11, 2015
1 parent efa6665 commit ca8d344
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 36 deletions.
2 changes: 1 addition & 1 deletion lib/dpctl.c
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct dpctl_params *dpctl_p)

odp_flow_key_to_flow(f.key, f.key_len, &flow);
odp_flow_key_to_mask(f.mask, f.mask_len, f.key, f.key_len,
&wc.masks, &flow);
&wc, &flow);
match_init(&match, &flow, &wc);

match_init(&match_filter, &flow_filter, &wc);
Expand Down
46 changes: 21 additions & 25 deletions lib/dpif-netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -1858,33 +1858,29 @@ dpif_netdev_mask_from_nlattrs(const struct nlattr *key, uint32_t key_len,
uint32_t mask_key_len, const struct flow *flow,
struct flow_wildcards *wc)
{
if (mask_key_len) {
enum odp_key_fitness fitness;

fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
key_len, &wc->masks, flow);
if (fitness) {
/* This should not happen: it indicates that
* odp_flow_key_from_mask() and odp_flow_key_to_mask()
* disagree on the acceptable form of a mask. Log the problem
* as an error, with enough details to enable debugging. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);

if (!VLOG_DROP_ERR(&rl)) {
struct ds s;

ds_init(&s);
odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
true);
VLOG_ERR("internal error parsing flow mask %s (%s)",
ds_cstr(&s), odp_key_fitness_to_string(fitness));
ds_destroy(&s);
}
enum odp_key_fitness fitness;

fitness = odp_flow_key_to_mask_udpif(mask_key, mask_key_len, key,
key_len, wc, flow);
if (fitness) {
/* This should not happen: it indicates that
* odp_flow_key_from_mask() and odp_flow_key_to_mask()
* disagree on the acceptable form of a mask. Log the problem
* as an error, with enough details to enable debugging. */
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);

if (!VLOG_DROP_ERR(&rl)) {
struct ds s;

return EINVAL;
ds_init(&s);
odp_flow_format(key, key_len, mask_key, mask_key_len, NULL, &s,
true);
VLOG_ERR("internal error parsing flow mask %s (%s)",
ds_cstr(&s), odp_key_fitness_to_string(fitness));
ds_destroy(&s);
}
} else {
flow_wildcards_init_for_packet(wc, flow);

return EINVAL;
}

return 0;
Expand Down
35 changes: 29 additions & 6 deletions lib/odp-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -5152,6 +5152,26 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
return odp_flow_key_to_flow__(key, key_len, NULL, 0, flow, flow, false);
}

static enum odp_key_fitness
odp_flow_key_to_mask__(const struct nlattr *mask_key, size_t mask_key_len,
const struct nlattr *flow_key, size_t flow_key_len,
struct flow_wildcards *mask,
const struct flow *src_flow,
bool udpif)
{
if (mask_key_len) {
return odp_flow_key_to_flow__(mask_key, mask_key_len,
flow_key, flow_key_len,
&mask->masks, src_flow, udpif);

} else {
/* A missing mask means that the flow should be exact matched.
* Generate an appropriate exact wildcard for the flow. */
flow_wildcards_init_for_packet(mask, src_flow);

return ODP_FIT_PERFECT;
}
}
/* Converts the 'mask_key_len' bytes of OVS_KEY_ATTR_* attributes in 'mask_key'
* to a mask structure in 'mask'. 'flow' must be a previously translated flow
* corresponding to 'mask' and similarly flow_key/flow_key_len must be the
Expand All @@ -5160,10 +5180,11 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
enum odp_key_fitness
odp_flow_key_to_mask(const struct nlattr *mask_key, size_t mask_key_len,
const struct nlattr *flow_key, size_t flow_key_len,
struct flow *mask, const struct flow *flow)
struct flow_wildcards *mask, const struct flow *flow)
{
return odp_flow_key_to_flow__(mask_key, mask_key_len, flow_key, flow_key_len,
mask, flow, false);
return odp_flow_key_to_mask__(mask_key, mask_key_len,
flow_key, flow_key_len,
mask, flow, false);
}

/* These functions are similar to their non-"_udpif" variants but output a
Expand All @@ -5185,10 +5206,12 @@ odp_flow_key_to_flow_udpif(const struct nlattr *key, size_t key_len,
enum odp_key_fitness
odp_flow_key_to_mask_udpif(const struct nlattr *mask_key, size_t mask_key_len,
const struct nlattr *flow_key, size_t flow_key_len,
struct flow *mask, const struct flow *flow)
struct flow_wildcards *mask,
const struct flow *flow)
{
return odp_flow_key_to_flow__(mask_key, mask_key_len, flow_key, flow_key_len,
mask, flow, true);
return odp_flow_key_to_mask__(mask_key, mask_key_len,
flow_key, flow_key_len,
mask, flow, true);
}

/* Returns 'fitness' as a string, for use in debug messages. */
Expand Down
4 changes: 2 additions & 2 deletions lib/odp-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ enum odp_key_fitness odp_flow_key_to_mask(const struct nlattr *mask_key,
size_t mask_key_len,
const struct nlattr *flow_key,
size_t flow_key_len,
struct flow *mask,
struct flow_wildcards *mask,
const struct flow *flow);

enum odp_key_fitness odp_flow_key_to_flow_udpif(const struct nlattr *, size_t,
Expand All @@ -249,7 +249,7 @@ enum odp_key_fitness odp_flow_key_to_mask_udpif(const struct nlattr *mask_key,
size_t mask_key_len,
const struct nlattr *flow_key,
size_t flow_key_len,
struct flow *mask,
struct flow_wildcards *mask,
const struct flow *flow);

const char *odp_key_fitness_to_string(enum odp_key_fitness);
Expand Down
2 changes: 1 addition & 1 deletion ofproto/ofproto-dpif-upcall.c
Original file line number Diff line number Diff line change
Expand Up @@ -1878,7 +1878,7 @@ revalidate_ukey(struct udpif *udpif, struct udpif_key *ukey,
}

if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, ukey->key,
ukey->key_len, &dp_mask.masks, &flow)
ukey->key_len, &dp_mask, &flow)
== ODP_FIT_ERROR) {
goto exit;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/test-odp.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ parse_filter(char *filter_parse)

odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
odp_flow_key_to_mask(odp_mask.data, odp_mask.size, odp_key.data,
odp_key.size, &wc.masks, &flow);
odp_key.size, &wc, &flow);
match_init(&match, &flow, &wc);

match_init(&match_filter, &flow_filter, &wc);
Expand Down

0 comments on commit ca8d344

Please sign in to comment.