On Thu, Feb 24, 2022 at 2:00 PM Frode Nordahl <[email protected]> wrote: > > On Thu, Feb 24, 2022 at 7:10 PM Mark Michelson <[email protected]> wrote: > > > > It can be desirable for replies to stateful ACLs to be logged. And in > > some cases, it can actually be a bit confusing why they aren't logged. > > Consider a situation where a port group called "port_group" exists and > > logical switch ports swp1 and swp2 belong to it. We create the following > > ACL, where logging is enabled: > > > > from-lport 100 'inport == @port_group' allow-stateless > > > > swp1 sends traffic to swp2 and swp2 responds within the same connection. > > In this case, the logs will show both the packets from swp1 to swp2, as > > well as the response packets from swp2 to swp1. > > > > Now change the ACL: > > > > from-lport 100 'inport == @port_group' allow-related > > > > Now with the same traffic pattern, the packets from swp1 to swp2 are > > logged, but the packets from swp2 to swp1 are not. Why is that? > > > > The reason is that as a shortcut, when stateful ACLs are used, a single > > priority 65532 flow is programmed to allow reply traffic to pass. When > > no stateful ACL is present, the reply traffic is at the mercy of the > > stateless ACL on the reply. Therefore, with the stateful ACL, the reply > > traffic is not actually hitting an ACL but is let through by default, > > but with the stateless ACL, the reply traffic does hit the ACL > > evaluation, so the reply traffic is logged. > > > > This change adds a feature that allows for reply traffic to be > > optionally logged for stateful ACLs, therefore allowing for the behavior > > to be similar for both ACL types. Since logging reply traffic requires > > adding more flows, it is not enabled by default. In order to have reply > > traffic logged, the ACL must have logging enabled, be stateful, have a > > label, and have the new log_related column set to true, > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2031150 > > > > Signed-off-by: Mark Michelson <[email protected]> > > Thanks for doing this work! > > Reviewed-by: Frode Nordahl <[email protected]>
Acked-by: Numan Siddique <[email protected]> Numan > > -- > Frode Nordahl > > > --- > > NEWS | 4 + > > northd/northd.c | 43 +++++ > > northd/ovn-northd.8.xml | 8 +- > > ovn-nb.ovsschema | 9 +- > > ovn-nb.xml | 16 ++ > > tests/automake.mk | 3 +- > > tests/check_acl_log.py | 107 ++++++++++++ > > tests/ovn-northd.at | 253 ++++++++++++++++++++++++++++ > > tests/system-ovn.at | 359 ++++++++++++++++++++++++++++++++++++++++ > > 9 files changed, 798 insertions(+), 4 deletions(-) > > create mode 100644 tests/check_acl_log.py > > > > diff --git a/NEWS b/NEWS > > index a309fe8df..da998904e 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -12,6 +12,10 @@ Post v21.12.0 > > ovn-controller-vtep will not process any DB changes. > > - When configured to log packtes matching ACLs, log the direction > > (logical > > pipeline) too. > > + - ACLs now have an "options" column for configuration of extra options. > > + - A new ACL option, "log-related" has been added that allows for reply > > and > > + related traffic to be logged for an ACL in addition to the traffic that > > + directly matches the ACL. > > > > OVN v21.12.0 - 22 Dec 2021 > > -------------------------- > > diff --git a/northd/northd.c b/northd/northd.c > > index 219398f96..36cb83c49 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -6259,6 +6259,49 @@ consider_acl(struct hmap *lflows, struct > > ovn_datapath *od, > > acl->priority + OVN_ACL_PRI_OFFSET, > > ds_cstr(match), ds_cstr(actions), > > &acl->header_); > > + > > + /* Related and reply traffic are universally allowed by > > priority > > + * 65532 flows created in build_acls(). If logging is enabled > > on > > + * the ACL, then we need to ensure that the related and reply > > + * traffic is logged, so we install a slightly higher-priority > > + * flow that matches the ACL, allows the traffic, and logs it. > > + */ > > + bool log_related = smap_get_bool(&acl->options, "log-related", > > + false); > > + if (acl->log && acl->label && log_related) { > > + /* Related/reply flows need to be set on the opposite > > pipeline > > + * from where the ACL itself is set. > > + */ > > + enum ovn_stage log_related_stage = ingress ? > > + S_SWITCH_OUT_ACL : > > + S_SWITCH_IN_ACL; > > + ds_clear(match); > > + ds_clear(actions); > > + > > + ds_put_format(match, "ct.est && !ct.rel && !ct.new%s && " > > + "ct.rpl && ct_label.blocked == 0 && " > > + "ct_label.label == %" PRId64, > > + use_ct_inv_match ? " && !ct.inv" : "", > > + acl->label); > > + build_acl_log(actions, acl, meter_groups); > > + ds_put_cstr(actions, "next;"); > > + ovn_lflow_add_with_hint(lflows, od, log_related_stage, > > + UINT16_MAX - 2, > > + ds_cstr(match), ds_cstr(actions), > > + &acl->header_); > > + > > + ds_clear(match); > > + ds_put_format(match, "!ct.est && ct.rel && !ct.new%s && " > > + "ct_label.blocked == 0 && " > > + "ct_label.label == %" PRId64, > > + use_ct_inv_match ? " && !ct.inv" : "", > > + acl->label); > > + ovn_lflow_add_with_hint(lflows, od, log_related_stage, > > + UINT16_MAX - 2, > > + ds_cstr(match), ds_cstr(actions), > > + &acl->header_); > > + } > > + > > } > > } else if (!strcmp(acl->action, "drop") > > || !strcmp(acl->action, "reject")) { > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > index 79f35bc16..cfe694831 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -746,7 +746,10 @@ > > go through the flows that implement the currently defined > > policy based on ACLs. If a connection is no longer allowed by > > policy, <code>ct_label.blocked</code> will get set and packets in > > the > > - reply direction will no longer be allowed, either. > > + reply direction will no longer be allowed, either. If ACL logging > > + and logging of related packets is enabled, then a companion > > priority- > > + 65533 flow will be installed that accomplishes the same thing but > > + also logs the traffic. > > </li> > > > > <li> > > @@ -754,6 +757,9 @@ > > related to a committed flow in the connection tracker (e.g., an > > ICMP Port Unreachable from a non-listening UDP port), as long > > as the committed flow does not have <code>ct_label.blocked</code> > > set. > > + If ACL logging and logging of related packets is enabled, then a > > + companion priority-65533 flow will be installed that accomplishes > > the > > + same thing but also logs the traffic. > > </li> > > > > <li> > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > > index eb17b4f4f..80b830629 100644 > > --- a/ovn-nb.ovsschema > > +++ b/ovn-nb.ovsschema > > @@ -1,7 +1,7 @@ > > { > > "name": "OVN_Northbound", > > - "version": "6.0.0", > > - "cksum": "1994796624 31020", > > + "version": "6.1.0", > > + "cksum": "4010776751 31237", > > "tables": { > > "NB_Global": { > > "columns": { > > @@ -267,6 +267,11 @@ > > "label": {"type": {"key": {"type": "integer", > > "minInteger": 0, > > "maxInteger": 4294967295}}}, > > + "options": { > > + "type": {"key": "string", > > + "value": "string", > > + "min": 0, > > + "max": "unlimited"}}, > > "external_ids": { > > "type": {"key": "string", "value": "string", > > "min": 0, "max": "unlimited"}}}, > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index 642e91845..c8473d3be 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -2070,6 +2070,22 @@ > > </group> > > > > <group title="Common Columns"> > > + <column name="options"> > > + This column provides general key/value settings. The supported > > + options are described individually below. > > + </column> > > + > > + <group title="ACL configuration options"> > > + <column name="options" key="log-related"> > > + If set to <code>true</code>, then log when reply or related > > + traffic is admitted from a stateful ACL. In order for this > > + option to function, the <ref column="log"/> option must be > > + set to <code>true</code> and a <ref column="label"/> must > > + be set. The label is necessary as it is the only means to > > associate > > + the reply traffic with the ACL to which it belongs. > > + </column> > > + </group> > > + > > <column name="external_ids"> > > See <em>External IDs</em> at the beginning of this document. > > </column> > > diff --git a/tests/automake.mk b/tests/automake.mk > > index a08dfa632..a5934d2b9 100644 > > --- a/tests/automake.mk > > +++ b/tests/automake.mk > > @@ -270,7 +270,8 @@ tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \ > > CHECK_PYFILES = \ > > tests/test-l7.py \ > > tests/uuidfilt.py \ > > - tests/test-tcp-rst.py > > + tests/test-tcp-rst.py \ > > + tests/check_acl_log.py > > > > EXTRA_DIST += $(CHECK_PYFILES) > > PYCOV_CLEAN_FILES += $(CHECK_PYFILES:.py=.py,cover) .coverage > > diff --git a/tests/check_acl_log.py b/tests/check_acl_log.py > > new file mode 100644 > > index 000000000..1dd9630c0 > > --- /dev/null > > +++ b/tests/check_acl_log.py > > @@ -0,0 +1,107 @@ > > +#!/usr/bin/env python3 > > +import argparse > > +import string > > + > > + > > +def strip(val): > > + """Strip whitespace and quotation marks from val""" > > + return val.strip(f"{string.whitespace}\"'") > > + > > + > > +def parse_acl_log(line): > > + """Convert an ACL log string into a dict""" > > + # First cut off the logging preamble. > > + # We're assuming the default log format. > > + acl_log = {} > > + _, _, details = line.rpartition("|") > > + > > + # acl_details are things like the acl name, direction, > > + # verdict, and severity. packet_details are things like > > + # the protocol, addresses, and ports of the packet being > > + # logged. > > + acl_details, _, packet_details = details.partition(":") > > + for datum in acl_details.split(","): > > + name, _, value = datum.rpartition("=") > > + acl_log[strip(name)] = strip(value) > > + > > + for datum in packet_details.split(","): > > + name, _, value = datum.rpartition("=") > > + if not name: > > + # The protocol is not preceded by "protocol=" > > + # so we need to add it manually. > > + name = "protocol" > > + acl_log[strip(name)] = strip(value) > > + > > + return acl_log > > + > > + > > +def get_acl_log(entry_num=1): > > + with open("ovn-controller.log", "r") as controller_log: > > + acl_logs = [line for line in controller_log if "acl_log" in line] > > + try: > > + return acl_logs[entry_num - 1] > > + except IndexError: > > + print( > > + f"There were not {entry_num} acl_log entries, \ > > + only {len(acl_logs)}" > > + ) > > + exit(1) > > + > > + > > +def add_parser_args(parser): > > + parser.add_argument("--entry-num", type=int, default=1) > > + > > + # There are other possible things that can be in an ACL log, > > + # and if we need those in the future, we can add them later. > > + parser.add_argument("--name") > > + parser.add_argument("--verdict") > > + parser.add_argument("--severity") > > + parser.add_argument("--protocol") > > + parser.add_argument("--vlan_tci") > > + parser.add_argument("--dl_src") > > + parser.add_argument("--dl_dst") > > + parser.add_argument("--nw_src") > > + parser.add_argument("--nw_dst") > > + parser.add_argument("--nw_tos") > > + parser.add_argument("--nw_ecn") > > + parser.add_argument("--nw_ttl") > > + parser.add_argument("--icmp_type") > > + parser.add_argument("--icmp_code") > > + parser.add_argument("--tp_src") > > + parser.add_argument("--tp_dst") > > + parser.add_argument("--tcp_flags") > > + parser.add_argument("--ipv6_src") > > + parser.add_argument("--ipv6_dst") > > + > > + > > +def main(): > > + parser = argparse.ArgumentParser() > > + add_parser_args(parser) > > + args = parser.parse_args() > > + > > + acl_log = get_acl_log(args.entry_num) > > + parsed_log = parse_acl_log(acl_log) > > + > > + # Express command line arguments as a dict, omitting any arguments that > > + # were not provided by the user. > > + expected = {k: v for k, v in vars(args).items() if v is not None} > > + del expected["entry_num"] > > + > > + for key, val in expected.items(): > > + try: > > + if parsed_log[key] != val: > > + print( > > + f"Expected log {key}={val} but got > > {key}={parsed_log[key]} \ > > + in:\n\t'{acl_log}" > > + ) > > + exit(1) > > + except KeyError: > > + print( > > + f"Expected log {key}={val} but {key} does not exist \ > > + in:\n\t'{acl_log}'" > > + ) > > + exit(1) > > + > > + > > +if __name__ == "__main__": > > + main() > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 170d015b2..a25ee16a2 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -5964,5 +5964,258 @@ AT_CHECK([grep -e "(lr_in_ip_routing ).*outport" > > lr0flows | sed 's/table=../ta > > table=??(lr_in_ip_routing ), priority=97 , match=(reg7 == 2 && > > ip4.dst == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = > > 192.168.0.20; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = > > "lrp0"; flags.loopback = 1; next;) > > ]) > > > > +AT_CLEANUP > > +]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([ACL log replies -- flows]) > > + > > +set_acl_options() { > > + local acl_name=$1 > > + local label=$2 > > + local log_related=$3 > > + > > + local acl_uuid=$(fetch_column nb:ACL _uuid name=$acl_name) > > + check ovn-nbctl set ACL $acl_uuid label=$label > > options:log-related=$log_related > > +} > > + > > +record_log_flows() { > > + ovn-sbctl lflow-list sw0 | grep -E 'ls_(out|in)_acl.*, priority=65533' > > | sed 's/table=../table=??/' | sort > log_flows > > +} > > + > > +check_log_flows_count() { > > + local expected=$1 > > + local table=$2 > > + local count= > > + > > + echo $table > > + if test -f log_flows; then > > + count=$(grep -c -E ls_${table}_acl log_flows) > > + else > > + count=$(ovn-sbctl lflow-list sw0 | grep -c -E "ls_$table_acl.*, > > priority=65533") > > + fi > > + > > + check test "$count" -eq "$expected" > > +} > > + > > +ovn_start > > + > > +check ovn-nbctl ls-add sw0 > > +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 > > "00:00:00:00:00:01 10.0.0.1" > > +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 > > "00:00:00:00:00:02 10.0.0.2" > > + > > +check ovn-nbctl pg-add pg1 sw0-p1 sw0-p2 > > +check ovn-nbctl pg-add pg2 sw0-p1 sw0-p2 > > +check ovn-nbctl pg-add pg3 sw0-p1 sw0-p2 > > + > > +check ovn-nbctl --log --name=allow_acl acl-add pg1 from-lport 100 > > 'inport=@pg1 && ip4' allow > > +set_acl_options allow_acl 1 true > > + > > +check ovn-nbctl --wait=sb sync > > + > > +# An allow ACL should *not* result in a priority 65533 log flow being > > installed > > +# since there are no stateful ACLs on the system. > > +check_log_flows_count 0 in > > +check_log_flows_count 0 out > > + > > +# Now add an allow-related ACL. This should result in both the > > allow-related > > +# ACL and the allow ACL having priority 65533 log flows added. > > +check ovn-nbctl --log --name=allow_related_acl acl-add pg2 from-lport 100 > > 'inport=@pg2 && ip4' allow-related > > +set_acl_options allow_related_acl 2 true > > +check ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count will be 4 since we have > > +# 2 flows for reply traffic for each ACL > > +# 2 flows for related traffic for each ACL > > +check_log_flows_count 4 out > > +# Since the ACLs are ingress, the ingress table > > +# should have no log flows > > +check_log_flows_count 0 in > > + > > +# Now ensure the flows are what we expect them to be for the ACLs we > > created > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2), > > action=(log(name="allow_related_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 2), action=(log(name="allow_related_acl", severity=info, verdict=allow); > > next;) > > +]) > > + > > +rm log_flows > > + > > +# Now add a stateless-allow ACL. > > +check ovn-nbctl --log --name=allow_stateless_acl acl-add pg3 from-lport > > 100 'inport=@pg3 && ip4' allow-stateless > > +set_acl_options allow_stateless_acl 3 true > > +check ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count will still be 4 since the stateless ACL should not have > > special log flows created > > +check_log_flows_count 4 out > > +check_log_flows_count 0 in > > + > > +# And the log flows will remain the same since the stateless ACL will not > > be represented. > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2), > > action=(log(name="allow_related_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 2), action=(log(name="allow_related_acl", severity=info, verdict=allow); > > next;) > > +]) > > + > > +rm log_flows > > + > > +# Now remove the label from the allow-related ACL. > > +set_acl_options allow_related_acl 0 true > > +ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count should now be 2 since the allow_related ACL will not have > > special > > +# log flows created. But since there there is an allow-related ACL > > present, the > > +# allow ACL will be stateful and have special log flows created. > > +check_log_flows_count 2 out > > +check_log_flows_count 0 in > > + > > +# And make sure only the allow ACL has the log flows installed > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > +]) > > + > > +rm log_flows > > + > > +# And now add the label back, but disable log_related on the allow-related > > ACL. > > +set_acl_options allow_related_acl 2 false > > + > > +record_log_flows > > + > > +# The count will again be 2 because only the allow ACL will have log flows > > installed. > > +check_log_flows_count 2 out > > +check_log_flows_count 0 in > > + > > +# And make sure only the allow ACL has the log flows installed > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_out_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_out_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > +]) > > + > > +rm log_flows > > + > > +# And just for sanity's sake, let's remove the allow-related ACL and make > > sure > > +# all the special log messages are gone. > > +check ovn-nbctl acl-del pg2 > > +check ovn-nbctl --wait=sb sync > > + > > +check_log_flows_count 0 out > > +check_log_flows_count 0 in > > + > > +# Now let's clear out all the ACLs, and re-do everything but with egress > > ACLs. > > +check ovn-nbctl acl-del pg1 > > +check ovn-nbctl acl-del pg3 > > +check_row_count nb:ACL 0 > > + > > +# Start again with an allow_acl only > > +check ovn-nbctl --log --name=allow_acl acl-add pg1 to-lport 100 > > 'inport=@pg1 && ip4' allow > > +set_acl_options allow_acl 1 true > > + > > +check ovn-nbctl --wait=sb sync > > + > > +# Again, the allow ACL is stateless, so no related log flows. > > +check_log_flows_count 0 in > > +check_log_flows_count 0 out > > + > > +# Adding a new allow-related ACL... > > +check ovn-nbctl --log --name=allow_related_acl acl-add pg2 to-lport 100 > > 'inport=@pg2 && ip4' allow-related > > +set_acl_options allow_related_acl 2 true > > +check ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count will be 4 since we have > > +# 2 flows for reply traffic for each ACL > > +# 2 flows for related traffic for each ACL > > +check_log_flows_count 4 in > > +# And this time, we should have no egress flows > > +check_log_flows_count 0 out > > + > > +# Now ensure the flows are what we expect them to be for the ACLs we > > created > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2), > > action=(log(name="allow_related_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 2), action=(log(name="allow_related_acl", severity=info, verdict=allow); > > next;) > > +]) > > + > > +rm log_flows > > + > > +# Now add a stateless-allow ACL. > > +check ovn-nbctl --log --name=allow_stateless_acl acl-add pg3 from-lport > > 100 'inport=@pg3 && ip4' allow-stateless > > +set_acl_options allow_stateless_acl 3 true > > +check ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count will still be 4 since the stateless ACL should not have > > special log flows created > > +check_log_flows_count 4 in > > +check_log_flows_count 0 out > > + > > +# And the log flows will remain the same since the stateless ACL will not > > be represented. > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 2), > > action=(log(name="allow_related_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 2), action=(log(name="allow_related_acl", severity=info, verdict=allow); > > next;) > > +]) > > + > > +rm log_flows > > + > > +# Now remove the label from the allow-related ACL. > > +set_acl_options allow_related_acl 0 true > > +ovn-nbctl --wait=sb sync > > + > > +record_log_flows > > + > > +# The count should now be 2 since the allow_related ACL will not have > > special > > +# log flows created. But since there there is an allow-related ACL > > present, the > > +# allow ACL will be stateful and have special log flows created. > > +check_log_flows_count 2 in > > +check_log_flows_count 0 out > > + > > +# And make sure only the allow ACL has the log flows installed > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > +]) > > + > > +rm log_flows > > + > > +# And now add the label back, but disable log_related on the allow-related > > ACL. > > +set_acl_options allow_related_acl 2 false > > + > > +record_log_flows > > + > > +# The count will again be 2 because only the allow ACL will have log flows > > installed. > > +check_log_flows_count 2 in > > +check_log_flows_count 0 out > > + > > +# And make sure only the allow ACL has the log flows installed > > +AT_CHECK([cat log_flows], [0], [dnl > > + table=??(ls_in_acl ), priority=65533, match=(!ct.est && ct.rel > > && !ct.new && !ct.inv && ct_label.blocked == 0 && ct_label.label == 1), > > action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > + table=??(ls_in_acl ), priority=65533, match=(ct.est && !ct.rel > > && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0 && ct_label.label > > == 1), action=(log(name="allow_acl", severity=info, verdict=allow); next;) > > +]) > > + > > +rm log_flows > > + > > +# And just for sanity's sake, let's remove the allow-related ACL and make > > sure > > +# all the special log messages are gone. > > +check ovn-nbctl acl-del pg2 > > +check ovn-nbctl --wait=sb sync > > + > > +check_log_flows_count 0 out > > +check_log_flows_count 0 in > > + > > + > > + > > AT_CLEANUP > > ]) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 2dcd7e906..f57d752d4 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -7006,3 +7006,362 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port > > patch-.*/d > > > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([ACL log_related]) > > + > > +CHECK_CONNTRACK() > > +ovn_start > > + > > +OVS_TRAFFIC_VSWITCHD_START() > > +ADD_BR([br-int]) > > + > > +set_acl_options() { > > + local acl_name=$1; shift > > + > > + local acl_uuid=$(fetch_column nb:ACL _uuid name=$acl_name) > > + check ovn-nbctl set ACL $acl_uuid "$@" > > +} > > + > > +clear_log() { > > + ovn-appctl -t ovn-controller vlog/close > > + rm ovn-controller.log > > + ovn-appctl -t ovn-controller vlog/reopen > > +} > > + > > +test_ping() { > > + NS_CHECK_EXEC([sw0-p1], [ping -q -c 1 -i 0.3 -w 2 10.0.0.2 | > > FORMAT_PING], \ > > +[0], [dnl > > +1 packets transmitted, 1 received, 0% packet loss, time 0ms > > +]) > > +} > > + > > +check_acl_log_count() { > > + local expected_count=$1 > > + > > + AT_CHECK_UNQUOTED([grep -c acl_log ovn-controller.log], [0], [dnl > > +$expected_count > > +]) > > +} > > + > > +# 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 ovn-controller > > +start_daemon ovn-controller > > + > > +check ovn-nbctl ls-add sw0 > > +check ovn-nbctl lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 > > "00:00:00:00:00:01 10.0.0.1" > > +check ovn-nbctl lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 > > "00:00:00:00:00:02 10.0.0.2" > > + > > +check ovn-nbctl pg-add pg1 sw0-p1 sw0-p2 > > + > > +ADD_NAMESPACES(sw0-p1) > > +ADD_VETH(sw0-p1, sw0-p1, br-int, "10.0.0.1/24", "00:00:00:00:00:01") > > +ADD_NAMESPACES(sw0-p2) > > +ADD_VETH(sw0-p2, sw0-p2, br-int, "10.0.0.2/24", "00:00:00:00:00:02") > > + > > +wait_for_ports_up > > + > > +check ovn-nbctl --log --name=allow_acl acl-add pg1 from-lport 100 'inport > > == @pg1 && ip4' allow > > + > > +check ovn-nbctl --wait=hv sync > > + > > +test_ping > > + > > +# The allow ACL should match on the request and reply traffic, resulting > > in 2 logs. > > +check_acl_log_count 2 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=2 \ > > + --name=allow_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:02 \ > > + --dl_dst=00:00:00:00:00:01 \ > > + --nw_src=10.0.0.2 \ > > + --nw_dst=10.0.0.1 \ > > + --icmp_type=0 \ > > + --icmp_code=0 > > + > > +# Now add a higher-priority stateful ACL that matches on the same > > +# parameters. Don't enable reply logging. > > +check ovn-nbctl --log --name=allow_related_acl acl-add pg1 from-lport 200 > > 'inport == @pg1 && ip4' allow-related > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# Since reply logging is not enabled, the allow-related ACL should match > > on the > > +# request, but the reply will not be logged. > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# As a control, set a label on the allow-related ACL, but still don't > > enable > > +# reply traffic logging. > > +set_acl_options allow_related_acl label=1 options:log-related=false > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# This should have the same result as the previous ping > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# As another control, remove the label but enable reply logging. > > +set_acl_options allow_related_acl label=0 options:log-related=true > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# This should have the same result as the previous ping > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# This time, add a label and enable reply logging on the allow_related ACL. > > +set_acl_options allow_related_acl label=1 options:log-related=true > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# Now we should have the request and reply logged. > > +check_acl_log_count 2 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=2 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:02 \ > > + --dl_dst=00:00:00:00:00:01 \ > > + --nw_src=10.0.0.2 \ > > + --nw_dst=10.0.0.1 \ > > + --icmp_type=0 \ > > + --icmp_code=0 > > + > > + > > +# And now, let's start from scratch but make sure everything works when > > +# using egress ACLs. > > +check ovn-nbctl acl-del pg1 > > +check_row_count nb:ACL 0 > > + > > +check ovn-nbctl --log --name=allow_acl acl-add pg1 to-lport 100 'outport > > == @pg1 && ip4' allow > > + > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# The allow ACL should match on the request and reply traffic, resulting > > in 2 logs. > > +check_acl_log_count 2 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=2 \ > > + --name=allow_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:02 \ > > + --dl_dst=00:00:00:00:00:01 \ > > + --nw_src=10.0.0.2 \ > > + --nw_dst=10.0.0.1 \ > > + --icmp_type=0 \ > > + --icmp_code=0 > > + > > +# Now add a higher-priority stateful ACL that matches on the same > > +# parameters. Don't enable reply logging. > > +check ovn-nbctl --log --name=allow_related_acl acl-add pg1 to-lport 200 > > 'outport == @pg1 && ip4' allow-related > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# Since reply logging is not enabled, the allow-related ACL should match > > on the > > +# request, but the reply will not be logged. > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# As a control, set a label on the allow-related ACL, but still don't > > enable > > +# reply traffic logging. > > +set_acl_options allow_related_acl label=1 options:log-related=false > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# This should have the same result as the previous ping > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# As another control, remove the label but enable reply logging. > > +set_acl_options allow_related_acl label=0 options:log-related=true > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# This should have the same result as the previous ping > > +check_acl_log_count 1 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +# This time, add a label and enable reply logging on the allow_related ACL. > > +set_acl_options allow_related_acl label=1 options:log-related=true > > +check ovn-nbctl --wait=hv sync > > + > > +clear_log > > +test_ping > > + > > +# Now we should have the request and reply logged. > > +check_acl_log_count 2 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=1 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:01 \ > > + --dl_dst=00:00:00:00:00:02 \ > > + --nw_src=10.0.0.1 \ > > + --nw_dst=10.0.0.2 \ > > + --icmp_type=8 \ > > + --icmp_code=0 > > + > > +check $PYTHON $srcdir/check_acl_log.py \ > > + --entry-num=2 \ > > + --name=allow_related_acl \ > > + --verdict=allow \ > > + --protocol=icmp \ > > + --dl_src=00:00:00:00:00:02 \ > > + --dl_dst=00:00:00:00:00:01 \ > > + --nw_src=10.0.0.2 \ > > + --nw_dst=10.0.0.1 \ > > + --icmp_type=0 \ > > + --icmp_code=0 > > + > > + > > +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([NORTHD_TYPE]) > > + > > +as > > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > > +/connection dropped.*/d"]) > > + > > +AT_CLEANUP > > +]) > > -- > > 2.31.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
