On Thu, Feb 05, 2026 at 02:04:42PM +0100, Ilya Maximets wrote:
> On 2/4/26 11:14 AM, Adrian Moreno via dev wrote:
> > The current way of calculating "next_rebalance" when there is a change
> > in the bond rebalance period applies the current rebalance interval
> > (again) and substracts the new one.
>
> *subtracts
>
> >
> > This yields an unexpected new value for "next_rebalance": say the
> > current interval is 60s and we reduce it to 10s (and rebalancing has
> > just occurred), the current logic will set the next rebalancing in 110s
> > which is probably not what the user expects.
> >
> > Swap current and new rebalance intervals in the offset calculation
> > logic.
> >
> > Fixes: 9a2169a3f7bb ("bond: Do not flag a revalidation when adjusting
> > time.")
> > Reported-by: Minxi Hou <[email protected]>
> > Signed-off-by: Adrian Moreno <[email protected]>
> > ---
> > ofproto/bond.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ofproto/bond.c b/ofproto/bond.c
> > index 584c38f07..ef481a360 100644
> > --- a/ofproto/bond.c
> > +++ b/ofproto/bond.c
> > @@ -468,9 +468,9 @@ bond_reconfigure(struct bond *bond, const struct
> > bond_settings *s)
> > * we can skip triggering the rest of the port
> > reconfiguration. */
> > if (bond->next_rebalance) {
> > long long int old_start_time =
> > - bond->next_rebalance - s->rebalance_interval;
> > + bond->next_rebalance - bond->rebalance_interval;
> > bond->next_rebalance =
> > - old_start_time + bond->rebalance_interval;
> > + old_start_time + s->rebalance_interval;
> > }
> > } else {
> > /* When the bond is doing a disable/enable of the rebalance
>
> Thanks, Adrian!
>
> The code change seems correct to me, but I think we must add a test for this.
> The original patch went in without a test because I assumed we'll have one
> once we fix all the other issues brought up during the v1 review of the
> original patch:
> https://mail.openvswitch.org/pipermail/ovs-dev/2025-August/425098.html
> But that never happened. This underlines the thought that we can't really
> afford tech debt and all patches should be complete as possible when they are
> merged.
>
> It's not particularly fair to ask for a test in the fix when the original code
> didn't have one, but I think it's a necessity at this point. Otherwise, we'll
> just never have a coverage for this functionality... I can help with
> preparing
> a test, if you don't have much time for it, just let me know.
>
Sure, I'll add a test and submit v2.
--
Adrián
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
