On Wed, Dec 4, 2024 at 10:53 PM Mark Michelson <mmich...@redhat.com> wrote:
> "allow-established" ACLs are identical to allow-related ACLs, with one
> exception. On allow-related ACLs, if the ACL changes and no longer
> matches an established session, then traffic will no longer
> automatically be allowed. Instead, traffic on the established session
> will be subject to ACL matching, and therefore the traffic may be
> dropped.
>
> For allow-established ACLs, this behavior is altered. We set a specific
> bit in the ct_mark to indicate this is an allow-established ACL. If this
> bit is set, then established session traffic will always be allowed,
> even if the ACL is altered to no longer match the traffic.
>
> Upcoming commits will put in place methods so that deleting the ACL or
> changing the action type on the ACL will remove the conntrack entry that
> allows the established traffic.
>
> Signed-off-by: Mark Michelson <mmich...@redhat.com>
> ---
>
Hi Mark,
thank you for the patch, I have a few minor comments down below.
> include/ovn/logical-fields.h | 1 +
> lib/logical-fields.c | 5 ++
> northd/en-ls-stateful.c | 3 +-
> northd/northd.c | 36 +++++++-
> ovn-nb.ovsschema | 7 +-
> ovn-nb.xml | 11 +++
> tests/automake.mk | 4 +-
> tests/client.py | 38 ++++++++
> tests/ovn-northd.at | 134 +++++++++++++++++++++++++++++
> tests/server.py | 30 +++++++
> tests/system-ovn.at | 162 +++++++++++++++++++++++++++++++++++
> 11 files changed, 424 insertions(+), 7 deletions(-)
> create mode 100755 tests/client.py
> create mode 100755 tests/server.py
>
> diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
> index d563e044c..c7e4baa51 100644
> --- a/include/ovn/logical-fields.h
> +++ b/include/ovn/logical-fields.h
> @@ -203,6 +203,7 @@ const struct ovn_field *ovn_field_from_name(const char
> *name);
> #define OVN_CT_LB_FORCE_SNAT_BIT 3
> #define OVN_CT_OBS_STAGE_1ST_BIT 4
> #define OVN_CT_OBS_STAGE_END_BIT 5
> +#define OVN_CT_ALLOW_ESTABLISHED_BIT 6
>
> #define OVN_CT_BLOCKED 1
> #define OVN_CT_NATTED 2
> diff --git a/lib/logical-fields.c b/lib/logical-fields.c
> index 5a8b53f2b..5b578b5c2 100644
> --- a/lib/logical-fields.c
> +++ b/lib/logical-fields.c
> @@ -171,6 +171,11 @@ ovn_init_symtab(struct shash *symtab)
> OVN_CT_STR(OVN_CT_OBS_STAGE_END_BIT)
> "]",
> WR_CT_COMMIT);
> + expr_symtab_add_subfield_scoped(symtab, "ct_mark.allow_established",
> NULL,
> + "ct_mark["
> +
> OVN_CT_STR(OVN_CT_ALLOW_ESTABLISHED_BIT)
> + "]",
> + WR_CT_COMMIT);
> expr_symtab_add_subfield_scoped(symtab, "ct_mark.obs_collector_id",
> NULL,
> "ct_mark[16..23]", WR_CT_COMMIT);
>
> diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
> index 44f74ea08..1dc8ce01f 100644
> --- a/northd/en-ls-stateful.c
> +++ b/northd/en-ls-stateful.c
> @@ -412,7 +412,8 @@ ls_stateful_record_set_acl_flags_(struct
> ls_stateful_record *ls_stateful_rec,
> ls_stateful_rec->max_acl_tier = acl->tier;
> }
> if (!ls_stateful_rec->has_stateful_acl
> - && !strcmp(acl->action, "allow-related")) {
> + && (!strcmp(acl->action, "allow-related")
> + || !strcmp(acl->action, "allow-established"))) {
>
nit: Formatting
ls_stateful_rec->has_stateful_acl = true;
> }
> if (ls_stateful_rec->has_stateful_acl &&
> diff --git a/northd/northd.c b/northd/northd.c
> index 3a488ff3d..0aae627a2 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -124,6 +124,7 @@ static bool vxlan_mode;
> #define REGBIT_ACL_HINT_ALLOW_REL "reg0[17]"
> #define REGBIT_FROM_ROUTER_PORT "reg0[18]"
> #define REGBIT_IP_FRAG "reg0[19]"
> +#define REGBIT_ACL_PERSIST_ID "reg0[20]"
>
> #define REG_ORIG_DIP_IPV4 "reg1"
> #define REG_ORIG_DIP_IPV6 "xxreg1"
> @@ -7121,7 +7122,8 @@ consider_acl(struct lflow_table *lflows, const
> struct ovn_datapath *od,
> }
>
> if (!strcmp(acl->action, "allow")
> - || !strcmp(acl->action, "allow-related")) {
> + || !strcmp(acl->action, "allow-related")
> + || !strcmp(acl->action, "allow-established")) {
> /* If there are any stateful flows, we must even commit "allow"
> * actions. This is because, while the initiater's
> * direction may not have any stateful rules, the server's
> @@ -7147,6 +7149,11 @@ consider_acl(struct lflow_table *lflows, const
> struct ovn_datapath *od,
> ds_truncate(actions, log_verdict_len);
> ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; ");
>
> + if (!strcmp(acl->action, "allow-established")) {
> + ds_put_format(actions,
> + REGBIT_ACL_PERSIST_ID " = 1; ");
> + }
> +
> /* For stateful ACLs sample "new" and "established" packets. */
> build_acl_sample_label_action(actions, acl, acl->sample_new,
> acl->sample_est, obs_stage);
> @@ -7626,6 +7633,26 @@ build_acls(const struct ls_stateful_record
> *ls_stateful_rec,
> REGBIT_ACL_HINT_ALLOW_REL" == 1",
> REGBIT_ACL_VERDICT_ALLOW " = 1; next;",
> lflow_ref);
> +
> + /* Ingress and egress ACL Table (Priority 65535).
> + *
> + * Allow traffic that is established if the ACL has a persistent
> + * conntrack ID configured.
> + */
> + ds_clear(&match);
> + ds_put_format(&match, "ct.est && ct_mark.allow_established == 1");
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX,
> + ds_cstr(&match),
> + REGBIT_ACL_VERDICT_ALLOW " = 1; next;",
> + lflow_ref);
> + ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_AFTER_LB_EVAL,
> UINT16_MAX,
> + ds_cstr(&match),
> + REGBIT_ACL_VERDICT_ALLOW " = 1; next;",
> + lflow_ref);
> + ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL_EVAL, UINT16_MAX,
> + ds_cstr(&match),
> + REGBIT_ACL_VERDICT_ALLOW " = 1; next;",
> + lflow_ref);
> }
>
> /* Ingress and Egress ACL Table (Priority 65532).
> @@ -8362,6 +8389,7 @@ build_stateful(struct ovn_datapath *od, struct
> lflow_table *lflows,
> ds_put_cstr(&actions,
> "ct_commit { "
> "ct_mark.blocked = 0; "
> + "ct_mark.allow_established = " REGBIT_ACL_PERSIST_ID
> "; "
>
>
nit: Extra space
> "ct_mark.obs_stage = " REGBIT_ACL_OBS_STAGE "; "
> "ct_mark.obs_collector_id = "
> REG_OBS_COLLECTOR_ID_EST "; "
> "ct_label.obs_point_id = " REG_OBS_POINT_ID_EST "; "
> @@ -8382,7 +8410,11 @@ build_stateful(struct ovn_datapath *od, struct
> lflow_table *lflows,
> * any packet that makes it this far is part of a connection we
> * want to allow to continue. */
> ds_clear(&actions);
> - ds_put_cstr(&actions, "ct_commit { ct_mark.blocked = 0; }; next;");
> + ds_put_cstr(&actions,
> + "ct_commit { "
> + "ct_mark.blocked = 0; "
> + "ct_mark.allow_established = " REGBIT_ACL_PERSIST_ID
> "; "
>
nit: Extra space
+ "}; next;");
> ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100,
> REGBIT_CONNTRACK_COMMIT" == 1 && "
> REGBIT_ACL_LABEL" == 0",
> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
> index c4a48183d..a02ea67b0 100644
> --- a/ovn-nb.ovsschema
> +++ b/ovn-nb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Northbound",
> - "version": "7.7.0",
> - "cksum": "116357561 38626",
> + "version": "7.8.0",
> + "cksum": "640528173 38695",
> "tables": {
> "NB_Global": {
> "columns": {
> @@ -295,7 +295,8 @@
> "enum": ["set",
> ["allow", "allow-related",
> "allow-stateless", "drop",
> - "reject", "pass"]]}}},
> + "reject", "pass",
> + "allow-established"]]}}},
> "log": {"type": "boolean"},
> "severity": {"type": {"key": {"type": "string",
> "enum": ["set",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 5114bbc2e..8f1ca302d 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -2489,6 +2489,17 @@ or
> (e.g. inbound replies to an outbound connection).
> </li>
>
> + <li>
> + <code>allow-established</code>: This is similar to
> + <code>allow-related</code>. The difference is that if the ACL
> + <code>match</code> is changed, then established traffic can
> + still pass, even if the traffic does not match the ACL any
> longer.
> + When the ACL is deleted, or if the action changes to something
> + other than <code>allow-established</code>, then the
> established
> + traffic is no longer automatically allowed. At that point, the
> + traffic is subject to other configured ACLs.
> + </li>
> +
> <li>
> <code>drop</code>: Silently drop the packet.
> </li>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 3899c9e80..940f5b923 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -313,7 +313,9 @@ CHECK_PYFILES = \
> tests/uuidfilt.py \
> tests/test-tcp-rst.py \
> tests/check_acl_log.py \
> - tests/scapy-server.py
> + tests/scapy-server.py \
> + tests/client.py \
> + tests/server.py
>
There are some flake8 errors in the new python files, please check it out:
tests/client.py:4:1: F401 'os' imported but unused
tests/client.py:24:80: E501 line too long (95 > 79 characters)
tests/server.py:15:80: E501 line too long (80 > 79 characters)
>
> EXTRA_DIST += $(CHECK_PYFILES)
> PYCOV_CLEAN_FILES += $(CHECK_PYFILES:.py=.py,cover) .coverage
> diff --git a/tests/client.py b/tests/client.py
> new file mode 100755
> index 000000000..7662347ff
> --- /dev/null
> +++ b/tests/client.py
> @@ -0,0 +1,38 @@
> +#!/usr/bin/env python3
> +
> +import socket
> +import os
> +import time
> +import argparse
> +
> +
> +def send_data_from_fifo_to_server(
> + fifo_path='/tmp/myfifo', host='127.0.0.1', port=10000
> +):
> + # Open the FIFO for reading (blocking mode)
> + with open(fifo_path, 'r') as fifo_file:
> + with socket.socket(
> + socket.AF_INET, socket.SOCK_STREAM
> + ) as client_socket:
> + client_socket.connect((host, port))
> + # Continuously read from the FIFO and send to the server
> + while True:
> + data = fifo_file.readline().strip()
> + if data:
> + client_socket.sendall(data.encode())
> + else:
> + # If no data is read (blocking mode), add a small
> delay or handle as needed
> + time.sleep(0.1)
> +
> +
> +if __name__ == "__main__":
> + parser = argparse.ArgumentParser()
> + group = parser.add_argument_group()
> + group.add_argument("-f", "--fifo_path")
> + group.add_argument("-i", "--server-host")
> + group.add_argument("-p", "--server-port", type=int)
> + args = parser.parse_args()
> +
> + send_data_from_fifo_to_server(
> + args.fifo_path, args.server_host, args.server_port
> + )
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 4eae1c67c..1d90622e1 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -14253,3 +14253,137 @@ AT_CHECK([grep "lr_in_dnat" lr1flows |
> ovn_strip_lflows | grep "30.0.0.1"], [0],
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([ACL persistent ID - Logical Flows])
> +ovn_start
> +
> +dnl For this test, we want to ensure that the logical flows for ACLs are
> +dnl what we expect.
> +dnl
> +dnl First, we'll ensure that an ACL that is not "allow-established" sets
> the
> +dnl relevant CT values to 0.
> +dnl
> +dnl Then we'll change the ACL to be "allow-established" and ensure the
> logical
> +dnl flows do set the appropriate values.
> +dnl
> +dnl Then finally, we'll set the ACL back away from "allow-established" and
> +dnl ensure that the logical flows set the relevant CT values to 0.
> +
> +check ovn-nbctl ls-add sw
> +
> +check ovn-nbctl acl-add sw from-lport 1001 "tcp" allow-related
> +check ovn-nbctl acl-add sw to-lport 1002 "ip" allow-related
> +check ovn-nbctl --apply-after-lb acl-add sw from-lport 1003 "udp"
> allow-related
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[7]] == 1 &&
> (tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
> + table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[8]] == 1 &&
> (tcp)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep
> priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[7]] ==
> 1 && (udp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[8]] ==
> 1 && (udp)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[7]] == 1 &&
> (ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; next;)
> + table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[8]] == 1 &&
> (ip)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +ingress_uuid=$(fetch_column nb:ACL _uuid priority=1001)
> +egress_uuid=$(fetch_column nb:ACL _uuid priority=1002)
> +after_lb_uuid=$(fetch_column nb:ACL _uuid priority=1003)
> +
> +check ovn-nbctl set acl $ingress_uuid action=allow-established
> +check ovn-nbctl set acl $egress_uuid action=allow-established
> +check ovn-nbctl set acl $after_lb_uuid action=allow-established
> +
> +dnl Now we should see the registers being set to the appropriate values.
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[7]] == 1 &&
> (tcp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;)
> + table=??(ls_in_acl_eval ), priority=2001 , match=(reg0[[8]] == 1 &&
> (tcp)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep
> priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[7]] ==
> 1 && (udp)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;)
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=(reg0[[8]] ==
> 1 && (udp)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[7]] == 1 &&
> (ip)), action=(reg8[[16]] = 1; reg0[[1]] = 1; reg0[[20]] = 1; next;)
> + table=??(ls_out_acl_eval ), priority=2002 , match=(reg0[[8]] == 1 &&
> (ip)), action=(reg8[[16]] = 1; next;)
> +])
> +
> +dnl Now try the other ACL verdicts and ensure that they do not
> +dnl try to set the values.
> +for verdict in allow allow-stateless
> +do
> + echo "verdict is $verdict"
> + check ovn-nbctl set acl $ingress_uuid action=$verdict
> + check ovn-nbctl set acl $egress_uuid action=$verdict
> + check ovn-nbctl set acl $after_lb_uuid action=$verdict
> +
> + AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=((tcp)),
> action=(reg8[[16]] = 1; next;)
> +])
> +
> + AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval |
> grep priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=((udp)),
> action=(reg8[[16]] = 1; next;)
> +])
> +
> + AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=((ip)),
> action=(reg8[[16]] = 1; next;)
> +])
> +done
> +
> +check ovn-nbctl set acl $ingress_uuid action=drop
> +check ovn-nbctl set acl $egress_uuid action=drop
> +check ovn-nbctl set acl $after_lb_uuid action=drop
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=((tcp)),
> action=(reg8[[17]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep
> priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=((udp)),
> action=(reg8[[17]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=((ip)),
> action=(reg8[[17]] = 1; next;)
> +])
> +
> +check ovn-nbctl set acl $ingress_uuid action=reject
> +check ovn-nbctl set acl $egress_uuid action=reject
> +check ovn-nbctl set acl $after_lb_uuid action=reject
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=((tcp)),
> action=(reg8[[18]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep
> priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=((udp)),
> action=(reg8[[18]] = 1; next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=((ip)),
> action=(reg8[[18]] = 1; next;)
> +])
> +
> +check ovn-nbctl set acl $ingress_uuid action=pass
> +check ovn-nbctl set acl $egress_uuid action=pass
> +check ovn-nbctl set acl $after_lb_uuid action=pass
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_eval | grep
> priority=2001 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_eval ), priority=2001 , match=((tcp)),
> action=(next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_in_acl_after_lb_eval | grep
> priority=2003 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_acl_after_lb_eval), priority=2003 , match=((udp)),
> action=(next;)
> +])
> +
> +AT_CHECK([ovn-sbctl lflow-list sw | grep ls_out_acl_eval | grep
> priority=2002 | ovn_strip_lflows], [0], [dnl
> + table=??(ls_out_acl_eval ), priority=2002 , match=((ip)),
> action=(next;)
> +])
> +
> +AT_CLEANUP
> +])
> diff --git a/tests/server.py b/tests/server.py
> new file mode 100755
> index 000000000..1eb378fd2
> --- /dev/null
> +++ b/tests/server.py
> @@ -0,0 +1,30 @@
> +#!/usr/bin/env python3
> +
> +import socket
> +import argparse
> +
> +
> +def start_server(host='127.0.0.1', port=10000):
> + # Create a TCP/IP socket
> + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as
> server_socket:
> + server_socket.bind((host, port))
> + server_socket.listen()
> + while True:
> + client_socket, client_address = server_socket.accept()
> + with client_socket:
> + # Receive the data from the client in chunks and write to
> a file
> + data = client_socket.recv(1024)
> + while data:
> + with open("output.txt", "a") as f:
> + f.write(data.decode() + "\n")
> + data = client_socket.recv(1024)
> +
> +
> +if __name__ == "__main__":
> + parser = argparse.ArgumentParser()
> + group = parser.add_argument_group()
> + group.add_argument("-i", "--bind-host")
> + group.add_argument("-p", "--bind-port", type=int)
> + args = parser.parse_args()
> +
> + start_server(args.bind_host, args.bind_port)
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 4452d5676..a6ddda0f7 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -14117,5 +14117,167 @@ as
> OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
> /failed to query port patch-.*/d
> /.*terminating with signal 15.*/d"])
> +
> +AT_CLEANUP
> +])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ACLs - persistent sessions])
> +
> +CHECK_CONNTRACK()
> +CHECK_CONNTRACK_NAT()
> +ovn_start
> +OVS_TRAFFIC_VSWITCHD_START()
> +ADD_BR([br-int])
> +
> +ovs-vsctl set-fail-mode br-ext standalone
> +# Set external-ids in br-int needed for ovn-controller
> +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_daemon ovn-controller
> +
> +# For this test, we want to ensure that established traffic
> +# is allowed on ACLs with allow-established.
> +#
> +# To start, we will set up allow-related ACLs.
> +# We will send traffic and ensure it is allowed. Then we will adjust
> +# the ACL so it no longer matches, and we will ensure that the traffic
> +# is no longer allowed.
> +#
> +# Next, we will reset the ACL to its initial state, but we will also
> +# change the ACL to be allow-established. We will flush conntrack,
> +# and rerun the test exactly as before. The difference this time is
> +# that after we adjust the ACL so it no longer matches, the traffic
> +# should still be allowed.
> +
> +check ovn-nbctl ls-add sw
> +check ovn-nbctl lsp-add sw swp1 -- lsp-set-addresses swp1
> "00:00:00:00:00:01 192.168.1.1"
> +check ovn-nbctl lsp-add sw swp2 -- lsp-set-addresses swp2
> "00:00:00:00:00:02 192.168.1.2"
> +
> +ADD_NAMESPACES(swp1)
> +ADD_VETH(swp1, swp1, br-int, "192.168.1.1/24", "00:00:00:00:00:01")
> +
> +ADD_NAMESPACES(swp2)
> +ADD_VETH(swp2, swp2, br-int, "192.168.1.2/24", "00:00:00:00:00:02")
> +
> +# Start a TCP server on swp2.
> +NETNS_DAEMONIZE(swp2, [server.py -i 192.168.1.2 -p 10000], [server.pid])
> +
> +# Make a FIFO and send its output to a client
> +# from swp1
> +mkfifo /tmp/myfifo
> +on_exit 'rm -rf /tmp/myfifo'
> +
> +NETNS_DAEMONIZE(swp1, [client.py -f "/tmp/myfifo" -i 192.168.1.2 -p
> 10000], [client.pid])
> +
> +# First, ensure that we have basic connectivity before we even start
> setting
> +# up ACLs.
> +AT_CHECK([printf "test\n" > /tmp/myfifo], [0], [dnl
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([cat output.txt], [0], [dnl
> +test
> +])
> +
> +: > output.txt
> +
> +check ovn-nbctl acl-add sw from-lport 1000 'ip4.dst == 192.168.1.2'
> allow-related
> +check ovn-nbctl acl-add sw from-lport 0 '1' drop
> +
> +# Do another basic connectivity check to ensure the ACL is allowing
> traffic as expected.
> +AT_CHECK([printf "test\n" > /tmp/myfifo], [0], [dnl
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([cat output.txt], [0], [dnl
> +test
> +])
> +
> +: > output.txt
> +
> +# At this point, I need to adjust the ACL so it no longer matches. We
> then need
> +# to ensure that the traffic does not pass. How we test this
> is...interesting. I'm
> +# not sure how to test for a negative condition accurately.
> +
> +acl_uuid=$(fetch_column nb:ACL _uuid priority=1000)
> +
> +# Update the ACL so that it no longer matches our client-server traffic
> +check ovn-nbctl set ACL $acl_uuid match="\"ip4.dst == 192.168.1.3\""
> +
> +# Send another packet from the client to the server.
> +AT_CHECK([printf "test\n" > /tmp/myfifo], [0], [dnl
> +])
> +
> +# The traffic should be blocked. We'll check the "drop" ACL to see if it
> has
> +# been hit. We can't predict the number of packets that will be seen, but
> we know
> +# it will be non-zero.
> +oftable=$(ovn-debug lflow-stage-to-oftable ls_in_acl_eval)
> +OVS_WAIT_FOR_OUTPUT([ovs-ofctl dump-flows br-int | grep table="$oftable"
> | grep "priority=1000" | grep -v commit | grep -c "n_packets=[[1-9]]"],
> [0], [dnl
> +1
> +])
> +
> +# And just to be safe, let's make sure the output file is still empty
> +AT_CHECK([cat output.txt], [0], [dnl
> +])
> +
> +# Reset the client and server processes so that we create a new connection
> +client_pid=$(cat client.pid)
> +server_pid=$(cat server.pid)
> +kill $server_pid
> +kill $client_pid
> +OVS_WAIT_WHILE([kill -0 $server_pid 2>/dev/null])
> +OVS_WAIT_WHILE([kill -0 $client_pid 2>/dev/null])
> +
> +NETNS_DAEMONIZE(swp2, [server.py -i 192.168.1.2 -p 20000], [server.pid])
> +NETNS_DAEMONIZE(swp1, [client.py -f "/tmp/myfifo" -i 192.168.1.2 -p
> 20000], [client.pid])
> +
> +# Now we'll re-set the ACL to allow the traffic.
> +check ovn-nbctl set ACL $acl_uuid match="\"ip4.dst == 192.168.1.2\""
> +
> +# We'll also change to allow-established
> +check ovn-nbctl set ACL $acl_uuid action=allow-established
> +
> +# Make sure traffic passes
> +AT_CHECK([printf "test\n" > /tmp/myfifo], [0], [dnl
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([cat output.txt], [0], [dnl
> +test
> +])
> +
> +: > output.txt
> +
> +# Adjust the ACL so that it no longer matches
> +check ovn-nbctl set ACL $acl_uuid match="\"ip4.dst == 192.168.1.3\""
> +
> +# Traffic should still pass
> +AT_CHECK([printf "test\n" > /tmp/myfifo], [0], [dnl
> +])
> +
> +OVS_WAIT_FOR_OUTPUT([cat output.txt], [0], [dnl
> +test
> +])
> +
> +: > output.txt
> +
> +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([ovn-northd])
> +
> +as
> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> +/connection dropped.*/d"])
> +
> AT_CLEANUP
> ])
> --
> 2.45.2
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With those addressed:
Reviewed-by: Ales Musil <amu...@redhat.com>
Thanks,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
