On Tue, Nov 11, 2025 at 02:27:53AM -0800, Breno Leitao wrote:
> > +
> > +   if [ "${STATE}" == "enabled" ]
> > +   then
> > +           ENABLED=1
> 
> Shouldn't they be local variables in here ?

Yes, good point.

> > +   else
> > +           ENABLED=0
> > +   fi
> > +
> > +   if [ ! -f "$FILE" ]; then
> 
>       if [ ! -f "${TARGET_PATH}" ]; then
> 
> > +           echo "FAIL: Target does not exist." >&2
> > +           exit "${ksft_fail}"
> > +   fi
> > +
> > +   slowwait 2 sh -c "test -n \"\$(grep \"${ENABLED}\" \"${FILE}\")\"" || {
> 
>       slowwait 2 sh -c "test -n \"\$(grep \"${ENABLED}\" 
> \"${TARGET_PATH}/enabled\")\"" || {
> 

Ack.

> > +           echo "FAIL: ${TARGET} is not ${STATE}." >&2
> > +   }
> > +}
> > +
> >  # A wrapper to translate protocol version to udp version
> >  function wait_for_port() {
> >     local NAMESPACE=${1}
> > diff --git a/tools/testing/selftests/drivers/net/netcons_resume.sh 
> > b/tools/testing/selftests/drivers/net/netcons_resume.sh
> > new file mode 100755
> > index 000000000000..404df7abef1b
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/netcons_resume.sh
> > @@ -0,0 +1,92 @@
> > +#!/usr/bin/env bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +# This test validates that netconsole is able to resume a target that was
> > +# deactivated when its interface was removed when the interface is brought
> > +# back up.
> 
> Comment above is a bit harder to understand.
> 

Agreed. What do you think of: 

# This test validates that netconsole is able to resume a previously deactivated
# target once its interface is brought back up. 

> > +for BINDMODE in "ifname" "mac"
> > +do
> > +   echo "Running with bind mode: ${BINDMODE}" >&2
> > +   # Set current loglevel to KERN_INFO(6), and default to KERN_NOTICE(5)
> > +   echo "6 5" > /proc/sys/kernel/printk
> > +
> > +   # Create one namespace and two interfaces
> > +   set_network
> > +   trap do_cleanup EXIT
> 
> can we keep these trap lines outside of the loop?
> 

Let me try to do that. I'm using different handlers depending on how far we are 
on
the test but instead I think I should be able to use a similar approach as you 
did
with cleanup_netcons() in 
https://lore.kernel.org/netdev/[email protected]/.

> > +   pkill_socat
> > +   # Cleanup & unload the module
> > +   cleanup "${NETCONS_CONFIGFS}/cmdline0"
> > +   rmmod netconsole
> 
> Why do we need to remove netconsole module in here?

We are removing the module here so we can load it on the second iteration of the
test with new cmdline. This is following a similar pattern to 
netcons_cmdline.sh.

> Thanks for this patch. This is solving a real issue we have right now.
> --breno

Thanks for the review!

-- 
Andre Carvalho

Reply via email to