[ovs-dev] [PATCH ovn] northd: I-P: fix BFD dependency.

Dumitru Ceara dceara at redhat.com
Wed Nov 24 16:18:40 UTC 2021


On 11/24/21 17:15, Numan Siddique wrote:
> On Tue, Nov 23, 2021 at 7:52 PM Dumitru Ceara <dceara at redhat.com> 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 <dceara at redhat.com>
> 
> 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



More information about the dev mailing list