[PATCH v5 0/2] Enhance network interface feature testing

[PATCH v5 0/2] Enhance network interface feature testing

[Abhinav Jain]

This small series includes fixes for creation of veth pairs for
networkless kernels & adds tests for turning the different network
interface features on and off in selftests/net/netdevice.sh script.

Changes in v5:
Rectify the syntax for ip add link.
Fix the veth_created condition check.

Changes in v4:
https://lore.kernel.org/all/20240807175717.7775-1-jain.abhinav177@gmail.com/

Move veth creation/removal to the main shell script.
Tested using vng on a networkless kernel and the script works, sample
output below the changes.

Changes in v3:
https://lore.kernel.org/all/20240614113240.41550-1-jain.abhinav177@gmail.com/

Add a check for netdev, create veth pair for testing.
Restore feature to its initial state.

Changes in v2:
https://lore.kernel.org/all/20240609132124.51683-1-jain.abhinav177@gmail.com/

Remove tail usage; use read to parse the features from temp file.

v1:
https://lore.kernel.org/all/20240606212714.27472-1-jain.abhinav177@gmail.com/

```
# selftests: net: netdevice.sh
# No valid network device found, creating veth pair
# PASS: veth0: set interface up
# PASS: veth0: set MAC address
# SKIP: veth0: set IP address
# PASS: veth0: ethtool list features
# PASS: veth0: Turned off feature: rx-checksumming
# PASS: veth0: Turned on feature: rx-checksumming
# PASS: veth0: Restore feature rx-checksumming to initial state on
# Actual changes:
# tx-checksum-ip-generic: off
# tx-tcp-segmentation: off [not requested]
....
....
....
# PASS: veth1: Restore feature tx-nocache-copy to initial state off
# PASS: veth1: Turned off feature: tx-vlan-stag-hw-insert
# PASS: veth1: Turned on feature: tx-vlan-stag-hw-insert
# PASS: veth1: Restore feature tx-vlan-stag-hw-insert to initial state on
# PASS: veth1: Turned off feature: rx-vlan-stag-hw-parse
# PASS: veth1: Turned on feature: rx-vlan-stag-hw-parse
# PASS: veth1: Restore feature rx-vlan-stag-hw-parse to initial state on
# PASS: veth1: Turned off feature: rx-gro-list
# PASS: veth1: Turned on feature: rx-gro-list
# PASS: veth1: Restore feature rx-gro-list to initial state off
# PASS: veth1: Turned off feature: rx-udp-gro-forwarding
# PASS: veth1: Turned on feature: rx-udp-gro-forwarding
# PASS: veth1: Restore feature rx-udp-gro-forwarding to initial state off
# Cannot get register dump: Operation not supported
# SKIP: veth1: ethtool dump not supported
# PASS: veth1: ethtool stats
# PASS: veth1: stop interface
# Removed veth pair
ok 12 selftests: net: netdevice.sh
```

Abhinav Jain (2):
  selftests: net: Create veth pair for testing in networkless kernel
  selftests: net: Add on/off checks for non-fixed features of interface

 tools/testing/selftests/net/netdevice.sh | 53 +++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

--
2.34.1

[PATCH v5 1/2] selftests: net: Create veth pair for testing in networkless kernel

[Abhinav Jain]

Check if the netdev list is empty and create veth pair to be used for
feature on/off testing.
Remove the veth pair after testing is complete.

Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
---
 tools/testing/selftests/net/netdevice.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh
index e3afcb424710..0c32950fdd17 100755
--- a/tools/testing/selftests/net/netdevice.sh
+++ b/tools/testing/selftests/net/netdevice.sh
@@ -129,6 +129,7 @@ kci_netdev_ethtool()
 
 	kci_netdev_ethtool_test 74 'dump' "ethtool -d $netdev"
 	kci_netdev_ethtool_test 94 'stats' "ethtool -S $netdev"
+
 	return 0
 }
 
@@ -196,10 +197,25 @@ if [ ! -e "$TMP_LIST_NETDEV" ];then
 fi
 
 ip link show |grep '^[0-9]' | grep -oE '[[:space:]].*eth[0-9]*:|[[:space:]].*enp[0-9]s[0-9]:' | cut -d\  -f2 | cut -d: -f1> "$TMP_LIST_NETDEV"
