| Message ID | 20230306111959.429680-1-po-hsu.lin@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: net: devlink_port_split.py: skip test if no suitable device available | expand |
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!
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__":
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(-)