On Fri, Apr 21, 2023 at 9:52 PM Mark Michelson <[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]> > --- > northd/northd.c | 96 +++++++++++++++++++------- > northd/northd.h | 1 + > ovn-nb.ovsschema | 5 +- > ovn-nb.xml | 20 ++++++ > tests/ovn-northd.at | 162 +++++++++++++++++++++++++++++++++++++++----- > tests/system-ovn.at | 107 +++++++++++++++++++++++++++++ > 6 files changed, 347 insertions(+), 44 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 1bafd46c5..647e70a0d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -245,6 +245,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 > @@ -5707,36 +5708,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; > } > } > } > @@ -6447,10 +6463,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; > } > @@ -6475,7 +6500,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); > > @@ -6498,7 +6523,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); > @@ -6519,7 +6544,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); > > @@ -6539,7 +6564,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); > > @@ -6693,6 +6718,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 [] = { > @@ -6705,6 +6731,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++) { > @@ -6745,6 +6775,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; > + 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=%d);", > + j + 1, ingress ? "ingress" : "egress", > + ovn_stage_get_table(stage) - 1); > + ovn_lflow_add(lflows, od, stage, 500, ds_cstr(match), > + ds_cstr(&tier_actions)); > + } > + ds_destroy(&tier_actions); > } > } > > @@ -7114,7 +7158,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 0552eff19..d5606ce7d 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2266,6 +2266,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 b/tests/ovn-northd.at > index eb18f82b0..d8562c9f1 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2161,10 +2161,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]) > @@ -2177,10 +2177,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]) > @@ -7437,7 +7437,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E "ls_.*_acl" > | sed 's/table=../table=??/ > table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1), > action=(next;) > table=??(ls_in_acl_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; 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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_in_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -7474,7 +7474,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E "ls_.*_acl" > | sed 's/table=../table=??/ > table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1), > action=(next;) > table=??(ls_in_acl_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; 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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_in_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -7511,7 +7511,7 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E "ls_.*_acl" > | sed 's/table=../table=??/ > table=??(ls_in_acl_after_lb_eval), priority=0 , match=(1), > action=(next;) > table=??(ls_in_acl_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; 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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_in_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -7619,7 +7619,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_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), 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;) > @@ -7656,7 +7656,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_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), 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;) > @@ -7693,7 +7693,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_after_lb_eval), priority=65532, match=(nd || nd_ra > || nd_rs || mldv1 || mldv2), 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;) > @@ -7815,7 +7815,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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_out_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -7852,7 +7852,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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_out_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -7889,7 +7889,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_eval ), priority=65532, match=(nd || nd_ra || > nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > table=??(ls_out_acl_hint ), priority=0 , match=(1), action=(next;) > @@ -9139,3 +9139,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 b/tests/system-ovn.at > index 8da88c9e4..7488d0a53 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -10667,3 +10667,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", "00:00:00:00:00:01") > +ADD_NAMESPACES(lsp2) > +ADD_VETH(lsp2, lsp2, br-int, "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.39.2 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Looks good to me, thanks. Reviewed-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