+
+if [ ! -s "$TMP_LIST_NETDEV" ]; then
+	echo "No valid network device found, creating veth pair"
+	ip link add veth0 type veth peer name veth1
+	echo "veth0" > "$TMP_LIST_NETDEV"
+	echo "veth1" >> "$TMP_LIST_NETDEV"
+	veth_created=1
+fi
+
 while read netdev
 do
 	kci_test_netdev "$netdev"
 done < "$TMP_LIST_NETDEV"
 
+#clean up veth interface pair if it was created
+if [ "$veth_created" ]; then
+	ip link delete veth0
+	echo "Removed veth pair"
+fi
+
 rm "$TMP_LIST_NETDEV"
 exit 0
-- 
2.34.1

Re: [PATCH v5 1/2] selftests: net: Create veth pair for testing in networkless kernel

[Jakub Kicinski]

On Thu,  8 Aug 2024 12:24:51 +0000 Abhinav Jain wrote:
> Check if the netdev list is empty and create veth pair to be used for
> feature on/off testing.
> Remove the veth pair after testing is complete.

A number of checks now return SKIP because veth doesn't support all
ethtool APIs.

In netdev selftests we try to make sure SKIP is only used when test
cannot be performed because of limitations of the environment.
For example some tool is not installed, kernel doesn't have a config.
Something that the person running the test is able to fix by fixing
how the test is run.

Running this test on veth will always SKIP, nothing CI system can do.
Please make the test use the keyword XFAIL instead of SKIP when
functionality is not supported by the underlying driver.
-- 
pw-bot: cr

Re: [PATCH v5 1/2] selftests: net: Create veth pair for testing in networkless kernel

[Abhinav Jain]

On Thu, 8 Aug 2024 09:23:09 -0700, Jakub Kicinski wrote:
> A number of checks now return SKIP because veth doesn't support all
> ethtool APIs.
>
> In netdev selftests we try to make sure SKIP is only used when test
> cannot be performed because of limitations of the environment.
> For example some tool is not installed, kernel doesn't have a config.
> Something that the person running the test is able to fix by fixing
> how the test is run.
>
> Running this test on veth will always SKIP, nothing CI system can do.
> Please make the test use the keyword XFAIL instead of SKIP when
> functionality is not supported by the underlying driver.

Ack, understood. I will do that, one clarification though.
Currently, the tests are using either PASS or FAIL and no SKIP. Based on
the above suggestion, it seems that I have replace FAIL with XFAIL for all
the tests that fail due to functionality not being supported by the
underlying driver.

Please confirm if my understanding is correct and I will send a v6 of the
series in accordance with netdev patch submission guidelines.
---

Re: [PATCH v5 1/2] selftests: net: Create veth pair for testing in networkless kernel

[Jakub Kicinski]

On Fri,  9 Aug 2024 16:53:26 +0000 Abhinav Jain wrote:
> On Thu, 8 Aug 2024 09:23:09 -0700, Jakub Kicinski wrote:
> > A number of checks now return SKIP because veth doesn't support all
> > ethtool APIs.
> >
> > In netdev selftests we try to make sure SKIP is only used when test
> > cannot be performed because of limitations of the environment.
> > For example some tool is not installed, kernel doesn't have a config.
> > Something that the person running the test is able to fix by fixing
> > how the test is run.
> >
> > Running this test on veth will always SKIP, nothing CI system can do.
> > Please make the test use the keyword XFAIL instead of SKIP when
> > functionality is not supported by the underlying driver.  
> 
> Ack, understood. I will do that, one clarification though.
> Currently, the tests are using either PASS or FAIL and no SKIP. Based on
> the above suggestion, it seems that I have replace FAIL with XFAIL for all
> the tests that fail due to functionality not being supported by the
> underlying driver.

Right, sorry for lack of clarity.

Our CI doesn't fully trust the exit codes, so even though the test
exits with zero the CI parses the output and finds the "SKIP: ..."
lines. You need to replace those "SKIP"s in the output with "XFAIL".

Re: [PATCH v5 1/2] selftests: net: Create veth pair for testing in networkless kernel

[Abhinav Jain]

On Fri, 9 Aug 2024 21:19:11 -0700, Jakub Kicinski wrote:

