Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Jay Vosburgh <jay.vosburgh@canonical.com>
To:     Jonathan Toppins <jtoppins@redhat.com>
cc:     "netdev @ vger . kernel . org" <netdev@vger.kernel.org>,
        pabeni@redhat.com, Veaceslav Falico <vfalico@gmail.com>,
        Andy Gospodarek <andy@greyhouse.net>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v2 2/2] bonding: fix link recovery in mode 2 when updelay is nonzero
In-reply-to: <f5a7893d8a83c6fb8233bb3127d301e12f45e9b3.1669147951.git.jtoppins@redhat.com>
References: <cover.1669147951.git.jtoppins@redhat.com> <f5a7893d8a83c6fb8233bb3127d301e12f45e9b3.1669147951.git.jtoppins@redhat.com>
Comments: In-reply-to Jonathan Toppins <jtoppins@redhat.com>
   message dated "Tue, 22 Nov 2022 16:24:29 -0500."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <15929.1669184720.1@famine>
Content-Transfer-Encoding: quoted-printable
Date:   Tue, 22 Nov 2022 22:25:20 -0800
Message-ID: <15930.1669184720@famine>
Precedence: bulk

Jonathan Toppins <jtoppins@redhat.com> wrote:

>Before this change when a bond in mode 2 lost link, all of its slaves
>lost link, the bonding device would never recover even after the
>expiration of updelay. This change removes the updelay when the bond
>currently has no usable links. Conforming to bonding.txt section 13.1
>paragraph 4.
>
>Fixes: 41f891004063 ("bonding: ignore updelay param when there is no acti=
ve slave")
>Signed-off-by: Jonathan Toppins <jtoppins@redhat.com>

	This looks correct, although I suspect it would affect more than
just balance-xor ("mode 2"); if memory serves, balance-rr mode operates
similarly.

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

	-J

>---
>
>Notes:
>    v2:
>     * added fixes tag and reposted to net tree
>
> drivers/net/bonding/bond_main.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_m=
ain.c
>index f298b9b3eb77..f747bd60d399 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2536,7 +2536,16 @@ static int bond_miimon_inspect(struct bonding *bon=
d)
> 	struct slave *slave;
> 	bool ignore_updelay;
> =

>-	ignore_updelay =3D !rcu_dereference(bond->curr_active_slave);
>+	if (BOND_MODE(bond) =3D=3D BOND_MODE_ACTIVEBACKUP) {
>+		ignore_updelay =3D !rcu_dereference(bond->curr_active_slave);
>+	} else {
>+		struct bond_up_slave *usable_slaves;
>+
>+		usable_slaves =3D rcu_dereference(bond->usable_slaves);
>+
>+		if (usable_slaves && usable_slaves->count =3D=3D 0)
>+			ignore_updelay =3D true;
>+	}
> =

> 	bond_for_each_slave_rcu(bond, slave, iter) {
> 		bond_propose_link_state(slave, BOND_LINK_NOCHANGE);
>-- =

>2.31.1
>