[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aHeLsNbhvqTFge3J@fedora>
Date: Wed, 16 Jul 2025 11:23:28 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Nikolay Aleksandrov <razor@...ckwall.org>,
Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 2/2] selftests: bonding: add test for passive LACP
mode
On Tue, Jul 15, 2025 at 11:37:54AM +0200, Paolo Abeni wrote:
> On 7/9/25 11:03 AM, Hangbin Liu wrote:
> > Add a selftest to verify bonding behavior when lacp_active is set to off.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> > ---
> > .../drivers/net/bonding/bond_passive_lacp.sh | 21 +++++
> > .../drivers/net/bonding/bond_topo_lacp.sh | 77 +++++++++++++++++++
> > 2 files changed, 98 insertions(+)
> > create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_passive_lacp.sh
> > create mode 100644 tools/testing/selftests/drivers/net/bonding/bond_topo_lacp.sh
>
> New test should be listed in the relevant makefile
Ah, yes, I forgot this.
> > diff --git a/tools/testing/selftests/drivers/net/bonding/bond_passive_lacp.sh b/tools/testing/selftests/drivers/net/bonding/bond_passive_lacp.sh
> > new file mode 100755
> > index 000000000000..4cf8a5999aaa
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/bonding/bond_passive_lacp.sh
> > @@ -0,0 +1,21 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Testing if bond works with lacp_active = off
> > +
> > +lib_dir=$(dirname "$0")
> > +source ${lib_dir}/bond_topo_lacp.sh
>
> shellcheck is not super happy about 'source' usage:
>
> In bond_passive_lacp.sh line 7:
> source ${lib_dir}/bond_topo_lacp.sh
> ^-- SC3046 (warning): In POSIX sh, 'source' in place of '.' is undefined.
> ^-- SC3051 (warning): In POSIX sh, 'source' in place of '.' is undefined.
>
> either switch to '. ' or use bash instead of 'sh'.
OK, I will fix this and other warns.
Thanks
Hangbin
>
> > +lacp_bond_reset "${c_ns}" "lacp_active off"
> > +# make sure the switch state is not expired [A,T,G,S,Ex]
> > +if slowwait 15 ip netns exec ${s_ns} grep -q 'port state: 143' /proc/net/bonding/bond0; then
>
> Shellcheck wants double quote everywhere. Since in many cases (all the
> blamed ones in this patch) we know the variable is really a single word,
> I think you could simply disable the warning with:
>
> #shellcheck disable=SC2086
>
> (same in the other test file)
>
> > + RET=1
> > +else
> > + RET=0
> > +fi
>
> /P
>
Powered by blists - more mailing lists