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

Dumitru Ceara dceara at redhat.com
Wed Nov 24 16:37:39 UTC 2021


On 11/24/21 17:21, Numan Siddique wrote:
> On Wed, Nov 24, 2021 at 11:19 AM Dumitru Ceara <dceara at redhat.com> wrote:
>>
>> 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.
> 
> Sounds good.  I applied this patch to the main branch.
> 

Thanks!



More information about the dev mailing list