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

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

quoted

resent the reply again with "plain text mode"

On Thu, Jul 24, 2025 at 2:25 PM Hangbin Liu[off-list ref] wrote:

quoted

Hi Xiumei,
On Thu, Jul 24, 2025 at 12:55:02PM +0800, Xiumei Mu wrote:

quoted

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.

quoted

quoted

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[off-list ref]. -----> Shannon Nelson
[off-list ref]

  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.

quoted

quoted

quoted

Signed-off-by: Xiumei Mu<redacted>
---
  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.

quoted

quoted

quoted

       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 <redacted> Cheers, sln

quoted

quoted

Thanks
Hangbin

quoted

       echo 0 > /sys/bus/netdevsim/del_device
       $probed && rmmod netdevsim

--
2.50.1