On Mon, Dec 16, 2024 at 8:03 PM Mark Michelson <[email protected]> wrote:
> 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. > > This behavior can be altered by setting the > "bypass_match_for_established" option on allow-related ACLs. > When set to true, we set a specific bit in the ct_mark when the > conntrack entry is committed. 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 <[email protected]> > --- > Hi Mark, thank you for the next version, it seems that it will need rebase. I have a few minor comments down below. > v2 -> v3: > * The configuration mechanism changed from a new ACL action to being an > option that supplements "allow-related" ACLs. The new option is > called bypass_match_for_established. A suggestion for the option was > "flush_ct_on_removal". I elected not to go with this because flushing > CT on removal isn't the real draw of the option. Admins set the > option so that the ACL does not have to be matched once the > connection is established. The flush of CT is a necessity of the > feature, but it's not why the admin is setting the option. > > v1 -> v2: > * Fixed formatting issues > * Fixed flake8 issues > * Check for CT label flush chassis feature > --- > include/ovn/logical-fields.h | 1 + > lib/logical-fields.c | 5 ++ > northd/en-ls-stateful.c | 4 +- > northd/northd.c | 49 ++++++++++- > ovn-nb.ovsschema | 4 +- > ovn-nb.xml | 26 ++++++ > tests/automake.mk | 4 +- > tests/client.py | 36 ++++++++ > tests/ovn-northd.at | 134 ++++++++++++++++++++++++++++ > tests/server.py | 31 +++++++ > tests/system-ovn.at | 163 +++++++++++++++++++++++++++++++++++ > 11 files changed, 448 insertions(+), 9 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..561f47695 100644 > --- a/northd/en-ls-stateful.c > +++ b/northd/en-ls-stateful.c > @@ -411,8 +411,8 @@ ls_stateful_record_set_acl_flags_(struct > ls_stateful_record *ls_stateful_rec, > if (acl->tier > ls_stateful_rec->max_acl_tier) { > ls_stateful_rec->max_acl_tier = acl->tier; > } > - if (!ls_stateful_rec->has_stateful_acl > - && !strcmp(acl->action, "allow-related")) { > + if (!ls_stateful_rec->has_stateful_acl && > + !strcmp(acl->action, "allow-related")) { > nit: Unrelated change. 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..eee5d55ff 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" > @@ -7061,7 +7062,8 @@ consider_acl(struct lflow_table *lflows, const > struct ovn_datapath *od, > const struct nbrec_acl *acl, bool has_stateful, > const struct shash *meter_groups, uint64_t max_acl_tier, > struct ds *match, struct ds *actions, > - struct lflow_ref *lflow_ref) > + struct lflow_ref *lflow_ref, > + const struct chassis_features *features) > { > bool ingress = !strcmp(acl->direction, "from-lport") ? true :false; > enum ovn_stage stage; > @@ -7147,6 +7149,20 @@ 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 (smap_get_bool(&acl->options, "bypass_match_for_established", > This option name is kind of long, how about "persist_established"? > + false)) { > + if (!features->ct_label_flush) { > + static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "OVS does not support ct label flush. " > nit: Capital CT + "bypass_match_for_established option cannot " > + "be honored for ACL "UUID_FMT".", > + UUID_ARGS(&acl->header_.uuid)); > + } else { > + 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 +7642,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). > @@ -7658,7 +7694,7 @@ build_acls(const struct ls_stateful_record > *ls_stateful_rec, > lflow_ref); > consider_acl(lflows, od, acl, has_stateful, > meter_groups, ls_stateful_rec->max_acl_tier, > - &match, &actions, lflow_ref); > + &match, &actions, lflow_ref, features); > build_acl_sample_flows(ls_stateful_rec, od, lflows, acl, > &match, &actions, sampling_apps, > features, lflow_ref); > @@ -7677,7 +7713,7 @@ build_acls(const struct ls_stateful_record > *ls_stateful_rec, > lflow_ref); > consider_acl(lflows, od, acl, has_stateful, > meter_groups, ls_stateful_rec->max_acl_tier, > - &match, &actions, lflow_ref); > + &match, &actions, lflow_ref, features); > build_acl_sample_flows(ls_stateful_rec, od, lflows, acl, > &match, &actions, sampling_apps, > features, lflow_ref); > @@ -8362,6 +8398,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 > "; " > "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 +8419,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 > "; " > + "}; 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..ab1cd4344 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": "3383352767 38626", > nit: Unrelated change > "tables": { > "NB_Global": { > "columns": { > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 5114bbc2e..e6eb2400b 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2558,6 +2558,32 @@ or > of all the ACLs and the default deny/allow ACLs if any. > </p> > </column> > + > + <column name="options" key="bypass_match_for_established"> > + <p> > + This option applies only to ACLs whose <ref column="action"/> > is set > + to <code>allow-related</code>. > + </p> > + > + <p> > + <code>allow-related</code> ACLs create a conntrack entry when a > + packet matches the ACL's <ref column="match"/> column. > Typically, > + traffic must continue to match these conditions in order to > continue > + to be allowed by the ACL. With this option set to > <code>true</code>, > + then the ACL match is bypassed once the original match occurs. > + Instead, a mark bit in the conntrack entry is used to allow the > + traffic. This means that traffic will continue to be allowed > even if > + the ACL's match changes and no longer matches the established > + traffic. > + </p> > + > + <p> > + The traffic will stop being allowed automatically if this > option is > + set to <code>false</code>, if the ACL's <ref column="action"/> > is > + changed to something other than <code>allow-related</code>, or > if the > + ACL is destroyed. > + </p> > + </column> > </group> > > <group title="Logging"> > 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 > > 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..1caabce7b > --- /dev/null > +++ b/tests/client.py > @@ -0,0 +1,36 @@ > +#!/usr/bin/env python3 > + > +import socket > +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: > + 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..a44b4c43c 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 does not have > +dnl "bypass_match_for_established" sets the relevant CT values to 0. > +dnl > +dnl Then we'll change the ACL to have "bypass_match_for_established" to > true > +dnl and ensure the logical flows do set the appropriate values. > +dnl > +dnl Then finally, we'll check other ACL actions and ensure that > +dnl "bypass_match_for_established" sets 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 > options:bypass_match_for_established=true > +check ovn-nbctl set acl $egress_uuid > options:bypass_match_for_established=true > +check ovn-nbctl set acl $after_lb_uuid > options:bypass_match_for_established=true > + > +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..857328a59 > --- /dev/null > +++ b/tests/server.py > @@ -0,0 +1,31 @@ > +#!/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") > Should we add the newline in the server? I suppose it's unlikely that the test will ever use more than 1024 characters per line, however we could just leave the line break encoded in the stream WDYT? > + 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..db134c2ab 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -14117,5 +14117,168 @@ 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 when the bypass_match_for_established option > +# is enabled. > +# > +# 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 have bypass_match_for_established enabled. > +# 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 > Maybe a more precise match would be to use cookie with "ovn-debug uuid-to-cookie". Also is it guaranteed that it will be a single digit number of packets? Maybe it's safer to allow more than one digit. +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 enable bypass_match_for_established. > +check ovn-nbctl --wait=hv set ACL $acl_uuid > options:bypass_match_for_established=true > + > +# 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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
