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). Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> --- lib/dpctl.c | 2 +- lib/dpif-netdev.c | 46 ++++++++++++++++++++----------------------- lib/odp-util.c | 35 ++++++++++++++++++++++++++------ lib/odp-util.h | 4 ++-- ofproto/ofproto-dpif-upcall.c | 2 +- tests/test-odp.c | 2 +- 6 files changed, 55 insertions(+), 36 deletions(-) diff --git a/lib/dpctl.c b/lib/dpctl.c index 438bfd3..26de23f 100644 --- a/lib/dpctl.c +++ b/lib/dpctl.c @@ -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); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 3bf130d..8794ca0 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -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; diff --git a/lib/odp-util.c b/lib/odp-util.c index 4db371d..3717aee 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -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 @@ -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 @@ -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. */ diff --git a/lib/odp-util.h b/lib/odp-util.h index 35849ae..51cf5c3 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -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, @@ -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); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index c191927..cd8a9f0 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -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; } diff --git a/tests/test-odp.c b/tests/test-odp.c index cdc761f..55ee97e 100644 --- a/tests/test-odp.c +++ b/tests/test-odp.c @@ -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); -- 2.1.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev