diff mbox series

[ovs-dev,branch-24.09] ovs: Bump submodule to latest OVS branch-3.4.

Message ID 20241013081912.75095-1-vlodintsov@k2.cloud
State Superseded
Headers show
Series [ovs-dev,branch-24.09] ovs: Bump submodule to latest OVS branch-3.4. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Vladislav Odintsov Oct. 13, 2024, 8:19 a.m. UTC
This picks up the following relevant OVS changes:
  a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
  76ba41b5c vconn: Always properly free flow stats reply.
  64cb90507 ovsdb-idl: Fix IDL memory leak.
  ... and others.

Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
---
 ovs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dumitru Ceara Oct. 14, 2024, 11:47 a.m. UTC | #1
On 10/13/24 10:19, Vladislav Odintsov wrote:
> This picks up the following relevant OVS changes:
>   a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>   76ba41b5c vconn: Always properly free flow stats reply.
>   64cb90507 ovsdb-idl: Fix IDL memory leak.
>   ... and others.
> 
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
> ---

Hi Vladislav,

>  ovs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ovs b/ovs
> index c598c05c8..a15ce086d 160000
> --- a/ovs
> +++ b/ovs
> @@ -1 +1 @@
> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0

This would make branch-24.09 use a newer submodule version than the OVN
main branch does.

I think we need this commit on main too, what do you think?

Thanks,
Dumitru
Vladislav Odintsov Oct. 14, 2024, 1:01 p.m. UTC | #2
On 14.10.2024 14:47, Dumitru Ceara wrote:
> On 10/13/24 10:19, Vladislav Odintsov wrote:
>> This picks up the following relevant OVS changes:
>>    a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>    76ba41b5c vconn: Always properly free flow stats reply.
>>    64cb90507 ovsdb-idl: Fix IDL memory leak.
>>    ... and others.
>>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>> ---
> Hi Vladislav,
>
>>   ovs | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ovs b/ovs
>> index c598c05c8..a15ce086d 160000
>> --- a/ovs
>> +++ b/ovs
>> @@ -1 +1 @@
>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
> This would make branch-24.09 use a newer submodule version than the OVN
> main branch does.
>
> I think we need this commit on main too, what do you think?

Hi Dumitru,

Agree.

The patch: 
https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/

>
> Thanks,
> Dumitru
>
Dumitru Ceara Oct. 14, 2024, 1:13 p.m. UTC | #3
On 10/14/24 15:01, Vladislav Odintsov wrote:
> 
> On 14.10.2024 14:47, Dumitru Ceara wrote:
>> On 10/13/24 10:19, Vladislav Odintsov wrote:
>>> This picks up the following relevant OVS changes:
>>>    a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>>    76ba41b5c vconn: Always properly free flow stats reply.
>>>    64cb90507 ovsdb-idl: Fix IDL memory leak.
>>>    ... and others.
>>>
>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>>> ---
>> Hi Vladislav,
>>
>>>   ovs | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/ovs b/ovs
>>> index c598c05c8..a15ce086d 160000
>>> --- a/ovs
>>> +++ b/ovs
>>> @@ -1 +1 @@
>>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
>> This would make branch-24.09 use a newer submodule version than the OVN
>> main branch does.
>>
>> I think we need this commit on main too, what do you think?
> 
> Hi Dumitru,
> 
> Agree.
> 
> The patch: 
> https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/
> 

I was talking offline to Ilya about all these bumps and he made a good
point: while the tip of OVS branch-3.y and the latest v3.y.z get almost
the same amount of testing in the OVS repo it might be a bit better to
use v3.y.z instead.  That's because likely external users of OVS run
tagged OVS releases in production so those might get more external testing.

I had a quick look at the main differences between choosing the tip of
branch-3.y and v3.y.z on all branches and I think we'd only miss:

99e7cf9cce1c vconn: Always properly free flow stats reply.
f59f19bf69a4 ovsdb-idl: Fix IDL memory leak.

which might be OK.

If you agree I can change your patches on all branches (no need to post
new ones) and apply them.

What do you think?

