On 9/1/25 4:12 PM, Ilya Maximets wrote:
> On 9/1/25 3:33 PM, Dumitru Ceara wrote:
>> Hi Ilya,
>>
>> Thanks for the review!
>>
>> On 9/1/25 3:23 PM, Ilya Maximets wrote:
>>> On 9/1/25 1:22 PM, Dumitru Ceara wrote:
>>>> Since 6919992d8781 ("Datapath_Binding: Separate type column and sync
>>>> NB.UUID to SB."), a new type column is added to SB datapaths to
>>>> differentiate between datapath types (switch vs router). The same
>>>> commit also changed northd such that newly created SB datapaths have as
>>>> UUID the same value as the NB switch/router UUID. All SB.Datapath
>>>> records were also updated and their 'type' field populated
>>>>
>>>> It also added a helper function, datapath_get_nb_uuid_and_type(), to
>>>> abstract out the extraction of a SB datapath's corresponding NB UUID and
>>>> type in the following way:
>>>> - if the SB.Datapath.type field has a value, the helper assumed that the
>>>> SB.Datapath.UUID is equal to the NB.Logical_Switch/Router.UUID so it
>>>> returned the 'type' value and the SB UUID
>>>> - else (for "old style") datapaths it extracted the NB UUID and type
>>>> from the SB.Datapath.external_ids (as used to be done before the
>>>> commit).
>>>>
>>>> This creates an upgrade issue thoughi: older (already existing before
>>>> upgrade) SB datapaths are not immediately recreated and their 'type'
>>>> is set; so the NB UUID value returned by datapath_get_nb_uuid_and_type()
>>>> for these records is incorrect.
>>>>
>>>> That stays like that until datapaths are finally recreated (due to other
>>>> events, e.g., full recompute of the datapath-sync nodes). All that time
>>>> the lflow manager will incorrectly determine that the existing lflows
>>>> that are stale. That's because the lflow manager uses
>>>> ovn_datapath_from_sbrec() which in turn relied on
>>>> datapath_get_nb_uuid_and_type() to determine the in-memory ovn_datapath
>>>> corresponding to the logical flow.
>>>>
>>>> There's no need to avoid re-creating SB.Datapath bindings as soon as
>>>> ovn-northd determines that they're not using the new scheme. In order
>>>> to achieve that we add two sets of helpers:
>>>> a. one to be used by ovn-northd, ovn_datapath_get_nb_uuid_and_type()
>>>> which only parses the new type of datapaths (everything else, i.e.
>>>> old-style datapaths, will be considered stale)
>>>> b. one to be used by all other readers of SB datapaths (e.g.,
>>>> ovn-controller, ovn-ic), datapath_get_nb_uuid_and_type_legacy()
>>>> which relies on parsing external IDs (even in the new type of
>>>> datapaths external IDs are still populated with the NB UUID in order
>>>> to maintain backwards compatibility).
>>>
>>> Can we ever switch all users to the non-leagacy? I'm not sure...
>>> Should probably be a TODO entry for this.
>>>
>>
>> We can, it should happen in the next LTS (26.03) though. And it should
>> be the plan.
>
> How the transition is done? i.e. is that guaranteed that the database
> is fully translated to the new format after the next upgrade? Or was it
> decided that the full database rewrite on upgrade is not an issue and it
> works this way?
>
Yes, it's guaranteed that after upgrade to 25.09.0 (or more recent) the
database is fully translated. It was decided that it's not a concern to
do that. Discussed here:
https://mail.openvswitch.org/pipermail/ovs-dev/2025-August/425327.html
>> Good point about adding a TODO. I can post a v3 or if
>> this is the only change that needs to happen maybe whoever merges this
>> series can add the following incremental:
>>
>> diff --git a/TODO.rst b/TODO.rst
>> index a9fe3ec4e8..4c7aa0c772 100644
>> --- a/TODO.rst
>> +++ b/TODO.rst
>> @@ -99,6 +99,12 @@ OVN To-do List
>> * Implement I-P for datapath groups.
>> * Implement I-P for route exchange relevant ports.
>>
>> +* ovn-northd Incremental processing
>> +
>> + * Remove datapath_get_nb_uuid_and_type_legacy() in the next LTS (26.03) as
>> + all deployments should be using the new SB.Datapath_Binding.type column
>> + at that point.
>> +
>> * ovn-northd parallel logical flow processing
>>
>> * Multi-threaded logical flow computation was optimized for the case
>> ---
>>
>>> If not, then we should just drop the "new" way and stop duplicating
>>> the same information in two places.
>>>
>>> Best regards, Ilya Maximets.
>>>
>>
>> Regards,
>> Dumitru
>>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
