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

[Shannon Nelson <sln@onemain.com>]

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