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

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

[Xiumei Mu]

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.

Fixes: 2766a11161cc ("selftests: rtnetlink: add ipsec offload API test")
Signed-off-by: Xiumei Mu <xmu@redhat.com>
---
 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 ! 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
 	echo 0 > /sys/bus/netdevsim/del_device
 	$probed && rmmod netdevsim
 
-- 
2.50.1

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

[Hangbin Liu]

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?

> 
> 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.

> Signed-off-by: Xiumei Mu <xmu@redhat.com>
> ---
>  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?

>  
>  	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
Hangbin
>  	echo 0 > /sys/bus/netdevsim/del_device
>  	$probed && rmmod netdevsim
>  
> -- 
> 2.50.1
> 

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

[Xiumei Mu]

resent the reply again with "plain text mode"

On Thu, Jul 24, 2025 at 2:25 PM Hangbin Liu <liuhangbin@gmail.com> 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@oracle.com>. -----> Shannon Nelson
<sln@onemain.com>

 get the information from here:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=a1113cefd7d6

>
> > Signed-off-by: Xiumei Mu <xmu@redhat.com>
> > ---
> >  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

>
> >
> >       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

>
> Thanks
> Hangbin
> >       echo 0 > /sys/bus/netdevsim/del_device
> >       $probed && rmmod netdevsim
> >
> > --
> > 2.50.1
> >
>

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@gmail.com> 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@oracle.com>. -----> Shannon Nelson
>> <sln@onemain.com>
>>
>>   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@redhat.com>
>>>> ---
>>>>   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@onemain.com> Cheers, sln
>>> Thanks
>>> Hangbin
>>>>        echo 0 > /sys/bus/netdevsim/del_device
>>>>        $probed && rmmod netdevsim
>>>>
>>>> --
>>>> 2.50.1
>>>>
>