On Thu, Feb 13, 2025 at 04:55:22PM +0100, Dumitru Ceara wrote: > On 2/13/25 3:22 PM, Felix Huettner wrote: > > On Thu, Feb 13, 2025 at 01:28:02PM +0100, Ilya Maximets wrote: > >> On 2/13/25 13:25, Ilya Maximets wrote: > >>> On 2/13/25 13:18, Felix Huettner wrote: > >>>> On Thu, Feb 13, 2025 at 01:12:29PM +0100, Ilya Maximets wrote: > >>>>> On 2/11/25 15:37, Dumitru Ceara wrote: > >>>>>> On 2/11/25 9:35 AM, Felix Huettner via dev wrote: > >>>>>>> in order to exchange routes between OVN and the network fabric we > >>>>>>> use the new Advertised_Route sb table. Northd here advertises all > >>>>>>> routes > >>>>>>> where the user explicitly opted-in. > >>>>>>> > >>>>>>> ovn-controller will later use this table to share these routes to the > >>>>>>> outside. > >>>>>>> > >>>>>>> Acked-by: Dumitru Ceara <[email protected]> > >>>>>>> Signed-off-by: Felix Huettner <[email protected]> > >>>>>>> --- > >>>>>> > >>>>>> Hi Felix, > >>>>>> > >>>>>> I applied this patch to main with the following minor style changes: > >>>>> > >>>>> <snip> > >>>>> > >>>>>>> +OVN_FOR_EACH_NORTHD_NO_HV([ > >>>>>>> +AT_SETUP([dynamic-routing incremental processing]) > >>>>>>> +AT_KEYWORDS([dynamic-routing]) > >>>>>>> +ovn_start > >>>>>>> + > >>>>>>> +# Test I-P for dynamic-routing. > >>>>>>> +# Presently ovn-northd has no I-P for Advertised_Route. > >>>>>>> +# Wait for sb to be connected before clearing stats. > >>>>>>> +check ovn-nbctl --wait=sb sync > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl lr-add lr0 > >>>>>>> +check ovn-nbctl --wait=sb set Logical_Router lr0 > >>>>>>> option:dynamic-routing=true > >>>>>>> + > >>>>>>> +check_engine_stats northd recompute nocompute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 > >>>>>>> 10.0.0.1/24 > >>>>>>> +check_engine_stats northd recompute compute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 > >>>>>>> 10.0.1.1/24 > >>>>>>> +check_engine_stats northd recompute compute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24 10.0.0.10 > >>>>>>> +check_engine_stats northd recompute nocompute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw2 00:00:00:00:ff:03 > >>>>>>> 2001:db8::1/64 fe80::1/64 > >>>>>>> +check_engine_stats northd recompute compute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb remove Logical_Router lr0 option > >>>>>>> dynamic-routing > >>>>>>> +check_engine_stats northd recompute nocompute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb set Logical_Router lr0 > >>>>>>> option:dynamic-routing=true > >>>>>>> +check_engine_stats northd recompute nocompute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lrp-del lr0-sw0 > >>>>>>> +check_engine_stats northd recompute compute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > >>>>>>> +check ovn-nbctl --wait=sb lr-del lr0 > >>>>>>> +check_engine_stats northd recompute nocompute > >>>>>>> +check_engine_stats routes recompute nocompute > >>>>>>> +check_engine_stats advertised_route_sync recompute nocompute > >>>>>>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE > >>>>>>> + > >>>>>>> +AT_CLEANUP > >>>>>>> +]) > >>>>> > >>>>> Hi, Felix and Dumitru. > >>>>> > >>>>> Just an FYI, this test seems to be unstable on arm with Cirrus CI and > >>>>> fails regularly. > >>>>> For example, on my fork: > >>>>> https://cirrus-ci.com/task/5960613278515200?logs=build#L2624 > >>>>> > >>>>> If someone could take a look, that would be great. > >>>> > >>>> Hi Ilya, > >>>> > >>>> i have an arm machine available and will try to reproduce the issue. > >>> > >> > >> CC: Felix (not sure why thunderbird dropped some people from To/Cc) > >> > >>> Thanks! FWIW, it's unlikely to be architecture-specific, more likely to > >>> just > >>> be a timing issue on a less powerful system. > >>> > >>> If you can't reproduce it yourself, cirrus allows to get a debug console > >>> access > >>> to the running test system for some decent, though limited, amount of > >>> time. > >>> See the 'Re-run with a Terminal Access' option. So, that can also be > >>> used for > >>> debugging right inside the CI env, if necessary. > > > > Hi Ilya, > > > > i managed to reproduce this issue on an arm node, if i only have 1 cpu > > and parallely run "stress --cpu 2" :) > > > > The issue seems to be that in some cases northd will batch multiple > > northbound/southbound changes together in one inc-engine run. > > Depending on what exactly that is that might mean that some runs will > > handle in one recompute what others handle in a recompute + compute. > > > > My current idea is to add a new option to check_engine_stats that allows > > you to ignore the compute value. > > > > I think we generally only care about 3 options on each engine node: > > 1. Something was recomputed (no full incremental handling) > > 2. Nothing was recomputed but something was computed (full incremental > > handling) > > 3. No recompute nor compute (no change at all) > > > > I think having a new option to ignore compute values would not interfere > > with the above if only used in combination with checking for recomputes. > > > > If that sounds reasonable to you then i would send out a patch for that. > > > > I think that's exactly the reason why some of incremental processing > tests for LB syncing to SB ignore "compute" stats: > > https://github.com/ovn-org/ovn/blob/1098d316be577f5bd5c2f9d4aedb192b6df08094/tests/ovn-northd.at#L10863 > > Added when ovn-northd LB I-P support was implemented: > > https://github.com/ovn-org/ovn/commit/5e5aeaa9f133ab021d48f61905fc3035ae4220b8#diff-ad86f2621d498f3f310f111c1a92fe47589f0c2d51529fa9ba98371b32c00a9e > > It sounds reasonable to me to ignore "compute" stats in these kinds of > cases.
Thanks a lot. I built a fix here https://patchwork.ozlabs.org/project/ovn/patch/408bc00c8a1158e4fdf5b1afd4ac0a816744a615.1739462687.git.felix.huettner@stackit.cloud/ If that approach seems generally reasonable i can also migrate the other testcases to this. Then the tests would be consistent. Thanks a lot, Felix > > Regards, > Dumitru > > > Thanks a lot, > > Felix > > > > > >>> > >>>> > >>>> Thanks for bringing that up, > >>>> Felix > >>>> > >>>>> > >>>>> Best regards, Ilya Maximets. > >>> > >> > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
