Thanks, pushed to master and branch-2.5
On 10/12/2015 15:08, "Jarno Rajahalme" <ja...@ovn.org> wrote:
>Awesome,
>
>Acked-by: Jarno Rajahalme <ja...@ovn.org>
>
>> On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto
>><diproiet...@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 <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
>>
>>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=
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
