[ovs-dev] [PATCH ovn] Document priority behavior for allow-stateless ACLs

Han Zhou zhouhan at gmail.com
Wed Jun 9 05:37:00 UTC 2021


On Tue, Jun 8, 2021 at 1:33 PM Ihar Hrachyshka <ihrachys at redhat.com> wrote:
>
> It's complex and probably impossible to split returning traffic for
> allow-related ACLs from stateless traffic, we don't fully implement
> ACL priority for allow-stateless rules. Meaning, allow-stateless rules
> always take precedence over stateful rules regardless of their
> relative priority order.
>
> This patch documents this behavior and covers it with explicit test
> cases.
>
> Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>
> ---
>  northd/ovn-northd.c  |  1 +
>  northd/ovn_northd.dl |  1 +
>  ovn-nb.xml           |  7 +++++
>  tests/ovn-northd.at  | 66 +++++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 71 insertions(+), 4 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 9652ce252..d872f6a3c 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5063,6 +5063,7 @@ build_pre_acls(struct ovn_datapath *od, struct hmap
*port_groups,
>                                       110, lflows);
>          }
>
> +        /* stateless filters always take precedence over stateful ACLs.
*/
>          build_stateless_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 cb8418540..3afa80a3b 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1841,6 +1841,7 @@ for (&Switch(._uuid =ls_uuid)) {
>           .external_ids     = map_empty())
>  }
>
> +/* stateless filters always take precedence over stateful ACLs. */
>  for (&SwitchACL(.sw = sw@&Switch{._uuid = ls_uuid}, .acl = &acl,
.has_fair_meter = fair_meter)) {
>      if (sw.has_stateful_acl) {
>          if (acl.action == "allow-stateless") {
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 47f25eac1..91b5e303a 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1761,6 +1761,13 @@
>          Return traffic from an <code>allow-related</code> flow is always
>          allowed and cannot be changed through an ACL.
>        </p>
> +
> +      <p>
> +        <code>allow-stateless</code> flows always take precedence before
> +        stateful ACLs, regardless of their priority. (Both
> +        <code>allow</code> and <code>allow-related</code> ACLs can be
> +        stateful.)
> +      </p>
>      </column>
>
>      <column name="direction">
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 818ff7a20..4692775ad 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -2722,7 +2722,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 4 tcp allow-stateless
>  done
>  ovn-nbctl --wait=sb sync
>
> @@ -2778,7 +2778,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 4 tcp allow-stateless
>  done
>  ovn-nbctl --wait=sb sync
>
> @@ -2858,7 +2858,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 4 tcp allow-stateless
>  done
>  ovn-nbctl --wait=sb sync
>
> @@ -2914,7 +2914,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 4 tcp allow-stateless
>  done
>  ovn-nbctl --wait=sb sync
>
> @@ -2943,6 +2943,64 @@ ct_lb {
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- ACL allow-stateless overrides stateful rules with
higher priority - Logical_Switch])
> +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
> +    ovn-nbctl acl-add ls ${direction}-lport 3 "udp" allow
> +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");
> +    };
> +};
> +])
> +
> +# 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
> +
> +# TCP packets should not go to conntrack anymore.
> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${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
> +output("lsp2");
> +])
> +
> +# UDP packets should not go to conntrack anymore.
> +flow="inport == \"lsp1\" && ${flow_eth} && ${flow_ip} && ${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
> +output("lsp2");
> +])
> +
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([ovn -- check BFD config propagation to SBDB])
>  AT_KEYWORDS([northd-bfd])
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Ihar. I applied this to master and also backported to branch-21.06
since the document is important to avoid confusion.


More information about the dev mailing list