[ovs-dev] [PATCH ovn v2 06/10] lflow: Do not cache non-conjunctive flows that use address sets/portgroups.

[Mark Michelson]

Hi Dumitru, just a small nit below

On 2/4/21 8:25 AM, Dumitru Ceara wrote:
> Caching the conjunction id offset for flows that refer to address sets
> and/or port groups but do not currently generate conjunctive matches,
> e.g., because the port group has only one locally bound port, is wrong.
> 
> If ports are later added to the port group and/or addresses are added to
> the address set, this flow will be reconsidered but at this point will
> generate conjunctive matches.  We cannot use the cached conjunction id
> offset because it might create collisions with other conjunction ids
> created for unrelated flows.
> 
> Fixes: 1213bc827040 ("ovn-controller: Cache logical flow expr matches.")
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
>   controller/lflow.c |    2 +-
>   tests/ovn.at       |   12 ++++++++++++
>   2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 3e3605a..c493652 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -884,7 +884,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow,
>                   lflow_cache_add_expr(l_ctx_out->lflow_cache, lflow,
>                                        conj_id_ofs, cached_expr);
>                   cached_expr = NULL;
> -            } else {
> +            } else if (n_conjs) {
>                   lflow_cache_add_conj_id(l_ctx_out->lflow_cache, lflow,
>                                           conj_id_ofs);
>               }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 0e114cf..4bb92d6 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -22300,6 +22300,18 @@ AT_CHECK([test "$(($conj_id_cnt + 2))" = "$(get_cache_count cache-conj-id)"], [0
>   AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], [])
>   AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], [])
>   
> +AS_BOX([Check no caching caching for non-conjunctive port group/address set matches])

s/caching caching/caching/

> +conj_id_cnt=$(get_cache_count cache-conj-id)
> +expr_cnt=$(get_cache_count cache-expr)
> +matches_cnt=$(get_cache_count cache-matches)
> +
> +check ovn-nbctl acl-add ls1 from-lport 1 'inport == @pg2 && outport == @pg2 && is_chassis_resident("lsp1")' drop
> +check ovn-nbctl --wait=hv sync
> +
> +AT_CHECK([test "$conj_id_cnt" = "$(get_cache_count cache-conj-id)"], [0], [])
> +AT_CHECK([test "$expr_cnt" = "$(get_cache_count cache-expr)"], [0], [])
> +AT_CHECK([test "$matches_cnt" = "$(get_cache_count cache-matches)"], [0], [])
> +
>   OVN_CLEANUP([hv1])
>   AT_CLEANUP
>   
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>