From: Numan Siddique <[email protected]>
After the commit [1], ovn-northd doesn't generate the required logical
flows for the tiered ACLs causing the higher tier ACLs to be skipped
from the evaluation. It should have used
'nbrec_acl_col_tier.type.key.integer.max' instead of
'nbrec_acl_col_tier.type.value.integer.max' to get the max tier
supported by the schema since the 'tier' column is not an smap.
'nbrec_acl_col_tier.type.value.integer.max' is 0 and the function
ls_acl_tiers_are_maxed_out() returns true for an ACL with tier 0.
This issue will only be seen if the first ACL in the acl column of
logical switch or port group has tier value 0. In this case
ls_stateful_record_set_acl_flags_() only evaluates the first ACL
and never updates the ls_stateful_rec->max_acl_tier to the actual
values, there by skipping the required logical flows for tier
evaluation.
This patch fixes it. It also enhances the existing test case
to cover this scenario.
[1] - 6aa66f9e1c58 ("northd: Track max ACL tiers more accurately.")
Fixes: 6aa66f9e1c58 ("northd: Track max ACL tiers more accurately.")
Reported-at: https://issues.redhat.com/browse/FDP-1154
Signed-off-by: Numan Siddique <[email protected]>
---
northd/en-ls-stateful.c | 2 +-
tests/ovn-northd.at | 28 ++++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
index 11e97aa9bb..69cda5008c 100644
--- a/northd/en-ls-stateful.c
+++ b/northd/en-ls-stateful.c
@@ -455,7 +455,7 @@ ls_stateful_record_set_acl_flags_(struct ls_stateful_record
*ls_stateful_rec,
if (ls_stateful_rec->has_stateful_acl &&
ls_acl_tiers_are_maxed_out(
&ls_stateful_rec->max_acl_tier,
- nbrec_acl_col_tier.type.value.integer.max)) {
+ nbrec_acl_col_tier.type.key.integer.max)) {
return true;
}
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 0ddb120279..07aac830d7 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -14636,6 +14636,34 @@ AT_CHECK([ovn-sbctl lflow-list S1 | grep
ls_out_acl_action | grep priority=500 |
table=??(ls_out_acl_action ), priority=500 , match=(reg8[[30..31]] == 1),
action=(reg8[[30..31]] = 2; next(pipeline=egress,table=??);)
])
+# Create a port group and add 2 ACLs.
+# First add an allow-related ACL with tier 1 with a predefine UUID and
+# make sure that there is a pri 500 flow to check for ACLs in tier 1.
+# Then add another ACL with tier 0 and make sure that the pri 500
+# flow still exists.
+check ovn-nbctl ls-add S2
+check ovn-nbctl lsp-add S2 S2-p1
+check ovn-nbctl lsp-add S2 S2-p2
+check ovn-nbctl pg-add pg1 S2-p1 S2-p2
+
+check_uuid ovn-nbctl --id=3f507ce6-f6e6-4b18-829b-80a18a8143cd create ACL \
+action=allow-related direction=from-lport match=tcp priority=1001 tier=1 -- \
+add port_group pg1 acls 3f507ce6-f6e6-4b18-829b-80a18a8143cd
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([ovn-sbctl lflow-list S2 | grep ls_in_acl_action | grep priority=500
| ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_acl_action ), priority=500 , match=(reg8[[30..31]] == 0),
action=(reg8[[30..31]] = 1; next(pipeline=ingress,table=??);)
+])
+
+check_uuid ovn-nbctl --id=3f507ce6-f6e6-4b18-829b-80a18a8143cc create ACL \
+action=allow-related direction=from-lport match=ip4 priority=2001 tier=0 -- \
+add port_group pg1 acls 3f507ce6-f6e6-4b18-829b-80a18a8143cc
+check ovn-nbctl --wait=sb sync
+
+AT_CHECK([ovn-sbctl lflow-list S2 | grep ls_in_acl_action | grep priority=500
| ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_acl_action ), priority=500 , match=(reg8[[30..31]] == 0),
action=(reg8[[30..31]] = 1; next(pipeline=ingress,table=??);)
+])
+
AT_CLEANUP
])
--
2.48.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
