On Mon, Apr 10, 2023 at 7:26 PM Mark Michelson <[email protected]
<mailto:[email protected]>> wrote:
With this commit, ACLs can now be arranged in hierarchical tiers. A tier
number can be assigned to an ACL. When evaluating ACLs, we first will
consider ACLs at tier 0. If no matching ACL is found, then we move to
tier 1. This continues until a matching ACL is found, or we reach the
maximum tier. If no match is found, then the default acl action is
applied.
Signed-off-by: Mark Michelson <[email protected]
<mailto:[email protected]>>
Hi Mark,
I have some small comments below.
---
northd/northd.c | 96 +++++++++++++++++++-------
northd/northd.h | 1 +
ovn-nb.ovsschema | 5 +-
ovn-nb.xml | 20 ++++++
tests/ovn-northd.at <http://ovn-northd.at> | 162
+++++++++++++++++++++++++++++++++++++++-----
tests/system-ovn.at <http://system-ovn.at> | 107
+++++++++++++++++++++++++++++
6 files changed, 347 insertions(+), 44 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 39de54e5b..36ca94ebb 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -242,6 +242,7 @@ enum ovn_stage {
#define REGBIT_ACL_VERDICT_ALLOW "reg8[16]"
#define REGBIT_ACL_VERDICT_DROP "reg8[17]"
#define REGBIT_ACL_VERDICT_REJECT "reg8[18]"
+#define REG_ACL_TIER "reg8[30..31]"
/* Indicate that this packet has been recirculated using egress
* loopback. This allows certain checks to be bypassed, such as a
@@ -5704,36 +5705,51 @@ ovn_ls_port_group_destroy(struct hmap *nb_pgs)
hmap_destroy(nb_pgs);
}
+static bool
+od_set_acl_flags(struct ovn_datapath *od, struct nbrec_acl **acls,
+ size_t n_acls)
+{
+ /* A true return indicates that there are no possible ACL flags
+ * left to set on od. A false return indicates that further ACLs
+ * should be explored in case more flags need to be set on od
+ */
+ if (!n_acls) {
+ return false;
+ }
+
+ od->has_acls = true;
+ for (size_t i = 0; i < n_acls; i++) {
+ const struct nbrec_acl *acl = acls[i];
+ if (acl->tier > od->max_acl_tier) {
+ od->max_acl_tier = acl->tier;
+ }
+ if (!od->has_stateful_acl && !strcmp(acl->action,
"allow-related")) {
+ od->has_stateful_acl = true;
+ }
+ if (od->has_stateful_acl &&
+ od->max_acl_tier ==
nbrec_acl_col_tier.type.value.integer.max) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
static void
ls_get_acl_flags(struct ovn_datapath *od)
{
od->has_acls = false;
od->has_stateful_acl = false;
+ od->max_acl_tier = 0;
- if (od->nbs->n_acls) {
- od->has_acls = true;
-
- for (size_t i = 0; i < od->nbs->n_acls; i++) {
- struct nbrec_acl *acl = od->nbs->acls[i];
- if (!strcmp(acl->action, "allow-related")) {
- od->has_stateful_acl = true;
- return;
- }
- }
+ if (od_set_acl_flags(od, od->nbs->acls, od->nbs->n_acls)) {
+ return;
}
struct ovn_ls_port_group *ls_pg;
HMAP_FOR_EACH (ls_pg, key_node, &od->nb_pgs) {
- if (ls_pg->nb_pg->n_acls) {
- od->has_acls = true;
-
- for (size_t i = 0; i < ls_pg->nb_pg->n_acls; i++) {
- struct nbrec_acl *acl = ls_pg->nb_pg->acls[i];
- if (!strcmp(acl->action, "allow-related")) {
- od->has_stateful_acl = true;
- return;
- }
- }
+ if (od_set_acl_flags(od, ls_pg->nb_pg->acls,
ls_pg->nb_pg->n_acls)) {
+ return;
}
}
}
@@ -6448,10 +6464,19 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
size_t log_verdict_len = actions->length;
uint16_t priority = acl->priority + OVN_ACL_PRI_OFFSET;
+ /* All ACLS will start by matching on their respective tier. */
+ size_t match_tier_len = 0;
+ ds_clear(match);
+ if (od->max_acl_tier) {
+ ds_put_format(match, REG_ACL_TIER " == %"PRId64" && ",
acl->tier);
+ match_tier_len = match->length;
+ }
+
if (!has_stateful || !strcmp(acl->action, "allow-stateless")) {
ds_put_cstr(actions, "next;");
+ ds_put_format(match, "(%s)", acl->match);
ovn_lflow_add_with_hint(lflows, od, stage, priority,
- acl->match, ds_cstr(actions),
+ ds_cstr(match), ds_cstr(actions),
&acl->header_);
return;
}
@@ -6476,7 +6501,7 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
* by ct_commit in the "stateful" stage) to indicate that the
* connection should be allowed to resume.
*/
- ds_clear(match);
+ ds_truncate(match, match_tier_len);
ds_put_format(match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 &&
(%s)",
acl->match);
@@ -6499,7 +6524,7 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
* Commit the connection only if the ACL has a label. This
is done
* to update the connection tracking entry label in case
the ACL
* allowing the connection changes. */
- ds_clear(match);
+ ds_truncate(match, match_tier_len);
ds_truncate(actions, log_verdict_len);
ds_put_format(match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)",
acl->match);
@@ -6520,7 +6545,7 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
* to the connection tracker with ct_commit. */
/* If the packet is not tracked or not part of an established
* connection, then we can simply reject/drop it. */
- ds_clear(match);
+ ds_truncate(match, match_tier_len);
ds_put_cstr(match, REGBIT_ACL_HINT_DROP " == 1");
ds_put_format(match, " && (%s)", acl->match);
@@ -6540,7 +6565,7 @@ consider_acl(struct hmap *lflows, struct
ovn_datapath *od,
* ct_commit() to the "stateful" stage, but since we're
* rejecting/dropping the packet, we go ahead and do it here.
*/
- ds_clear(match);
+ ds_truncate(match, match_tier_len);
ds_put_cstr(match, REGBIT_ACL_HINT_BLOCK " == 1");
ds_put_format(match, " && (%s)", acl->match);
@@ -6692,6 +6717,7 @@ static void
build_acl_action_lflows(struct ovn_datapath *od, struct hmap *lflows,
const char *default_acl_action,
const struct shash *meter_groups,
+ struct ds *match,
struct ds *actions)
{
enum ovn_stage stages [] = {
@@ -6704,6 +6730,10 @@ build_acl_action_lflows(struct ovn_datapath
*od, struct hmap *lflows,
ds_put_cstr(actions, REGBIT_ACL_VERDICT_ALLOW " = 0; "
REGBIT_ACL_VERDICT_DROP " = 0; "
REGBIT_ACL_VERDICT_REJECT " = 0; ");
+ if (od->max_acl_tier) {
+ ds_put_cstr(actions, REG_ACL_TIER " = 0; ");
+ }
+
size_t verdict_len = actions->length;
for (size_t i = 0; i < ARRAY_SIZE(stages); i++) {
@@ -6744,6 +6774,20 @@ build_acl_action_lflows(struct ovn_datapath
*od, struct hmap *lflows,
ds_truncate(actions, verdict_len);
ds_put_cstr(actions, default_acl_action);
ovn_lflow_add(lflows, od, stage, 0, "1", ds_cstr(actions));
+
+ struct ds tier_actions = DS_EMPTY_INITIALIZER;
I don't think we actually need to allocate another ds.
The "actions" variable is not used at this point anymore, it can be used
instead.
+ for (size_t j = 0; j < od->max_acl_tier; j++) {
+ ds_clear(match);
+ ds_put_format(match, REG_ACL_TIER " == %"PRIuSIZE, j);
+ ds_clear(&tier_actions);
+ ds_put_format(&tier_actions, REG_ACL_TIER " =
%"PRIuSIZE"; "
+ "next(pipeline=%s,table=%"PRIu8");",
+ j + 1, ingress ? "ingress" : "egress",
+ ovn_stage_get_table(stage) - 1);
nit: This formatting is not uniform.
+ ovn_lflow_add(lflows, od, stage, 500, ds_cstr(match),
+ ds_cstr(&tier_actions));
+ }
+ ds_destroy(&tier_actions);
}
}
@@ -7107,7 +7151,7 @@ build_acls(struct ovn_datapath *od, const
struct chassis_features *features,
}
build_acl_action_lflows(od, lflows, default_acl_action,
meter_groups,
- &actions);
+ &match, &actions);
ds_destroy(&match);
ds_destroy(&actions);
diff --git a/northd/northd.h b/northd/northd.h
index a503f4a66..ad6ccef5e 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -230,6 +230,7 @@ struct ovn_datapath {
bool has_lb_vip;
bool has_unknown;
bool has_acls;
+ uint64_t max_acl_tier;
bool has_vtep_lports;
bool has_arp_proxy_port;
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index 4836a219f..f12d39542 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
"version": "7.0.0",
- "cksum": "94023179 33468",
+ "cksum": "3195094080 33650",
"tables": {
"NB_Global": {
"columns": {
@@ -272,6 +272,9 @@
"label": {"type": {"key": {"type": "integer",
"minInteger": 0,
"maxInteger":
4294967295}}},
+ "tier": {"type": {"key": {"type": "integer",
+ "minInteger": 0,
+ "maxInteger": 3}}},
"options": {
"type": {"key": "string",
"value": "string",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index d6694778f..19cbd191d 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2257,6 +2257,26 @@ or
</ul>
</column>
+ <column name="tier">
+ <p>The hierarchical tier that this ACL belongs to.</p>
+
+ <p>
+ ACLs can be assigned to numerical tiers. When evaluating
ACLs, an
+ internal counter is used to determine which tier of ACLs
should be
+ evaluated. Tier 0 ACLs are evaluated first. If no verdict
can be
+ determined, then tier 1 ACLs are evaluated next. This continues
+ until the maximum tier value is reached. If all tiers of
ACLs are
+ evaluated and no verdict is reached, then the <ref
column="options"
+ key="default_acl_drop" table="NB_Global" /> option from table
+ <ref table="NB_Global" /> is used to determine how to proceed.
+ </p>
+
+ <p>
+ In this version of OVN, the maximum tier value for ACLs is 3,
+ meaning there are 4 tiers of ACLs allowed (0-3).
+ </p>
+ </column>
+
<group title="options">
<p>
ACLs options.
diff --git a/tests/ovn-northd.at <http://ovn-northd.at>
b/tests/ovn-northd.at <http://ovn-northd.at>
index 19a0e630f..507db7e7a 100644
--- a/tests/ovn-northd.at <http://ovn-northd.at>
+++ b/tests/ovn-northd.at <http://ovn-northd.at>
@@ -2165,10 +2165,10 @@ AT_CAPTURE_FILE([sw1flows])
AT_CHECK(
[grep -E 'ls_(in|out)_acl' sw0flows sw1flows | grep pg0 | sort],
[0], [dnl
-sw0flows: table=4 (ls_out_acl_eval ), priority=2003 ,
match=(outport == @pg0 && ip6 && udp), action=(reg8[[18]] = 1; next;)
-sw0flows: table=8 (ls_in_acl_eval ), priority=2002 ,
match=(inport == @pg0 && ip4 && tcp && tcp.dst == 80),
action=(reg8[[18]] = 1; next;)
-sw1flows: table=4 (ls_out_acl_eval ), priority=2003 ,
match=(outport == @pg0 && ip6 && udp), action=(reg8[[18]] = 1; next;)
-sw1flows: table=8 (ls_in_acl_eval ), priority=2002 ,
match=(inport == @pg0 && ip4 && tcp && tcp.dst == 80),
action=(reg8[[18]] = 1; next;)
+sw0flows: table=4 (ls_out_acl_eval ), priority=2003 ,
match=((outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
+sw0flows: table=8 (ls_in_acl_eval ), priority=2002 ,
match=((inport == @pg0 && ip4 && tcp && tcp.dst == 80)),
action=(reg8[[18]] = 1; next;)
+sw1flows: table=4 (ls_out_acl_eval ), priority=2003 ,
match=((outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
+sw1flows: table=8 (ls_in_acl_eval ), priority=2002 ,
match=((inport == @pg0 && ip4 && tcp && tcp.dst == 80)),
action=(reg8[[18]] = 1; next;)
])
AS_BOX([2])
@@ -2181,10 +2181,10 @@ ovn-sbctl dump-flows sw1 > sw1flows2
AT_CAPTURE_FILE([sw1flows2])
AT_CHECK([grep "ls_out_acl" sw0flows2 sw1flows2 | grep pg0 |
sort], [0], [dnl
-sw0flows2: table=4 (ls_out_acl_eval ), priority=2002 ,
match=(outport == @pg0 && ip4 && udp), action=(reg8[[18]] = 1; next;)
-sw0flows2: table=4 (ls_out_acl_eval ), priority=2003 ,
match=(outport == @pg0 && ip6 && udp), action=(reg8[[18]] = 1; next;)
-sw1flows2: table=4 (ls_out_acl_eval ), priority=2002 ,
match=(outport == @pg0 && ip4 && udp), action=(reg8[[18]] = 1; next;)
-sw1flows2: table=4 (ls_out_acl_eval ), priority=2003 ,
match=(outport == @pg0 && ip6 && udp), action=(reg8[[18]] = 1; next;)
+sw0flows2: table=4 (ls_out_acl_eval ), priority=2002 ,
match=((outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; next;)
+sw0flows2: table=4 (ls_out_acl_eval ), priority=2003 ,
match=((outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
+sw1flows2: table=4 (ls_out_acl_eval ), priority=2002 ,
match=((outport == @pg0 && ip4 && udp)), action=(reg8[[18]] = 1; next;)
+sw1flows2: table=4 (ls_out_acl_eval ), priority=2003 ,
match=((outport == @pg0 && ip6 && udp)), action=(reg8[[18]] = 1; next;)
])
AS_BOX([3])
@@ -7324,7 +7324,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -7358,7 +7358,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -7392,7 +7392,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -7497,7 +7497,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[17]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; /* drop */)
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_after_lb_eval), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
@@ -7531,7 +7531,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[17]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; /* drop */)
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_after_lb_eval), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
@@ -7565,7 +7565,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[17]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; /* drop */)
table=??(ls_in_acl_after_lb_action), priority=1000 ,
match=(reg8[[18]] == 1), action=(reg8[[16]] = 0; reg8[[17]] = 0;
reg8[[18]] = 0; reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst
<-> ip.src; is implicit. */ outport <-> inport;
next(pipeline=egress,table=6); };)
table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1),
action=(next;)
- table=??(ls_in_acl_after_lb_eval), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_in_acl_after_lb_eval), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_eval ), priority=0 , match=(1),
action=(next;)
table=??(ls_in_acl_eval ), priority=34000, match=(eth.dst ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_in_acl_hint ), priority=0 , match=(1),
action=(next;)
@@ -7681,7 +7681,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[17]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; /*
drop */)
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[18]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; reg0
= 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is
implicit. */ outport <-> inport; next(pipeline=ingress,table=27); };)
table=??(ls_out_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_out_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_out_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_eval ), priority=34000, match=(eth.src ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_out_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -7715,7 +7715,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[17]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; /*
drop */)
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[18]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; reg0
= 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is
implicit. */ outport <-> inport; next(pipeline=ingress,table=27); };)
table=??(ls_out_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_out_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_out_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_eval ), priority=34000, match=(eth.src ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_out_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -7749,7 +7749,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
"ls_.*_acl" | sed 's/table=../table=??/
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[17]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; /*
drop */)
table=??(ls_out_acl_action ), priority=1000 , match=(reg8[[18]]
== 1), action=(reg8[[16]] = 0; reg8[[17]] = 0; reg8[[18]] = 0; reg0
= 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is
implicit. */ outport <-> inport; next(pipeline=ingress,table=27); };)
table=??(ls_out_acl_eval ), priority=0 , match=(1),
action=(next;)
- table=??(ls_out_acl_eval ), priority=1001 , match=(ip4 &&
tcp), action=(reg8[[16]] = 1; next;)
+ table=??(ls_out_acl_eval ), priority=1001 , match=((ip4 &&
tcp)), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_eval ), priority=34000, match=(eth.src ==
$svc_monitor_mac), action=(reg8[[16]] = 1; next;)
table=??(ls_out_acl_hint ), priority=0 , match=(1),
action=(next;)
table=??(ls_out_pre_acl ), priority=0 , match=(1),
action=(next;)
@@ -8998,3 +8998,131 @@ mac_binding_timestamp: true
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Tiered ACL logical flows])
+AT_KEYWORDS([acl])
+
+ovn_start
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp
+check ovn-nbctl pg-add pg lsp
+
+m4_define([ACL_FLOWS], [grep -w $1 lflows | grep "$2" | sed
's/table=../table=??/' | sed "s/\($1[[^)]]*\)/$1/" | sort])
+
+acl_test() {
+ direction=$1
+ options=$2
+ thing=$3
+ eval_stage=$4
+ action_stage=$5
+ eval_stage_table=$6
+
+ if test "$direction" = "from-lport" ; then
+ pipeline=ingress
+ else
+ pipeline=egress
+ fi
+
+ # Baseline test. Ensure that no ACL evaluation or tier-related
flows are
+ # installed.
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [])
+
+ AT_CHECK([ACL_FLOWS([$action_stage], [priority=500])], [0], [])
+
+ # Add an untiered ACL. Ensure that the ACL appears in the eval
stage, and
+ # that no tier-related flows appear in the action stage.
+ check ovn-nbctl --wait=sb $options acl-add $thing $direction
1000 "ip4.addr == 80.111.111.112" drop
+ acl1_uuid=$(ovn-nbctl --bare --columns _uuid find ACL
priority=1000)
+
+ ovn-sbctl lflow-list ls > lflows
+ AT_CAPTURE_FILE([lflows])
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=((ip4.addr ==
80.111.111.112)), action=(reg8[[17]] = 1; next;)
+])
+
+ AT_CHECK([ACL_FLOWS([$action_stage], [priority=500])], [0], [])
+
+ # Explicitly name the tier on the ACL to be tier 0. This should
have no
+ # effect on the logical flows.
+ check ovn-nbctl --wait=sb set ACL $acl1_uuid tier=0
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=((ip4.addr ==
80.111.111.112)), action=(reg8[[17]] = 1; next;)
+])
+ AT_CHECK([ACL_FLOWS([$action_stage], [priority=500])], [0], [])
+
+ # Change the ACL to tier 1. Now we should see the tier as part
of the ACL
+ # match, and we should see a flow in the action stage to bump
the tier
+ # to 1 if there was no match on tier 0.
+ check ovn-nbctl --wait=sb set ACL $acl1_uuid tier=1
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=(reg8[[30..31]] == 1
&& (ip4.addr == 80.111.111.112)), action=(reg8[[17]] = 1; next;)
+])
+
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$action_stage], [priority=500])],
[0], [dnl
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
0), action=(reg8[[30..31]] = 1;
next(pipeline=$pipeline,table=$eval_stage_table);)
+])
+
+ # Change the ACL to tier 3. Ensure the tier match on the ACL
has been
+ # updated, and ensure we see three flows present for
incrementing the
+ # tier value in the action stage.
+ check ovn-nbctl --wait=sb set ACL $acl1_uuid tier=3
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=(reg8[[30..31]] == 3
&& (ip4.addr == 80.111.111.112)), action=(reg8[[17]] = 1; next;)
+])
+
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$action_stage], [priority=500])],
[0], [dnl
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
0), action=(reg8[[30..31]] = 1;
next(pipeline=$pipeline,table=$eval_stage_table);)
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
1), action=(reg8[[30..31]] = 2;
next(pipeline=$pipeline,table=$eval_stage_table);)
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
2), action=(reg8[[30..31]] = 3;
next(pipeline=$pipeline,table=$eval_stage_table);)
+])
+
+ # Add an untiered ACL. Ensure that it matches on tier 0, but
otherwise,
+ # nothing else should have changed in the logical flows.
+ check ovn-nbctl --wait=sb $options acl-add $thing $direction
1000 "ip4.addr == 83.104.105.116" allow
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=(reg8[[30..31]] == 0
&& (ip4.addr == 83.104.105.116)), action=(reg8[[16]] = 1; next;)
+ table=??($eval_stage), priority=2000 , match=(reg8[[30..31]] == 3
&& (ip4.addr == 80.111.111.112)), action=(reg8[[17]] = 1; next;)
+])
+
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$action_stage], [priority=500])],
[0], [dnl
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
0), action=(reg8[[30..31]] = 1;
next(pipeline=$pipeline,table=$eval_stage_table);)
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
1), action=(reg8[[30..31]] = 2;
next(pipeline=$pipeline,table=$eval_stage_table);)
+ table=??($action_stage), priority=500 , match=(reg8[[30..31]] ==
2), action=(reg8[[30..31]] = 3;
next(pipeline=$pipeline,table=$eval_stage_table);)
+])
+
+ # Remove the tier 3 ACL. The remaining ACL is untiered, and
there are no
+ # other tiered ACLs. So we should go back to not checking the tier
+ # number in the ACL match, and there should be no tier-related
flows
+ # in the action stage.
+ check ovn-nbctl --wait=sb acl-del $thing $direction 1000
"ip4.addr == 80.111.111.112"
+ ovn-sbctl lflow-list ls > lflows
+ AT_CHECK_UNQUOTED([ACL_FLOWS([$eval_stage], [priority=2000])],
[0], [dnl
+ table=??($eval_stage), priority=2000 , match=((ip4.addr ==
83.104.105.116)), action=(reg8[[16]] = 1; next;)
+])
+
+ AT_CHECK([ACL_FLOWS([$action_stage], [priority=500])], [0], [])
+
+ check ovn-nbctl --wait=sb acl-del $thing
+ ovn-sbctl lflow-list ls > lflows
+
+ AT_CHECK([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [])
+
+ AT_CHECK([ACL_FLOWS([$action_stage], [priority=500])], [0], [])
+}
+
+acl_test from-lport "" ls ls_in_acl_eval ls_in_acl_action 8
+acl_test from-lport "--apply-after-lb" ls ls_in_acl_after_lb_eval
ls_in_acl_after_lb_action 18
+acl_test to-lport "" ls ls_out_acl_eval ls_out_acl_action 4
+acl_test from-lport "" pg ls_in_acl_eval ls_in_acl_action 8
+acl_test from-lport "--apply-after-lb" pg ls_in_acl_after_lb_eval
ls_in_acl_after_lb_action 18
+acl_test to-lport "" pg ls_out_acl_eval ls_out_acl_action 4
+
+AT_CLEANUP
+])
diff --git a/tests/system-ovn.at <http://system-ovn.at>
b/tests/system-ovn.at <http://system-ovn.at>
index 6d1ce9577..620e29cf6 100644
--- a/tests/system-ovn.at <http://system-ovn.at>
+++ b/tests/system-ovn.at <http://system-ovn.at>
@@ -10784,3 +10784,110 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to
query port patch-.*/d
/connection dropped.*/d"])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([Tiered ACLs])
+AT_KEYWORDS([acl])
+
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+check ovs-vsctl \
+ -- set Open_vSwitch . external-ids:system-id=hv1 \
+ -- set Open_vSwitch .
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+ -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+ -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+ -- set bridge br-int fail-mode=secure
other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp1 -- lsp-set-addresses lsp1
"00:00:00:00:00:01 10.0.0.1"
+check ovn-nbctl lsp-add ls lsp2 -- lsp-set-addresses lsp2
"00:00:00:00:00:02 10.0.0.2"
+
+check ovn-nbctl pg-add pg lsp1 lsp2
+
+ADD_NAMESPACES(lsp1)
+ADD_VETH(lsp1, lsp1, br-int, "10.0.0.1/24 <http://10.0.0.1/24>",
"00:00:00:00:00:01")
+ADD_NAMESPACES(lsp2)
+ADD_VETH(lsp2, lsp2, br-int, "10.0.0.2/24 <http://10.0.0.2/24>",
"00:00:00:00:00:02")
+
+m4_define([PING_PCT], [grep -o "[[0-9]]\{1,3\}% packet loss"])
+
+acl_test() {
+ direction=$1
+ options=$2
+ thing=$3
+
+ # First a baseline. If traffic isn't being allowed, then
something is
+ # very wrong.
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+0% packet loss
+])
+ # Add an untiered drop ACL. This should cause pings to fail.
+ check ovn-nbctl --wait=sb $options acl-add $thing $direction
1000 "ip4.dst == 10.0.0.2" drop
+ acl1_uuid=$(ovn-nbctl --bare --columns _uuid find ACL
priority=1000)
+
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+100% packet loss
+])
+
+ # Change the tier to 3. Despite there being "holes" in tiers 0,
1, and 2,
+ # the ACL should still apply, and pings should fail.
+ check ovn-nbctl --wait=sb set ACL $acl1_uuid tier=3
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+100% packet loss
+])
+
+ # Add a tier-0 ACL that allows the traffic. The priority is
only 4, but
+ # since it is a higher tier, the traffic should be allowed.
+ check ovn-nbctl --wait=sb $options acl-add $thing $direction 4
"ip4.dst == 10.0.0.2" allow
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+0% packet loss
+])
+
+ # Removing the 0-tier ACL should make traffic go back to being
dropped.
+ check ovn-nbctl --wait=sb acl-del $thing $direction 4 "ip4.dst
== 10.0.0.2"
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+100% packet loss
+])
+
+ # Removing all ACLs should make traffic go back to passing.
+ check ovn-nbctl --wait=sb acl-del $thing
+ NS_CHECK_EXEC([lsp1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 |
PING_PCT], \
+[0], [dnl
+0% packet loss
+])
+}
+
+acl_test from-lport "" ls
+acl_test from-lport "--apply-after-lb" ls
+acl_test to-lport "" ls
+acl_test from-lport "" pg
+acl_test from-lport "--apply-after-lb" pg
+acl_test to-lport "" pg
+
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
--
2.38.1
_______________________________________________
dev mailing list
[email protected] <mailto:[email protected]>
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
<https://mail.openvswitch.org/mailman/listinfo/ovs-dev>
Other than that and the error that causes the CI failure it looks good.
Thanks,
Ales
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
[email protected] <mailto:[email protected]> IM: amusil
<https://red.ht/sig>
