From patchwork Wed Dec 1 12:56:08 2021
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Vladislav Odintsov
X-Patchwork-Id: 1562162
Return-Path:
X-Original-To: incoming@patchwork.ozlabs.org
Delivered-To: patchwork-incoming@bilbo.ozlabs.org
Authentication-Results: bilbo.ozlabs.org;
dkim=fail reason="signature verification failed" (2048-bit key;
unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256
header.s=20210112 header.b=dHEsPuwz;
dkim-atps=neutral
Authentication-Results: ozlabs.org;
spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org
(client-ip=140.211.166.136; helo=smtp3.osuosl.org;
envelope-from=ovs-dev-bounces@openvswitch.org; receiver=)
Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest
SHA256)
(No client certificate requested)
by bilbo.ozlabs.org (Postfix) with ESMTPS id 4J3zcG1Hg7z9sCD
for ; Wed, 1 Dec 2021 23:56:30 +1100 (AEDT)
Received: from localhost (localhost [127.0.0.1])
by smtp3.osuosl.org (Postfix) with ESMTP id 3C9A561BC3;
Wed, 1 Dec 2021 12:56:27 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Received: from smtp3.osuosl.org ([127.0.0.1])
by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
with ESMTP id wRLtZTGojDmH; Wed, 1 Dec 2021 12:56:23 +0000 (UTC)
Received: from lists.linuxfoundation.org (lf-lists.osuosl.org
[IPv6:2605:bc80:3010:104::8cd3:938])
by smtp3.osuosl.org (Postfix) with ESMTPS id 2372B61BC5;
Wed, 1 Dec 2021 12:56:22 +0000 (UTC)
Received: from lf-lists.osuosl.org (localhost [127.0.0.1])
by lists.linuxfoundation.org (Postfix) with ESMTP id BF128C003C;
Wed, 1 Dec 2021 12:56:20 +0000 (UTC)
X-Original-To: dev@openvswitch.org
Delivered-To: ovs-dev@lists.linuxfoundation.org
Received: from smtp2.osuosl.org (smtp2.osuosl.org [IPv6:2605:bc80:3010::133])
by lists.linuxfoundation.org (Postfix) with ESMTP id C93BDC003F
for ; Wed, 1 Dec 2021 12:56:19 +0000 (UTC)
Received: from localhost (localhost [127.0.0.1])
by smtp2.osuosl.org (Postfix) with ESMTP id A69414033C
for ; Wed, 1 Dec 2021 12:56:19 +0000 (UTC)
X-Virus-Scanned: amavisd-new at osuosl.org
Authentication-Results: smtp2.osuosl.org (amavisd-new);
dkim=pass (2048-bit key) header.d=gmail.com
Received: from smtp2.osuosl.org ([127.0.0.1])
by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)
with ESMTP id Op0GCI-Qp5JN for ;
Wed, 1 Dec 2021 12:56:18 +0000 (UTC)
X-Greylist: whitelisted by SQLgrey-1.8.0
Received: from mail-ed1-x534.google.com (mail-ed1-x534.google.com
[IPv6:2a00:1450:4864:20::534])
by smtp2.osuosl.org (Postfix) with ESMTPS id 11A4F400FB
for ; Wed, 1 Dec 2021 12:56:17 +0000 (UTC)
Received: by mail-ed1-x534.google.com with SMTP id v1so101170689edx.2
for ; Wed, 01 Dec 2021 04:56:17 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112;
h=from:to:cc:subject:date:message-id:in-reply-to:references
:mime-version:content-transfer-encoding;
bh=GduZQm5a726nI1lN0MDPF0x1VLr8YR9kC2mMA7TaW4E=;
b=dHEsPuwzTkVGtyx2vXCqk5X0bgAx/fN4AvwhujaaUmF44EUBmNll6knVZQPyEaAinJ
Dgyi87/KACWh/naPVCvk8TofNNQkWb7u3M3hjMIXmcOyVsOLAI7O+GKeY7sCmTAUgJKv
NxSN+vOVmH8se4hXzcoqIFSBy64oW8Mqm+YcBoQKodPCNX9FYwiocKli/vZnxLLgzQdA
HfllSxIcrBm82qOi45AyBowfYDhaN3QilHtLLCC5iPLwa2dWpJaCkE2wIidvmQribSq+
sCxk+9/IZDOnkES9X62+k1YNlijFn3bUe6WomlKeOTbL32kvhpcgw8SbCZTd7/c08j9j
dT7g==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20210112;
h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to
:references:mime-version:content-transfer-encoding;
bh=GduZQm5a726nI1lN0MDPF0x1VLr8YR9kC2mMA7TaW4E=;
b=sEHVsRF/Ox6aMOScO7rTbJ4tzs8aE2C4HecsoOQU/ldf2xhp/L0DANjccnVMRUyZFJ
J262uIAN82WyzeFL3jAG1GmGYm8QQuCV8IBRSrre/UWjw7XJTCiXBF8Ipcau01a78E3V
F8QNmhnTJjyx3z5RRe7TaegV3/125cObnvorf/+KKD/E6nWO+JJhatVHWTaH1AEJH9VP
phLPqN9iUa7sqQUumuFuMThV3/61BU8pz0fcfU83Om/uSHOvovCSx9mdQ3RvXdu+Qvjr
bbdTrsNzz+rZlE2hRsna73mplXhJ1cPHABakZdNcG8U1tzHibxE+wwtXDkVum8lb92lm
8dQw==
X-Gm-Message-State: AOAM530L36yg1A1RRrmI0MPnjxNFnAOmgCmgDz57PJCv4BS4Dei3WcWf
gA7BTndulmJoKTGFtl4SR8YfONTVtxnx7w==
X-Google-Smtp-Source:
ABdhPJy5AiVVsUL/fTEJudrE06yLFFDDSXwv9jju1T9y4JJFVoMmRRiB3mPqqD/nFexQsNB7ThIqTw==
X-Received: by 2002:a05:6402:339:: with SMTP id
q25mr8402130edw.251.1638363375481;
Wed, 01 Dec 2021 04:56:15 -0800 (PST)
Received: from localhost.localdomain (109-252-131-59.dynamic.spd-mgts.ru.
[109.252.131.59])
by smtp.gmail.com with ESMTPSA id dm6sm6499907ejc.89.2021.12.01.04.56.14
(version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128);
Wed, 01 Dec 2021 04:56:15 -0800 (PST)
From: Vladislav Odintsov
To: dev@openvswitch.org
Date: Wed, 1 Dec 2021 15:56:08 +0300
Message-Id: <20211201125608.36918-4-odivlad@gmail.com>
X-Mailer: git-send-email 2.30.0
In-Reply-To: <20211201125608.36918-1-odivlad@gmail.com>
References: <20211201125608.36918-1-odivlad@gmail.com>
MIME-Version: 1.0
Cc: Vladislav Odintsov
Subject: [ovs-dev] [PATCH ovn 3/3] northd: support mix of stateless ACL with
lower priority than stateful
X-BeenThere: ovs-dev@openvswitch.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id:
List-Unsubscribe: ,
List-Archive:
List-Post:
List-Help:
List-Subscribe: ,
Errors-To: ovs-dev-bounces@openvswitch.org
Sender: "dev"
It was unsupported - to have a mix of stateless ACLs with lower than
stateful priority at the same time with stateful (related) ACLs.
This patch changes stateless ACLs behaviour, but this change should not
be harmful for users. Likely nobody mixed rules in the described manner,
so this change should be okay.
Signed-off-by: Vladislav Odintsov
---
northd/northd.c | 92 +++++++++++++++++++++++++----------------
northd/ovn-northd.8.xml | 4 +-
tests/ovn-northd.at | 48 ++++++++++++---------
3 files changed, 87 insertions(+), 57 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 2efc4bb1f..1831e6e79 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -592,6 +592,7 @@ struct ovn_datapath {
uint32_t port_key_hint;
bool has_stateful_acl;
+ bool has_stateless_acl;
bool has_lb_vip;
bool has_unknown;
bool has_acls;
@@ -5416,15 +5417,26 @@ ls_get_acl_flags(struct ovn_datapath *od)
{
od->has_acls = false;
od->has_stateful_acl = false;
+ od->has_stateless_acl = false;
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")) {
+ if (!od->has_stateful_acl &&
+ !strcmp(acl->action, "allow-related")) {
od->has_stateful_acl = true;
- return;
+ if (od->has_stateless_acl) {
+ return;
+ }
+ }
+ if (!od->has_stateless_acl &&
+ !strcmp(acl->action, "allow-stateless")) {
+ od->has_stateless_acl = true;
+ if (od->has_stateful_acl) {
+ return;
+ }
}
}
}
@@ -5436,9 +5448,19 @@ ls_get_acl_flags(struct ovn_datapath *od)
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")) {
+ if (!od->has_stateful_acl &&
+ !strcmp(acl->action, "allow-related")) {
od->has_stateful_acl = true;
- return;
+ if (od->has_stateless_acl) {
+ return;
+ }
+ }
+ if (!od->has_stateless_acl &&
+ !strcmp(acl->action, "allow-stateless")) {
+ od->has_stateless_acl = true;
+ if (od->has_stateful_acl) {
+ return;
+ }
}
}
}
@@ -5647,45 +5669,42 @@ skip_port_from_conntrack(struct ovn_datapath *od, struct ovn_port *op,
}
static void
-build_stateless_filter(struct ovn_datapath *od,
- const struct nbrec_acl *acl,
- struct hmap *lflows)
+build_acl_filter(struct ovn_datapath *od, const struct nbrec_acl *acl,
+ struct hmap *lflows)
{
+ int direction;
+ char *actions;
+
if (!strcmp(acl->direction, "from-lport")) {
- ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
- acl->priority + OVN_ACL_PRI_OFFSET,
- acl->match,
- "next;",
- &acl->header_);
+ direction = S_SWITCH_IN_PRE_ACL;
} else {
- ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
- acl->priority + OVN_ACL_PRI_OFFSET,
- acl->match,
- "next;",
- &acl->header_);
+ direction = S_SWITCH_OUT_PRE_ACL;
+ }
+
+ if (!strcmp(acl->action, "allow-stateless")) {
+ actions = "next;";
+ } else {
+ actions = REGBIT_CONNTRACK_DEFRAG" = 1; next;";
}
+
+ ovn_lflow_add_with_hint(lflows, od, direction,
+ acl->priority + OVN_ACL_PRI_OFFSET, acl->match,
+ actions, &acl->header_);
}
static void
-build_stateless_filters(struct ovn_datapath *od,
- const struct hmap *port_groups,
- struct hmap *lflows)
+build_acl_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);
- }
+ build_acl_filter(od, od->nbs->acls[i], lflows);
}
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);
- }
+ build_acl_filter(od, pg->nb_pg->acls[i], lflows);
}
}
}
@@ -5700,10 +5719,10 @@ build_pre_acls(struct ovn_datapath *od, const struct hmap *port_groups,
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;");
- ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 65535,
"eth.dst == $svc_monitor_mac", "next;");
- ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+ ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 65535,
"eth.src == $svc_monitor_mac", "next;");
/* If there are any stateful ACL rules in this datapath, we may
@@ -5713,25 +5732,26 @@ build_pre_acls(struct ovn_datapath *od, const struct hmap *port_groups,
for (size_t i = 0; i < od->n_router_ports; i++) {
skip_port_from_conntrack(od, od->router_ports[i],
S_SWITCH_IN_PRE_ACL, S_SWITCH_OUT_PRE_ACL,
- 110, lflows);
+ 65535, lflows);
}
for (size_t i = 0; i < od->n_localnet_ports; i++) {
skip_port_from_conntrack(od, od->localnet_ports[i],
S_SWITCH_IN_PRE_ACL, S_SWITCH_OUT_PRE_ACL,
- 110, lflows);
+ 65535, lflows);
}
- /* stateless filters always take precedence over stateful ACLs. */
- build_stateless_filters(od, port_groups, lflows);
+ if (od->has_stateless_acl) {
+ build_acl_filters(od, port_groups, lflows);
+ }
/* Ingress and Egress Pre-ACL Table (Priority 110).
*
* Not to do conntrack on ND and ICMP destination
* unreachable packets. */
- ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 65535,
"nd || nd_rs || nd_ra || mldv1 || mldv2 || "
"(udp && udp.src == 546 && udp.dst == 547)", "next;");
- ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110,
+ ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 65535,
"nd || nd_rs || nd_ra || mldv1 || mldv2 || "
"(udp && udp.src == 546 && udp.dst == 547)", "next;");
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index bd3c3aa26..f92906e5b 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -459,7 +459,9 @@
priority-110 flow is added to skip over stateful ACLs. IPv6 Neighbor
Discovery and MLD traffic also skips stateful ACLs. For "allow-stateless"
ACLs, a flow is added to bypass setting the hint for connection tracker
- processing.
+ processing. If datapath has "allow-stateless" and "allow-related" ACLs,
+ a flow for each ACL would be added with priority value calculated as 1000
+ plus ACL's priority and action reg0[0] = 1; next;
.
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c4424ab14..b168b79c9 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3100,7 +3100,7 @@ AT_CLEANUP
])
OVN_FOR_EACH_NORTHD([
-AT_SETUP([ACL allow-stateless overrides stateful rules with higher priority - Logical_Switch])
+AT_SETUP([ACL allow-stateless doesn't override stateful rules with higher priority - Logical_Switch])
ovn_start
ovn-nbctl ls-add ls
@@ -3110,22 +3110,29 @@ 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
- ovn-nbctl acl-add ls ${direction}-lport 3 "udp" allow
+ ovn-nbctl acl-add ls ${direction}-lport 3 "ip4.src == 10.0.0.0/24 && tcp" allow-related
+ ovn-nbctl acl-add ls ${direction}-lport 3 "ip4.src == 10.0.0.0/24 && udp" allow
+done
+
+# Allow stateless with *lower* priority. It should not override stateful rules.
+for direction in from to; do
+ ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
+ ovn-nbctl acl-add ls ${direction}-lport 1 udp allow-stateless
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_ip_10='ip.ttl==64 && ip4.src == 10.0.0.1 && ip4.dst == 66.66.66.66'
+flow_ip_20='ip.ttl==64 && ip4.src == 20.0.0.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}"
+# TCP and UDP packets should go to conntrack.
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip_10} && ${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
+# 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=10.0.0.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");
@@ -3133,24 +3140,25 @@ ct_next(ct_state=new|trk) {
};
])
-# Allow stateless with *lower* priority. It always beats stateful rules.
-for direction in from to; do
- ovn-nbctl acl-add ls ${direction}-lport 1 tcp allow-stateless
- ovn-nbctl acl-add ls ${direction}-lport 1 udp allow-stateless
-done
-ovn-nbctl --wait=sb sync
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip_10} && ${flow_udp}"
+AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
+# udp,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=10.0.0.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
+ct_next(ct_state=new|trk) {
+ ct_next(ct_state=new|trk) {
+ output("lsp2");
+ };
+};
+])
-# TCP packets should not go to conntrack anymore.
-flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_tcp}"
+# TCP and UDP packets from 20.0.0.1 should go to conntrack.
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip_20} && ${flow_tcp}"
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
+# 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=20.0.0.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");
])
-
-# UDP packets should not go to conntrack anymore.
-flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${flow_udp}"
+flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip_20} && ${flow_udp}"
AT_CHECK_UNQUOTED([ovn-trace --ct new --ct new --minimal ls "${flow}"], [0], [dnl
-# udp,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
+# udp,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=20.0.0.1,nw_dst=66.66.66.66,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=0,tp_dst=80
output("lsp2");
])