For each allow-stateless ACL, a rule was added earlier in the pipeline
that circumvented setting REGBIT_CONNTRACK_DEFRAG regardless of
whether other, e.g. allow-related ACLs with higher priority were
present.
Now, when allow-stateless ACLs are present on the switch, for each
allow-related, insert an early pipeline rule that would set DEFRAG bit
for the corresponding match and priority.
Fixes: 3187b9fef1 ("ovn-northd: introduce new allow-stateless ACL verb")
Signed-off-by: Ihar Hrachyshka <[email protected]>
---
northd/lswitch.dl | 20 +++++++++
northd/ovn-northd.c | 91 +++++++++++++++++++++++++++++++--------
northd/ovn_northd.dl | 24 ++++++++++-
tests/ovn-northd.at | 100 ++++++++++++++++++++++++++++++++++++++++---
4 files changed, 210 insertions(+), 25 deletions(-)
diff --git a/northd/lswitch.dl b/northd/lswitch.dl
index a1aaebb6d..b73cfd047 100644
--- a/northd/lswitch.dl
+++ b/northd/lswitch.dl
@@ -129,6 +129,23 @@ LogicalSwitchHasStatefulACL(ls, false) :-
nb::Logical_Switch(._uuid = ls),
not LogicalSwitchStatefulACL(ls, _).
+relation LogicalSwitchStatelessACL(ls: uuid, acl: uuid)
+
+LogicalSwitchStatelessACL(ls, acl) :-
+ LogicalSwitchACL(ls, acl),
+ nb::ACL(._uuid = acl, .action = "allow-stateless").
+
+// "Pitfalls of projections" in ddlog-new-feature.rst explains why this
+// is an output relation:
+output relation LogicalSwitchHasStatelessACL(ls: uuid, has_stateless_acl: bool)
+
+LogicalSwitchHasStatelessACL(ls, true) :-
+ LogicalSwitchStatelessACL(ls, _).
+
+LogicalSwitchHasStatelessACL(ls, false) :-
+ nb::Logical_Switch(._uuid = ls),
+ not LogicalSwitchStatelessACL(ls, _).
+
// "Pitfalls of projections" in ddlog-new-feature.rst explains why this
// is an output relation:
output relation LogicalSwitchHasACLs(ls: uuid, has_acls: bool)
@@ -208,6 +225,7 @@ LogicalSwitchHasNonRouterPort(ls, false) :-
relation &Switch(
ls: nb::Logical_Switch,
has_stateful_acl: bool,
+ has_stateless_acl: bool,
has_acls: bool,
has_lb_vip: bool,
has_dns_records: bool,
@@ -235,6 +253,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
&Switch(.ls = ls,
.has_stateful_acl = has_stateful_acl,
+ .has_stateless_acl = has_stateless_acl,
.has_acls = has_acls,
.has_lb_vip = has_lb_vip,
.has_dns_records = has_dns_records,
@@ -247,6 +266,7 @@ function ipv6_parse_prefix(s: string): Option<in6_addr> {
.is_vlan_transparent = is_vlan_transparent) :-
nb::Logical_Switch[ls],
LogicalSwitchHasStatefulACL(ls._uuid, has_stateful_acl),
+ LogicalSwitchHasStatelessACL(ls._uuid, has_stateless_acl),
LogicalSwitchHasACLs(ls._uuid, has_acls),
LogicalSwitchHasLBVIP(ls._uuid, has_lb_vip),
LogicalSwitchHasDNSRecords(ls._uuid, has_dns_records),
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 7464b7fba..26b723165 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4983,6 +4983,38 @@ skip_port_from_conntrack(struct ovn_datapath *od, struct
ovn_port *op,
ds_destroy(&match_out);
}
+static bool
+apply_to_each_acl_of_action(struct ovn_datapath *od,
+ const struct hmap *port_groups,
+ struct hmap *lflows, const char *action,
+ void (*func)(struct ovn_datapath *,
+ const struct nbrec_acl *,
+ struct hmap *))
+{
+ bool applied = false;
+ for (size_t i = 0; i < od->nbs->n_acls; i++) {
+ const struct nbrec_acl *acl = od->nbs->acls[i];
+ if (!strcmp(acl->action, action)) {
+ func(od, acl, lflows);
+ applied = true;
+ }
+ }
+
+ struct ovn_port_group *pg;
+ HMAP_FOR_EACH (pg, key_node, port_groups) {
+ if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
+ for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
+ const struct nbrec_acl *acl = pg->nb_pg->acls[i];
+ if (!strcmp(acl->action, action)) {
+ func(od, acl, lflows);
+ applied = true;
+ }
+ }
+ }
+ }
+ return applied;
+}
+
static void
build_stateless_filter(struct ovn_datapath *od,
const struct nbrec_acl *acl,
@@ -5003,28 +5035,47 @@ build_stateless_filter(struct ovn_datapath *od,
}
}
-static void
-build_stateless_filters(struct ovn_datapath *od, struct hmap *port_groups,
+static bool
+build_stateless_filters(struct ovn_datapath *od,
+ const struct hmap *port_groups,
struct hmap *lflows)
{
- for (size_t i = 0; i < od->nbs->n_acls; i++) {
- const struct nbrec_acl *acl = od->nbs->acls[i];
- if (!strcmp(acl->action, "allow-stateless")) {
- build_stateless_filter(od, acl, lflows);
- }
- }
+ return apply_to_each_acl_of_action(
+ od, port_groups, lflows, "allow-stateless", build_stateless_filter);
+}
- struct ovn_port_group *pg;
- HMAP_FOR_EACH (pg, key_node, port_groups) {
- if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) {
- for (size_t i = 0; i < pg->nb_pg->n_acls; i++) {
- const struct nbrec_acl *acl = pg->nb_pg->acls[i];
- if (!strcmp(acl->action, "allow-stateless")) {
- build_stateless_filter(od, acl, lflows);
- }
- }
- }
+static void
+build_stateful_override_filter(struct ovn_datapath *od,
+ const struct nbrec_acl *acl,
+ struct hmap *lflows)
+{
+ struct ds match = DS_EMPTY_INITIALIZER;
+ ds_put_format(&match, "ip && %s", acl->match);
+
+ if (!strcmp(acl->direction, "from-lport")) {
+ ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
+ acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match),
+ REGBIT_CONNTRACK_DEFRAG" = 1; next;",
+ &acl->header_);
+ } else {
+ ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
+ acl->priority + OVN_ACL_PRI_OFFSET,
+ ds_cstr(&match),
+ REGBIT_CONNTRACK_DEFRAG" = 1; next;",
+ &acl->header_);
}
+ ds_destroy(&match);
+}
+
+static void
+build_stateful_override_filters(struct ovn_datapath *od,
+ const struct hmap *port_groups,
+ struct hmap *lflows)
+{
+ apply_to_each_acl_of_action(
+ od, port_groups, lflows, "allow-related",
+ build_stateful_override_filter);
}
static void
@@ -5057,7 +5108,9 @@ build_pre_acls(struct ovn_datapath *od, struct hmap
*port_groups,
110, lflows);
}
- build_stateless_filters(od, port_groups, lflows);
+ if (build_stateless_filters(od, port_groups, lflows)) {
+ build_stateful_override_filters(od, port_groups, lflows);
+ }
/* Ingress and Egress Pre-ACL Table (Priority 110).
*
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 5e1788351..7c2402be4 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1822,7 +1822,7 @@ for (&Switch(.ls =ls)) {
.external_ids = map_empty())
}
-for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl, .has_fair_meter =
fair_meter)) {
+for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl, .has_fair_meter = _))
{
if (sw.has_stateful_acl) {
if (acl.action == "allow-stateless") {
if (acl.direction == "from-lport") {
@@ -1844,6 +1844,28 @@ for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl,
.has_fair_meter = fair_
}
}
+for (&SwitchACL(.sw = sw@&Switch{.ls = ls}, .acl = &acl, .has_fair_meter = _))
{
+ if (sw.has_stateless_acl) {
+ if (acl.action == "allow-related") {
+ if (acl.direction == "from-lport") {
+ Flow(.logical_datapath = ls._uuid,
+ .stage = s_SWITCH_IN_PRE_ACL(),
+ .priority = acl.priority + oVN_ACL_PRI_OFFSET(),
+ .__match = "ip && ${acl.__match}",
+ .actions = "${rEGBIT_CONNTRACK_DEFRAG()} = 1;
next;",
+ .external_ids = stage_hint(acl._uuid))
+ } else {
+ Flow(.logical_datapath = ls._uuid,
+ .stage = s_SWITCH_OUT_PRE_ACL(),
+ .priority = acl.priority + oVN_ACL_PRI_OFFSET(),
+ .__match = "ip && ${acl.__match}",
+ .actions = "${rEGBIT_CONNTRACK_DEFRAG()} = 1;
next;",
+ .external_ids = stage_hint(acl._uuid))
+ }
+ }
+ }
+}
+
/* If there are any stateful ACL rules in this datapath, we must
* send all IP packets through the conntrack action, which handles
* defragmentation, in order to match L4 headers. */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 6c2745ed1..da75434b6 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2664,6 +2664,97 @@ sed 's/reg8\[[0..15\]] == [[0-9]]*/reg8\[[0..15\]] ==
<cleared>/' | sort], [0],
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- ACL priority: allow-stateless vs allow-related])
+ovn_start
+
+ovn-nbctl ls-add ls
+ovn-nbctl lsp-add ls lsp1
+ovn-nbctl lsp-set-addresses lsp1 00:00:00:00:00:01
+ovn-nbctl lsp-add ls lsp2
+ovn-nbctl lsp-set-addresses lsp2 00:00:00:00:00:02
+
+for direction in from to; do
+ ovn-nbctl acl-add ls ${direction}-lport 3 "tcp" allow-related
+done
+ovn-nbctl --wait=sb sync
+
+flow_eth='eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
+flow_ip='ip.ttl==64 && ip4.src == 42.42.42.1 && ip4.dst == 66.66.66.66'
+flow_tcp='tcp && tcp.dst == 80'
+flow_udp='udp && udp.dst == 80'
+lsp1_inport=$(fetch_column Port_Binding tunnel_key logical_port=lsp1)
+
+# TCP packets should go to conntrack.
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0],
[dnl
+#
tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+ct_next(ct_state=new|trk) {
+ ct_next(ct_state=new|trk) {
+ output("lsp2");
+ };
+};
+])
+
+# Add allow-stateless with lower priority.
+for direction in from to; do
+ ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should still go to conntrack.
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0],
[dnl
+#
tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+ct_next(ct_state=new|trk) {
+ ct_next(ct_state=new|trk) {
+ output("lsp2");
+ };
+};
+])
+
+# Add another allow-stateless with higher priority.
+for direction in from to; do
+ ovn-nbctl acl-add ls ${direction}-lport 5 tcp allow-stateless
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should no longer go to conntrack
+AT_CHECK_UNQUOTED([ovn-trace --minimal ls "${flow}"], [0], [dnl
+#
tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+output("lsp2");
+])
+
+# Remove the higher priority allow-stateless.
+for direction in from to; do
+ ovn-nbctl acl-del ls ${direction}-lport 5 tcp
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should again go to conntrack.
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0],
[dnl
+#
tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+ct_next(ct_state=new|trk) {
+ ct_next(ct_state=new|trk) {
+ output("lsp2");
+ };
+};
+])
+
+# Remove the allow-related ACL.
+for direction in from to; do
+ ovn-nbctl acl-del ls ${direction}-lport 3 tcp
+done
+ovn-nbctl --wait=sb sync
+
+# TCP packets should no longer go to conntrack
+AT_CHECK_UNQUOTED([ovn-trace --minimal ls "${flow}"], [0], [dnl
+#
tcp,reg14=0x${lsp1_inport},vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:00:00:00:02,nw_src=42.42.42.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80,tcp_flags=0
+output("lsp2");
+])
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn -- ACL allow-stateless omit conntrack - Logical_Switch])
ovn_start
@@ -2712,7 +2803,7 @@ ct_next(ct_state=new|trk) {
# Allow stateless for TCP.
for direction in from to; do
- ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
+ ovn-nbctl acl-add ls ${direction}-lport 5 tcp allow-stateless
done
ovn-nbctl --wait=sb sync
@@ -2768,7 +2859,7 @@ ct_lb {
# Allow stateless for TCP.
for direction in from to; do
- ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
+ ovn-nbctl acl-add ls ${direction}-lport 5 tcp allow-stateless
done
ovn-nbctl --wait=sb sync
@@ -2817,7 +2908,6 @@ done
ovn-nbctl --wait=sb sync
lsp1_inport=$(fetch_column Port_Binding tunnel_key logical_port=lsp1)
-echo $lsp1_inport
flow_eth='eth.src == 00:00:00:00:00:01 && eth.dst == 00:00:00:00:00:02'
flow_ip='ip.ttl==64 && ip4.src == 42.42.42.1 && ip4.dst == 66.66.66.66'
@@ -2848,7 +2938,7 @@ ct_next(ct_state=new|trk) {
# Allow stateless for TCP.
for direction in from to; do
- ovn-nbctl acl-add pg ${direction}-lport 1 tcp allow-stateless
+ ovn-nbctl acl-add pg ${direction}-lport 5 tcp allow-stateless
done
ovn-nbctl --wait=sb sync
@@ -2904,7 +2994,7 @@ ct_lb {
# Allow stateless for TCP.
for direction in from to; do
- ovn-nbctl acl-add pg ${direction}-lport 1 tcp allow-stateless
+ ovn-nbctl acl-add pg ${direction}-lport 5 tcp allow-stateless
done
ovn-nbctl --wait=sb sync
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
