On Wed, Aug 16, 2023 at 4:18 PM Ilya Maximets <[email protected]> wrote: > > On 8/9/23 19:00, Mike Pattrick wrote: > > Currently a bond will not always revalidate when an active member > > changes. This can result in counter-intuitive behaviors like the fact > > that using ovs-appctl bond/set-active-member will cause the bond to > > revalidate but changing other_config:bond-primary will not trigger a > > revalidate in the bond. > > Uhm, is that true? If primary changes in the database, > bond_reconfigure() should return 'true' and the bundle_set > function should set need_revalidate = REV_RECONFIGURE. > Is that not happening? > > Can we have a test for this case?
I'll resubmit with a test. -M > > Best regards, Ilya Maximets. > > > > > When revalidation is not set but the active member changes in an > > unbalanced bond, OVS may send traffic out of previously active member > > instead of the new active member. > > > > This change will always mark unbalanced bonds for revalidation if the > > active member changes. > > > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2214979 > > Signed-off-by: Mike Pattrick <[email protected]> > > --- > > ofproto/bond.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/ofproto/bond.c b/ofproto/bond.c > > index cfdf44f85..fb108d30a 100644 > > --- a/ofproto/bond.c > > +++ b/ofproto/bond.c > > @@ -193,6 +193,7 @@ static void bond_update_post_recirc_rules__(struct bond > > *, bool force) > > static bool bond_is_falling_back_to_ab(const struct bond *); > > static void bond_add_lb_output_buckets(const struct bond *); > > static void bond_del_lb_output_buckets(const struct bond *); > > +static bool bond_is_balanced(const struct bond *bond) > > OVS_REQ_RDLOCK(rwlock); > > > > > > /* Attempts to parse 's' as the name of a bond balancing mode. If > > successful, > > @@ -552,11 +553,15 @@ bond_find_member_by_mac(const struct bond *bond, > > const struct eth_addr mac) > > > > static void > > bond_active_member_changed(struct bond *bond) > > + OVS_REQ_WRLOCK(rwlock) > > { > > if (bond->active_member) { > > struct eth_addr mac; > > netdev_get_etheraddr(bond->active_member->netdev, &mac); > > bond->active_member_mac = mac; > > + if (!bond_is_balanced(bond)) { > > + bond->bond_revalidate = true; > > + } > > } else { > > bond->active_member_mac = eth_addr_zero; > > } > > @@ -1124,7 +1129,7 @@ bond_get_recirc_id_and_hash_basis(struct bond *bond, > > uint32_t *recirc_id, > > /* Rebalancing. */ > > > > static bool > > -bond_is_balanced(const struct bond *bond) OVS_REQ_RDLOCK(rwlock) > > +bond_is_balanced(const struct bond *bond) > > { > > return bond->rebalance_interval > > && (bond->balance == BM_SLB || bond->balance == BM_TCP) > > @@ -1728,7 +1733,6 @@ bond_unixctl_set_active_member(struct unixctl_conn > > *conn, > > } > > > > if (bond->active_member != member) { > > - bond->bond_revalidate = true; > > bond->active_member = member; > > VLOG_INFO("bond %s: active member is now %s", > > bond->name, member->name); > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
