I pushed this change to main and all branches back to 24.03. Thanks again! On Tue, Jan 20, 2026 at 9:17 AM Mark Michelson <[email protected]> wrote: > > Thanks for the update, Xavier! > > Acked-by: Mark Michelson <[email protected]> > > On Tue, Jan 20, 2026 at 6:55 AM Xavier Simonart via dev > <[email protected]> wrote: > > > > Test was sometimes failing as logging two ACLs (one from request > > traffic and the other one from reply traffic) misordered. > > PACKET_IN from OVS might be misordered in this case as being generated > > by two different OVS threads. > > > > Fixes: b988d5fa62d9 ("northd: Add feature to log reply and related ACL > > traffic.") > > Signed-off-by: Xavier Simonart <[email protected]> > > > > --- > > -v2: - Updated based on Mark's feedback i.e. update check_acl_log.py > > to make all tests consistent and ignore fields such as nw_tos. > > - Rebased. > > - Added Fixes tag. > > --- > > tests/check_acl_log.py | 10 +++++----- > > tests/system-ovn.at | 38 ++++++++++++++++++++++++++------------ > > 2 files changed, 31 insertions(+), 17 deletions(-) > > > > diff --git a/tests/check_acl_log.py b/tests/check_acl_log.py > > index 0c1968b2e..2cc241459 100644 > > --- a/tests/check_acl_log.py > > +++ b/tests/check_acl_log.py > > @@ -10,16 +10,13 @@ def strip(val): > > > > 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(":") > > + acl_details, _, packet_details = line.partition(":") > > for datum in acl_details.split(","): > > name, _, value = datum.rpartition("=") > > acl_log[strip(name)] = strip(value) > > @@ -37,8 +34,10 @@ def parse_acl_log(line): > > > > def get_acl_log(entry_num=1): > > with open("ovn-controller.log", "r") as controller_log: > > - acl_logs = [line.rstrip() for line in controller_log > > + # Cut off the logging preamble, so we can sort logs properly. > > + acl_logs = [line.split('|', 4)[-1].rstrip() for line in > > controller_log > > if "acl_log" in line] > > + acl_logs.sort() > > try: > > return acl_logs[entry_num - 1] > > except IndexError: > > @@ -55,6 +54,7 @@ def add_parser_args(parser): > > # 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("--direction") > > parser.add_argument("--verdict") > > parser.add_argument("--severity") > > parser.add_argument("--protocol") > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index 5b3dc47fd..636b1e4d9 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -8241,6 +8241,7 @@ check_acl_log_count 2 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8253,6 +8254,7 @@ check $PYTHON $srcdir/check_acl_log.py \ > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=2 \ > > --name=allow_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:02 \ > > @@ -8277,6 +8279,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8300,6 +8303,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8322,6 +8326,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8339,11 +8344,13 @@ clear_log > > test_ping > > > > # Now we should have the request and reply logged. > > +# Note that reply traffic is now logged as to-lport. > > check_acl_log_count 2 > > > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=from-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8356,6 +8363,7 @@ check $PYTHON $srcdir/check_acl_log.py \ > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=2 \ > > --name=allow_related_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:02 \ > > @@ -8365,7 +8373,6 @@ check $PYTHON $srcdir/check_acl_log.py \ > > --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 > > @@ -8384,6 +8391,7 @@ check_acl_log_count 2 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8396,6 +8404,7 @@ check $PYTHON $srcdir/check_acl_log.py \ > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=2 \ > > --name=allow_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:02 \ > > @@ -8420,6 +8429,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8443,6 +8453,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8465,6 +8476,7 @@ check_acl_log_count 1 > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=to-lport \ > > --verdict=allow \ > > --protocol=icmp \ > > --dl_src=00:00:00:00:00:01 \ > > @@ -8482,33 +8494,35 @@ clear_log > > test_ping > > > > # Now we should have the request and reply logged. > > +# Note that reply traffic is from-lport, so it is sorted first. > > check_acl_log_count 2 > > > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=1 \ > > --name=allow_related_acl \ > > + --direction=from-lport \ > > --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 \ > > + --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 > > > > check $PYTHON $srcdir/check_acl_log.py \ > > --entry-num=2 \ > > --name=allow_related_acl \ > > + --direction=to-lport \ > > --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 \ > > + --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 > > > > - > > OVN_CLEANUP_CONTROLLER([hv1]) > > > > OVN_CLEANUP_NORTHD > > -- > > 2.47.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
