Re: [PATCH net] selftests: rtnetlink.sh: remove esp4_offload after test

[Shannon Nelson]

(yeah, I made the same non-ASCII mistake...)

On 7/24/25 10:49 AM, Shannon Nelson wrote:
> On 7/24/25 1:20 AM, Xiumei Mu wrote:
> > resent the reply again with "plain text mode"
> >
> > On Thu, Jul 24, 2025 at 2:25 PM Hangbin Liu<liuhangbin@xxxxxxxxx> wrote:
> > > Hi Xiumei,
> > > On Thu, Jul 24, 2025 at 12:55:02PM +0800, Xiumei Mu wrote:
> > > > The esp4_offload module, loaded during IPsec offload tests, should
> > > > be reset to its default settings after testing.
> > > > Otherwise, leaving it enabled could unintentionally affect subsequence
> > > > test cases by keeping offload active.
> > > Would you please show which subsequence test will be affected?
> > Any general ipsec case, which expects to be tested by default
> > behavior(without offload).
> > esp4_offload will affect the performance.
> >
> > > > Fixes: 2766a11161cc ("selftests: rtnetlink: add ipsec offload API test")
> > > It would be good to Cc the fix commit author. You can use
> > > `./scripts/get_maintainer.pl your_patch_file` to get the contacts you
> > > need to Cc.
> > I used the script to generate the cc list.
> > and I double checked the old email of the author is invalid
> > added his personal email in the cc list:
> >
> > Shannon Nelson<shannon.nelson@xxxxxxxxxx>. -----> Shannon Nelson
> > <sln@xxxxxxxxxxx>
> >
> >   get the information from here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a1113cefd7d6
>
> Yep, that was me a couple of corporate email addresses ago. Thanks for 
> digging up the new email address.  Luckily I have a couple of mail 
> filters watching for old email addresses.
>
> > > > Signed-off-by: Xiumei Mu<xmu@xxxxxxxxxx>
> > > > ---
> > > >   tools/testing/selftests/net/rtnetlink.sh | 6 ++++++
> > > >   1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
> > > > index 2e8243a65b50..5cc1b5340a1a 100755
> > > > --- a/tools/testing/selftests/net/rtnetlink.sh
> > > > +++ b/tools/testing/selftests/net/rtnetlink.sh
> > > > @@ -673,6 +673,11 @@ kci_test_ipsec_offload()
> > > >        sysfsf=$sysfsd/ipsec
> > > >        sysfsnet=/sys/bus/netdevsim/devices/netdevsim0/net/
> > > >        probed=false
> > > > +     esp4_offload_probed_default=false
> > > > +
> > > > +     if lsmod | grep -q esp4_offload; then
> > > > +             esp4_offload_probed_default=true
> > > > +     fi
> > > If the mode is loaded by default, how to avoid the subsequence test to be
> > > failed?
> > The module is not loaded by default, but some users or testers may
> > need to load esp4_offload in their own environments.
> > Therefore, resetting it to the default configuration is the best
> > practice to prevent this self-test case from impacting subsequent
> > tests
>
> Seems reasonable to me.
>
> > > >        if ! mount | grep -q debugfs; then
> > > >                mount -t debugfs none /sys/kernel/debug/ &> /dev/null
> > > > @@ -766,6 +771,7 @@ EOF
> > > >        fi
> > > >
> > > >        # clean up any leftovers
> > > > +     [ $esp4_offload_probed_default == false ] && rmmod esp4_offload
> > > The new patch need to pass shellcheck. We need to double quote the variable.
> > Thanks your comment, I will add double quote in patchv2
>
> Or you keep with the existing style as done a line or two later:
>     $esp4_offload_probed_default && rmmod esp4_offload Either way, 
> Reviewed-by: Shannon Nelson <sln@xxxxxxxxxxx> Cheers, sln
> > > Thanks
> > > Hangbin
> > > >        echo 0 > /sys/bus/netdevsim/del_device
> > > >        $probed && rmmod netdevsim
> > > >
> > > > --
> > > > 2.50.1