On 11/24/21 17:15, Numan Siddique wrote: > On Tue, Nov 23, 2021 at 7:52 PM Dumitru Ceara <[email protected]> wrote: >> >> BFD entries are updated and their ->ref field is set to 'true' when >> static routes are parsed, within build_lflows(), in the 'en_lflow' I-P >> node. Therefore we cannot clean up BFD entries in 'en_northd' which is >> an input of 'en_lflow'. >> >> To fix this we now move all BFD handling in the 'en_lflow' node. >> >> This fixes the CI failures that we've recently started hitting when >> running the ovn-kubernetes jobs, for example: >> https://github.com/ovn-org/ovn/runs/4294661249?check_suite_focus=true#step:10:9154 >> >> Fixes: 1fa6612ffebf ("northd: Add lflow node") >> Signed-off-by: Dumitru Ceara <[email protected]> > > Hi Dumitru, >
Hi Numan, > Thanks for fixing this issue. > Thanks for reviewing this. > In my opinion en_northd engine node should handle the DB syncs and > en_lflow engine node > should handle the flow generation. I think it would be better to move > the syncing of > NB bfd->state from en_lflow engine node (ie frombuild_lflows()) to > en_northd engine node. > This would mean en_northd engine node should iterate the logical > static routes and set the > status. Which would mean we need to move out the function > parsed_routes_add() from > build_static_route_flows_for_lrouter(). > > What do you think ? Would you agree with this ? > Sounds like this would be the ideal way to implement this, yes. > I'm not too sure about how easy it is ? I'm inclined to accept this I don't see a very clean way of implementing it though. > patch and then revisit this > as a follow up or once we branch out of 21.12. I think this would be the safest way to go for now. > > Let me know your thoughts. > > Thanks > Numan > Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
