Logical flow matches that always evaluate to false should never be
programmed by ovn-northd (or as ACLs).  If that's the case, log a
warning.

Note: matches that cannot be parsed because they refer to empty
or non existent port_groups/address_sets/templates don't generate
a valid expression structure so they trigger a different warning.
This behavior was already present before the current commit.

Signed-off-by: Dumitru Ceara <[email protected]>
---
 controller/lflow.c      | 15 +++++++++++----
 tests/ovn-controller.at | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index 4b1cfe3188..be67a0b025 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -320,6 +320,15 @@ store_lflow_template_refs(struct objdep_mgr 
*lflow_deps_mgr,
     }
 }
 
+static void
+log_lflow_empty_matches(const struct sbrec_logical_flow *lflow)
+{
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+    VLOG_WARN_RL(&rl, "lflow "UUID_FMT" match always evaluates to false: "
+                 "\"%s\"", UUID_ARGS(&lflow->header_.uuid),
+                 lflow->match);
+}
+
 static bool
 lflow_parse_actions(const struct sbrec_logical_flow *lflow,
                     const struct lflow_ctx_in *l_ctx_in,
@@ -499,8 +508,7 @@ consider_lflow_for_added_as_ips__(
     uint32_t n_conjs = 0;
     n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, &matches);
     if (hmap_is_empty(&matches)) {
-        VLOG_DBG("lflow "UUID_FMT" matches are empty, skip",
-                 UUID_ARGS(&lflow->header_.uuid));
+        log_lflow_empty_matches(lflow);
         goto done;
     }
 
@@ -1222,8 +1230,7 @@ consider_logical_flow__(const struct sbrec_logical_flow 
*lflow,
         matches = xmalloc(sizeof *matches);
         n_conjs = expr_to_matches(expr, lookup_port_cb, &aux, matches);
         if (hmap_is_empty(matches)) {
-            VLOG_DBG("lflow "UUID_FMT" matches are empty, skip",
-                     UUID_ARGS(&lflow->header_.uuid));
+            log_lflow_empty_matches(lflow);
             goto done;
         }
         if (n_conjs) {
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index bbe142ae30..71b6b6e0df 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -2527,3 +2527,44 @@ check test "$zone_num" -eq 666
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - Log always false lflow matches])
+ovn_start
+
+net_add n1
+sim_add hv1
+as hv1
+check ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+check ovn-nbctl ls-add ls -- lsp-add ls lsp
+as hv1
+check ovs-vsctl \
+    -- add-port br-int vif \
+    -- set Interface vif external_ids:iface-id=lsp
+
+check ovn-nbctl --wait=hv sync
+wait_for_ports_up
+
+check ovn-nbctl pg-add pg
+check ovn-nbctl --wait=hv acl-add ls from-lport 1 \
+    'inport == @pg && ip4.dst == 1.1.1.1 && ip6.dst == 2::2' drop
+
+dnl Should complain about invalid port group name as there's no local 'pg'.
+OVS_WAIT_UNTIL([
+    grep -q 'expecting port group name.' hv1/ovn-controller.log
+])
+AT_CHECK([
+    grep -q 'match always evaluates to false' hv1/ovn-controller.log
+], [1])
+
+check ovn-nbctl --wait=hv pg-set-ports pg lsp
+
+dnl Should complain about match always evaluating to false.
+OVS_WAIT_UNTIL([
+    grep -q 'match always evaluates to false' hv1/ovn-controller.log
+])
+
+AT_CLEANUP
+])
-- 
2.31.1

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

Reply via email to