Thanks,
Dumitru
Vladislav Odintsov Oct. 14, 2024, 2:41 p.m. UTC | #4
On 14.10.2024 16:13, Dumitru Ceara wrote:
> On 10/14/24 15:01, Vladislav Odintsov wrote:
>> On 14.10.2024 14:47, Dumitru Ceara wrote:
>>> On 10/13/24 10:19, Vladislav Odintsov wrote:
>>>> This picks up the following relevant OVS changes:
>>>>     a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>>>     76ba41b5c vconn: Always properly free flow stats reply.
>>>>     64cb90507 ovsdb-idl: Fix IDL memory leak.
>>>>     ... and others.
>>>>
>>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>>>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>>>> ---
>>> Hi Vladislav,
>>>
>>>>    ovs | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/ovs b/ovs
>>>> index c598c05c8..a15ce086d 160000
>>>> --- a/ovs
>>>> +++ b/ovs
>>>> @@ -1 +1 @@
>>>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>>>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
>>> This would make branch-24.09 use a newer submodule version than the OVN
>>> main branch does.
>>>
>>> I think we need this commit on main too, what do you think?
>> Hi Dumitru,
>>
>> Agree.
>>
>> The patch:
>> https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/
>>
> I was talking offline to Ilya about all these bumps and he made a good
> point: while the tip of OVS branch-3.y and the latest v3.y.z get almost
> the same amount of testing in the OVS repo it might be a bit better to
> use v3.y.z instead.  That's because likely external users of OVS run
> tagged OVS releases in production so those might get more external testing.
>
> I had a quick look at the main differences between choosing the tip of
> branch-3.y and v3.y.z on all branches and I think we'd only miss:
>
> 99e7cf9cce1c vconn: Always properly free flow stats reply.
> f59f19bf69a4 ovsdb-idl: Fix IDL memory leak.
>
> which might be OK.
>
> If you agree I can change your patches on all branches (no need to post
> new ones) and apply them.
>
> What do you think?

Well, I'm totally fine with this. Please feel free to modify my patches.

Also, shouldn't we update documentation to reflect this approach in 
Documentation/internals/ovs_submodule.rst for further bumps? And if 
talking about documentation, I've got one note, which should be covered 
by new process. Imagine situation, where quite old OVS branch (let's 
say, 3.0) has a wanted commit (for example, fix for build with new 
compiler or latest libs), but the new patch release is not created 
because it is not a critical problem). I'd say we either need to request 
OVS community to bump patch release or bump from release to commit sha. 
What do you think here? Or, just leave it as is and decide how to bump 
in flexible manner in each individual case?

>
> Thanks,
> Dumitru
>
Dumitru Ceara Oct. 15, 2024, 3:34 p.m. UTC | #5
On 10/14/24 16:41, Vladislav Odintsov wrote:
> 
> On 14.10.2024 16:13, Dumitru Ceara wrote:
>> On 10/14/24 15:01, Vladislav Odintsov wrote:
>>> On 14.10.2024 14:47, Dumitru Ceara wrote:
>>>> On 10/13/24 10:19, Vladislav Odintsov wrote:
>>>>> This picks up the following relevant OVS changes:
>>>>>     a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>>>>     76ba41b5c vconn: Always properly free flow stats reply.
>>>>>     64cb90507 ovsdb-idl: Fix IDL memory leak.
>>>>>     ... and others.
>>>>>
>>>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>>>>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>>>>> ---
>>>> Hi Vladislav,
>>>>
>>>>>    ovs | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/ovs b/ovs
>>>>> index c598c05c8..a15ce086d 160000
>>>>> --- a/ovs
>>>>> +++ b/ovs
>>>>> @@ -1 +1 @@
>>>>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>>>>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
>>>> This would make branch-24.09 use a newer submodule version than the OVN
>>>> main branch does.
>>>>
>>>> I think we need this commit on main too, what do you think?
>>> Hi Dumitru,
>>>
>>> Agree.
>>>
>>> The patch:
>>> https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/
>>>
>> I was talking offline to Ilya about all these bumps and he made a good
>> point: while the tip of OVS branch-3.y and the latest v3.y.z get almost
>> the same amount of testing in the OVS repo it might be a bit better to
>> use v3.y.z instead.  That's because likely external users of OVS run
>> tagged OVS releases in production so those might get more external testing.
>>
>> I had a quick look at the main differences between choosing the tip of
>> branch-3.y and v3.y.z on all branches and I think we'd only miss:
>>
>> 99e7cf9cce1c vconn: Always properly free flow stats reply.
>> f59f19bf69a4 ovsdb-idl: Fix IDL memory leak.
>>
>> which might be OK.
>>
>> If you agree I can change your patches on all branches (no need to post
>> new ones) and apply them.
>>
>> What do you think?
> 
> Well, I'm totally fine with this. Please feel free to modify my patches.
> 

I prepared them here, it would be great if you could double check:

https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-24.03
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.09
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.03
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.12
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.09
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.06
https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.03

