Re: [RFC PATCH net-next] selftests: forwarding: add a test for MAC Merge layer

[Petr Machata <petrm@nvidia.com>]

Vladimir Oltean <vladimir.oltean@nxp.com> writes:

> Hi Petr,
>
> On Mon, Feb 13, 2023 at 11:51:37AM +0100, Petr Machata wrote:
>> Vladimir Oltean <vladimir.oltean@nxp.com> writes:
>> 
>> > +# Borrowed from mlxsw/qos_lib.sh, message adapted.
>> > +bail_on_lldpad()
>> > +{
>> > +	if systemctl is-active --quiet lldpad; then
>> > +		cat >/dev/stderr <<-EOF
>> > +		WARNING: lldpad is running
>> > +
>> > +			lldpad will likely autoconfigure the MAC Merge layer,
>> > +			while this test will configure it manually. One of them
>> > +			is arbitrarily going to overwrite the other. That will
>> > +			cause spurious failures (or, unlikely, passes) of this
>> > +			test.
>> > +		EOF
>> > +		exit 1
>> > +	fi
>> > +}
>> 
>> This would be good to have unified. Can you make the function reusable,
>> with a generic or parametrized message? I should be able to carve a bit
>> of time later to move it to lib.sh, migrate the mlxsw selftests, and
>> drop the qos_lib.sh copy.
>
> Maybe like this?

Yes, for most of them, but the issue is that...

> diff --git a/tools/testing/selftests/drivers/net/mlxsw/sch_tbf_ets.sh b/tools/testing/selftests/drivers/net/mlxsw/sch_tbf_ets.sh
> index c6ce0b448bf3..bf57400e14ee 100755
> --- a/tools/testing/selftests/drivers/net/mlxsw/sch_tbf_ets.sh
> +++ b/tools/testing/selftests/drivers/net/mlxsw/sch_tbf_ets.sh
> @@ -2,7 +2,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  source qos_lib.sh
> -bail_on_lldpad
> +bail_on_lldpad "configure DCB" "configure Qdiscs"

... lib.sh isn't sourced at this point yet. `source
$lib_dir/sch_tbf_ets.sh' brings that in later in the file, so the bail
would need to be below that. But if it is, it won't run until after the
test, which is useless.

Maybe all it takes is to replace that `source qos_lib.sh' with
`NUM_NETIFS=0 source $lib_dir/lib.sh', as we do in in_ns(), but I'll
need to check.

That's why I proposed to do it myself, some of it is fiddly and having a
switch handy is going to be... um, handy.