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. 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
