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.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
