| Message ID | 20210204132548.9958.61101.stgit@dceara.remote.csb |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ovn-controller: Make lflow cache size configurable. | expand |
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@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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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]) +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
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@redhat.com> --- controller/lflow.c | 2 +- tests/ovn.at | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-)