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 <[email protected]>
---
  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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to