| Message ID | 20240609132124.51683-1-jain.abhinav177@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] sefltests: net: Add on/off checks for network interface non fixed features | expand |
On Sun, 9 Jun 2024 13:21:24 +0000 Abhinav Jain wrote: > This patch addresses the TODO (add non fixed feature on/off check). > I have tested it manually on my system and made changes as suggested in v1 > 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" line > + [ "$FIXED" != "[fixed]" ] || continue # Skip fixed features > + feature = "${FEATURE%:*}" > + > + 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 You do off then on, so you assume the feature was on to begin with. Not all features will be on. You gotta change the order based on the initial state so that the feature goes back to what it was. > + if [ $? -eq 0 ]; then > + echo "PASS: $netdev: Turned on feature: $feature" > + else > + echo "FAIL: $netdev: Failed to turn on feature: $feature" > + fi > + done < "$TMP_ETHTOOL_FEATURES" More importantly, tho, currently in our CI this test passes: https://netdev.bots.linux.dev/contest.html?test=netdevice-sh But it doesn't do anything: https://netdev-3.bots.linux.dev/vmksft-net/results/638724/98-netdevice-sh/stdout https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/638544/90-netdevice-sh/stdout Because I'm guessing the VM does not have any interface to test. Before we extend the test could you please make the test automatically add a veth or netdevsim device if there is no testable device on the system? Make sure you remove it before exiting. You can look at tests in tools/testing/selftests/drivers/net/netdevsim for inspiration and library scripts which can be reused.
On Thu, 13 Jun 2024 18:49:38 -0700, Jakub Kicinski wrote: > You do off then on, so you assume the feature was on to begin with. > Not all features will be on. You gotta change the order based on > the initial state so that the feature goes back to what it was. Thanks for sharing feedback, I have sent a patch v3 addressing this. I save the initial state of the feature and set it back after I toggle it. > Because I'm guessing the VM does not have any interface to test. > > Before we extend the test could you please make the test automatically > add a veth or netdevsim device if there is no testable device on the > system? Make sure you remove it before exiting. > > You can look at tests in tools/testing/selftests/drivers/net/netdevsim > for inspiration and library scripts which can be reused. I did the same, I have added a veth pair in case if the network device in the positional argument is not there. If it's there and just a bad device name though, I am printing an error. Let me know if I should use the veth pair in those cases as well for testing. I have sent patch v3 here, please review: https://lore.kernel.org/all/20240614113240.41550-1-jain.abhinav177@gmail.com/ -Abhinav
diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh index e3afcb424710..196b7f985db4 100755 --- a/tools/testing/selftests/net/netdevice.sh +++ b/tools/testing/selftests/net/netdevice.sh @@ -124,7 +124,27 @@ 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" line + [ "$FIXED" != "[fixed]" ] || continue # Skip fixed features + feature = "${FEATURE%:*}" + + 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 + done < "$TMP_ETHTOOL_FEATURES" + rm "$TMP_ETHTOOL_FEATURES" kci_netdev_ethtool_test 74 'dump' "ethtool -d $netdev"
This patch addresses the TODO (add non fixed feature on/off check). I have tested it manually on my system and made changes as suggested in v1 Signed-off-by: Abhinav Jain <jain.abhinav177@gmail.com> --- PATCH v1: https://lore.kernel.org/all/20240606212714.27472-1-jain.abhinav177@gmail.com/ Changes since v1: - Removed the addition of tail command as it was not required after below change - Used read to parse the temp features file rather than using for loop and took out awk/grep/sed from v1 --- tools/testing/selftests/net/netdevice.sh | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)