| Message ID | 1449714462-31831-6-git-send-email-diproiettod@vmware.com |
|---|---|
| State | Accepted |
| Headers | show |
Awesome, Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto <diproiettod@vmware.com> wrote: > > 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 <diproiettod@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
Thanks, pushed to master and branch-2.5 On 10/12/2015 15:08, "Jarno Rajahalme" <jarno@ovn.org> wrote: >Awesome, > >Acked-by: Jarno Rajahalme <jarno@ovn.org> > >> On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto >><diproiettod@vmware.com> wrote: >> >> 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 <diproiettod@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 >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=BQIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=OllXIN0J2B_ZDYttKntxT9QkWyo >>4wdKJmRkA3QPOIr8&s=EQiD8IiUF34OepgGOh5djAgCN_F-PZqfknbsWSuW72M&e= >
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);
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 <diproiettod@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(-)