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
