Re: [PATCH net 2/2] selftests: bonding: add test for passive LACP mode

[Hangbin Liu]

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@xxxxxxxxx>
> > ---
> >  .../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
>