Messages in this thread | | | Date | Wed, 15 Mar 2023 17:52:20 +0200 | From | Vladimir Oltean <> | Subject | Re: [RFC PATCH net-next] selftests: forwarding: add a test for MAC Merge layer |
| |
Hi Petr,
On Tue, Feb 14, 2023 at 12:53:06PM +0100, Petr Machata wrote: > > 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.
Are you planning to take a look and see whether the callers of bail_on_lldpad() can also source $lib_dir/lib.sh? Should I resend a blind change here?
| |