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

From: Petr Machata <[email-masked]>
Date: Mon, 20 Mar 2023 11:20:19 +0100

Vladimir Oltean <[email-masked]> writes:

> On Fri, Mar 17, 2023 at 04:35:58PM +0100, Petr Machata wrote:
>> 
>> Petr Machata <[email-masked]> writes:
>> 
>> > Petr Machata <[email-masked]> writes:
>> >
>> >> Petr Machata <[email-masked]> writes:
>> >>
>> >>> Vladimir Oltean <[email-masked]> writes:
>> >>>
>> >>>> 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.
>> >
>> > I added a shim as shown below. Comments welcome. Your patch then needs a
>> > bit of adaptation, plus I've dropped all the now-useless imports of
>> > qos_lib.sh. I'll pass this through our regression, and if nothing
>> > explodes, I'll point you at a branch tomorrow, and you can make the two
>> > patches a part of your larger patchset?
>> 
>> (I only pushed this to our regression today. The patches are the top two
>> ones here:
>> 
>>     https://github.com/pmachata/linux_mlxsw/commits/petrm_selftests_bail_on_lldpad_move
>> 
>> I'll let you know on Monday whether anything exploded in regression.)
>
> Thanks.
>
> Side question. Your bail_on_lldpad logic wants to avoid lldpad from
> being involved at all in the selftest. In my case, I want to make sure
> that the service is disabled system-wide, so that the selftest can start
> it by itself. Is that the best way to make use of lldpad in general in
> the kselftest framework?

^o^ I don't see why not. You could bail_on_lldpad, then proceed to start
it, then stop it after the test again. Maybe the error message needs
tweaking to work well with this use-case though.