The `devlink -j dev show` command output may not contain the "flavour" key, for example: $ devlink -j dev show {"dev":{"pci/0001:00:00.0":{},"pci/0002:00:00.0":{}}}
This will cause a KeyError exception. Fix this by checking the key existence first.
Also, if max lanes is 0 the port splitting won't be tested at all. but the script will end normally and thus causing a false-negative test result.
Use a test_ran flag to determine if these tests were skipped and return KSFT_SKIP accordingly.
Link: https://bugs.launchpad.net/bugs/1937133 Fixes: f3348a82e727 ("selftests: net: Add port split test") Signed-off-by: Po-Hsu Lin po-hsu.lin@canonical.com --- tools/testing/selftests/net/devlink_port_split.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/devlink_port_split.py b/tools/testing/selftests/net/devlink_port_split.py index 2b5d6ff..462f3df 100755 --- a/tools/testing/selftests/net/devlink_port_split.py +++ b/tools/testing/selftests/net/devlink_port_split.py @@ -61,7 +61,7 @@ class devlink_ports(object):
for port in ports: if dev in port: - if ports[port]['flavour'] == 'physical': + if 'flavour' in ports[port] and ports[port]['flavour'] == 'physical': arr.append(Port(bus_info=port, name=ports[port]['netdev']))
return arr @@ -231,6 +231,7 @@ def make_parser():
def main(cmdline=None): + test_ran = False parser = make_parser() args = parser.parse_args(cmdline)
@@ -277,6 +278,11 @@ def main(cmdline=None): split_splittable_port(port, lane, max_lanes, dev)
lane //= 2 + test_ran = True + + if not test_ran: + print("No suitable device for the test, test skipped") + sys.exit(KSFT_SKIP)
if __name__ == "__main__":
On Mon, 6 Mar 2023 19:19:59 +0800 Po-Hsu Lin wrote:
The `devlink -j dev show` command output may not contain the "flavour" key, for example: $ devlink -j dev show {"dev":{"pci/0001:00:00.0":{},"pci/0002:00:00.0":{}}}
It's not dev that's supposed to have the flavor, it's port.
devlink -j port show
Are you running with old kernel or old user space? Flavor is not an optional attribute.
This will cause a KeyError exception. Fix this by checking the key existence first.
Also, if max lanes is 0 the port splitting won't be tested at all. but the script will end normally and thus causing a false-negative test result.
Use a test_ran flag to determine if these tests were skipped and return KSFT_SKIP accordingly.
Link: https://bugs.launchpad.net/bugs/1937133 Fixes: f3348a82e727 ("selftests: net: Add port split test") Signed-off-by: Po-Hsu Lin po-hsu.lin@canonical.com
Could you factor out the existing skipping logic from main() (the code under "if not dev:") and add the test for flavors to the same function? It'll be a bit more code but cleaner result IMHO.
On Tue, Mar 7, 2023 at 2:33 AM Jakub Kicinski kuba@kernel.org wrote:
On Mon, 6 Mar 2023 19:19:59 +0800 Po-Hsu Lin wrote:
The `devlink -j dev show` command output may not contain the "flavour" key, for example: $ devlink -j dev show {"dev":{"pci/0001:00:00.0":{},"pci/0002:00:00.0":{}}}
It's not dev that's supposed to have the flavor, it's port.
devlink -j port show
Ah yes, it's using output from this command, thanks for catching this.
Are you running with old kernel or old user space? Flavor is not an optional attribute.
This was from a s390x LPAR instance with Ubuntu 22.10 (5.19.0-37-generic) iproute2-5.15.0
$ devlink -j port show {"port":{"pci/0001:00:00.0/1":{"type":"eth","netdev":"ens301"},"pci/0001:00:00.0/2":{"type":"eth","netdev":"ens301d1"},"pci/0002:00:00.0/1":{"type":"eth","netdev":"ens317"},"pci/0002:00:00.0/2":{"type":"eth","netdev":"ens317d1"}}}
This will cause a KeyError exception. Fix this by checking the key existence first.
Also, if max lanes is 0 the port splitting won't be tested at all. but the script will end normally and thus causing a false-negative test result.
Use a test_ran flag to determine if these tests were skipped and return KSFT_SKIP accordingly.
Link: https://bugs.launchpad.net/bugs/1937133 Fixes: f3348a82e727 ("selftests: net: Add port split test") Signed-off-by: Po-Hsu Lin po-hsu.lin@canonical.com
Could you factor out the existing skipping logic from main() (the code under "if not dev:") and add the test for flavors to the same function? It'll be a bit more code but cleaner result IMHO.
Sure, will do with V2. Thanks!
linux-kselftest-mirror@lists.linaro.org