[ovs-dev] [PATCH ovn 2/3] northd: update stage-name if changed

[Mark Gray]

On 24/06/2021 18:36, Dumitru Ceara wrote:
> On 6/24/21 5:56 PM, Ilya Maximets wrote:
>> On 6/24/21 5:44 PM, Dumitru Ceara wrote:
>>> On 6/22/21 7:25 AM, Han Zhou wrote:
>>>> On Sat, Jun 19, 2021 at 2:51 AM Mark Gray <mark.d.gray at redhat.com> wrote:
>>>>> If a new table is added to a logical flow pipeline, the mapping between
>>>>> 'external_ids:stage-name' from the 'Logical_Flow' table in the
>>>>> 'OVN_Southbound' database and the 'stage' number may change for some
>>>> tables.
>>>>> If 'ovn-northd' is started against a populated Southbound database,
>>>>> 'external_ids:stage-name' will not be updated to reflect the new, correct
>>>>> name. This will cause the stage name to be incorrectly displayed by some
>>>>> tools and commands such as `ovn-sbctl dump-flows`.
>>>>>
>>>>> This commit, reconciles changes to the stage name as part of
>>>> build_lflows().
>>>> This is interesting. It means a flow F existed in stage S (named "foo") of
>>>> ovn-northd version V1, and in version V2, S becomes S + 1 ("foo"), but in
>>>> the V2's stage S ("bar") there happens to be an exactly same flow like F
>>>> existing, but the stage name shown in ovn-sbctl dump-flows will be "foo"
>>>> instead of "bar". Is this the scenario the patch is trying to fix?

Yes, exactly this.

>>>>
>>>> If so, I think it is better not only fixing the "stage-name" but also other
>>>> external_ids, including "source" and "stage-hint", for the same reason.
>>>>
>>>
>>> Hi Mark, Han,
>>>
>>> Wouldn't this cause lots of additional string comparisons?  What if we
>>> restrict ourselves to checking one flow per stage (for each ovn-northd
>>> iteration).  Would that be enough to detect that we need to update all
>>> of them?
>>>
>>> This might be an ugly hack though. :)
>>
>> Maybe we can check if 'northd_internal_version' changed in SB and re-check
>> all external-ids only in this case?
>>
> 
> Nice idea!  We just need to make sure the ovn_internal_version changes
> when logical flow stages are added.  We do something similar for OVN
> actions.
> 

I have done this as Ilya suggested. Thanks for the suggestion.