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 |
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 |
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
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 >
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
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 >
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
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 >
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 --git a/ovs b/ovs index c598c05c8..a15ce086d 160000 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit c598c05c85b2d38874a0ce8f7f088f6aae4fdabc +Subproject commit a15ce086d41f9dfe6c1589333413b8e777401ef0
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(-)