On 10/11/24 21:20, Odintsov Vladislav wrote:
> On 11.10.2024 14:00, Dumitru Ceara wrote:
> 
>> On 10/7/24 12:45, Vladislav Odintsov wrote:
>>> From: Dumitru Ceara <[email protected]>
>>>
>>> This picks up the following relevant commit:
>>>   cd8ffc956c3c ovs-atomic: Fix inclusion of Clang header by GCC 14.
>>>
>>> Without this builds on Fedora 40 (rawhide) are broken due to failing to
>>> compile the submodule.
>>>
>>> Signed-off-by: Dumitru Ceara <[email protected]>
>>> Acked-by: Numan Siddique <[email protected]>
>>> Signed-off-by: Numan Siddique <[email protected]>
>>> (cherry picked from commit f224c6e5f69c099ddb008f99dba2e19a902a612f)
>>> Signed-off-by: Vladislav Odintsov <[email protected]>
>>> ---
>>> Without this patch there are errors building OVN on a modern systems.
>>>
>>> I kindly request for this patch to be backported down to 22.03 LTS
>>> including already officially unsupported branches 23.03, 22.09 and 22.06,
>>> since we internally still need to base on 22.09 branch in development.
>>>
>>> Thanks in advance if it is possible to make an exception and ignore
>>> backport rules for non-LTS releases and patch them too.
>>> ---
>> Hi Vladislav,
>>
>> I see this patch was marked as "Changes Requested" in patchwork but I'm
>> not sure who did that (either one of the other maintainers or yourself).
>>
>> In any case, I assume the request still applies, right?
>>
>>>  ovs | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/ovs b/ovs
>>> index 8fd5f77cd..49e64f13b 160000
>>> --- a/ovs
>>> +++ b/ovs
>>> @@ -1 +1 @@
>>> -Subproject commit 8fd5f77cd84ea04667f987c7b84181604dc99f60
>>> +Subproject commit 49e64f13b2c965f5b53a65eeab70ac2e3f0bf69a
>> Shouldn't we bump to 3ff343a96b4c instead?  AFAICT that's the tip of
>> branch-3.2 in OVS today.  While we're at it we should probably bump
>> branch-23.09 to the same version too.
>>
>> What do you think?
> 
> Am I understanding you correctly that it's worth to bump OVN
> branch-23.03's OVS submodule to 120050c26 (which is the backport commit
> of proposed by you 3ff343a96b4c, but from branch-3.1)? I'm not sure if
> it is good idea to bump minor OVS version between (possible)
> patch/hotfix OVN versions.
> 

Sorry for the confusion.  I was trying to suggest to bump the
submodule on all OVN stable branches to the current tip of a
stable OVS release branch.  That's what I think our documentation
suggests:

https://github.com/ovn-org/ovn/blob/main/Documentation/internals/ovs_submodule.rst#submodules-for-releases

Right now we have this:
24.09: c598c05c85b2 ("Set release date for 3.4.0.")
24.03: f19448b86189 ("github: Update python to 3.12.") - branch-3.3
23.09: c88a35fc29f0 ("github: Update python to 3.12.") - branch-3.2
23.03: 8fd5f77cd84e ("ovsdb-idl: Preserve change_seqno when deleting rows.") - 
branch-3.1
22.12: 8fd5f77cd84e ("ovsdb-idl: Preserve change_seqno when deleting rows.") - 
branch-3.1
22.09: 94191b7a4926 ("ovsdb-idl: Preserve change_seqno when deleting rows.") - 
branch-3.0
22.06: 94191b7a4926 ("ovsdb-idl: Preserve change_seqno when deleting rows.") - 
branch-3.0
22.03: 94191b7a4926 ("ovsdb-idl: Preserve change_seqno when deleting rows.") - 
branch-3.0

We could bump the submodule on all these branches to the tip of the
OVS branch they're already tracking and I think we should be fine.

OVS commit 335a5deac3ff ("ovs-atomic: Fix inclusion of Clang header by
GCC 14.") was backported to all branches down to branch-3.0.

> If yes, I've got a question: shouldn't we bump to f59f19bf6 (ovsdb-idl:
> Fix IDL memory leak.) instead?
> 

If we do what I suggest above we'll also get this fix.

Our documentation also says:
"
For choice 1, the decision of whether to update the submodule commit to OVS 
branch-Z is based on several factors.
    Is OVN release X still being supported?
    Is there any known benefit to updating the submodule? E.g., are there 
performance improvements we could take advantage of by updating the submodule?
    Is there risk in updating the submodule?
"

While I understand it's riskier to bump to the tip of the stable OVS
branch (potentially across minor OVS versions) the alternative of
bumping strictly to the commit that fixes the compilation error also
carries some risks: we might be missing follow up fixes that went in
afterwards.

However, I didn't do a thorough review yet of what other commits we'd
be picking up on each branch.  I'll try to do that when the patches
are posted.

What do you think?

Thanks,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to