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? 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 <m...@redhat.com> > --- > 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 d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
