On 2/11/26 4:08 PM, Matteo Perin via dev wrote: > Tests including routing rule checks were failing on systems with > non-standard routing rules, causing issues when trying to run them on > some deployments. > These failures occurred because tests performed exact output matching > on the full output of 'ovs-appctl ovs/route/rule/show', which includes > both user-added and system-cached rules. > > When the system has additional routing rules that meet certain criteria > (FR_ACT_TO_TBL action without unsupported selectors like fwmark, dport, > sport, iif, ipproto, or tun_id), OVS caches them, as expected, causing > them to appear in the "Cached:" section of the output. > Tests that expect exact output fail when encountering these legitimate, > albeit unexpected, cached rules.
Hi, Matteo. Thanks for the patch! > > The following affected tests were modified to verify only the expected > behavior rather than requiring exact output matches: > > - system-route.at (ovs-route - unsupported rules): > Captures full initial cache state before adding test rules, verifies that > the cache state remains unchanged after adding unsupported rules (keeping > the intent of the test). This one makes sense to me... > > - ovs-router.at: > For standard rule existence checks, changed from exact match to verifying > each expected rule exists independently. > For user rule verification, added filtering to show only "User:" rules, > ignoring "Cached:" system rules entirely. > > 3. tunnel-push-pop.at (tunnel configuration with routing rules): > Added filter to output to show only "User:" rules when verifying routing > rule configuration. ...but these two do not. These are not system tests, they are running with the dummy datapath and with system routes disabled. So, it should be perfectly fine to do an exact match comparison. And we should keep doing that. Or do you see these tests actually fail in your setup? If so, we need to find the reason and fix it. But the tests should stay as they are. > > Signed-off-by: Matteo Perin <[email protected]> > --- > tests/ovs-router.at | 114 ++++++++------------------------------- > tests/system-route.at | 51 +++++++----------- > tests/tunnel-push-pop.at | 5 +- > 3 files changed, 43 insertions(+), 127 deletions(-) > <snip> > --- a/tests/system-route.at > +++ b/tests/system-route.at > @@ -343,45 +343,34 @@ on_exit 'ip link del p1-route' > AT_CHECK([ip tuntap add name p1-route mode tap]) > AT_CHECK([ip link set p1-route up]) > > -dnl Check there are no non-standard rules cached in OVS. > -AT_CHECK([ovs-appctl ovs/route/rule/show], [0], [dnl > -Cached: 0: from all lookup local > -Cached: 32766: from all lookup main > -Cached: 32767: from all lookup default > -]) > -AT_CHECK([ovs-appctl ovs/route/rule/show -6], [0], [dnl > -Cached: 0: from all lookup local > -Cached: 32766: from all lookup main > -]) > - > -dnl Add unsupported rules to kernel. > +dnl Register cleanup for all rules we are going to add. > on_exit 'ip rule del priority 100 fwmark 0x16 lookup 42' > -AT_CHECK([ip rule add priority 100 fwmark 0x16 lookup 42]) > on_exit 'ip rule del priority 101 from 10.0.0.1 dport 22 lookup 42' > -AT_CHECK([ip rule add priority 101 from 10.0.0.1 dport 22 lookup 42]) > on_exit 'ip rule del priority 102 from 10.0.0.1 sport 22 lookup 42' > -AT_CHECK([ip rule add priority 102 from 10.0.0.1 sport 22 lookup 42]) > on_exit 'ip rule del priority 103 from 10.0.0.1 tun_id 22 lookup 42' > -AT_CHECK([ip rule add priority 103 from 10.0.0.1 tun_id 22 lookup 42]) > on_exit 'ip rule del priority 104 iif p1-route lookup 42' > -AT_CHECK([ip rule add priority 104 iif p1-route lookup 42]) > on_exit 'ip rule del priority 105 ipproto udp lookup 42' > -AT_CHECK([ip rule add priority 105 ipproto udp lookup 42]) > on_exit 'ip rule del priority 106 from all tun_id 22 lookup 42' > -AT_CHECK([ip rule add priority 106 from all tun_id 22 lookup 42]) > > -dnl Give the main thread a chance to act. > -AT_CHECK([ovs-appctl revalidator/wait]) > -dnl Check OVS rules cache hasn't changed. > -AT_CHECK([ovs-appctl ovs/route/rule/show], [0], [dnl > -Cached: 0: from all lookup local > -Cached: 32766: from all lookup main > -Cached: 32767: from all lookup default > -]) > -AT_CHECK([ovs-appctl ovs/route/rule/show -6], [0], [dnl > -Cached: 0: from all lookup local > -Cached: 32766: from all lookup main > -]) > +dnl Capture initial cache state, add unsupported rules, and verify OVS cache > +dnl hasn't changed. This test verifies that unsupported routing rules (with > +dnl selectors like fwmark, dport, sport, iif, ipproto, tun_id) are not cached > +dnl by OVS, even though the kernel accepts them. > +AT_CHECK([ > + initial_v4=$(ovs-appctl ovs/route/rule/show) > + initial_v6=$(ovs-appctl ovs/route/rule/show -6) > + ip rule add priority 100 fwmark 0x16 lookup 42 > + ip rule add priority 101 from 10.0.0.1 dport 22 lookup 42 > + ip rule add priority 102 from 10.0.0.1 sport 22 lookup 42 > + ip rule add priority 103 from 10.0.0.1 tun_id 22 lookup 42 > + ip rule add priority 104 iif p1-route lookup 42 > + ip rule add priority 105 ipproto udp lookup 42 > + ip rule add priority 106 from all tun_id 22 lookup 42 > + ovs-appctl revalidator/wait > + final_v4=$(ovs-appctl ovs/route/rule/show) > + final_v6=$(ovs-appctl ovs/route/rule/show -6) > + test "$initial_v4" = "$final_v4" && test "$initial_v6" = "$final_v6" > +], [0]) We should not write tests this way. The block above, IIRC, will only fail if the last command fails. If any of the 'ip rule add' will fail, the test will still pass, even though it didn't test want it supposed to test. Please, keep the commands where they were originally. Just capture the state before and after and then compare. You also removed some comments on that we're explaining why certain calls are made. We should keep them. I'd also suggest to not use plain shell comparisons. Instead, capture the output with AT_CHECK([...], [0], [stdout]), then rename the stdout file into something more meaningfull (under AT_CHECK as well), then do the same at the end and do a diif -u comparison of two files (also under AT_CHECK). This way all the commands and outputs will end up in the log and it will be clear what exactly is different. Also, it may be good to sort the outputs. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