> > On Thu, 8 Aug 2024 09:23:09 -0700, Jakub Kicinski wrote:
> > > A number of checks now return SKIP because veth doesn't support all
> > > ethtool APIs.
> > >
> > > In netdev selftests we try to make sure SKIP is only used when test
> > > cannot be performed because of limitations of the environment.
> > > For example some tool is not installed, kernel doesn't have a config.
> > > Something that the person running the test is able to fix by fixing
> > > how the test is run.
> > >
> > > Running this test on veth will always SKIP, nothing CI system can do.
> > > Please make the test use the keyword XFAIL instead of SKIP when
> > > functionality is not supported by the underlying driver.  
> > 
> > Ack, understood. I will do that, one clarification though.
> > Currently, the tests are using either PASS or FAIL and no SKIP. Based on
> > the above suggestion, it seems that I have replace FAIL with XFAIL for all
> > the tests that fail due to functionality not being supported by the
> > underlying driver.
>
> Right, sorry for lack of clarity.
>
> Our CI doesn't fully trust the exit codes, so even though the test
> exits with zero the CI parses the output and finds the "SKIP: ..."
> lines. You need to replace those "SKIP"s in the output with "XFAIL".

I re-tested and found that currently only two APIs are tested, "dump" and
"stats". For veth pair, the only test that fails currently with a SKIP is
the dump operation.

```
# Cannot get register dump: Operation not supported
# SKIP: veth1: ethtool dump not supported
```

This is present in kci_netdev_ethtool_test function, please confirm if this
is the one that I need to change to XFAIL. The logic to incorporate
the failure exit code for dump operation (74) is already in place.

```
kci_netdev_ethtool_test 74 'dump' "ethtool -d $netdev"
```

I just need to change the SKIP in this function to XFAIL. In case, if you
were referring to any other tests that are failing (features on/off),
please let me know. Thank you for prompt feedbacks, really appreciated.

[PATCH v5 2/2] selftests: net: Add on/off checks for non-fixed features of interface

[Abhinav Jain]

Implement on/off testing for all non-fixed features via while loop.
Save the initial state so that it can be restored after on/off checks.

Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com>
---
 tools/testing/selftests/net/netdevice.sh | 37 +++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh
index 0c32950fdd17..50f7b9d1163d 100755
--- a/tools/testing/selftests/net/netdevice.sh
+++ b/tools/testing/selftests/net/netdevice.sh
@@ -124,7 +124,42 @@ kci_netdev_ethtool()
 		return 1
 	fi
 	echo "PASS: $netdev: ethtool list features"
-	#TODO for each non fixed features, try to turn them on/off
+
+	while read -r FEATURE VALUE FIXED; do
+		[ "$FEATURE" != "Features" ] || continue # Skip "Features"
+		[ "$FIXED" != "[fixed]" ] || continue # Skip fixed features
+		feature="${FEATURE%:*}"
+
+		initial_state=$(ethtool -k "$netdev" | grep "$feature:" \
+			| awk '{print $2}')
+		ethtool --offload "$netdev" "$feature" off
+		if [ $? -eq 0 ]; then
+			echo "PASS: $netdev: Turned off feature: $feature"
+		else
+			echo "FAIL: $netdev: Failed to turn off feature:" \
+				"$feature"
+		fi
+
+		ethtool --offload "$netdev" "$feature" on
+		if [ $? -eq 0 ]; then
+			echo "PASS: $netdev: Turned on feature: $feature"
+		else
+			echo "FAIL: $netdev: Failed to turn on feature:" \
+				"$feature"
+		fi
+
+		#restore the feature to its initial state
+		ethtool --offload "$netdev" "$feature" "$initial_state"
+		if [ $? -eq 0 ]; then
+			echo "PASS: $netdev: Restore feature $feature" \
+				"to initial state $initial_state"
+		else
+			echo "FAIL: $netdev: Failed to restore feature" \
+				"$feature to default $initial_state"
+		fi
+
+	done < "$TMP_ETHTOOL_FEATURES"
+
 	rm "$TMP_ETHTOOL_FEATURES"
 
 	kci_netdev_ethtool_test 74 'dump' "ethtool -d $netdev"
-- 
2.34.1