I skipped main and branch-24.09 because those are already on the latest
OVS v3.4.z.


> Also, shouldn't we update documentation to reflect this approach in 
> Documentation/internals/ovs_submodule.rst for further bumps? And if 

We should, you're right, I'll prepare a patch.

> talking about documentation, I've got one note, which should be covered 
> by new process. Imagine situation, where quite old OVS branch (let's 
> say, 3.0) has a wanted commit (for example, fix for build with new 
> compiler or latest libs), but the new patch release is not created 
> because it is not a critical problem). I'd say we either need to request 
> OVS community to bump patch release or bump from release to commit sha. 
> What do you think here? Or, just leave it as is and decide how to bump 
> in flexible manner in each individual case?
> 

I think this is not the common case so maybe we can leave it flexible
for now.

Regards,
Dumitru
Vladislav Odintsov Oct. 18, 2024, 5:42 a.m. UTC | #6
Hi Dumitru,

On 15.10.2024 18:34, Dumitru Ceara wrote:
> On 10/14/24 16:41, Vladislav Odintsov wrote:
>> On 14.10.2024 16:13, Dumitru Ceara wrote:
>>> On 10/14/24 15:01, Vladislav Odintsov wrote:
>>>> On 14.10.2024 14:47, Dumitru Ceara wrote:
>>>>> On 10/13/24 10:19, Vladislav Odintsov wrote:
>>>>>> This picks up the following relevant OVS changes:
>>>>>>      a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>>>>>      76ba41b5c vconn: Always properly free flow stats reply.
>>>>>>      64cb90507 ovsdb-idl: Fix IDL memory leak.
>>>>>>      ... and others.
>>>>>>
>>>>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>>>>>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>>>>>> ---
>>>>> Hi Vladislav,
>>>>>
>>>>>>     ovs | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/ovs b/ovs
>>>>>> index c598c05c8..a15ce086d 160000
>>>>>> --- a/ovs
>>>>>> +++ b/ovs
>>>>>> @@ -1 +1 @@
>>>>>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>>>>>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
>>>>> This would make branch-24.09 use a newer submodule version than the OVN
>>>>> main branch does.
>>>>>
>>>>> I think we need this commit on main too, what do you think?
>>>> Hi Dumitru,
>>>>
>>>> Agree.
>>>>
>>>> The patch:
>>>> https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/
>>>>
>>> I was talking offline to Ilya about all these bumps and he made a good
>>> point: while the tip of OVS branch-3.y and the latest v3.y.z get almost
>>> the same amount of testing in the OVS repo it might be a bit better to
>>> use v3.y.z instead.  That's because likely external users of OVS run
>>> tagged OVS releases in production so those might get more external testing.
>>>
>>> I had a quick look at the main differences between choosing the tip of
>>> branch-3.y and v3.y.z on all branches and I think we'd only miss:
>>>
>>> 99e7cf9cce1c vconn: Always properly free flow stats reply.
>>> f59f19bf69a4 ovsdb-idl: Fix IDL memory leak.
>>>
>>> which might be OK.
>>>
>>> If you agree I can change your patches on all branches (no need to post
>>> new ones) and apply them.
>>>
>>> What do you think?
>> Well, I'm totally fine with this. Please feel free to modify my patches.
>>
> I prepared them here, it would be great if you could double check:
>
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-24.03
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.09
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.03
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.12
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.09

Could you please update commit subject to "ovs: Bump submodule to OVS 
3.0.7." for patches within branches branch-22.09, branch-22.06, 
branch-22.03?

> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.06
> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.03
>
> I skipped main and branch-24.09 because those are already on the latest
> OVS v3.4.z.
Thanks for the update, other than commit subject, these patches look 
good to me!
>
>
>> Also, shouldn't we update documentation to reflect this approach in
>> Documentation/internals/ovs_submodule.rst for further bumps? And if
> We should, you're right, I'll prepare a patch.
>
>> talking about documentation, I've got one note, which should be covered
>> by new process. Imagine situation, where quite old OVS branch (let's
>> say, 3.0) has a wanted commit (for example, fix for build with new
>> compiler or latest libs), but the new patch release is not created
>> because it is not a critical problem). I'd say we either need to request
>> OVS community to bump patch release or bump from release to commit sha.
>> What do you think here? Or, just leave it as is and decide how to bump
>> in flexible manner in each individual case?
>>
> I think this is not the common case so maybe we can leave it flexible
> for now.
>
> Regards,
> Dumitru
>
Dumitru Ceara Oct. 18, 2024, 7:58 a.m. UTC | #7
On 10/18/24 07:42, Vladislav Odintsov wrote:
> Hi Dumitru,
> 
> On 15.10.2024 18:34, Dumitru Ceara wrote:
>> On 10/14/24 16:41, Vladislav Odintsov wrote:
>>> On 14.10.2024 16:13, Dumitru Ceara wrote:
>>>> On 10/14/24 15:01, Vladislav Odintsov wrote:
>>>>> On 14.10.2024 14:47, Dumitru Ceara wrote:
>>>>>> On 10/13/24 10:19, Vladislav Odintsov wrote:
>>>>>>> This picks up the following relevant OVS changes:
>>>>>>>      a15ce086d ofproto-dpif: Improve load balancing in dp_hash select groups.
>>>>>>>      76ba41b5c vconn: Always properly free flow stats reply.
>>>>>>>      64cb90507 ovsdb-idl: Fix IDL memory leak.
>>>>>>>      ... and others.
>>>>>>>
>>>>>>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417627.html
>>>>>>> Signed-off-by: Vladislav Odintsov <vlodintsov@k2.cloud>
>>>>>>> ---
>>>>>> Hi Vladislav,
>>>>>>
>>>>>>>     ovs | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/ovs b/ovs
>>>>>>> index c598c05c8..a15ce086d 160000
>>>>>>> --- a/ovs
>>>>>>> +++ b/ovs
>>>>>>> @@ -1 +1 @@
>>>>>>> -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
>>>>>>> +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
>>>>>> This would make branch-24.09 use a newer submodule version than the OVN
>>>>>> main branch does.
>>>>>>
>>>>>> I think we need this commit on main too, what do you think?
>>>>> Hi Dumitru,
>>>>>
>>>>> Agree.
>>>>>
>>>>> The patch:
>>>>> https://patchwork.ozlabs.org/project/ovn/patch/20241014125151.74094-1-vlodintsov@k2.cloud/
>>>>>
>>>> I was talking offline to Ilya about all these bumps and he made a good
>>>> point: while the tip of OVS branch-3.y and the latest v3.y.z get almost
>>>> the same amount of testing in the OVS repo it might be a bit better to
>>>> use v3.y.z instead.  That's because likely external users of OVS run
>>>> tagged OVS releases in production so those might get more external testing.
>>>>
>>>> I had a quick look at the main differences between choosing the tip of
>>>> branch-3.y and v3.y.z on all branches and I think we'd only miss:
>>>>
>>>> 99e7cf9cce1c vconn: Always properly free flow stats reply.
>>>> f59f19bf69a4 ovsdb-idl: Fix IDL memory leak.
>>>>
>>>> which might be OK.
>>>>
>>>> If you agree I can change your patches on all branches (no need to post
>>>> new ones) and apply them.
>>>>
>>>> What do you think?
>>> Well, I'm totally fine with this. Please feel free to modify my patches.
>>>
>> I prepared them here, it would be great if you could double check:
>>
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-24.03
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.09
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-23.03
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.12
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.09
> 
> Could you please update commit subject to "ovs: Bump submodule to OVS 
> 3.0.7." for patches within branches branch-22.09, branch-22.06, 
> branch-22.03?
> 
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.06
>> https://github.com/dceara/ovn/tree/refs/heads/tmp-branch-22.03
>>
>> I skipped main and branch-24.09 because those are already on the latest
>> OVS v3.4.z.
> Thanks for the update, other than commit subject, these patches look 
> good to me!

Thanks for double checking!  My original intention was to avoid posting
a new set of patches and getting this done "quickly" but I kind of failed.

For better tracking I decided to post the patches again on the mailing
list (after fixing up the commit message on 22.09, 22.06 and 22.03) so
we have tracking of what actually happened:

24.03:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075003.996714-1-dceara@redhat.com/
23.09:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075116.997019-1-dceara@redhat.com/
23.03:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075211.997262-1-dceara@redhat.com/
22.12:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075317.997485-1-dceara@redhat.com/
22.09:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075406.997921-1-dceara@redhat.com/
22.06:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075442.998098-1-dceara@redhat.com/
22.03:
https://patchwork.ozlabs.org/project/ovn/patch/20241018075518.998310-1-dceara@redhat.com/

I plan to merge these once ovsrobot CI is green.

Thanks again!

Dumitru
diff mbox series

Patch

diff --git a/ovs b/ovs
index c598c05c8..a15ce086d 160000
--- a/ovs
+++ b/ovs
@@ -1 +1 @@ 
-Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc
+Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0